squishykid / solax

🌞 Solax Inverter API Wrapper
MIT License
102 stars 61 forks source link

Units are lacking required information #62

Closed VadimKraus closed 2 years ago

VadimKraus commented 2 years ago

The unit defined in this library is used to determine the state class in HomeAssistant. However, not all values which have the kWh unit are "total increasing". Without additional information it is impossible to determine the proper state_class.

For example: https://github.com/home-assistant/core/blob/dev/homeassistant/components/solax/sensor.py#L48

        elif unit == "kWh":
            device_class = SensorDeviceClass.ENERGY
            state_class = SensorStateClass.TOTAL_INCREASING

https://github.com/squishykid/solax/blob/master/solax/inverter.py#L152

152:        'Today\'s Energy':            (8, 'kWh'),
...
164:        'Month\'s Energy':            (19, 'kWh'),

Suggestion:

  1. Enum Units
  2. Unit with a classifier for the measurement type (monotonic or not )
ppetru commented 2 years ago

Thanks for finding & working on resolving this.

Somewhat related, I got an interesting suggestion from HA a while back when adding energy support for Solax (and thus those unit-based device_class and state_class settings): https://github.com/home-assistant/core/pull/54654#discussion_r690143683

I didn't get to implement it so far and I'm not asking you to, but I thought you might find it useful.

VadimKraus commented 2 years ago

70 merged, so this is obsolete