Fix Home Assistant Energy Management #2584
Conversation
Prevents Home Assistant Energy Management from interpreting data before all inverters were polled. This happens regularly when OpenDTU is rebooted, especially at night.
At night, getEnablePolling() returns false when "poll at night" is off. This caused the default isReachable value to fall thrugh, erroneously reporting all inverters to be reachable when in reality none of them are. This is fixed by checking cfg->Poll_Enable instead.
…was sent isReachable() was returning true even though we never received any response. This was causing ac/is_valid, among other mqtt topics, to be set 1 in immediately.
prevents home assistant from interpreting invalid data after startup. the order of publishing topics is important. when going from available -> unavailable, is_valid must be published first, before any invalid data is sent.
functionpointer
force-pushed
the
availability
branch
from
661cc9b to
293cfe3
Compare
src/Datastore.cpp
Outdated
| isReachable++; | ||
| } else { | ||
| if (inv->getEnablePolling()) { | ||
| if (cfg->Poll_Enable) { |
There was a problem hiding this comment.
Why is this necessary here but not elsewhere above and below?
There was a problem hiding this comment.
It very well might, but this is the only one relevant for ac/is_valid, as that depends on isAllEnabledReachable().
The other things don't affect Home Assistant Energy Management. In my opinion, they should go into their own PR.
There was a problem hiding this comment.
The original logic was to set all available if they are suposed to be available. That means if you don't fetch inverter values at night, all other inverters are still valid because you intentional don't pull this specific inverter.
Example: You have two inverters, one at a solar panel, one at a battery (which can run the whole night). If you don't fetch the first one at night, the ac_total should be still valid because the second inverter works as expected (as so does the first one as it is suspected to be offline)
There was a problem hiding this comment.
I see, i hadn't considered that scenario.
ac/yieldtotal won't actually be valid after a reboot. The solar panel inverter's yieldtotal is assumed to be 0, causing two jumps in the ac/yieldtotal value: One jump down on reboot at night, and one jump back up in the morning.
To keep backwards compatibility, ac/is_valid behavior could be kept original, except at night we must additionally have nonzero data from all inverters with cfg->Poll_Enable && !cfg->Poll_Night_Enable and have at least one inverter with cfg->Poll_Enable & cfg->Poll_Night_Enable.
That would mean we stay available until reboot. Afterward, we will be unavailable until morning.
There was a problem hiding this comment.
I have implemented that strategy now
include/MqttHandleInverterTotal.h
Outdated
|
|
||
| Task _loopTask; | ||
|
|
||
| unsigned int _connected_iterations = 0; |
There was a problem hiding this comment.
Style: This should be CamelCase.
There was a problem hiding this comment.
I have replaced this with an even better implementation. Instead of blindly waiting a few iterations, ac/is_valid is published with QOS 1, and the data is only published once the topic is confirmed to be published.
A less complex option would be to simply not publish data when ac/is_valid=false. However, that would be backwards incompatible for non-HA uses of mqtt.
| bool InverterAbstract::isReachable() | ||
| { | ||
| return _enablePolling && Statistics()->getRxFailureCount() <= _reachableThreshold; | ||
| return _enablePolling && Statistics()->getRxFailureCount() <= _reachableThreshold && Statistics()->getLastUpdate() > 0; |
There was a problem hiding this comment.
This makes sense and should have gone into its own PR 😉
when not valid, data topics are only published after we get confirmation of ac/is_valid being published. when valid, data topics are published first, followed by ac/is_valid
It now stays available at night when a battery powered inverter is reachable.
|
I think I addressed all comments. This PR is ready for another review. |
3 participants
YieldTotal in Home Assistant Energy Management is somewhat broken, due to YieldTotal intermittently jumping to 0 or incomplete values from a subset of inverters. This happens after reboots, especially at night, and also sometimes during connectivity issues.
This PR fixes it by introducing availability topics to most MQTT topics.
Each inverter's topics are only available if that inverter is reachable.
ac/yieldtotalandac/yielddayare only available when all inverters are reachable.isReachable()andisAllEnabledReachable()are fixed.unavailablefor a few seconds. This ensures no invalid data is accidentally taken as valid. Previously, rebooting would causeac/yieldtotal=0 to be published beforeac/is_valid= false.