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

2016-11-20 16:10:23 +01:00
parent 54e3ad872b
commit e0f02d7d26
3 changed files with 140 additions and 56 deletions
@@ -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<unsigned>(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<string, string>::iterator it = m_grabbedUnknownMessages.begin(); it != m_grabbedUnknownMessages.end(); it++) {
-			if (first)
+		for (map<unsigned long long, GrabbedMessage>::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;
+			}
 		}
 	}
 }
@@ -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<unsigned char, string> 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<string, string> m_grabbedUnknownMessages;
+	/** the grabbed messages by key.*/
+	map<unsigned long long, GrabbedMessage> m_grabbedMessages;

 };

-
 #endif // BUSHANDLER_H_
@@ -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<string> &args)

 string MainLoop::executeGrab(vector<string> &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<string> &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"