From e0f02d7d262b8bed7620f0d29789164550f352b8 Mon Sep 17 00:00:00 2001 From: john30 Date: Sun, 20 Nov 2016 16:10:23 +0100 Subject: [PATCH] added helper class for grabbing message counts and changed grab command to allow selection of all/unknown messages in result, fixed potential memory leaks during shutdown --- src/ebusd/bushandler.cpp | 72 ++++++++++++++++++++----------- src/ebusd/bushandler.h | 91 ++++++++++++++++++++++++++++++++-------- src/ebusd/mainloop.cpp | 33 ++++++++------- 3 files changed, 140 insertions(+), 56 deletions(-) diff --git a/src/ebusd/bushandler.cpp b/src/ebusd/bushandler.cpp index 873030ee..563ef425 100644 --- a/src/ebusd/bushandler.cpp +++ b/src/ebusd/bushandler.cpp @@ -181,6 +181,36 @@ bool ActiveBusRequest::notify(result_t result, SymbolString& slave) return false; } +void GrabbedMessage::setLastData(SymbolString& master, SymbolString& slave) +{ + m_lastMaster.clear(false); + m_lastMaster.addAll(master); + m_lastSlave.clear(false); + m_lastSlave.addAll(slave); + m_count++; +} + +bool GrabbedMessage::dump(const bool unknown, MessageMap* messages, bool first, ostringstream& output) +{ + Message* message = messages->find(m_lastMaster); + if (unknown && message) { + return false; + } + if (!first) { + output << endl; + } + unsigned char dstAddress = m_lastMaster[1]; + output << m_lastMaster.getDataStr(); + if (dstAddress != BROADCAST && !isMaster(dstAddress)) { + output << " / " << m_lastSlave.getDataStr(); + } + output << " = " << static_cast(m_count); + if (message) { + output << ": " << message->getCircuit() << " " << message->getName(); + } + return true; +} + void BusHandler::clear() { @@ -840,19 +870,14 @@ void BusHandler::receiveCompleted() logInfo(lf_update, "update MS cmd: %s / %s", m_command.getDataStr().c_str(), m_response.getDataStr().c_str()); } Message* message = m_messages->find(m_command); - if (m_grabUnknownMessages==gr_all || (message==NULL && m_grabUnknownMessages==gr_unknown)) { - string data; - string key = data = m_command.getDataStr(); - if (key.length() > 2*(1+1+2+1+4)) { - key = key.substr(0, 2*(1+1+2+1+4)); // QQZZPBSBNN + up to 4 DD bytes - } - if (dstAddress != BROADCAST && !master) { - data += " / " + m_response.getDataStr(); - } + if (m_grabMessages) { + unsigned long long key; if (message) { - data += " = "+message->getCircuit()+" "+message->getName(); + key = message->getKey(); + } else { + key = Message::createKey(m_command, 4); // up to 4 DD bytes } - m_grabbedUnknownMessages[key] = data; + m_grabbedMessages[key].setLastData(m_command, m_response); } if (message == NULL) { if (dstAddress == BROADCAST) { @@ -1057,29 +1082,28 @@ result_t BusHandler::scanAndWait(unsigned char dstAddress, SymbolString& slave) return scanMessage->storeLastData(pt_slaveData, slave, 0); // update the cache } -bool BusHandler::enableGrab(bool enable, bool all) +bool BusHandler::enableGrab(bool enable) { - GrabRequest request = enable ? (all ? gr_all : gr_unknown) : gr_none; - if (request==m_grabUnknownMessages) + if (enable==m_grabMessages) { return false; - if (m_grabUnknownMessages==gr_none) - m_grabbedUnknownMessages.clear(); - m_grabUnknownMessages = request; + } + if (!enable) { + m_grabbedMessages.clear(); // TODO check + } + m_grabMessages = enable; return true; } -void BusHandler::formatGrabResult(ostringstream& output) +void BusHandler::formatGrabResult(const bool unknown, ostringstream& output) { - if (!m_grabUnknownMessages) { + if (!m_grabMessages) { output << "grab disabled"; } else { bool first = true; - for (map::iterator it = m_grabbedUnknownMessages.begin(); it != m_grabbedUnknownMessages.end(); it++) { - if (first) + for (map::iterator it = m_grabbedMessages.begin(); it != m_grabbedMessages.end(); it++) { + if (it->second.dump(unknown, m_messages, first, output)) { first = false; - else - output << endl; - output << it->second; + } } } } diff --git a/src/ebusd/bushandler.h b/src/ebusd/bushandler.h index 4cd0e92e..3d899442 100644 --- a/src/ebusd/bushandler.h +++ b/src/ebusd/bushandler.h @@ -72,13 +72,6 @@ enum BusState { bs_sendSyn, //!< send SYN for completed transfer [active set+get] }; -/** the possible grab request kinds. */ -enum GrabRequest { - gr_none, //!< no grabbing at all - gr_unknown, //!< grab unknown messages only - gr_all, //!< grab all messages -}; - /** bit for the seen state: seen. */ #define SEEN 0x01 @@ -288,6 +281,59 @@ private: }; +/** + * Helper class for keeping track of grabbed messages. + */ +class GrabbedMessage +{ + +public: + + /** + * Construct a new instance. + */ + GrabbedMessage() : m_count(0) {} + + /** + * Copy constructor. + * @param other the @a GrabbedMessage to copy from. + */ + GrabbedMessage(const GrabbedMessage& other) : m_count(other.m_count) { + m_lastMaster.addAll(other.m_lastMaster); + m_lastSlave.addAll(other.m_lastSlave); + } + + /** + * Set the last received data. + * @param master the last master @a SymbolString. + * @param slave the last slave @a SymbolString. + */ + void setLastData(SymbolString& master, SymbolString& slave); + + /** + * Dump the last received data and message count to the output. + * @param unknown whether to dump only if this message is unknown. + * @param messages the @a MessageMap instance for resolving known @a Message instances. + * @param first whether this is the first message to be added to the output. + * @param output the @a ostringstream to format the messages to. + * @return whether the message was added to the output. + */ + bool dump(const bool unknown, MessageMap* messages, bool first, ostringstream& output); + +private: + + /** the last master @a SymbolString. */ + SymbolString m_lastMaster; + + /** the last slave @a SymbolString. */ + SymbolString m_lastSlave; + + /** the number of times this message was seen. */ + unsigned int m_count; + +}; + + /** * Handles input from and output to the bus with respect to the eBUS protocol. */ @@ -327,7 +373,8 @@ public: m_symPerSec(0), m_maxSymPerSec(0), m_state(bs_noSignal), m_repeat(false), m_command(false), m_commandCrcValid(false), m_response(false), m_responseCrcValid(false), - m_grabUnknownMessages(gr_unknown) { + m_grabMessages(true) + { memset(m_seenAddresses, 0, sizeof(m_seenAddresses)); } @@ -336,6 +383,17 @@ public: */ virtual ~BusHandler() { stop(); + BusRequest* req; + while ((req = m_finishedRequests.pop())!=NULL) { + delete req; + } + while ((req = m_nextRequests.pop())!=NULL) { + delete req; + } + if (m_currentRequest!=NULL) { + delete m_currentRequest; + m_currentRequest = NULL; + } } /** @@ -407,16 +465,16 @@ public: /** * Start or stop grabbing unknown messages. * @param enable true to enable grabbing, false to disable it. - * @param all true to grab all messages, false to grab unknown messages only (only relevant if @a enable is true). * @return true when the grabbing was changed. */ - bool enableGrab(bool enable=true, bool all=false); + bool enableGrab(bool enable=true); /** - * Format the grabbed unknown messages to the @a ostringstream. + * Format the grabbed messages to the @a ostringstream. + * @param unknown whether to dump only unknown messages. * @param output the @a ostringstream to format the messages to. */ - void formatGrabResult(ostringstream& output); + void formatGrabResult(const bool unknown, ostringstream& output); /** * Return true when a signal on the bus is available. @@ -593,13 +651,12 @@ private: /** the scan results by slave address. */ map m_scanResults; - /** whether to grab unknown messages. */ - GrabRequest m_grabUnknownMessages; + /** whether to grab messages. */ + bool m_grabMessages; - /** the grabbed unknown messages by ID prefix (QQZZPBSBNNDD with up to 4 DD bytes).*/ - map m_grabbedUnknownMessages; + /** the grabbed messages by key.*/ + map m_grabbedMessages; }; - #endif // BUSHANDLER_H_ diff --git a/src/ebusd/mainloop.cpp b/src/ebusd/mainloop.cpp index 50bcc0ec..3b68ecde 100644 --- a/src/ebusd/mainloop.cpp +++ b/src/ebusd/mainloop.cpp @@ -98,6 +98,10 @@ MainLoop::~MainLoop() delete m_device; m_device = NULL; } + NetMessage* msg; + while ((msg = m_netQueue.pop())!=NULL) { + delete msg; + } } void MainLoop::run() @@ -996,23 +1000,22 @@ string MainLoop::executeState(vector &args) string MainLoop::executeGrab(vector &args) { - bool all = args.size() == 2 && strcasecmp(args[1].c_str(), "ALL") == 0; - if (args.size() == 1 || all) - return m_busHandler->enableGrab(true, all) ? "grab started" : "grab continued"; - - if (args.size() == 2) { - if (strcasecmp(args[1].c_str(), "STOP") == 0) - return m_busHandler->enableGrab(false) ? "grab stopped" : "grab not running"; - - if (strcasecmp(args[1].c_str(), "RESULT") == 0) { + if (args.size() == 1) { + return m_busHandler->enableGrab(true) ? "grab started" : "grab continued"; + } + if (args.size() == 2 && strcasecmp(args[1].c_str(), "STOP") == 0) { + return m_busHandler->enableGrab(false) ? "grab stopped" : "grab not running"; + } + if (args.size() >= 2 && strcasecmp(args[1].c_str(), "RESULT") == 0) { + if (args.size() == 2 || strcasecmp(args[2].c_str(), "ALL") == 0) { ostringstream result; - m_busHandler->formatGrabResult(result); + m_busHandler->formatGrabResult(args.size() == 2, result); return result.str(); } } - return "usage: grab [all|stop]\n" - " or: grab result\n" - " Start or stop grabbing unknown or all messages, or report the grabbed messages."; + return "usage: grab [stop]\n" + " or: grab result [all]\n" + " Start or stop grabbing, or report unknown or all grabbed messages."; } string MainLoop::executeScan(vector &args) @@ -1183,8 +1186,8 @@ string MainLoop::executeHelp() " listen|l Listen for updates: listen [stop]\n" " state|s Report bus state\n" " info|i Report information about the daemon, the configuration, and seen devices.\n" - " grab|g Grab messages: grab [all|stop]\n" - " Report the messages: grab result\n" + " grab|g Grab messages: grab [stop]\n" + " Report the messages: grab result [all]\n" " scan Scan slaves: scan [full|ZZ]\n" " Report scan result: scan result\n" " log Set log area/level: log [AREA[,AREA]*] [LEVEL]\n"