Check serial number on device open, get correctly for PCAP devices

communication/communication.cpp

@@ -68,19 +68,20 @@ bool Communication::getSettingsSync(std::vector<uint8_t>& data, std::chrono::mil
 	return true;
 }
 
-bool Communication::getSerialNumberSync(std::string& serial, std::chrono::milliseconds timeout) {
+std::shared_ptr<SerialNumberMessage> Communication::getSerialNumberSync(std::chrono::milliseconds timeout) {
 	sendCommand(Command::RequestSerialNumber);
-	std::shared_ptr<Message> msg = waitForMessageSync(MessageFilter(Network::NetID::RED_OLDFORMAT), timeout);
-	if(!msg)
-		return false;
-
-	std::cout << "Got " << msg->data.size() << " bytes" << std::endl;
-	for(size_t i = 0; i < msg->data.size(); i++) {
-		std::cout << std::hex << (int)msg->data[i] << ' ' << std::dec;
-		if(i % 16 == 15)
-			std::cout << std::endl;
+	std::shared_ptr<Message> msg = waitForMessageSync(std::make_shared<Main51MessageFilter>(Command::RequestSerialNumber), timeout);
+	if(!msg) {
+		std::cout << "Didn't get a message in time" << std::endl;
+		return std::shared_ptr<SerialNumberMessage>();
 	}
-	return true;
+	auto m51 = std::dynamic_pointer_cast<Main51Message>(msg);
+	if(!m51) {
+		std::cout << "msg could not be cast to main51 " << msg->network << std::endl;
+		return std::shared_ptr<SerialNumberMessage>();
+	}
+	
+	return std::dynamic_pointer_cast<SerialNumberMessage>(m51);
 }
 
 int Communication::addMessageCallback(const MessageCallback& cb) {
@@ -97,7 +98,7 @@ bool Communication::removeMessageCallback(int id) {
 	}
 }
 
-std::shared_ptr<Message> Communication::waitForMessageSync(MessageFilter f, std::chrono::milliseconds timeout) {
+std::shared_ptr<Message> Communication::waitForMessageSync(std::shared_ptr<MessageFilter> f, std::chrono::milliseconds timeout) {
 	std::mutex m;
 	std::condition_variable cv;
 	std::shared_ptr<Message> returnedMessage;

communication/include/communication.h

@@ -33,13 +33,14 @@ public:
 	virtual bool sendCommand(Command cmd, bool boolean) { return sendCommand(cmd, std::vector<uint8_t>({ (uint8_t)boolean })); }
 	virtual bool sendCommand(Command cmd, std::vector<uint8_t> arguments = {});
 	bool getSettingsSync(std::vector<uint8_t>& data, std::chrono::milliseconds timeout = std::chrono::milliseconds(50));
-	bool getSerialNumberSync(std::string& serial, std::chrono::milliseconds timeout = std::chrono::milliseconds(50));
+	std::shared_ptr<SerialNumberMessage> getSerialNumberSync(std::chrono::milliseconds timeout = std::chrono::milliseconds(50));
 	
 	int addMessageCallback(const MessageCallback& cb);
 	bool removeMessageCallback(int id);
-	std::shared_ptr<Message> waitForMessageSync(MessageFilter f = MessageFilter(), std::chrono::milliseconds timeout = std::chrono::milliseconds(50));
-
-	void setAlign16Bit(bool enable) { align16bit = enable; }
+	std::shared_ptr<Message> waitForMessageSync(MessageFilter f = MessageFilter(), std::chrono::milliseconds timeout = std::chrono::milliseconds(50)) {
+		return waitForMessageSync(std::make_shared<MessageFilter>(f), timeout);
+	}
+	std::shared_ptr<Message> waitForMessageSync(std::shared_ptr<MessageFilter> f, std::chrono::milliseconds timeout = std::chrono::milliseconds(50));
 
 	std::shared_ptr<Packetizer> packetizer;
 	std::shared_ptr<MessageDecoder> decoder;
@@ -52,7 +53,6 @@ protected:
 
 private:
 	bool isOpen = false;
-	bool align16bit = true; // Not needed for Gigalog, Galaxy, etc and newer
 
 	std::thread readTaskThread;
 	void readTask();

communication/messagedecoder.cpp

@@ -1,5 +1,6 @@
 #include "communication/include/messagedecoder.h"
 #include "communication/include/communication.h"
+#include "communication/message/include/serialnumbermessage.h"
 #include "communication/include/command.h"
 #include "device/include/device.h"
 #include <iostream>
@@ -33,7 +34,42 @@ std::shared_ptr<Message> MessageDecoder::decodePacket(const std::shared_ptr<Pack
 			return msg;
 		}
 		default:
-			// TODO Implement others
+			switch(packet->network.getNetID()) {
+				case Network::NetID::Main51: {
+					switch((Command)packet->data[0]) {
+						case Command::RequestSerialNumber: {
+							auto msg = std::make_shared<SerialNumberMessage>();
+							msg->network = packet->network;
+							uint64_t serial = GetUInt64FromLEBytes(packet->data.data() + 1);
+							// The device sends 64-bits of serial number, but we never use more than 32-bits.
+							msg->deviceSerial = Device::SerialNumToString((uint32_t)serial);
+							msg->hasMacAddress = packet->data.size() >= 15;
+							if(msg->hasMacAddress)
+								memcpy(msg->macAddress, packet->data.data() + 9, sizeof(msg->macAddress));
+							msg->hasPCBSerial = packet->data.size() >= 31;
+							if(msg->hasPCBSerial)
+								memcpy(msg->pcbSerial, packet->data.data() + 15, sizeof(msg->pcbSerial));
+							return msg;
+						}
+					}
+					break;
+				}
+				case Network::NetID::RED_OLDFORMAT: {
+					/* So-called "old format" messages are a "new style, long format" wrapper around the old short messages.
+					 * They consist of a 16-bit LE length first, then the 8-bit length and netid combo byte, then the payload
+					 * with no checksum. The upper-nibble length of the combo byte should be ignored completely, using the
+					 * length from the first two bytes in it's place. Ideally, we never actually send the oldformat messages
+					 * out to the rest of the application as they can recursively get decoded to another message type here.
+					 * Feed the result back into the decoder in case we do something special with the resultant netid.
+					 */
+					uint16_t length = packet->data[0] | (packet->data[1] << 8);
+					packet->network = Network(packet->data[2] & 0xF);
+					packet->data.erase(packet->data.begin(), packet->data.begin() + 3);
+					if(packet->data.size() != length)
+						packet->data.resize(length);
+					return decodePacket(packet);
+				}
+			}
 			break;
 	}
 

communication/packetizer.cpp

@@ -54,7 +54,7 @@ bool Packetizer::input(const std::vector<uint8_t>& inputBytes) {
 					headerSize = 6;
 				} else {
 					state = ReadState::GetData;
-					checksum = true;
+					checksum = true; // Even if checksum is not explicitly disallowed, we enable it here, as this goes into length calculation
 					headerSize = 2;
 					packetLength += 2; // The packet length given in short packets does not include header
 				}
@@ -99,10 +99,10 @@ bool Packetizer::input(const std::vector<uint8_t>& inputBytes) {
 				while(currentIndex < packetLength)
 					packet.data[i++] = bytes[currentIndex++];
 
-				if(!checksum || bytes[currentIndex] == Communication::ICSChecksum(packet.data)) {
+				if(disableChecksum || !checksum || bytes[currentIndex] == ICSChecksum(packet.data)) {
 					// Got a good packet
 					gotGoodPackets = true;
-					processedPackets.push_back(std::make_shared<Communication::Packet>(packet));
+					processedPackets.push_back(std::make_shared<Packet>(packet));
 					for (auto a = 0; a < packetLength; a++)
 						bytes.pop_front();
 				} else {

device/device.cpp

@@ -123,10 +123,37 @@ bool Device::open() {
 	if(!com)
 		return false;
 
+	if(!com->open())
+		return false;
+
+	auto serial = com->getSerialNumberSync();
+	int i = 0;
+	while(!serial) {
+		serial = com->getSerialNumberSync();
+		if(i++ > 5)
+			break;
+	}
+	if(!serial) {
+		std::cout << "Failed to get serial number in " << i << " tries" << std::endl;
+		return false;
+	}
+	
+	if(i != 0)
+		std::cout << "Took " << i << " tries to get the serial number" << std::endl;
+	
+	std::string currentSerial = getNeoDevice().serial;
+	if(currentSerial == SERIAL_FIND_ON_OPEN) {
+		strncpy(getWritableNeoDevice().serial, serial->deviceSerial.c_str(), sizeof(getNeoDevice().serial));
+		getWritableNeoDevice().serial[sizeof(getWritableNeoDevice().serial) - 1] = '\0';
+	} else if(currentSerial != serial->deviceSerial) {
+		std::cout << "NeoDevice has serial " << getNeoDevice().serial << " but device has serial " << serial->deviceSerial.c_str() << "!" << std::endl;
+		return false;
+	}
+	
 	if(settings)
 		settings->refresh();
 
-	return com->open();
+	return true;
 }
 
 bool Device::close() {
@@ -141,7 +168,7 @@ bool Device::close() {
 bool Device::goOnline() {
 	if(!com->sendCommand(Command::EnableNetworkCommunication, true))
 		return false;
-	
+
 	online = true;
 	return true;
 }

device/include/device.h

@@ -15,6 +15,8 @@ namespace icsneo {
 
 class Device {
 public:
+	static constexpr char* SERIAL_FIND_ON_OPEN = "xxxxxx";
+
 	Device(neodevice_t neodevice = {}) {
 		data = neodevice;
 		data.device = this;

platform/windows/pcap.cpp

@@ -134,13 +134,15 @@ std::vector<neodevice_t> PCAP::FindByProduct(int product) {
 				if(product != packet.srcMAC[3]) // This is where the PID is stored in the MAC
 					continue; // This is not a product we're currently looking for
 
-				std::string serialFromMAC = GetEthDevSerialFromMacAddress(packet.srcMAC[3], ((packet.srcMAC[4] << 8) | packet.srcMAC[5]));
 				neodevice_t neodevice;
-				#pragma warning(push)
-				#pragma warning(disable:4996)
-				strncpy(neodevice.serial, serialFromMAC.c_str(), sizeof(neodevice.serial));
-				neodevice.serial[sizeof(neodevice.serial) - 1] = 0;
-				#pragma warning(pop)
+				/* Unlike other transport layers, we can't get the serial number here as we
+				 * actually need to open the device in the find method and then request it
+				 * over a working communication layer. We could technically create a communication
+				 * layer to parse the packet we have in `data` at this point, but we'd need to
+				 * know information about the device to correctly instantiate a packetizer and
+				 * decoder. I'm intentionally avoiding passing that information down here for
+				 * code quality's sake.
+				 */
 				neodevice.handle = (neodevice_handle_t)((i << 24) | (packet.srcMAC[3] << 16) | (packet.srcMAC[4] << 8) | (packet.srcMAC[5]));
 				bool alreadyExists = false;
 				for(auto& dev : foundDevices)