fixed wrong topic, added more logging with identifier [SSDR]

Author: KrX3D

usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp

@@ -16,10 +16,11 @@
     bool colonsDone = false;
     bool lightDone = false;
     _setAllFalse();
-    
+
     for (int index = 0; index < displayMaskLen; index++) {
       int timeVar = 0;
-      switch (umSSDRDisplayMask[index]) {
+      char c = umSSDRDisplayMask[index];
+      switch (c) {
         case 'h':
           timeVar = hourFormat12(localTime);
           _showElements(&umSSDRHours, timeVar, 0, !umSSDRLeadingZero);
@@ -68,6 +69,7 @@
             lightDone = true;
           }
           break;
+        _logWithPrefix("Unknown mask char '%c'", c);
       }
     }
     _setMaskToLeds();
@@ -205,15 +207,27 @@
   void UsermodSSDR::_publishMQTTint_P(const char *subTopic, int value)
   {
     #ifndef WLED_DISABLE_MQTT
+    _logWithPrefix("Entering _publishMQTTint_P topic='%s' value=%d", subTopic, value);
     if (WLED_MQTT_CONNECTED) {
-      if(mqtt == NULL) return;
+      if(mqtt == NULL){
+        _logWithPrefix("MQTT pointer NULL");
+        return;
+      }
 
-      char buffer[64];
-      char valBuffer[12];
-      int result = snprintf_P(buffer, sizeof(buffer), PSTR("%s/%S/%S"), mqttDeviceTopic, _str_name, subTopic);
-      if (result < 0 || result >= sizeof(buffer)) return;  // Buffer overflow check
-      
+      char buffer[64], nameBuffer[30], topicBuffer[30], valBuffer[12];
+
+      // Copy PROGMEM strings to local buffers
+      strlcpy(nameBuffer, reinterpret_cast<const char *>(_str_name), sizeof(nameBuffer));
+      strlcpy(topicBuffer, reinterpret_cast<const char *>(subTopic), sizeof(topicBuffer));
+
+      int result = snprintf(buffer, sizeof(buffer), "%s/%s/%s", mqttDeviceTopic, nameBuffer, topicBuffer);
+      if (result < 0 || result >= sizeof(buffer)) {
+        _logWithPrefix("Buffer overflow in _publishMQTTint_P");
+        return;  // Buffer overflow check
+      }
       snprintf_P(valBuffer, sizeof(valBuffer), PSTR("%d"), value);
+
+      _logWithPrefix("Publishing INT to: '%s', value: '%s'", buffer, valBuffer);
       mqtt->publish(buffer, 2, true, valBuffer);
     }
     #endif
@@ -222,49 +236,84 @@
   void UsermodSSDR::_publishMQTTstr_P(const char *subTopic, String Value)
   {
     #ifndef WLED_DISABLE_MQTT
+    _logWithPrefix("Entering _publishMQTTstr_P topic='%s' value='%s'", subTopic, Value.c_str());
     if (WLED_MQTT_CONNECTED) {
-      if(mqtt == NULL) return;
-      char buffer[64];
-      int result = snprintf_P(buffer, sizeof(buffer), PSTR("%s/%S/%S"), mqttDeviceTopic, _str_name, subTopic);
-      if (result < 0 || result >= sizeof(buffer)) return;  // Buffer overflow check
-      
+      if(mqtt == NULL){
+        _logWithPrefix("MQTT pointer NULL");
+        return;
+      }
+
+      char buffer[64], nameBuffer[30], topicBuffer[30];
+
+      strlcpy(nameBuffer, reinterpret_cast<const char *>(_str_name), sizeof(nameBuffer));
+      strlcpy(topicBuffer, reinterpret_cast<const char *>(subTopic), sizeof(topicBuffer));
+
+      int result = snprintf(buffer, sizeof(buffer), "%s/%s/%s", mqttDeviceTopic, nameBuffer, topicBuffer);
+      if (result < 0 || result >= sizeof(buffer)) {
+        _logWithPrefix("Buffer overflow in _publishMQTTstr_P");
+        return;  // Buffer overflow check
+      }
+
+      _logWithPrefix("Publishing STR to: '%s', value: '%s'", buffer, Value.c_str());
       mqtt->publish(buffer, 2, true, Value.c_str(), Value.length());
     }
     #endif
   }
 
   bool UsermodSSDR::_cmpIntSetting_P(char *topic, char *payload, const char *setting, void *value)
   {
-    if (strcmp_P(topic, setting) == 0)
+    _logWithPrefix("._cmpIntSetting topic='%s' payload='%s' setting='%s'", topic, payload, reinterpret_cast<const char*>(setting));
+    char settingBuffer[30];
+    strlcpy(settingBuffer, reinterpret_cast<const char *>(setting), sizeof(settingBuffer));
+
+    if (strcmp_P(topic, settingBuffer) == 0)
     {
+      int oldValue = *((int *)value);
       *((int *)value) = strtol(payload, nullptr, 10);  // Changed NULL to nullptr
+      _logWithPrefix("Setting updated from %d to %d", oldValue, *((int *)value));
       _publishMQTTint_P(setting, *((int *)value));
       return true;
     }
     return false;
   }
 
   bool UsermodSSDR::_handleSetting(char *topic, char *payload) {
+    _logWithPrefix("Handling setting. Topic='%s', Payload='%s'", topic, payload);
+
     if (_cmpIntSetting_P(topic, payload, _str_timeEnabled, &umSSDRDisplayTime)) {
+      _logWithPrefix("Updated timeEnabled");
       return true;
     }
     if (_cmpIntSetting_P(topic, payload, _str_ldrEnabled, &umSSDREnableLDR)) {
+      _logWithPrefix("Updated ldrEnabled");
       return true;
     }
     if (_cmpIntSetting_P(topic, payload, _str_inverted, &umSSDRInverted)) {
+      _logWithPrefix("Updated inverted");
       return true;
     }
     if (_cmpIntSetting_P(topic, payload, _str_colonblink, &umSSDRColonblink)) {
+      _logWithPrefix("Updated colonblink");
       return true;
     }
     if (_cmpIntSetting_P(topic, payload, _str_leadingZero, &umSSDRLeadingZero)) {
+      _logWithPrefix("Updated leadingZero");
       return true;
     }
+
+    char displayMaskBuffer[30];
+    strlcpy(displayMaskBuffer, reinterpret_cast<const char *>(_str_displayMask), sizeof(displayMaskBuffer));
+
+    _logWithPrefix("Comparing '%s' with '%s'", topic, displayMaskBuffer);
+
     if (strcmp_P(topic, _str_displayMask) == 0) {
       umSSDRDisplayMask = String(payload);
+
+      _logWithPrefix("Updated displayMask to '%s'", umSSDRDisplayMask.c_str());
       _publishMQTTstr_P(_str_displayMask, umSSDRDisplayMask);
       return true;
     }
+    _logWithPrefix("No matching setting found");
     return false;
   }
 
@@ -329,7 +378,7 @@
     umSSDRMask = (bool*) calloc(umSSDRLength, sizeof(bool));  // Use calloc to initialize to 0
 
     if (umSSDRMask == nullptr) {
-      DEBUG_PRINTLN(F("Failed to allocate memory for SSDR mask"));
+      _logWithPrefix("Failed to allocate memory for SSDR mask");
       return;  // Early return on memory allocation failure
     }
     _setAllFalse();
@@ -340,7 +389,7 @@
     #ifdef USERMOD_BH1750
       bh1750 = (Usermod_BH1750*) UsermodManager::lookup(USERMOD_ID_BH1750);
     #endif
-    DEBUG_PRINTLN(F("SSDR setup done"));
+    _logWithPrefix("Setup done");
   }
 
   // Clean up any allocated memory in the destructor
@@ -360,7 +409,7 @@
     }
 
     #if defined(USERMOD_SN_PHOTORESISTOR) || defined(USERMOD_BH1750)
-      if(bri != 0 && umSSDREnableLDR && (millis() - umSSDRLastRefresh > umSSDRRefreshTime) && !disableUmLedControl) {
+      if(bri != 0 && umSSDREnableLDR && (millis() - umSSDRLastRefresh > umSSDRRefreshTime) && !externalLedOutputDisabled) {
         float lux = -1; // Initialize lux with an invalid value
 
         #ifdef USERMOD_SN_PHOTORESISTOR
@@ -385,9 +434,10 @@
           } else {
             brightness = map(constrainedLux, umSSDRLuxMin, umSSDRLuxMax, umSSDRBrightnessMax, umSSDRBrightnessMin);
           }
+          _logWithPrefix("Lux=%.2f?brightness=%d",lux,br);
 
           if (bri != brightness) {
-            DEBUG_PRINTF("Adjusting brightness based on lux value: %.2f lx, new brightness: %d\n", lux, brightness);
+            _logWithPrefix("Adjusting brightness based on lux value: %.2f lx, new brightness: %d", lux, brightness);
             bri = brightness;
             stateUpdated(1);
           }
@@ -398,7 +448,7 @@
   }
 
   void UsermodSSDR::handleOverlayDraw() {
-    if (umSSDRDisplayTime && !disableUmLedControl && umSSDRMask != nullptr) {
+    if (umSSDRDisplayTime && !externalLedOutputDisabled && umSSDRMask != nullptr) {
       _overlaySevenSegmentDraw();
     }
   }
@@ -482,24 +532,34 @@
   }
 
   void UsermodSSDR::onMqttConnect(bool sessionPresent) {
+    _logWithPrefix("onMqttConnect sessionPresent=%d", sessionPresent);
     if (!umSSDRDisplayTime) {
+      _logWithPrefix("DisplayTime disabled, skipping subscribe");
       return;
     }
     #ifndef WLED_DISABLE_MQTT
     if (WLED_MQTT_CONNECTED) {
-      char subBuffer[48];
+      char subBuffer[48], nameBuffer[30];
+
+      // Copy PROGMEM string to local buffer
+      strlcpy(nameBuffer, reinterpret_cast<const char *>(_str_name), sizeof(nameBuffer));
+
+      _logWithPrefix("Connected. DeviceTopic='%s', GroupTopic='%s'", mqttDeviceTopic, mqttGroupTopic);
+
       if (mqttDeviceTopic[0] != 0)
       {
         _updateMQTT();
         //subscribe for sevenseg messages on the device topic
-        sprintf_P(subBuffer, PSTR("%s/%S/+/set"), mqttDeviceTopic, _str_name);
+        sprintf(subBuffer, "%s/%s/+/set", mqttDeviceTopic, nameBuffer);
+        _logWithPrefix("Subscribing to device topic: '%s'", subBuffer);
         mqtt->subscribe(subBuffer, 2);
       }
 
       if (mqttGroupTopic[0] != 0)
       {
         //subscribe for sevenseg messages on the group topic
-        sprintf_P(subBuffer, PSTR("%s/%S/+/set"), mqttGroupTopic, _str_name);
+        sprintf(subBuffer, "%s/%s/+/set", mqttGroupTopic, nameBuffer);
+        _logWithPrefix("Subscribing to group topic: '%s'", subBuffer);
         mqtt->subscribe(subBuffer, 2);
       }
     }
@@ -512,7 +572,9 @@
       //If topic begins with sevenSeg cut it off, otherwise not our message.
       size_t topicPrefixLen = strlen_P(PSTR("/wledSS/"));
       if (strncmp_P(topic, PSTR("/wledSS/"), topicPrefixLen) == 0) {
+        _logWithPrefix("onMqttMessage topic='%s' payload='%s'", topic, payload);
         topic += topicPrefixLen;
+        _logWithPrefix("Topic starts with /wledSS/, modified topic: '%s'", topic);
       } else {
         return false;
       }
@@ -526,7 +588,9 @@
       {
         //Trim /set and handle it
         topic[topicLen - 4] = '\0';
-        _handleSetting(topic, payload);
+        _logWithPrefix("Processing setting. Setting='%s', Value='%s'", topic, payload);
+        bool result = _handleSetting(topic, payload);
+        _logWithPrefix("Handling result: %s", result ? "success" : "failure");
       }
     }
     return true;
@@ -543,8 +607,7 @@
     JsonObject top = root[FPSTR(_str_name)];
 
     if (top.isNull()) {
-      DEBUG_PRINT(FPSTR(_str_name));
-      DEBUG_PRINTLN(F(": No config found. (Using defaults.)"));
+      _logWithPrefix("%s: No config found. (Using defaults.)", reinterpret_cast<const char*>(_str_name));
       return false;
     }
     umSSDRDisplayTime			= (top[FPSTR(_str_timeEnabled)] | umSSDRDisplayTime);
@@ -567,8 +630,7 @@
     umSSDRLuxMax                = top[FPSTR(_str_luxMax)] | umSSDRLuxMax;
     umSSDRInvertAutoBrightness  = top[FPSTR(_str_invertAutoBrightness)] | umSSDRInvertAutoBrightness;
 
-    DEBUG_PRINT(FPSTR(_str_name));
-    DEBUG_PRINTLN(F(" config (re)loaded."));
+    _logWithPrefix("%s: config (re)loaded.)", reinterpret_cast<const char*>(_str_name));
 
     return true;
   }

usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h

@@ -1,6 +1,10 @@
 #pragma once
 #include "wled.h"
 
+// logging macro:
+#define _logWithPrefix(fmt, ...) \
+	DEBUG_PRINTF("[SSDR] " fmt "\n", ##__VA_ARGS__)
+
 //#define REFRESHTIME 497
 
 class UsermodSSDR : public Usermod {
@@ -157,7 +161,7 @@ class UsermodSSDR : public Usermod {
     String umSSDRYears = umSSDR_YEARS;
 
     bool* umSSDRMask = nullptr;
-    bool disableUmLedControl = false;
+    bool externalLedOutputDisabled = false;
 
     // Forward declarations of private methods
     void _overlaySevenSegmentDraw();
@@ -224,10 +228,15 @@ class UsermodSSDR : public Usermod {
     uint16_t getId() override;
 
     // Public function that can be called from other usermods
+    /**
+    * Allows external usermods to temporarily disable the LED output of this usermod.
+    * This is useful when multiple usermods might be trying to control the same LEDs.
+    * @param state true to disable LED output, false to enable
+    */
     void disableOutputFunction(bool state) {
-      disableUmLedControl = state;
+	  externalLedOutputDisabled = state;
 	  #ifdef DEBUG_PRINTF
-	  	DEBUG_PRINTF("disableOutputFunction was triggered by an external Usermod: %s\n", disableUmLedControl ? "true" : "false");
+	  	_logWithPrefix("disableOutputFunction was triggered by an external Usermod: %s\n", externalLedOutputDisabled ? "true" : "false");
 	  #endif
     }