Conversation
tspopp
left a comment
There was a problem hiding this comment.
Overall this looks very solid 👍 I have not installed this yet, but will most probably try it out the next days. There are a few spelling things which we can resolve later. I did a first review of the code and I believe I like what I see and how you added this to the codebase.
I think we should prepare this PR to remove the entire Configuration from the firmware, so we will be able to provide firmware images which does not contain any custom configuration.
Still, I do not undestand the need for the javascript and the css files. If you could provide some quick explanation and background and maybe why you chosen that, would be very helpful.
Actually I thought about going along with https://github.com/tzapu/WiFiManager. What was the reason for not taking this library into accout?
Co-authored-by: Thomas Popp <66408890+tspopp@users.noreply.github.com>
|
Hello, |
…popp#75) * fix(next): fix heating element enabled switch * chore(version): bump version to v1.7.1
* added a guide inside of HOMEASSISTANT.md` to setup a climat entity * added custom climate card showcase images * Updated HOMEASSISTANT.md with 3 showcase images from ./img
…heatpumps The mLastStateUpdate variable was declared as time_t (signed integer) while being compared with millis() return values (unsigned long). This caused incorrect behavior after millis() exceeded the maximum signed integer value (2,147,483,647ms / ~24.8 days), causing time updates to stop being sent. Changed mLastStateUpdate to unsigned long to match the type returned by millis() and ensure proper overflow handling in time comparisons.
|
Hello, any chance for this PR to be merged ? ;-) |
|
First I think you need to rebase. I am quite open to merge this, but as suggested I think it would be nice to get rid of the entire configuration.h file. Don't you agree? I thought maybe you will go in that direction, but you were running into an "out of time" error ;) |
Co-authored-by: Thomas Popp <66408890+tspopp@users.noreply.github.com>
|
Hello, |
|
Hey, sorry for the long delay, been quite busy these days. I try go give this a test run soon and provide comments. |
|
I think this would be a real changer for this project. |
Have not looked yet into the code, but if you can move all the business logic unrelated to the Arduino framework into a separate internal library, you can use the
Maybe me make these kalman parameters just const, not changeable through the web-if. Only very advanced users will change them anyway. What do you think? |
Will have a look to unittests, hope I'll manage to handle them...
Actually, I added all parameters from Configuration.h (except WATCHDOG_TIMEOUT_MS, WIFI_RECONNECT_CYCLE_S, MQTT_MAX_TOPIC_SIZE, MQTT_MAX_PAYLOAD_SIZE which are unlikely to change) to almost remove the entire Configuration from the firmware. Do you envision any other settings that are not very relevant from web config ? |




This PR cancel and replace #71.
Even the purpose is the same, implementation is different and use a rest API architecture for better/easier extensibility.
Other changes are :
fix feature request: Web interface #55 , fix-partially feature(wifimanager): get rid of configuration files #63