MindFreeze / ha-sankey-chart

A Home Assistant lovelace card to display a sankey chart. For example for power consumption
MIT License
348 stars 15 forks source link

fix: connection_entity_id and add_entities/subtract_entities #199

Closed madsciencetist closed 1 month ago

madsciencetist commented 1 month ago
MindFreeze commented 1 month ago

The connection_entity_id fix looks good but the other one is tricky and I want to test it first. Hopefully I'll have time for it this weekend. Thanks for the fixes

madsciencetist commented 1 month ago

I agree that _getUnitOfMeasurement() is a hack and that the solution is to refactor such that units are always tracked with the values. It got tricky in that original unit conversion PR because kWh -> gCO2 happens in getStatistics(), but then gCO2 -> kgCO2 happens in normalizeStateValue(), which has no real tracking of the unit of the value it is given.

The new problem I found is as follows. Assume the following configuration:

convert_units_to: monetary
monetary_unit: USD
electricity_price: 0.5
sections:
  - entities:
      - entity_id: sensor.energy_1         # 100 kWh
        add_entities:
          - entity_id: sensor.energy_2     #  20 kWh

We want the bar to evaluate to $60. Instead, this happened:

MindFreeze commented 1 month ago

I understand now. The problem is that we don't use this._getUnitOfMeasurement there.

What confused me is that you also changed subEntity.attributes.unit_of_measurement || unit_of_measurement to entityConf.unit_of_measurement || subEntity.attributes.unit_of_measurement. Basically reversed the order, which I think is a problem. Can you change it to this._getUnitOfMeasurement(subEntity.attributes.unit_of_measurement || unit_of_measurement) so the order is preserved?

madsciencetist commented 1 month ago

Yes, that makes sense