safepay / sensor.fronius

A Fronius Sensor for Home Assistant
MIT License
80 stars 31 forks source link

kWh instead of MWh in day energy #67

Open Sloneczna17 opened 2 years ago

Sloneczna17 commented 2 years ago

From the new update I have MWh in fronius day energy. It should be kWh. Even if I change to kWh in customize.yaml Istill have MWh count. Before new update I had kWh and it was fine to me.

colwilliamsnz commented 2 years ago

The day energy entity was changed a couple of releases ago to bring it in line with all the other energy entities (to support long term statistics). Looks like that has inadvertently caused this issue. If you add this to the components config yaml, does it resolve this for you?

units: kWh

Sloneczna17 commented 2 years ago

No, if I change in config yaml it only change tke name MWh to kWh but value is in MWh. For example my doday's prodeced energy was 35 kWh but I only see 0,03 kWh (or MWh depends what I use). If I change the the unit of energy in castomize.yaml I said that the configuration.yaml join incorectly castomize yaml. I attached my configuration yaml and castomize yaml and entities after I change the unit in castomize. Sorry for polish language. castomize yaml configuration yaml fronis day energy1

colwilliamsnz commented 2 years ago

I reverted the change made in my branch and I'm seeing no difference (see screen shot which is identical value BEFORE and AFTER). The only difference being there are no "long term stats" attributes in the reverted version which is why I changed it in the first place).

Screen Shot 2021-09-07 at 9 08 20 AM

I'm not sure that using customise.yaml is the right way of going about fixing this. How about changing the units for the Fronius sensor by adding "units: kWh"? Here's mine for reference:

Sloneczna17 commented 2 years ago

You have smartmeter and I don't but this is not the point. I set in configutation yaml units: kWh and after this I have every entitis in kWh but I have to change manually units to kWh because its stayed in MWh. Before the upgrade of HA and Hacs integraton I didn't have to set the units in configutation yaml and have entities: Fronius power in W, day energy in kWh, year energy in MWh, total energy in MWh. Now when I set units: kWh I have day, year and total energy in kWh. Why? I don't know it is because of HA upgrade to 2021.9 or Fronius HACS integration. I did whole updates the same day.

nilrog commented 2 years ago

Yes, @Sloneczna17, you are correct in that this is a change of behavior in this integration. It was introduced when support for long term statistics was added to comply with the new energy feature in HA. That said, having the different energy sensors report their values in different units as it was done before is not logical or practical.

If there is a need to have the DAY_ENERGY in kWh and the other energy sensors in MWh then, imo, the user can set units to kWh and define new template sensors to convert those sensors to MWh.

@colwilliamsnz The reason why this behavior was changed is because the convert_units field was changed from false to energy for DAY_ENERGY to support the long term sensors. And as you can see here also the default units for these sensors are not equal (there are historical reasons for that).

    'year_energy': ['inverter', True, 'YEAR_ENERGY', 'Year Energy', 'MWh', 'energy', 'mdi:solar-power'],
    'total_energy': ['inverter', True, 'TOTAL_ENERGY', 'Total Energy', 'MWh', 'energy', 'mdi:solar-power'],
    'day_energy': ['inverter', True, 'DAY_ENERGY', 'Day Energy', 'kWh', 'energy', 'mdi:solar-power'],

If you look further down in the code you can see here there is special handling for the DAY_ENERGY sensor and that is no longer happening. So now the DAY_ENERGY will always be converted to units. And I believe that the current behavior is better and less confusing. But it will affect a number of people who were running this integration without specifying units or specifying it as Wh.

            if self._convert_units == "energy":
                _LOGGER.debug("Converting energy ({}) to {}".format(state, self._units))
                if self._units == "MWh":
                    self._state = round(state / 1000000, 2)
                elif self._units == "kWh":
                    self._state = round(state / 1000, 2)
                else:
                    self._state = round(state, 2)
            elif self._convert_units == "power":
                _LOGGER.debug("Converting power ({}) to {}".format(state, self._units))
                if self._units == "MW":
                    self._state = round(state / 1000000, 2)
                elif self._units == "kW":
                    self._state = round(state / 1000, 2)
                else:
                    self._state = round(state, 2)
            elif self._json_key == "DAY_ENERGY":
                # day energy always gets converted to kWh
                _LOGGER.debug("Converting day energy to kWh ({})".format(state))
                self._state = round(state / 1000, 2)
            else:
                _LOGGER.debug("Rounding ({}) to two decimals".format(state))
                self._state = round(state, 2)

If we want to support the old behavior where DAY_ENERGY was treated special then that if-statement needs to go into the energy if-statement.

The way forward now is either:

  1. Update the README to state that all energy sensors now use the same, configurable, unit (breaking change, but more consistent)
  2. Update the code to restore the old behavior while maintaining the support for long term sensors (non breaking change, but confusing with different units).

For me it doesn't matter as I have always used units: 'kWh'. But from a maintenance perspective and the new energy feature in HA I say that option 1 is the preferred one.

Gollam commented 2 years ago

For me, it is more confusing that instead of 9.984 kWh, I now see 0.1 MWh for half a day, the granularity has decreased a lot!

UdoK commented 2 years ago

The Energy Configuration is even complaining about the Fronius Day Energy in MWh Solar Production energy should be in kWh