Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some pieces of code in mitsubishi2mqtt.ino that implement delay by comparing to millis() can be improved to work better on a wrap #257

Open
xalier1 opened this issue Feb 6, 2024 · 0 comments

Comments

@xalier1
Copy link

xalier1 commented Feb 6, 2024

In mitsubishi2mqtt.ino the variable wifi_timeout is compared to millis() to implement a delay in a few places in a way that doesn't work properly when millis() nears the unsigned long wrap point.

There are lots of examples of properly implemented millis() delay code in mitsubishi2mqtt.ino. Delays using variables lastTempSend, lastRemoteTemp , lastMqttRetry, etc in mitsubishi2mqtt.ino are all implemented in a way that works at the unsigned long wrap point. The delays associated with the variable wifi_timeout should copy the way a delay is done using delay code associated with the variable lastTempSend as an example.

When looking for correct examples of code, I avoided the code associated with the variable lastHpSync copied to here as follows:

if (((millis() - lastHpSync > durationNextSync) or lastHpSync == 0)) {
lastHpSync = millis();

It may seem to convenient to use lastHpSync == 0 as a "do immediately without delay" flag but remember that in the statement "lastHpSync = millis();", millis() can return a value of 0 which would cause an unintended "do immediately without delay" in a subsequent pass. The probability of a 0 being returned by millis() is low but if possible, it's best to implement code that works as intended all the time rather than most of the time. In an environment that is not memory constrained and is not performance constrained, it is better to replace "lastHpSync == 0" with an explicit "do immediately" flag that is initialized to true at startup and is set to false once the first pass has occurred. An example of the replacement code could be:

if (((millis() - lastHpSync > durationNextSync) or firstHpSync)) {
lastHpSync = millis();
firstHpSync = false;

where firstHpSync is declared as a global bool initialized to "true"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant