weltmeyer / ha_sonnenbatterie

Homeassistant integration to show many stats of Sonnenbatterie
GNU General Public License v3.0
51 stars 24 forks source link

Some meters report a value of `undefined` when their actual value is `0` #59

Closed RustyDust closed 1 month ago

RustyDust commented 1 month ago

@Xitee1 Since the update I'm seeing these meters report undefined when their actual value should be 0:

To me this looks like a division by zero error somewhere in the code but I can't find any logs to that effect.

Bildschirmfoto 2024-05-21 um 08 47 30

Xitee1 commented 1 month ago

Probably is not None needs to be appended to this check https://github.com/weltmeyer/ha_sonnenbatterie/blob/22483c9c398fd5e51b85ec7d3648ce34ee17352a/custom_components/sonnenbatterie/sensor_list.py#L70 I'll try it tonight.

RustyDust commented 1 month ago

I'd rather try

    else 0

here

'cause I think the None is what causes the undefined result.

Xitee1 commented 1 month ago

But then if the value is actually None for whatever reason, the sensor will still be set to 0 (and created if it does not exist). Because the powermeters are a hardcoded list, there is a possibility that they are just not supported. With else 0 they will still be created in HA, which they will not if we return None if the reported value is equal None.

Xitee1 commented 1 month ago

cause I think the None is what causes the undefined result.

exactly, which is good if returned correctly.

RustyDust commented 1 month ago

Try running

print("bla") if (val := 0) else print("blubb")

;)

Xitee1 commented 1 month ago
>>> print(val if (val := 0) else 0)
0
>>> print(val if (val := None) else 0)
0

...which means that if the val is None (if e.g. the sensor is not supported), 0 will be reported instead of None, which is wrong.

This is what we want:

>>> print(val if (val := 0) is not None else None)
0
>>> print(val if (val := None) is not None else None)
None
RustyDust commented 1 month ago

Ah, sorry. I skimmed over your initial statement about the possibility of those meters not being supported. I have yet to find a system that doesn't but if there are this is definitely the cleaner solution.

Xitee1 commented 1 month ago

Hat jetzt leider doch etwas länger gedauert, hab aber gerade den PR erstellt.

RustyDust commented 1 month ago

NP, bin auch nicht dazu gekommen