polish SMA HomeManager integration

* remove/comment unused variables to avoid compiler warnings

* cleanups: fix indention and style, make variable private, implement
  getters in header and make const.

* optimize message output: respect verbose logging setting. prefix
  output with SMA_HM.

* use newly introduced mutex in PowerMeterClass also for SMA HomeManager

* refactor code for readibility, unindent where possible.

include/SMA_HM.h

@@ -9,21 +9,28 @@
 
 class SMA_HMClass {
 public:
-    void init(Scheduler& scheduler);
+    void init(Scheduler& scheduler, bool verboseLogging);
     void loop();
     void event1();
-    float getPowerTotal();
+    float getPowerTotal() const { return _powerMeterPower; }
-    float getPowerL1();
+    float getPowerL1() const { return _powerMeterL1; }
-    float getPowerL2();
+    float getPowerL2() const { return _powerMeterL2; }
-    float getPowerL3();
+    float getPowerL3() const { return _powerMeterL3; }
-    uint32_t serial = 0;
+
 private:
-    uint32_t _lastUpdate = 0;
+    void Soutput(int kanal, int index, int art, int tarif,
+            char const* name, float value, uint32_t timestamp);
+
+    uint8_t* decodeGroup(uint8_t* offset, uint16_t grouplen);
+
+    bool _verboseLogging = false;
     float _powerMeterPower = 0.0;
     float _powerMeterL1 = 0.0;
     float _powerMeterL2 = 0.0;
     float _powerMeterL3 = 0.0;
-    uint32_t previousMillis = 0;
+    uint32_t _previousMillis = 0;
+    uint32_t _serial = 0;
     Task _loopTask;
 };
+
 extern SMA_HMClass SMA_HM;

src/PowerMeter.cpp

@@ -75,7 +75,7 @@ void PowerMeterClass::init(Scheduler& scheduler)
         break;
 
     case SOURCE_SMAHM2:
-        SMA_HM.init(scheduler);
+        SMA_HM.init(scheduler, config.PowerMeter.VerboseLogging);
         break;
     }
 }
@@ -229,6 +229,7 @@ void PowerMeterClass::readPowerMeter()
         }
     }
     else if (config.PowerMeter.Source == SOURCE_SMAHM2) {
+        std::lock_guard<std::mutex> l(_mutex);
         _powerMeter1Power = SMA_HM.getPowerL1();
         _powerMeter2Power = SMA_HM.getPowerL2();
         _powerMeter3Power = SMA_HM.getPowerL3();

src/SMA_HM.cpp

@@ -13,18 +13,22 @@ unsigned int multicastPort = 9522;  // local port to listen on
 IPAddress multicastIP(239, 12, 255, 254);
 WiFiUDP SMAUdp;
 
-const uint32_t interval = 1000;
+constexpr uint32_t interval = 1000;
-
-static void Soutput(int kanal, int index, int art, int tarif, String Bezeichnung, double value, int timestamp){
-  MessageOutput.print(Bezeichnung);
-  MessageOutput.print('=');
-  MessageOutput.println(value);
-}
 
 SMA_HMClass SMA_HM;
 
-void SMA_HMClass::init(Scheduler& scheduler)
+void SMA_HMClass::Soutput(int kanal, int index, int art, int tarif,
+        char const* name, float value, uint32_t timestamp)
 {
+    if (!_verboseLogging) { return; }
+
+    MessageOutput.printf("SMA_HM: %s = %.1f (timestamp %d)\r\n",
+            name, value, timestamp);
+}
+
+void SMA_HMClass::init(Scheduler& scheduler, bool verboseLogging)
+{
+    _verboseLogging = verboseLogging;
     scheduler.addTask(_loopTask);
     _loopTask.setCallback(std::bind(&SMA_HMClass::loop, this));
     _loopTask.setIterations(TASK_FOREVER);
@@ -35,158 +39,165 @@ void SMA_HMClass::init(Scheduler& scheduler)
 
 void SMA_HMClass::loop()
 {
-  uint32_t currentMillis = millis();
+    uint32_t currentMillis = millis();
-  if (currentMillis - previousMillis >= interval) {
+    if (currentMillis - _previousMillis >= interval) {
-    previousMillis = currentMillis;
+        _previousMillis = currentMillis;
-      event1();
+        event1();
-  }
+    }
+}
+
+uint8_t* SMA_HMClass::decodeGroup(uint8_t* offset, uint16_t grouplen)
+{
+    float Pbezug = 0;
+    float BezugL1 = 0;
+    float BezugL2 = 0;
+    float BezugL3 = 0;
+    float Peinspeisung = 0;
+    float EinspeisungL1 = 0;
+    float EinspeisungL2 = 0;
+    float EinspeisungL3 = 0;
+
+    uint8_t* endOfGroup = offset + grouplen;
+
+    // not used: uint16_t protocolID = (offset[0] << 8) + offset[1];
+    offset += 2;
+
+    // not used: uint16_t susyID = (offset[0] << 8) + offset[1];
+    offset += 2;
+
+    _serial = (offset[0] << 24) + (offset[1] << 16) + (offset[2] << 8) + offset[3];
+    offset += 4;
+
+    uint32_t timestamp = (offset[0] << 24) + (offset[1] << 16) + (offset[2] << 8) + offset[3];
+    offset += 4;
+
+    unsigned count = 0;
+    while (offset < endOfGroup) {
+        uint8_t kanal = offset[0];
+        uint8_t index = offset[1];
+        uint8_t art = offset[2];
+        uint8_t tarif = offset[3];
+        offset += 4;
+
+        if (kanal == 144) {
+            // Optional: Versionsnummer auslesen... aber interessiert die?
+            offset += 4;
+            continue;
+        }
+
+        if (art == 8) {
+            offset += 8;
+            continue;
+        }
+
+        if (art == 4) {
+            uint32_t data = (offset[0] << 24) +
+                (offset[1] << 16) +
+                (offset[2] << 8) +
+                offset[3];
+            offset += 4;
+
+            switch (index) {
+                case (1):
+                    Pbezug = data * 0.1;
+                    ++count;
+                    break;
+                case (2):
+                    Peinspeisung = data * 0.1;
+                    ++count;
+                    break;
+                case (21):
+                    BezugL1 = data * 0.1;
+                    ++count;
+                    break;
+                case (22):
+                    EinspeisungL1 = data * 0.1;
+                    ++count;
+                    break;
+                case (41):
+                    BezugL2 = data * 0.1;
+                    ++count;
+                    break;
+                case (42):
+                    EinspeisungL2 = data * 0.1;
+                    ++count;
+                    break;
+                case (61):
+                    BezugL3 = data * 0.1;
+                    ++count;
+                    break;
+                case (62):
+                    EinspeisungL3 = data * 0.1;
+                    ++count;
+                    break;
+                default:
+                    break;
+            }
+
+            if (count == 8) {
+                _powerMeterPower = Peinspeisung - Pbezug;
+                _powerMeterL1 = EinspeisungL1 - BezugL1;
+                _powerMeterL2 = EinspeisungL2 - BezugL2;
+                _powerMeterL3 = EinspeisungL3 - BezugL3;
+                Soutput(kanal, index, art, tarif, "Leistung", _powerMeterPower, timestamp);
+                Soutput(kanal, index, art, tarif, "Leistung L1", _powerMeterL1, timestamp);
+                Soutput(kanal, index, art, tarif, "Leistung L2", _powerMeterL2, timestamp);
+                Soutput(kanal, index, art, tarif, "Leistung L3", _powerMeterL3, timestamp);
+                count = 0;
+            }
+
+            continue;
+        }
+
+        MessageOutput.printf("SMA_HM: Skipped unknown measurement: %d %d %d %d\r\n",
+                kanal, index, art, tarif);
+        offset += art;
+    }
+
+    return offset;
 }
 
 void SMA_HMClass::event1()
 {
-    uint8_t buffer[1024];
     int packetSize = SMAUdp.parsePacket();
-    float Pbezug,BezugL1,BezugL2,BezugL3;
+    if (!packetSize) { return; }
-    Pbezug = 0;
+
-    BezugL1 = 0;
+    uint8_t buffer[1024];
-    BezugL2 = 0;
+    int rSize = SMAUdp.read(buffer, 1024);
-    BezugL3 = 0;
+    if (buffer[0] != 'S' || buffer[1] != 'M' || buffer[2] != 'A') {
-    float Peinspeisung,EinspeisungL1,EinspeisungL2,EinspeisungL3;
+        MessageOutput.println("SMA_HM: Not an SMA packet?");
-    Peinspeisung = 0;
+        return;
-    EinspeisungL1 = 0;
-    EinspeisungL2 = 0;
-    EinspeisungL3 = 0;
-    int count =0;
-    if (packetSize) {
-        int rSize = SMAUdp.read(buffer, 1024);
-        if (buffer[0] != 'S' || buffer[1] != 'M' || buffer[2] != 'A') {
-            MessageOutput.println("Not an SMA packet?");
-            return;
-        }
-        uint16_t grouplen;
-        uint16_t grouptag;
-        uint8_t* offset = buffer + 4;
-        do {
-            grouplen = (offset[0] << 8) + offset[1];
-            grouptag = (offset[2] << 8) + offset[3];
-            offset += 4;
-            if (grouplen == 0xffff) return;
-            if (grouptag == 0x02A0 && grouplen == 4) {
-                offset += 4;
-            } else if (grouptag == 0x0010) {
-                uint8_t* endOfGroup = offset + grouplen;
-                uint16_t protocolID = (offset[0] << 8) + offset[1];
-                offset += 2;
-                uint16_t susyID = (offset[0] << 8) + offset[1];
-                offset += 2;
-                uint32_t serial = (offset[0] << 24) + (offset[1] << 16) + (offset[2] << 8) + offset[3];
-                SMA_HM.serial=serial;
-                offset += 4;
-                uint32_t timestamp = (offset[0] << 24) + (offset[1] << 16) + (offset[2] << 8) + offset[3];
-                offset += 4;
-                while (offset < endOfGroup) {
-                    uint8_t kanal = offset[0];
-                    uint8_t index = offset[1];
-                    uint8_t art = offset[2];
-                    uint8_t tarif = offset[3];
-                    offset += 4;
-                    if (art == 8) {
-                      uint64_t data = ((uint64_t)offset[0] << 56) +
-                                      ((uint64_t)offset[1] << 48) +
-                                      ((uint64_t)offset[2] << 40) +
-                                      ((uint64_t)offset[3] << 32) +
-                                      ((uint64_t)offset[4] << 24) +
-                                      ((uint64_t)offset[5] << 16) +
-                                      ((uint64_t)offset[6] << 8) +
-                                      offset[7];
-                        offset += 8;
-                    } else if (art == 4) {
-                      uint32_t data = (offset[0] << 24) +
-                      (offset[1] << 16) +
-                      (offset[2] << 8) +
-                      offset[3];
-                      offset += 4;
-                      switch (index) {
-                      case (1):
-                        Pbezug = data * 0.1;
-                        count +=1;
-                        break;
-                      case (2):
-                        Peinspeisung = data * 0.1;
-                        count +=1;
-                        break;
-                      case (21):
-                        BezugL1 = data * 0.1;
-                        count +=1;
-                        break;
-                      case (22):
-                        EinspeisungL1 = data * 0.1;
-                        count +=1;
-                        break;
-                      case (41):
-                        BezugL2 = data * 0.1;
-                        count +=1;
-                        break;
-                      case (42):
-                        EinspeisungL2 = data * 0.1;
-                        count +=1;
-                        break;
-                      case (61):
-                        BezugL3 = data * 0.1;
-                        count +=1;
-                        break;
-                      case (62):
-                        count +=1;
-                        EinspeisungL3 = data * 0.1;
-                        break;
-                      default:
-                        break; // Wird nicht benötigt, wenn Statement(s) vorhanden sind
-                      }
-                      if (count == 8){
-                        _powerMeterPower = Peinspeisung - Pbezug;
-                        _powerMeterL1=EinspeisungL1-BezugL1;
-                        _powerMeterL2=EinspeisungL2-BezugL2;
-                        _powerMeterL3=EinspeisungL3-BezugL3;
-                        Soutput(kanal, index, art, tarif, "Leistung", _powerMeterPower, timestamp);
-                        Soutput(kanal, index, art, tarif, "Leistung L1", _powerMeterL1, timestamp);
-                        Soutput(kanal, index, art, tarif, "Leistung L2", _powerMeterL2, timestamp);
-                        Soutput(kanal, index, art, tarif, "Leistung L3", _powerMeterL3, timestamp);
-                        count=0;
-                      }
-                    } else if (kanal==144) {
-                      // Optional: Versionsnummer auslesen... aber interessiert die?
-                      offset += 4;
-                    } else {
-                        offset += art;
-                        MessageOutput.println("Strange measurement skipped");
-                    }
-                }
-            } else if (grouptag == 0) {
-                // end marker
-                offset += grouplen;
-            } else {
-                MessageOutput.print("unhandled group ");
-                MessageOutput.print(grouptag);
-                MessageOutput.print(" with len=");
-                MessageOutput.println(grouplen);
-                offset += grouplen;
-            }
-        } while (grouplen > 0 && offset + 4 < buffer + rSize);
     }
-}
+
-float SMA_HMClass::getPowerTotal()
+    uint16_t grouplen;
-{
+    uint16_t grouptag;
-    return _powerMeterPower;
+    uint8_t* offset = buffer + 4; // skips the header 'SMA\0'
-}
+
-float SMA_HMClass::getPowerL1()
+    do {
-{
+        grouplen = (offset[0] << 8) + offset[1];
-    return _powerMeterL1;
+        grouptag = (offset[2] << 8) + offset[3];
-}
+        offset += 4;
-float SMA_HMClass::getPowerL2()
+
-{
+        if (grouplen == 0xffff) return;
-    return _powerMeterL2;
+
-}
+        if (grouptag == 0x02A0 && grouplen == 4) {
-float SMA_HMClass::getPowerL3()
+            offset += 4;
-{
+            continue;
-    return _powerMeterL3;
+        }
+
+        if (grouptag == 0x0010) {
+            offset = decodeGroup(offset, grouplen);
+            continue;
+        }
+
+        if (grouptag == 0) {
+            // end marker
+            offset += grouplen;
+            continue;
+        }
+
+        MessageOutput.printf("SMA_HM: Unhandled group 0x%04x with length %d\r\n",
+                grouptag, grouplen);
+        offset += grouplen;
+    } while (grouplen > 0 && offset + 4 < buffer + rSize);
 }