home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
69.81k stars 28.94k forks source link

[dsmr-reader] Tariff 1 isn't necessarily the "low" tariff #66084

Open Glodenox opened 2 years ago

Glodenox commented 2 years ago

The problem

The integration assumes that tariff 1 from DSMR is always the "low" tariff, but that isn't always the case (example: Fluvius metres in Belgium). We should be able to invert these values with a configuration setting. Or making it possible to override these two texts in the configuration is probably even better as a solution, as then we can also translate them to our preferred language.

I consider this a bug as I'm receiving wrong data, but I must admit it's not too far off from being a feature request. If I find some time, I'll try to send in a pull request. My python-fu isn't that great, but how hard can it be? (famous last words...)

What version of Home Assistant Core has the issue?

2022.2.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

dsmr-reader

Link to integration documentation on our website

No response

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

The function causing this issue: https://github.com/home-assistant/core/blob/dev/homeassistant/components/dsmr_reader/definitions.py#L34

probot-home-assistant[bot] commented 2 years ago

dsmr_reader documentation dsmr_reader source (message by IssueLinks)

probot-home-assistant[bot] commented 2 years ago

Hey there @depl0y, mind taking a look at this issue as it has been labeled with an integration (dsmr_reader) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

nikmrx commented 2 years ago

How I discovered this: low tariff started this Monday morning at 7, after a whole weekend in high tariff.

They go high, we go low

Thank you for reporting, @Glodenox.

vdborght commented 2 years ago

Since this integration is only reading the values posted on MQTT by DSMR Reader (https://dsmr-reader.readthedocs.io/en/latest/), I think the fix should be done there and not in this integration.

nikmrx commented 2 years ago

This integration is not "only reading the values posted on MQTT by DSMR Reader", @vdborght. In fact, the values posted by DSMR Reader are either 0001 or 0002. image

As far as I can see, it is this integration that decides 0001 is low, and 0002 is high. In my case incorrectly, as you can clearly see in this screenshot. The shorter green ones are nights, the longer (marked 0002) are weekends. image

Glodenox commented 2 years ago

The way I have it set up is with a remote datalogger that sends the data to a DSMR-reader instance, which then posts the messages to the MQTT broker. The DSMR-reader instance just receives a tariff 1 or 2 and I've had to rename the tariff names for the front-end there. The DSMR webinterface component won't be able to affect the way the messages are generated for MQTT, so I don't think it can be handled in that part of DSMR-reader. That said, I do think it would be a good solution for DSMR reader to support a flag that inverts the values, as "1" usually means the low tariff.

As said before in this conversation: it's just tariff 1 or 2 that is being sent, which doesn't directly imply a "low" and "high" tariff by itself. That is currently being implied in this integration. But I'm less inclined to say we should be able to invert the values here, and more that we should be able to override the text used in the number transformation in the code. Then it's not just useful for people who want to switch the values, but also for those who want to personalize/translate the text.

Glodenox commented 2 years ago

EDIT: this workaround for the sensor names is no longer needed since the entity_ids are properly set and allow for overriding in the HA GUI directly.

Oh, I forgot to mention a somewhat decent workaround I'm currently using to at least fix the names used for the entities exposed by this integration. It is perfectly possible to customize the names of these entities in the configuration.yaml. I added the following values under the homeassistant object:

  customize:
    sensor.dsmr_reading_electricity_delivered_1:
      friendly_name: Dagtarief verbruik
    sensor.dsmr_reading_electricity_delivered_2:
      friendly_name: Nachttarief verbruik
    sensor.dsmr_reading_electricity_returned_1:
      friendly_name: Dagtarief teruglevering
    sensor.dsmr_reading_electricity_returned_2:
      friendly_name: Nachttarief teruglevering
    sensor.dsmr_reading_electricity_currently_delivered:
      friendly_name: Huidig verbruik
    sensor.dsmr_reading_electricity_currently_returned:
      friendly_name: Huidige teruglevering
    sensor.dsmr_meter_stats_electricity_tariff:
      friendly_name: Tarief elektriciteit
    sensor.dsmr_day_consumption_electricity_merged:
      friendly_name: Vandaag verbruikt
    sensor.dsmr_day_consumption_electricity_returned_merged:
      friendly_name: Vandaag teruggeleverd

I only renamed entities that I'm actually using in dashboards, so you may want to rename others. The only thing this can't fix is the value of the sensor.dsmr_meter_stats_electricity_tariff sensor entity, which contains name of the currently active tariff. Also, normally you'd be able to adjust these friendly names in the UI, but it would seem there's some sort of unique ID missing, which causes the UI to not be available for these entities.

Glodenox commented 2 years ago

Would someone who uses the DSMR Reader integration be able to help me out by validating this pull request? This would require installing the development environment and running the changes locally.

Before I can make the names of the two tariffs configurable, the integration needs to migrate to a ConfigEntry-based installation. That is what that pull request is doing. We're not supposed to add new configuration through configuration.yaml any more, so it's not as easy as just providing some options there.

Glodenox commented 1 year ago

With the config flow merged, I can start looking into making these names configurable.

nikmrx commented 1 year ago

I look forward to it, but incidentally I managed today to solve the problem in my own way.

I have installed Mosquitto on a Raspberry Pi Zero (relay), and I have installed Node-RED on my original broker ,with a very simple flow that reads messages from broker, switches the _1 and _2 suffix, and posts the messages again on relay. (There is no discernible delay.) All I had to do was switch MQTT in HA from broker to relay (and get rid of all the customisation in dashboards, etc.), and everything is now as it was meant to be.

issue-triage-workflows[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment ๐Ÿ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Glodenox commented 1 year ago

This is still an issue I intend to solve.

ver1tyn commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment ๐Ÿ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

It is still an issue on March 3d 2023.

ver1tyn commented 1 year ago

This is still an issue I intend to solve.

Thank you, looking forward to it. I will try the workaround renaming the entities for now...

issue-triage-workflows[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment ๐Ÿ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Glodenox commented 1 year ago

I still need to reserve some time to fix this. Potentially this will be possible to override in HA itself in the future though, if I look at how the development for entity value translation is going.

hazcod commented 10 months ago

I was wondering, if one does not make use of different tarriffs, should we make a sum of both tarriffs for the consumption stats?

Glodenox commented 10 months ago

I personally consider that outside of the scope of this integration. A template sensor that consists of the sum of the several sensors you'd want to sum up should work fine for this.

issue-triage-workflows[bot] commented 7 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment ๐Ÿ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Glodenox commented 7 months ago

The issue is still present, but I'm somewhat awaiting the current evolution that makes it easier to rename/translate sensor names. If HA makes it possible to do the same to the enumeration values of sensors, this addition might not be needed.

kennysaelen commented 5 months ago

I just actually went and changed the entity names to fix it. The annoying thing is now that I have to reinstall DSMR reader integration as I have added 2 other meter readers (Gas and Water) and it does not automatically add them / updates them.

GylRon1 commented 2 months ago

Still a problem for more than 2 years now. The only thing that has to change is the firmware of the man in the middle for the Belgian meters. No changes have to be made in HA that way. This can't be that difficult for SmarthomeGateways is it? Also new firmware versions lack a list of the changes in new firmware versions on the website, how are we supposed to know what has changed/fixed/added/removed?