softypit / esp32_mqtt_eq3

esp32-based mqtt node to control EQ-3 BLE TRVs
146 stars 48 forks source link

Issue with settime validation? #66

Open borys-wwa opened 2 years ago

borys-wwa commented 2 years ago

Hi. I have two ESP32 set up with Your code to monitor and control 7 EQ-3 valves with use of Home Assistant. For now, HA executes script every few minutes sending "\<bleaddr> settime" command for each of the connected EQ3s, to the ESP32 just to trigger a status response, so that it could see, how much valve is open etc. I noticed, that after few hours o running without issues, one of the ESPs starts throwing following error in the console: "I (26386629) EQ3_MAIN: Invalid time argument �?M��?ğ�?"

The situation goes away, when I start sending the same command, but with a white space after "settime". I'm not the great c++ expert, but I tried to analyze the code to see if I can find the root cause, and I believe it might be caused by the fact, that this line is looking beyond actual command string, when the command only contains "settime". I assume, that as long, as the memory behind it is not occupied by anything, all works fine, but as soon as some data is stored just adjacent to the cmdstr, it then messes that check up. That would (I think) explain, why adding white space to the command fixes it, as it reserves that additional one byte, that this validation routine looks into, preventing it from being allocated to other data. These are just assumptions, but I thought I would share them with You so that You could look into this.

Kind Regards, Darek.

PS. Thanks for the great software :)

softypit commented 2 years ago

Thanks Darek, I guess during development this wasn't tested too thoroughly. You are correct that it should be checking cmdptr[7] for a 0 and not cmdptr[8].

I finally got around to pushing a new commit to the repository (v1.63) and included a fix for this. There isn't really a compelling reason to use the new code unless you really need the fix. There are a number of changes including a new esp-idf version, a new networking library and some additional config items although functionally the application works the same. Regards. Paul.