ekutner / home-connect-hass

Alternative (and improved) Home Connect integration for Home Assistant
MIT License
572 stars 56 forks source link

Remaining program time sensor: wrong unit? #286

Closed TjallingO closed 9 months ago

TjallingO commented 10 months ago

Hi,

Thank you for your work on this project; I am using it with great pleasure.

However, from my testing I've found that one of the sensors enabled by this integration interprets one value (BSH.Common.Option.RemainingProgramTime) as being a timestamp, while according to the API documentation it should be seen as an integer, or more specifically as a timespan. As far as I can tell, the line linked here is where it should be changed, should you agree to do so. Implementing this change would also make this sensor function as expected, at least from my perspective.

Thanks!

ekutner commented 10 months ago

Unfortunately there is no native support for timespan in HA so the options are to define this sensor as an int, a string or a timestamp. Out of these options I believe most people would find the timestamp most useful. If you wish to convert it back to seconds you can use a template like this: {{ as_datetime(states("sensor.<you_appliance_ID>_bsh_common_option_remainingprogramtime"))-now() }}

TjallingO commented 10 months ago

Thanks for the quick response.

I understand your perspective about it being useful as a timestamp too. However, the API natively returns the number of seconds until the program is finished, therefore any timestamp that is formed from it will be off - which is how I noticed this issue in the first place. Moreover, in my personal opinion, interpreting this variable as a timestamp also presents a bit of a incongruity with its name. When I see remaining program time, my mind thinks of a timespan, rather than a timestamp, which would be called something like ProgramCompletionTime or something.

As such, I changed line 31 of the .py as follows:

From: "BSH.Common.Option.RemainingProgramTime": {"unit": None, "class": "timestamp" },

To: "BSH.Common.Option.RemainingProgramTime": { "unit": None, "class": f"{DOMAIN}__timespan"},

To be in line with the other timespan values in the _CONF_ENTITYSETTINGS block. However, of course this totally up to you to implement or not.

ekutner commented 10 months ago
  1. In the UI the timestamp class is actually displayed as a timespan, so it will show something like: "In 2 hours", which is consistent with the name of the sensor.
  2. The HA UI is rounding the time to full hours for values greater than 45 minutes, but the sensor itself is accurate to the second. You can verify that using the template.
  3. Changing the behavior now would be a breaking changing for existing automatons that rely on this sensor.

If you want to override the default class definition you can do that using entity_settings in your configuration.yaml file. Check the README for more information.

I suggest that you open this as a discussion topic and try to get some feedback from other users. If this suggested change gets a wide enough support I'll change it.

seidler2547 commented 9 months ago

I would support a better format, if possible. Right now I've come up with this monstrosity to display the estimated finishing time also when delayed start is active:

{{ as_timestamp(now() + timedelta(seconds=as_timestamp(states('sensor.01311xxx_bsh_common_option_remainingprogramtime'))-as_timestamp(states.sensor['01311xxx_bsh_common_option_remainingprogramtime'].last_changed)) + as_timedelta('0 '+states('sensor.01311xxx_bsh_common_option_startinrelative')+':00'))|timestamp_custom('%H:%M',now()) }}
nireins commented 9 months ago

I hope I am addressing my concerns correctly at this point.

Using this integration, my new NEFF S157ZBX01D dishwasher shows a remaining program running time of 4 hours when switched off. Only when I switch on the device and start a program is the correct remaining program running time displayed.

Am I doing something wrong? Can someone help me?

nireins commented 9 months ago

Thanks for the quick response.

I understand your perspective about it being useful as a timestamp too. However, the API natively returns the number of seconds until the program is finished, therefore any timestamp that is formed from it will be off - which is how I noticed this issue in the first place. Moreover, in my personal opinion, interpreting this variable as a timestamp also presents a bit of a incongruity with its name. When I see remaining program time, my mind thinks of a timespan, rather than a timestamp, which would be called something like ProgramCompletionTime or something.

As such, I changed line 31 of the .py as follows:

From: "BSH.Common.Option.RemainingProgramTime": {"unit": None, "class": "timestamp" },

To: "BSH.Common.Option.RemainingProgramTime": { "unit": None, "class": f"{DOMAIN}__timespan"},

To be in line with the other timespan values in the _CONF_ENTITYSETTINGS block. However, of course this totally up to you to implement or not.

I changed it too and actually find the 00:00 display better than the timestamp. This means I no longer need the sensor I created myself, which converts the previous display into HH:MM.

Thank you @TjallingO

ekutner commented 9 months ago

There is no reason to change source files, see my note above:

If you want to override the default class definition you can do that using entity_settings in your configuration.yaml file. Check the README for more information.

I'm going to close this issue now since I don't expect new developments here. As also noted above, if you want this changed by default please open a discussion about it and get some other people to vote for for or against that change.