Add kostal model filtering and handle interrupted discovery

libnymea-sunspec/sunspecconnection.cpp

@@ -374,8 +374,8 @@ void SunSpecConnection::scanNextSunspecBaseRegister()
 
 void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset)
 {
-    qCDebug(dcSunSpec()) << "Reading SunSpec models header" << this << "using SunSpec base register" << m_baseRegister << "offset:" << offset;
     quint16 startRegisterAddress = m_baseRegister + offset;
+    qCDebug(dcSunSpec()) << "Reading SunSpec model header" << this << "using SunSpec base register" << m_baseRegister << "offset:" << offset << "=" << startRegisterAddress;
     QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::HoldingRegisters, startRegisterAddress, 2);
     QModbusReply *reply = m_modbusTcpClient->sendReadRequest(request, m_slaveId);
 
@@ -391,7 +391,7 @@ void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset)
     }
 
     connect(reply, &QModbusReply::finished, reply, &QModbusReply::deleteLater);
-    connect(reply, &QModbusReply::finished, this, [this, reply, offset] {
+    connect(reply, &QModbusReply::finished, this, [this, reply, offset, startRegisterAddress] {
         if (reply->error() == QModbusDevice::NoError) {
 
             const QModbusDataUnit unit = reply->result();
@@ -406,7 +406,7 @@ void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset)
                 return;
             }
 
-            qCDebug(dcSunSpec()) << "Discovered SunSpec model on" << this << modelId << "with length" << modelLength;
+            qCDebug(dcSunSpec()) << "Discovered SunSpec model on" << this << "[" << startRegisterAddress + 2 << "-" << startRegisterAddress + 2 + modelLength << "]" << "(base: " << m_baseRegister << "offset:" << offset << "length:" << modelLength << ") | Model ID:" << modelId << static_cast<SunSpecModelFactory::ModelId>(modelId);
             ModuleDiscoveryResult result;
             result.modbusStartRegister = modbusStartRegister;
             result.modelId = modelId;
@@ -417,9 +417,14 @@ void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset)
             scanModelsOnBaseRegister(offset + 2 + modelLength);
         } else {
             qCWarning(dcSunSpec()) << "Error occured while reading model header from" << this << "using offset" << offset << m_modbusTcpClient->errorString();
-            // FIXME: check if models have already been found, finish with success in that case so we show at least the models found so far...
+            if (!m_modelDiscoveryResult.isEmpty()) {
-            setDiscoveryRunning(false);
+                qCWarning(dcSunSpec()) << "Error occured but already discovered" << m_modelDiscoveryResult.count() << "models. Continue with the discovered models, but the discovery may be incomplete due to header reading errors.";
-            emit discoveryFinished(false);
+                qCDebug(dcSunSpec()) << "Scan for SunSpec models on" << this << m_baseRegister << "finished successfully";
+                processDiscoveryResult();
+            } else {
+                setDiscoveryRunning(false);
+                emit discoveryFinished(false);
+            }
         }
     });
 }

sunspec/README.md

@@ -15,6 +15,12 @@ Connect nymea to SunSpec devices over Modbus TCP
 * SunSpec Storage 
 	* Model ID 124
 
+
+## Tested connections
+
+* SolarEdge (with custom battery)
+* Kostal [Documentation](
+
 ## Requirements
 
 * The package 'nymea-plugin-sunspec' must be installed.

sunspec/integrationpluginsunspec.cpp

@@ -167,9 +167,8 @@ void IntegrationPluginSunSpec::discoverThings(ThingDiscoveryInfo *info)
                     title += networkDeviceInfo.address().toString() + " (" + networkDeviceInfo.hostName() + ")";
                 }
             } else {
-                // Kostal does not provide usefull information for filterin in the discovery
-
                 // Generic or not discoverable sunspec connection, show all network results
+                // - Kostal does not provide usefull information for filterin in the discovery
                 if (networkDeviceInfo.hostName().isEmpty()) {
                     title += networkDeviceInfo.address().toString();
                 } else {
@@ -210,7 +209,8 @@ void IntegrationPluginSunSpec::discoverThings(ThingDiscoveryInfo *info)
 void IntegrationPluginSunSpec::setupThing(ThingSetupInfo *info)
 {
     Thing *thing = info->thing();
-    qCDebug(dcSunSpec()) << "Setup thing" << thing->name();
+    qCDebug(dcSunSpec()) << "Setup thing" << thing;
+    qCDebug(dcSunSpec()) << thing->params();
 
     if (thing->thingClassId() == sunspecConnectionThingClassId || thing->thingClassId() == solarEdgeConnectionThingClassId  || thing->thingClassId() == kostalConnectionThingClassId) {
         setupConnection(info);
@@ -359,48 +359,90 @@ void IntegrationPluginSunSpec::processDiscoveryResult(Thing *thing, SunSpecConne
 {
     qCDebug(dcSunSpec()) << "Processing discovery result from" << thing->name() << connection;
 
-    // Now process the other models and check if we can create any auto device if not already added
+    // Note: from kostal devices is known, that they add inverter
-    foreach (SunSpecModel *model, connection->models()) {
+    // as normal and float version, but we need only one.
+    // Lets filter the duplicated information for kostal connections
+    if (thing->thingClassId() == kostalConnectionThingClassId) {
+        QHash<quint16, SunSpecModel *> filteredModels;
+        foreach (SunSpecModel *model, connection->models()) {
+            switch (model->modelId()) {
+            case SunSpecModelFactory::ModelIdInverterSinglePhaseFloat:
+                if (filteredModels.contains(SunSpecModelFactory::ModelIdInverterSinglePhase)) {
+                    qCDebug(dcSunSpec()) << "Kostal: Filter out" << model;
+                } else {
+                    filteredModels.insert(model->modelId(), model);
+                }
+                break;
+            case SunSpecModelFactory::ModelIdInverterSplitPhaseFloat:
+                if (filteredModels.contains(SunSpecModelFactory::ModelIdInverterSplitPhase)) {
+                    qCDebug(dcSunSpec()) << "Kostal: Filter out" << model;
+                } else {
+                    filteredModels.insert(model->modelId(), model);
+                }
+                break;
+            case SunSpecModelFactory::ModelIdInverterThreePhaseFloat:
+                if (filteredModels.contains(SunSpecModelFactory::ModelIdInverterThreePhase)) {
+                    qCDebug(dcSunSpec()) << "Kostal: Filter out" << model;
+                } else {
+                    filteredModels.insert(model->modelId(), model);
+                }
+                break;
+            default:
+                filteredModels.insert(model->modelId(), model);
+                break;
+            }
+        }
+
+        // Process the filtered list
+        checkAutoSetupModels(thing, filteredModels.values());
+    } else {
+        // Process all models
+        checkAutoSetupModels(thing, connection->models());
+    }
+}
+
+void IntegrationPluginSunSpec::checkAutoSetupModels(Thing *connectionThing, QList<SunSpecModel *> models)
+{
+    // Process the models and check if we can create any auto device if not already added
+    foreach (SunSpecModel *model, models) {
         // Make sure we have not added this model yet
-        if (sunspecThingAlreadyAdded(model->modelId(), model->modbusStartRegister(), thing->id())) {
+        if (sunspecThingAlreadyAdded(model->modelId(), model->modbusStartRegister(), connectionThing->id())) {
             qCDebug(dcSunSpec()) << "Thing already set up for" << model;
             continue;
         }
 
-        // TODO: Make sure to not add duplicated models like inverter + inverter float..
-
         switch (model->modelId()) {
         case SunSpecModelFactory::ModelIdCommon:
             // Skip the common model, we already handled this one for each thing model
             break;
         case SunSpecModelFactory::ModelIdInverterSinglePhase:
         case SunSpecModelFactory::ModelIdInverterSinglePhaseFloat:
-            autocreateSunSpecModelThing(sunspecSinglePhaseInverterThingClassId, QT_TR_NOOP("Single Phase Inverter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecSinglePhaseInverterThingClassId, QT_TR_NOOP("Single Phase Inverter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdInverterSplitPhase:
         case SunSpecModelFactory::ModelIdInverterSplitPhaseFloat:
-            autocreateSunSpecModelThing(sunspecSplitPhaseInverterThingClassId, QT_TR_NOOP("Split Phase Inverter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecSplitPhaseInverterThingClassId, QT_TR_NOOP("Split Phase Inverter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdInverterThreePhase:
         case SunSpecModelFactory::ModelIdInverterThreePhaseFloat:
-            autocreateSunSpecModelThing(sunspecThreePhaseInverterThingClassId, QT_TR_NOOP("Three Phase Inverter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecThreePhaseInverterThingClassId, QT_TR_NOOP("Three Phase Inverter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdMeterSinglePhase:
         case SunSpecModelFactory::ModelIdMeterSinglePhaseFloat:
-            autocreateSunSpecModelThing(sunspecSinglePhaseMeterThingClassId, QT_TR_NOOP("Single Phase Meter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecSinglePhaseMeterThingClassId, QT_TR_NOOP("Single Phase Meter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdMeterSplitSinglePhaseAbn:
         case SunSpecModelFactory::ModelIdMeterSplitSinglePhaseFloat:
-            autocreateSunSpecModelThing(sunspecSplitPhaseMeterThingClassId, QT_TR_NOOP("Split Phase Meter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecSplitPhaseMeterThingClassId, QT_TR_NOOP("Split Phase Meter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdMeterThreePhase:
         case SunSpecModelFactory::ModelIdDeltaConnectThreePhaseAbcMeter:
         case SunSpecModelFactory::ModelIdMeterThreePhaseWyeConnect:
         case SunSpecModelFactory::ModelIdMeterThreePhaseDeltaConnect:
-            autocreateSunSpecModelThing(sunspecThreePhaseMeterThingClassId, QT_TR_NOOP("Three Phase Meter"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecThreePhaseMeterThingClassId, QT_TR_NOOP("Three Phase Meter"), connectionThing->id(), model);
             break;
         case SunSpecModelFactory::ModelIdStorage:
-            autocreateSunSpecModelThing(sunspecStorageThingClassId, QT_TR_NOOP("Storage"), thing->id(), model);
+            autocreateSunSpecModelThing(sunspecStorageThingClassId, QT_TR_NOOP("Storage"), connectionThing->id(), model);
             break;
         default:
             qCWarning(dcSunSpec()) << "Plugin has no implementation for detected" << model;

sunspec/integrationpluginsunspec.h

@@ -83,6 +83,7 @@ private:
 
     bool sunspecThingAlreadyAdded(uint modelId, uint modbusAddress, const ThingId &parentId);
     void processDiscoveryResult(Thing *thing, SunSpecConnection *connection);
+    void checkAutoSetupModels(Thing *connectionThing, QList<SunSpecModel *> models);
 
     // SunSpec things
     void setupConnection(ThingSetupInfo *info);

sunspec/integrationpluginsunspec.json

@@ -1495,14 +1495,14 @@
                             "name":"port",
                             "displayName": "Port",
                             "type": "int",
-                            "defaultValue": 502
+                            "defaultValue": 1502
                         },
                         {
                             "id": "48b133da-cce3-47f3-9c7c-470026af7829",
                             "name":"slaveId",
                             "displayName": "Slave ID",
                             "type": "int",
-                            "defaultValue": 1
+                            "defaultValue": 71
                         }
                     ],
                     "stateTypes":[