From 03d8da3b7d454bb69615225ad0d790532033fe51 Mon Sep 17 00:00:00 2001 From: Scott Penrose Date: Mon, 29 Dec 2025 19:12:47 +1100 Subject: [PATCH] Cleaning up by using structs and reusing data blocks --- examples/MultiDevice/platformio.ini | 9 -- examples/MultiDevice/src/main.cpp | 18 --- experiment/src/main.cpp | 2 +- src/VictronBLE.cpp | 165 +++++++++++----------------- src/VictronBLE.h | 6 +- 5 files changed, 72 insertions(+), 128 deletions(-) diff --git a/examples/MultiDevice/platformio.ini b/examples/MultiDevice/platformio.ini index 5ca7b1b..0d345cd 100644 --- a/examples/MultiDevice/platformio.ini +++ b/examples/MultiDevice/platformio.ini @@ -119,11 +119,6 @@ build_flags = lib_deps = M5StickC elapsedMillis - TaskScheduler - Button2 - ArduinoJson - https://github.com/scottp/PsychicHttp.git - [env:tough] board = m5stack-core2 @@ -146,7 +141,3 @@ build_flags = lib_deps = M5Unified elapsedMillis - TaskScheduler - Button2 - ArduinoJson - https://github.com/scottp/PsychicHttp.git diff --git a/examples/MultiDevice/src/main.cpp b/examples/MultiDevice/src/main.cpp index 76b323a..8306fb2 100644 --- a/examples/MultiDevice/src/main.cpp +++ b/examples/MultiDevice/src/main.cpp @@ -208,24 +208,6 @@ void setup() { DEVICE_TYPE_SOLAR_CHARGER // Device type ); - - /* - * - [VictronBLE] Encrypted data: A0 01 83 2C 0E CF D6 04 89 72 6E 81 56 E4 2D F1 83 - [VictronBLE] IV: 02 58 00 00 00 00 00 00 00 00 00 00 00 00 00 00 - [VictronBLE] Decrypted data: E1 1C 99 32 D5 7E 81 A3 EB 8C 25 97 3E 0E DD 2D C4 - [VictronBLE] Unknown device type: 0x10 - [VictronBLE] BLE Device: 3ffd0148:3ffd014e, RSSI: -27 dBm - [VictronBLE] BLE Device: 3ffd0148:3ffd014e, RSSI: -81 dBm, Mfg ID: 0x2e1 (Victron) - [VictronBLE] Processing data from: Rainbow48Vc - [VictronBLE] Encrypted data: A0 01 83 2C 0E CF D6 04 89 72 6E 81 56 E4 2D F1 83 - [VictronBLE] IV: 02 58 00 00 00 00 00 00 00 00 00 00 00 00 00 00 - [VictronBLE] Decrypted data: E1 1C 99 32 D5 7E 81 A3 EB 8C 25 97 3E 0E DD 2D C4 - [VictronBLE] Unknown device type: 0x10 - [VictronBLE] BLE Device: 3ffd0148:3ffd014e, RSSI: -49 dBm, Mfg ID: 0x75 - [VictronBLE] BLE Device: 3ffd0148:3ffd014e, RSSI: -26 dBm - */ - // Example: Solar Charger #1 /* victron.addDevice( diff --git a/experiment/src/main.cpp b/experiment/src/main.cpp index 78f13f7..c2c3744 100644 --- a/experiment/src/main.cpp +++ b/experiment/src/main.cpp @@ -1,6 +1,6 @@ /* -TODO +Scott's original test code - this does work for MPPT chargers - use it as a base */ diff --git a/src/VictronBLE.cpp b/src/VictronBLE.cpp index 88ca72e..01879ac 100644 --- a/src/VictronBLE.cpp +++ b/src/VictronBLE.cpp @@ -132,100 +132,89 @@ void VictronBLE::loop() { // BLE callback implementation void VictronBLEAdvertisedDeviceCallbacks::onResult(BLEAdvertisedDevice advertisedDevice) { + // XXX why victronBLE and not just this? or nothing since inside same object - to do with callback? if (victronBLE) { - // Debug: Log all discovered BLE devices - if (victronBLE->debugEnabled) { - String mac = victronBLE->macAddressToString(advertisedDevice.getAddress()); - String debugMsg = "BLE Device: " + mac; - debugMsg += ", RSSI: " + String(advertisedDevice.getRSSI()) + " dBm"; - - if (advertisedDevice.haveName()) { - debugMsg += ", Name: " + String(advertisedDevice.getName().c_str()); - } - - if (advertisedDevice.haveManufacturerData()) { - std::string mfgData = advertisedDevice.getManufacturerData(); - if (mfgData.length() >= 2) { - uint16_t mfgId = (uint8_t)mfgData[1] << 8 | (uint8_t)mfgData[0]; - debugMsg += ", Mfg ID: 0x" + String(mfgId, HEX); - if (mfgId == VICTRON_MANUFACTURER_ID) { - debugMsg += " (Victron)"; - } - } - } - - victronBLE->debugPrint(debugMsg); - } - victronBLE->processDevice(advertisedDevice); } } // Process advertised device void VictronBLE::processDevice(BLEAdvertisedDevice advertisedDevice) { + // XXX Improve mac address handling into somewhere... String mac = macAddressToString(advertisedDevice.getAddress()); + // XXX normalize mac not working here, but is in experiment String normalizedMAC = normalizeMAC(mac); + // TODO: Consider skipping with no manufacturer data? + memset(&manufacturerData, 0, sizeof(manufacturerData)); + if (advertisedDevice.haveManufacturerData()) { + std::string mfgData = advertisedDevice.getManufacturerData(); + // XXX Storing it this way is not thread safe - is that issue on this ESP32? + mfgData.copy((char*)&manufacturerData, (mfgData.length() > sizeof(manufacturerData) ? sizeof(manufacturerData) : mfgData.length())); + // XXX Rather than copy, we can use pointers to .data + // Delete string? Or keep, or alternative buuffer handling + } + + // Pointer? XXX + + // Debug: Log all discovered BLE devices + if (debugEnabled) { + String debugMsg = ""; + + debugMsg += "BLE Device: " + mac; + debugMsg += ", RSSI: " + String(advertisedDevice.getRSSI()) + " dBm"; + if (advertisedDevice.haveName()) + debugMsg += ", Name: " + String(advertisedDevice.getName().c_str()); + + debugMsg += ", Mfg ID: 0x" + String(manufacturerData.vendorID, HEX); + if (manufacturerData.vendorID == VICTRON_MANUFACTURER_ID) { + debugMsg += " (Victron)"; + } + + debugPrint(debugMsg); + } + // Check if this is one of our configured devices auto it = devices.find(normalizedMAC); if (it == devices.end()) { // XXX Check if the device is a Victron device // This needs lots of improvemet and only do in debug - if (advertisedDevice.haveManufacturerData()) { - std::string mfgData = advertisedDevice.getManufacturerData(); - if (mfgData.length() >= 2) { - uint16_t mfgId = (uint8_t)mfgData[1] << 8 | (uint8_t)mfgData[0]; - if (mfgId == VICTRON_MANUFACTURER_ID) { - debugPrint("Found unmonitored Victron Device: " + normalizeMAC(mac)); - // DeviceInfo* deviceInfo = new DeviceInfo(mac, advertisedDevice.getName()); - // devices.insert({normalizedMAC, deviceInfo}); - // XXX What type of Victron device is it? - // Check if it's a Victron Energy device - /* - if (advertisedDevice.haveServiceData()) { - std::string serviceData = advertisedDevice.getServiceData(); - if (serviceData.length() >= 2) { - uint16_t serviceId = (uint8_t)serviceData[1] << 8 | (uint8_t)serviceData[0]; - if (serviceId == VICTRON_ENERGY_SERVICE_ID) { - debugPrint("Found Victron Energy Device: " + mac); - } - } + if (manufacturerData.vendorID == VICTRON_MANUFACTURER_ID) { + debugPrint("Found unmonitored Victron Device: " + normalizeMAC(mac)); + // DeviceInfo* deviceInfo = new DeviceInfo(mac, advertisedDevice.getName()); + // devices.insert({normalizedMAC, deviceInfo}); + // XXX What type of Victron device is it? + // Check if it's a Victron Energy device + /* + if (advertisedDevice.haveServiceData()) { + std::string serviceData = advertisedDevice.getServiceData(); + if (serviceData.length() >= 2) { + uint16_t serviceId = (uint8_t)serviceData[1] << 8 | (uint8_t)serviceData[0]; + if (serviceId == VICTRON_ENERGY_SERVICE_ID) { + debugPrint("Found Victron Energy Device: " + mac); } - */ - } } + */ } - return; // Not a device we're monitoring } DeviceInfo* deviceInfo = it->second; - // Check if device has manufacturer data - if (!advertisedDevice.haveManufacturerData()) { - return; - } - - std::string mfgData = advertisedDevice.getManufacturerData(); - if (mfgData.length() < 2) { - return; - } - debugPrint("Manufacturer Length = " + String(mfgData.length())); - // XXX Use struct like code in Sh3dNg // Check if it's Victron (manufacturer ID 0x02E1) - uint16_t mfgId = (uint8_t)mfgData[1] << 8 | (uint8_t)mfgData[0]; - if (mfgId != VICTRON_MANUFACTURER_ID) { + if (manufacturerData.vendorID != VICTRON_MANUFACTURER_ID) { + debugPrint("Skipping non VICTRON"); return; } debugPrint("Processing data from: " + deviceInfo->config.name); // Parse the advertisement - if (parseAdvertisement((const uint8_t*)mfgData.data(), mfgData.length(), normalizedMAC)) { + if (parseAdvertisement(normalizedMAC)) { // Update RSSI if (deviceInfo->data) { deviceInfo->data->rssi = advertisedDevice.getRSSI(); @@ -235,8 +224,8 @@ void VictronBLE::processDevice(BLEAdvertisedDevice advertisedDevice) { } // Parse advertisement data -bool VictronBLE::parseAdvertisement(const uint8_t* manufacturerData, size_t len, - const String& macAddress) { +bool VictronBLE::parseAdvertisement(const String& macAddress) { + // XXX We already searched above - try not to again? auto it = devices.find(macAddress); if (it == devices.end()) { debugPrint("parseAdvertisement: Device not found"); @@ -245,69 +234,48 @@ bool VictronBLE::parseAdvertisement(const uint8_t* manufacturerData, size_t len, DeviceInfo* deviceInfo = it->second; - // Verify minimum size for victronManufacturerData struct - if (len < sizeof(victronManufacturerData)) { - debugPrint("Manufacturer data too short: " + String(len) + " bytes, expected: " + String(sizeof(victronManufacturerData))); - return false; - } - - // Cast manufacturer data to struct for easy access - const victronManufacturerData* vicData = (const victronManufacturerData*)manufacturerData; - if (debugEnabled) { - debugPrint("Vendor ID: 0x" + String(vicData->vendorID, HEX)); - debugPrint("Beacon Type: 0x" + String(vicData->beaconType, HEX)); - debugPrint("Model ID: 0x" + String(vicData->modelID, HEX)); - debugPrint("Readout Type: 0x" + String(vicData->readoutType, HEX)); - debugPrint("Record Type: 0x" + String(vicData->victronRecordType, HEX)); - debugPrint("Nonce: 0x" + String(vicData->nonceDataCounter, HEX)); + debugPrint("Vendor ID: 0x" + String(manufacturerData.vendorID, HEX)); + debugPrint("Beacon Type: 0x" + String(manufacturerData.beaconType, HEX)); + debugPrint("Model ID: 0x" + String(manufacturerData.modelID, HEX)); + debugPrint("Readout Type: 0x" + String(manufacturerData.readoutType, HEX)); + debugPrint("Record Type: 0x" + String(manufacturerData.victronRecordType, HEX)); + debugPrint("Nonce: 0x" + String(manufacturerData.nonceDataCounter, HEX)); } // Get device type from record type field - uint8_t deviceType = vicData->victronRecordType; + uint8_t deviceType = manufacturerData.victronRecordType; // Build IV (initialization vector) from nonce // IV is 16 bytes: nonce (2 bytes little-endian) + zeros (14 bytes) uint8_t iv[16] = {0}; - iv[0] = vicData->nonceDataCounter & 0xFF; // Low byte - iv[1] = (vicData->nonceDataCounter >> 8) & 0xFF; // High byte + iv[0] = manufacturerData.nonceDataCounter & 0xFF; // Low byte + iv[1] = (manufacturerData.nonceDataCounter >> 8) & 0xFF; // High byte // Remaining bytes stay zero - // Get pointer to encrypted data - const uint8_t* encryptedData = vicData->victronEncryptedData; - size_t encryptedLen = sizeof(vicData->victronEncryptedData); - - if (debugEnabled) { - debugPrintHex("IV", iv, 16); - debugPrintHex("Encrypted data", encryptedData, encryptedLen); - } - // Decrypt the data uint8_t decrypted[32]; // Max expected size - if (!decryptAdvertisement(encryptedData, encryptedLen, + if (!decryptAdvertisement(manufacturerData.victronEncryptedData, + sizeof(manufacturerData.victronEncryptedData), deviceInfo->encryptionKeyBytes, iv, decrypted)) { lastError = "Decryption failed"; return false; } - if (debugEnabled) { - debugPrintHex("Decrypted data", decrypted, encryptedLen); - } - // Parse based on device type bool parseOk = false; switch (deviceType) { case DEVICE_TYPE_SOLAR_CHARGER: if (deviceInfo->data && deviceInfo->data->deviceType == DEVICE_TYPE_SOLAR_CHARGER) { - parseOk = parseSolarCharger(decrypted, encryptedLen, + parseOk = parseSolarCharger(decrypted, sizeof(decrypted), *(SolarChargerData*)deviceInfo->data); } break; case DEVICE_TYPE_BATTERY_MONITOR: if (deviceInfo->data && deviceInfo->data->deviceType == DEVICE_TYPE_BATTERY_MONITOR) { - parseOk = parseBatteryMonitor(decrypted, encryptedLen, + parseOk = parseBatteryMonitor(decrypted, sizeof(decrypted), *(BatteryMonitorData*)deviceInfo->data); } break; @@ -317,14 +285,14 @@ bool VictronBLE::parseAdvertisement(const uint8_t* manufacturerData, size_t len, case DEVICE_TYPE_MULTI_RS: case DEVICE_TYPE_VE_BUS: if (deviceInfo->data && deviceInfo->data->deviceType == DEVICE_TYPE_INVERTER) { - parseOk = parseInverter(decrypted, encryptedLen, + parseOk = parseInverter(decrypted, sizeof(decrypted), *(InverterData*)deviceInfo->data); } break; case DEVICE_TYPE_DCDC_CONVERTER: if (deviceInfo->data && deviceInfo->data->deviceType == DEVICE_TYPE_DCDC_CONVERTER) { - parseOk = parseDCDCConverter(decrypted, encryptedLen, + parseOk = parseDCDCConverter(decrypted, sizeof(decrypted), *(DCDCConverterData*)deviceInfo->data); } break; @@ -363,6 +331,7 @@ bool VictronBLE::parseAdvertisement(const uint8_t* manufacturerData, size_t len, } // Decrypt advertisement using AES-128-CTR +// XX Compare mbedlts_aes vs esp_aes bool VictronBLE::decryptAdvertisement(const uint8_t* encrypted, size_t encLen, const uint8_t* key, const uint8_t* iv, uint8_t* decrypted) { diff --git a/src/VictronBLE.h b/src/VictronBLE.h index d3d4cef..40b106e 100644 --- a/src/VictronBLE.h +++ b/src/VictronBLE.h @@ -296,13 +296,15 @@ private: uint32_t scanDuration; bool initialized; + // XXX Experiment with actual victron data + victronManufacturerData manufacturerData; + // Internal methods bool hexStringToBytes(const String& hex, uint8_t* bytes, size_t len); bool decryptAdvertisement(const uint8_t* encrypted, size_t encLen, const uint8_t* key, const uint8_t* iv, uint8_t* decrypted); - bool parseAdvertisement(const uint8_t* manufacturerData, size_t len, - const String& macAddress); + bool parseAdvertisement(const String& macAddress); void processDevice(BLEAdvertisedDevice advertisedDevice); VictronDeviceData* createDeviceData(VictronDeviceType type);