dewenni / ESP_Buderus_KM271

Control your Buderus Logamatic 2107 or HS-2105 with MQTT or Home Assistant
MIT License
54 stars 10 forks source link

compiler warning #72

Closed Franck78 closed 4 months ago

Franck78 commented 4 months ago

I tend to pay attention to compiler warnings. Under Arduino IDE, some are fatal (error). I don't know the setting with PlatformIO

Can you try a build without any warnings ?

Anyway, here is a list of 'fixes' required for Arduino IDE.

apparently not enough size here:
km271_3.31/basics.cpp
@@ -251,7 +251,7 @@
-  static char ret_str[4];
+  static char ret_str[5];
   snprintf(ret_str, sizeof(ret_str), "%i", value);
   return ret_str;
 }

Nearly all the memset on structures are problematic
https://stackoverflow.com/questions/66368061/error-clearing-an-object-of-non-trivial-type-with-memset
+++ km271_3.31/config.cpp
@@ -85,8 +85,21 @@
 void configInitValue(){

-    memset(&config, 0, sizeof(config));
+    memset((void*)&config, 0, sizeof(config));

Here, tmpMessage is 'Unused' when no debug. Declaration should be moved inside the ifdef
Other vars are really unused.

+++ km271_3.31/km271.cpp
@@ -254,9 +254,8 @@
-  char            tmpMessage[300]={'\0'};
-  char            errorMsg[300]={'\0'};
-  double          oil_consumption;

   #ifdef DEBUG_ON 
+    char            tmpMessage[300]={'\0'};
     if (kmregister != 0x0400 && kmregister != 0x882e && kmregister != 0x882f){
       if ( config.debug.enable && compareHexValues(config.debug.filter, data) ){ 

Another problematic memset
@@ -1380,7 +1396,7 @@

   // initialize structs
   memset(&kmStatus, 0, sizeof(s_km271_status));
-  memset(&kmConfigStr, 0, sizeof(s_km271_config_str));
+  //memset(&kmConfigStr, 0, sizeof(s_km271_config_str));
   memset(&kmConfigNum, 0, sizeof(s_km271_config_num));
   return RET_OK;  
 }

Another problematic memset
@@ -2111,7 +2127,8 @@
-    strncpy(timerInfo, "--", sizeof(timerInfo));
+    //strncpy(timerInfo, "--", sizeof(timerInfo));
+    strcpy(timerInfo, "--");
   }
 }

Some warnings that this buffer is too small!
+++ km271_3.31/km271.h
@@ -114,7 +115,7 @@
-#define CFG_MAX_CHAR_TIMER 128
+#define CFG_MAX_CHAR_TIMER 322 /*128*/

Compiler complaint about data format 
+++ km271_3.31/oilmeter.cpp
@@ -55,7 +55,7 @@   
-  snprintf(tmpMsg, sizeof(tmpMsg), "oilcounter was set to: %i", data.oilcounter);
+  snprintf(tmpMsg, sizeof(tmpMsg), "oilcounter was set to: %ld", data.oilcounter);

@@ -88,7 +88,7 @@
-  snprintf(tmpMsg, sizeof(tmpMsg), "oilcounter was set to: %i", data.oilcounter);
+  snprintf(tmpMsg, sizeof(tmpMsg), "oilcounter was set to: %ld", data.oilcounter);
 }

Sometime I don't understand the problem.
+++ km271_3.31/webTools.cpp
@@ -195,7 +195,7 @@
  * @return  none
  * *******************************************************************/
 void deleteFile(fs::FS &fs, const String& path) {
-  Serial.printf("Deleting file: %s\r\n", path);
+  Serial.printf("Deleting file: %s\r\n", (char*)&path);

And sometimes, memset is the correct choice ;)
+++ km271_3.31/webUI.cpp
@@ -49,7 +49,8 @@
-  snprintf(elementBuffer, sizeof(elementBuffer), "");
+  //snprintf(elementBuffer, sizeof(elementBuffer), 0);
+  memset(elementBuffer, 0, sizeof(elementBuffer));

@@ -830,9 +831,12 @@
   /*-------------------------------------------------------------------------
   // initialize structs
   -------------------------------------------------------------------------*/
-  memset(&kmStatusCpy, 0, sizeof(s_km271_status));
-  memset(&kmConfigNumCpy, 0, sizeof(s_km271_config_num));
-  memset(&kmConfigStrCpy, 0, sizeof(s_km271_config_str));
+  //memset(&kmStatusCpy, 0, sizeof(s_km271_status));
+  //memset(&kmConfigNumCpy, 0, sizeof(s_km271_config_num));
+  //memset(&kmConfigStrCpy, 0, sizeof(s_km271_config_str));
dewenni commented 4 months ago

Thanks for the tips. Interestingly, I don't get the warnings in PlatformIO. I'll have to see how I can make the compiler more sensitive. However, I have tried to take all the hints into account. I don't know whether the "memset" is really necessary, but I have changed it.

I am currently working on a new version where I will also implement the log in the WebUI #70

Franck78 commented 4 months ago

all of above gone.