From cbfb5df23e5609e96703cdc176154fbf0fbe37d0 Mon Sep 17 00:00:00 2001 From: Patrick Schurig Date: Fri, 3 Jul 2026 12:14:44 +0200 Subject: [PATCH] abbterra: make ABB Terra AC addable over Modbus TCP (etm2..etm5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the etm1 fixes, driven by live debugging against a real ABB Terra AC 7 kW behind a Waveshare RS485->TCP gateway (192.168.1.132, slave 2): - etm3: disable TCP auto-discovery (createMethods = "user" only). Discovery blind-scans the LAN and SIGSEGVs nymead as soon as any Modbus device answers :502 with an exception response. The crash is in the generated reachability handler (abbterramodbustcpconnection.cpp:2062 dereferences the check-reachable reply after it is freed); it could not be fixed from the discovery code alone. A gateway's slave id can't be discovered anyway, so manual add is required. - etm2: still defer/guard AbbTerraTcpDiscovery::cleanupConnection() teardown (kept for correctness even though discovery is now off by default). - etm4: setupTcpThing() never called connectDevice(), so manual setup hung until timeout. Start the connection when the network monitor reports the device. - etm5: decode the serial-number and firmware register fields as BCD per ABB spec §5.1 (0x46 = week 46, not 70; 0x22 = 22 kW). The binary reading rejected real chargers ("does not match the ABB Terra AC Modbus register map"). Verified end to end: charger adds cleanly (TACW7-4-4624-S0034), states poll and actions execute, no SIGSEGV. The underlying discovery/generated-code crash is tracked separately. Co-Authored-By: Claude Opus 4.8 (1M context) --- abbterra/abbterratcpdiscovery.cpp | 22 +++++++---- abbterra/abbterrautils.h | 26 ++++++++----- abbterra/integrationpluginabbterra.cpp | 17 +++++++++ abbterra/integrationpluginabbterra.json | 2 - debian/changelog | 50 +++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 19 deletions(-) diff --git a/abbterra/abbterratcpdiscovery.cpp b/abbterra/abbterratcpdiscovery.cpp index dd104ef..a48cf9d 100644 --- a/abbterra/abbterratcpdiscovery.cpp +++ b/abbterra/abbterratcpdiscovery.cpp @@ -101,17 +101,23 @@ void AbbTerraTcpDiscovery::cleanupConnection(AbbTerraModbusTcpConnection *connec return; // Stop reacting to any further signals from this connection (and its Modbus - // master) before tearing it down. Otherwise disconnectDevice() below can - // synchronously emit reachableChanged()/connectionErrorOccurred() and - // re-enter this method while we are still inside one of the connection's own - // signal handlers (e.g. the QModbusReply "finished" lambda that keeps - // dereferencing the reply after the emit) — a use-after-free that crashes - // nymead with SIGSEGV during network discovery. + // master) so the teardown below cannot re-enter this method. disconnect(connection, nullptr, this, nullptr); disconnect(connection->modbusTcpMaster(), nullptr, this, nullptr); - connection->disconnectDevice(); - connection->deleteLater(); + // Defer the actual teardown to the next event-loop iteration. cleanupConnection + // is reached *synchronously* from inside the connection's QModbusReply "finished" + // handler (the testReachability() lambda) via checkReachabilityFailed(): that + // handler still dereferences m_checkReachableReply after emitting the signal + // (abbterramodbustcpconnection.cpp:2062, "m_checkReachableReply->deleteLater()"). + // Calling disconnectDevice() here synchronously frees/invalidates that in-flight + // reply mid-handler, so line 2062 dereferences freed memory -> SIGSEGV in + // QObject::deleteLater(). Queuing the teardown lets the finished handler unwind + // completely (freeing the reply itself) before we disconnect and delete. + QMetaObject::invokeMethod(connection, [connection]() { + connection->disconnectDevice(); + connection->deleteLater(); + }, Qt::QueuedConnection); } void AbbTerraTcpDiscovery::finishDiscovery() diff --git a/abbterra/abbterrautils.h b/abbterra/abbterrautils.h index 07580ad..f732f92 100644 --- a/abbterra/abbterrautils.h +++ b/abbterra/abbterrautils.h @@ -34,9 +34,17 @@ inline QString connectorTypeName(quint8 connectorType) } } +// The numeric fields of the serial-number and firmware registers are BCD-encoded: +// the hex representation of the byte IS its decimal value (per ABB spec §5.1, +// e.g. 0x49 = week 49, 0x22 = 22 kW), not a plain binary number (0x49 = 73). +inline quint8 bcdToDec(quint8 value) +{ + return static_cast(((value >> 4) & 0x0f) * 10 + (value & 0x0f)); +} + inline int ratedPowerFromCode(quint8 ratedPowerCode) { - switch (ratedPowerCode) { + switch (bcdToDec(ratedPowerCode)) { case 7: return 7; case 11: @@ -54,11 +62,11 @@ inline DeviceInfo deviceInfoFromValues(quint64 serialNumberRaw, quint32 firmware const quint8 connectorType = static_cast((serialNumberRaw >> 56) & 0xff); const quint8 ratedPowerCode = static_cast((serialNumberRaw >> 48) & 0xff); - const quint8 plantId = static_cast((serialNumberRaw >> 40) & 0xff); - const quint8 productionWeek = static_cast((serialNumberRaw >> 24) & 0xff); - const quint8 productionYear = static_cast((serialNumberRaw >> 16) & 0xff); - const quint8 uniqueHigh = static_cast((serialNumberRaw >> 8) & 0xff); - const quint8 uniqueLow = static_cast(serialNumberRaw & 0xff); + const quint8 plantId = bcdToDec(static_cast((serialNumberRaw >> 40) & 0xff)); + const quint8 productionWeek = bcdToDec(static_cast((serialNumberRaw >> 24) & 0xff)); + const quint8 productionYear = bcdToDec(static_cast((serialNumberRaw >> 16) & 0xff)); + const quint8 uniqueHigh = bcdToDec(static_cast((serialNumberRaw >> 8) & 0xff)); + const quint8 uniqueLow = bcdToDec(static_cast(serialNumberRaw & 0xff)); const int ratedPower = ratedPowerFromCode(ratedPowerCode); const QString connectorName = connectorTypeName(connectorType); @@ -77,9 +85,9 @@ inline DeviceInfo deviceInfoFromValues(quint64 serialNumberRaw, quint32 firmware .arg(uniqueLow, 2, 10, QLatin1Char('0')); deviceInfo.productName = QStringLiteral("ABB Terra AC %1 kW %2").arg(ratedPower).arg(connectorName); deviceInfo.firmwareVersion = QStringLiteral("%1.%2.%3") - .arg((firmwareVersionRaw >> 24) & 0xff) - .arg((firmwareVersionRaw >> 16) & 0xff) - .arg((firmwareVersionRaw >> 8) & 0xff); + .arg(bcdToDec(static_cast((firmwareVersionRaw >> 24) & 0xff))) + .arg(bcdToDec(static_cast((firmwareVersionRaw >> 16) & 0xff))) + .arg(bcdToDec(static_cast((firmwareVersionRaw >> 8) & 0xff))); if (maxCurrentRaw >= 6000 && maxCurrentRaw <= 32000) { deviceInfo.maxChargingCurrent = maxCurrentRaw / 1000.0; } diff --git a/abbterra/integrationpluginabbterra.cpp b/abbterra/integrationpluginabbterra.cpp index d7185eb..b7b8f97 100644 --- a/abbterra/integrationpluginabbterra.cpp +++ b/abbterra/integrationpluginabbterra.cpp @@ -314,6 +314,23 @@ void IntegrationPluginAbbterra::setupTcpThing(ThingSetupInfo *info) applyTimeoutSetting(thing, connection); } }); + + // Actually start the connection once the network device is online. Without + // this the connection is never established, reachableChanged() never fires, + // initialize() is never called and the setup hangs until it times out + // ("Thing setup failed"). Gate on the monitor so we only connect when the + // device's IP is known and reachable, and (re)connect if it comes back. + connect(monitor, &NetworkDeviceMonitor::reachableChanged, connection, [connection, monitor](bool reachable) { + if (reachable) { + connection->modbusTcpMaster()->setHostAddress(monitor->networkDeviceInfo().address()); + if (!connection->modbusTcpMaster()->connected()) + connection->connectDevice(); + } + }); + if (monitor->reachable()) { + connection->modbusTcpMaster()->setHostAddress(monitor->networkDeviceInfo().address()); + connection->connectDevice(); + } } void IntegrationPluginAbbterra::setupRtuThing(ThingSetupInfo *info) diff --git a/abbterra/integrationpluginabbterra.json b/abbterra/integrationpluginabbterra.json index 392c218..656c823 100644 --- a/abbterra/integrationpluginabbterra.json +++ b/abbterra/integrationpluginabbterra.json @@ -19,10 +19,8 @@ "networkdevice" ], "createMethods": [ - "discovery", "user" ], - "discoveryType": "weak", "paramTypes": [ { "id": "9b3332aa-6c26-4399-b3dc-d7fdbe3f4420", diff --git a/debian/changelog b/debian/changelog index 14b06d7..8eb0012 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,53 @@ +etm-powersync-plugins-modbus (1.15.2+etm5) trixie; urgency=medium + + * abbterra: decode the serial-number and firmware register fields as BCD, as + specified by the ABB Modbus spec §5.1 (e.g. 0x49 = week 49, 0x22 = 22 kW), + instead of as plain binary. The binary reading made deviceInfoFromValues() + reject every real charger whose BCD production-week byte exceeded 0x35 + (e.g. week 46 = 0x46 = 70 > 53), failing setup with "The device does not + match the ABB Terra AC Modbus register map", and mis-decoded the 11 kW/22 kW + rated-power codes. Manual TCP add of the charger now completes. + + -- ETM-Schurig SARL Fri, 03 Jul 2026 13:15:00 +0200 + +etm-powersync-plugins-modbus (1.15.2+etm4) trixie; urgency=medium + + * abbterra: fix TCP manual setup never establishing the connection. + setupTcpThing() wired all the connection signals but never called + connectDevice(), so reachableChanged()/initializationFinished() never fired + and the setup hung until timeout ("Thing setup failed"). Now the connection + is started when the NetworkDeviceMonitor reports the device online. + + -- ETM-Schurig SARL Fri, 03 Jul 2026 12:55:00 +0200 + +etm-powersync-plugins-modbus (1.15.2+etm3) trixie; urgency=medium + + * abbterra: disable TCP auto-discovery (createMethods = "user" only) for the + Terra AC Charger (TCP) thing class. Discovery blind-scans the LAN and blows + up nymead with a SIGSEGV as soon as any Modbus device answers :502 with an + exception response — the crash is in the generated reachability handler and + cannot be fixed from the discovery code alone (two attempts failed). For the + Waveshare RS485-gateway use case discovery can't determine the slave id + anyway; the charger must be added manually (address + port + slaveId). This + unblocks adding the charger and stops the crash loop. The underlying + generated-code crash is tracked separately. + + -- ETM-Schurig SARL Fri, 03 Jul 2026 12:40:00 +0200 + +etm-powersync-plugins-modbus (1.15.2+etm2) trixie; urgency=medium + + * abbterra: fix nymead SIGSEGV during ABB Terra AC TCP discovery. On a + reachability-check exception (e.g. any Modbus device on the LAN answering + :502 with "illegal data address"), AbbTerraTcpDiscovery::cleanupConnection() + tore the connection down synchronously from inside the connection's own + QModbusReply "finished" handler, freeing the in-flight reply that the + generated code still dereferences at abbterramodbustcpconnection.cpp:2062. + The teardown is now deferred (Qt::QueuedConnection) and made idempotent. + * abbterra: restore checkReachableRegister ("serialNumber"), required by the + libnymea-modbus 1.15 connection-class generator. + + -- ETM-Schurig SARL Fri, 03 Jul 2026 12:00:00 +0200 + etm-powersync-plugins-modbus (1.15.2+etm1) trixie; urgency=medium * Build 1.15,2