fix home assistant complaining GBP/kWH is an invalid currency

si458 commented 3 years ago

Description

Motivation and Context

How Has This Been Tested?

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

ColinRobbins commented 3 years ago

Thanks - I thought I had fixed that.

si458 commented 3 years ago

@ColinRobbins this is a different line 😆 you fixed the elec tariff standing, this is for elec tariff rate

si458 commented 3 years ago

@ColinRobbins if you dont change GBP/kWh to GBP the web ui in home assistant complains in the console saying RangeError: invalid currency code in NumberFormat(): GBP/kWh so this needs to be set as GBP

ColinRobbins commented 3 years ago

I don’t think this is right. The Energy integration needs the unit rate to by GBP/kWh, otherwise it won’t accept it. The standing rate is GBP.

ColinRobbins commented 3 years ago

Which page / card are you looking at when you see the console error? I’m not seeing anything in my console!

si458 commented 3 years ago

BEFORE: Screenshot 2021-11-09 at 18 35 52

AFTER: Screenshot 2021-11-09 at 18 42 16

CONSOLE ERRORS BEFORE PATCH: Screenshot 2021-11-09 at 18 37 31

EDIT: uploading right picture does help haha

this is just a standard entities added to my home dashboard

type: entities
entities:
  - sensor.electric_consumption_today
  - sensor.electric_consumption_year
  - sensor.electric_cost_today
  - sensor.electric_tariff_rate
  - sensor.electric_tariff_standing

EDIT AGAIN: i dont believe the kWh is needed as thats being used/calculated in the energy tab and not on the standard dashboards

ColinRobbins commented 3 years ago

Ah, OK. I think that probably needs a different fix then. I think GBP/kWh is correct. The issue is the device class being DEVICE_CLASS_MONETARY.

ColinRobbins commented 3 years ago

Sorry, device class, not state class - corrected post above. No sure what the “correct” value should be - looking…

ColinRobbins commented 3 years ago

Not sure I can see anything better than “DEVICE_CLASS_ENERGY”

si458 commented 3 years ago

@ColinRobbins im keeping my pi running for now with this patch set as GBP and ill see if any issues happen over the next few days none so fair, but case of wait n see 🤞

ColinRobbins commented 3 years ago

With it set to GBP, you will not be able to add it as an entity with current price in the energy dashboard. It will probably work if set up, but would not work if you remove it and try to add it back. The energy dashboard needs GBP/kWh.

si458 commented 3 years ago

With it set to GBP, you will not be able to add it as an entity with current price in the energy dashboard. It will probably work if set up, but would not work if you remove it and try to add it back. The energy dashboard needs GBP/kWh.

surely the entity with current price option would be sensor.electric_cost_today and not sensor.electric_tariff_rate ?

si458 commented 3 years ago

i currently have mine set to use an entity tracking the total costs and use sensor.electric_cost_today and no issues here

ColinRobbins commented 3 years ago

If you look at the code in https://github.com/home-assistant/core/blob/dev/homeassistant/components/energy/sensor.py About line 300, the integration is expecting the price to end in /wh, /mwh or by default /kWh. My proposed alternative fix, resolves the issue you reported, and meets the expectation of the energy dashboard.

ColinRobbins commented 3 years ago

The energy dashboard documentation gives an example in ‘USD/kWh’, not ‘USD’.

si458 commented 3 years ago

@ColinRobbins if you look at the developer docs too for the sensor entites it says the values need to be either kWh or a GBP (currency) for either energy or monetary so i think this fix is good enough :) https://developers.home-assistant.io/docs/core/entity/sensor#available-device-classes

ColinRobbins commented 3 years ago

We’ll have to agree to differ, and let HandyHat decide which approach to take.

ColinRobbins commented 2 years ago

Revisiting this, I think if you use “rate” in the energy dashboard with GBP as the unit, this PR will lead to the error…

Unexpected unit of measurement

Translation Error: The intl string context variable "currency" was not provided to the string "The following entities do not have the expected units of measurement ''{currency}/kWh'' or ''{currency}/Wh'':"

Hence why I believe the current units are correct at GBP/kWh.

si458 commented 2 years ago

Revisiting this, I think if you use “rate” in the energy dashboard with GBP as the unit, this PR will lead to the error…
Unexpected unit of measurement

Translation Error: The intl string context variable "currency" was not provided to the string "The following entities do not have the expected units of measurement ''{currency}/kWh'' or ''{currency}/Wh'':"
Hence why I believe the current units are correct at GBP/kWh.

Where are you seeing this error? I've had no issues/no errors with this pr on my pi for awhile now?

ColinRobbins commented 2 years ago

This error is in the HA Logs. It occurs if you use the “rate” sensor, with this PR, in the Energy integration to supply the current cost of energy.

si458 commented 2 years ago

How does ur elec look as no issues here at all? Screenshot_20211122-183416_Home Assistant.jpg

ColinRobbins commented 2 years ago

The error occurs if you use the current tariff rate sensor for “use an entity with the current price”. ( I know using the cost sensor is better, but the using the tariff sensor should not result in an error).

robertalexa commented 2 years ago

Don't mean to intervene but I am with @ColinRobbins on this one.

Here is my reasoning:

yesterday i have played with this, tried using use an entity with the current price and tried it with Electric Cost (Today) which is basically GBP (unit of measurement) and that indeed throws an error.

Revisiting this, I think if you use “rate” in the energy dashboard with GBP as the unit, this PR will lead to the error…
Unexpected unit of measurement

Translation Error: The intl string context variable "currency" was not provided to the string "The following entities do not have the expected units of measurement ''{currency}/kWh'' or ''{currency}/Wh'':"
Hence why I believe the current units are correct at GBP/kWh.

So i can confirm changing tariff rate (the entity refered in this PR) to GBP and using it for Energy will error later.

I agree that the tariff rate unit of measurement should be GBP/kWh, and not GBP. This is NOT a sum of money (currency) but rather a unit of measurement, like fuel consumption. Fuel consumption 5L makes no sense if you do not match it against a distance e.g. 100km. (same logic applies to miles per galon obviously). Same things is when you go shop for anything, the price for meat is £/kg, otherwise how do you know how much you are getting for your £?
So that being said, the approach should be to change the device class rather than change the unit of measurement, as already suggested by Colin

Rather than changing line 336, I suggest trying adding the following at line 332:
    @property
    def device_class(self) -> str:
        """Return None as the device class."""
        return None
I've tried it on my test systems, and it seems to solve the issue. I think anything other that None will case issues somewhere.

PS: I have landed here by basically discovering the same error in the console, see here https://github.com/HandyHat/ha-hildebrandglow-dcc/issues/88#issuecomment-976615782

HandyHat commented 2 years ago

Thank you for the insightful discussion here everyone! I'm going to close this in favour of @ColinRobbins' solution of unsetting the device class (#138), as I think that makes more sense

HandyHat / ha-hildebrandglow-dcc