ksheumaker / homeassistant-apsystems_ecur

Home Assistant custom component for local querying of APSystems ECU-R Solar System
Apache License 2.0
186 stars 43 forks source link

When inverters are off, inverter sensors entities should not update #214

Closed wildekek closed 11 months ago

wildekek commented 1 year ago

When the inverters are off, the following sensors report a value of 0:

The following sensors report their last know state:

The correct behavior is not updating the state, like the temperature sensor:

Wrong:

image

Expected behavior:

image
HAEdwin commented 1 year ago

The temperature sensor(s) of the inverter(s) also reported zero when switching off the inverter(s). This zero value does not reflect the actual temperature of the inverter. That is why the preference arose not to update these graphs.

wildekek commented 1 year ago

Yes, that makes total sense. So you agree this behavior should apply to the other graphs as well? (Since the inverter has no idea what the Signal, Frequency, Voltage are, Power is debatable.)

HAEdwin commented 1 year ago

I created a poll to see how others think about this idea: https://github.com/ksheumaker/homeassistant-apsystems_ecur/discussions/215 freq

HAEdwin commented 11 months ago

I'll try to make it optional like so: image

wildekek commented 11 months ago

Sweet, that we would be a great addition!

HAEdwin commented 11 months ago

@wildekek I'm currently testing... If you would like to test, you can find the sources here: https://github.com/HAEdwin/homeassistant-apsystems_ecur.

Personally I like to update the graphs when the inverters are offline because this morning it showed power being zero on two channels with the inverter online, showing that these parameters are indeed zero and not a guess. image

HAEdwin commented 11 months ago

@wildekek With some delay due to the busy Christmas holidays and a software bug the 1.3.0b version includes the feature - ready for further testing 🎉

wildekek commented 11 months ago

Nice work @HAEdwin ! I've installed it and will check out the results in 24h. Thanks a bunch and happy holidays to you.

HAEdwin commented 11 months ago

Fixed: Caught a bug on a fresh install; when configuring the integration it throws an error on the no_graphs because there was no default so the key and value (False) could not be created.

Release was updated just now. @wildekek, All is in working as expected?

wildekek commented 11 months ago

Yep, thanks for your work!