OpenTherm Gateway integration limits precision of floating point values

[FlorianOosterhof]

The problem

The "Room Temperature" sensor from the OpenTherm Gateway integration is calculated from a 16-bit value by intepreting it as a fixed-point value with 8 bits of integer part, 8 bits of fractional part, i.e. the floating point value is the 16-bit integer value divided by 256.

However, the file sensor.py explicitly formats all floating point values to have 1 significant digit (line 101 at time of writing). Since division by 256 leads to a minimum difference between values of 0.00390625. The explicit formatting loses insightful precision of at least the Room Temperature sensor.

Suggestion is to remove the explicit number of digits while formatting, or else increase it to the maximum amount of significant digits supported by the floating point type (e.g. 18 for double).

What version of Home Assistant Core has the issue?

core-2023.8.4

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

OpenTherm Gateway

Link to integration documentation on our website

https://www.home-assistant.io/integrations/opentherm_gw/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

I manually changed the python code to format to 3 significant digits on the mentioned line, restarted HA and my sensor now had more precision. So if accepted, the fix is really in that one line of code.

[home-assistant[bot]]

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

Code owner commands

Code owners of `opentherm_gw` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign opentherm_gw` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

_{^{(message by CodeOwnersMention)}}

---

opentherm_gw documentation opentherm_gw source _{^{(message by IssueLinks)}}

[mvn23]

Thank you for the report. The next overhaul of the integration will include things like native_value and suggested_display_precision as per the developer docs.

[FlorianOosterhof]

I see. I guess that would fix this problem.

Is there an expected time when this overhaul will take place/be released? If it is not in the near future, would it be a big hassle to apply this change regardless of the mentioned overhaul?

[mvn23]

The overhaul has been on my to-do list for a while, but I've been rather busy with other things lately. My current estimate is probably somewhere in the first few months of next year. Since I don't consider this particular issue to have a very high priority - I don't really see a use case for more than 0.1°C precision and most temperature sensors aren't that accurate anyway - I would rather focus on getting the overhaul done instead. I will review any PR that comes along though.

[damige]

I face this same challenge. When switching from openhab to HA i'm missing the second digit precision. For me its relevant as i'm troubleshooting issues with a thermostat over-correcting. And without this precision the cause and effect are not evident.

[FlorianOosterhof]

The overhaul has been on my to-do list for a while, but I've been rather busy with other things lately. My current estimate is probably somewhere in the first few months of next year. Since I don't consider this particular issue to have a very high priority - I don't really see a use case for more than 0.1°C precision and most temperature sensors aren't that accurate anyway - I would rather focus on getting the overhaul done instead. I will review any PR that comes along though.

I understand this does not have much priority. However, I disagree with there being no use-case for more than 0.1 degree precision, because I see one specifically for the "Room temperature"; This is a closed-loop system (i.e. there is a control loop) with a one-way correction, and for proper working of a control loop there needs to be sufficient precision so that a proper control algorithm can be created. In the case of my specific thermostat I see that the precision of the temperature sensor is much more than 0.1 degree, and even on 0.01 degree there is little jitter (noise) on the temperature signal; When the temperature is relatively stable, it always switches between 2 values, not more than that, indicating that potentially a larger precision could even be used. (However, I also see using OpenTherm Monitor that all values that the Thermostat sends to the Boiler are multiples of 1/256 that are the closest value to a multiple of 0.1, i.e. the thermostat seems to first round to 0.1, then to the nearest multiple of 1/256, meaning there is not more precision to gain for me by allowing 0.001.)

I face this same challenge. When switching from openhab to HA i'm missing the second digit precision. For me its relevant as i'm troubleshooting issues with a thermostat over-correcting. And without this precision the cause and effect are not evident.

Currently I have manually changed the running HA code (and then restarted HA) to remove the hard-coded precision truncation. However, this needs to be applied again after every HA update. This results in all temperature sensors that have more precision showing more precision, but also has a drawback: The "climate" entity still rounds to 0.1, so the history graphs when including this entity do not match up nicely with the room temperature sensor.

Starting next year (i.e. in 2 weeks) I have quite some time to try and make a proper solution for this, i.e. add the option for 0.01 degree precision in the OTGW climate entity and all (temperature) sensors in general. However, I have little experience in "proper" python development (only for small helper scripts), and no experience in HA development. So I do not know whether this process will yield any (significant) results on a short time basis.

[damige]

Would it be wrong to submit this pull request? https://github.com/damige/core/commit/3294de22eeb9863062d41ca5eb47eca16e9ab87e

I am testing this currently, it works but i'm not a programmer.

[joostlek]

Haven't really read the rest of the issue, but have you considered just returning the full value and then suggesting a precision? We can apply that for the sensors that require this.

[damige]

Haven't really read the rest of the issue, but have you considered just returning the full value and then suggesting a precision? We can apply that for the sensors that require this.

This makes sense to me. I believe that to be similar to what @mvn23 was suggesting:

Thank you for the report. The next overhaul of the integration will include things like native_value and suggested_display_precision as per the developer docs.

[FlorianOosterhof]

Would it be wrong to submit this pull request? damige@3294de2

I am testing this currently, it works but i'm not a programmer.

This is my current hacky approach to achieve temporary goals. But I dislike it because it does not reflect the same changes in the Climate entity, and I also agree with @joostlek that it would be a better approach to not limit the precision at all, and let the display precision fix it.

I guess splitting the changes in 2 PRs would make sense:

• Unlock the sensor value precision
• Allow the climate entity to use the same precision, or at least more than what is used now.

@mvn23 regarding the second, I have tried to update the code for the Climate entity to support higher precision, and it works, but the solution is not elegant due to an additional feature you have implemented, the "floor temperature" feature. Could you elaborate (or link to somewhere where you do) about this design decision? Why did you add this feature? What use does it have to you?

[joostlek]

Splitting them won't make sense as it could break automations of people, so this has to go in one PR

[FlorianOosterhof]

Splitting them won't make sense as it could break automations of people, so this has to go in one PR

Can you elaborate on what kind of scenario you see breaking by adding precision to a set of sensors but not adding precision to a climate entity at the same time?

[joostlek]

Ooh, never mind, i thought we were talking about the changes on the same platform.

Your explanation looks good

[mvn23]

* Allow the climate entity to use the same precision, or at least more than what is used now.

Not sure if this is a good idea since it may affect the UI as well. On top of that, the climate platform defaults to tenths or whole numbers depending on the unit of measurement, so it's probably best to stick with that. More precision can then still be obtained from the individual sensors if required.

@mvn23 regarding the second, I have tried to update the code for the Climate entity to support higher precision, and it works, but the solution is not elegant due to an additional feature you have implemented, the "floor temperature" feature. Could you elaborate (or link to somewhere where you do) about this design decision? Why did you add this feature? What use does it have to you?

This was introduced to mimic the room temperature on the room unit as closely as possible. Some thermostats seem to do this internally as well. It doesn't serve any other purpose.

[FlorianOosterhof]

* Allow the climate entity to use the same precision, or at least more than what is used now.

Not sure if this is a good idea since it may affect the UI as well. On top of that, the climate platform defaults to tenths or whole numbers depending on the unit of measurement, so it's probably best to stick with that. More precision can then still be obtained from the individual sensors if required.

Do you mean that you think that non-component and specifically UI code needs to be updated? Because I dont think so. I have tried this locally and it works by only adapting OTGW integration code and 1 generic piece of code which has nothing to do with UI (directly, it is the code that rounds to the required precision, and it only needs to be updated to allow more values for the "precision" argument).

Also, I dont see why adding the option to use more precision would be a bad idea. Adding options is never a bad idea, I would say, specifically if it is adding an option-option, i.e. adding an additional choice to an existing option (the 2 radio button options in the OTGW config flow)

@mvn23 regarding the second, I have tried to update the code for the Climate entity to support higher precision, and it works, but the solution is not elegant due to an additional feature you have implemented, the "floor temperature" feature. Could you elaborate (or link to somewhere where you do) about this design decision? Why did you add this feature? What use does it have to you?

This was introduced to mimic the room temperature on the room unit as closely as possible. Some thermostats seem to do this internally as well. It doesn't serve any other purpose.

Okay, but it is an existing option now, so removing it would be a breaking change. We could suggest that climate entities in general receive a "rounding mode" option, so that this non-elegant code becomes elegant again :) (at least in code, no need to update all other integrations that create a climate entity, just make those use the default round mode; "round to nearest, tie to even") However, that would be a bit of a larger change, and I can only assume it would therefore require the consent of "Home Assistant", i.e. some major contributors.

[emontnemery]

It seems there's an agreement that opentherm_gw sensors should be updated to set their native_value to an unrounded float and also set a suggested_display_precision as needed.

It's not clear to me why the opentherm_gw climate entities need the same change. @FlorianOosterhof you mention you want to track your indoor temperature with high precision, that use case should be solved if the sensor is updated?

[FlorianOosterhof]

It seems there's an agreement that opentherm_gw sensors should be updated to set their native_value to an unrounded float and also set a suggested_display_precision as needed.

It's not clear to me why the opentherm_gw climate entities need the same change. @FlorianOosterhof you mention you want to track your indoor temperature with high precision, that use case should be solved if the sensor is updated?

Yes you are right, unlocking the precision should be the solution to the problem of this ticket. I have opened a (small) PR that fixes this now (https://github.com/home-assistant/core/pull/107227)

However, with unlocked precision, I am seeing the following in History when looking at the sensor + climate entity:

And I really like how the climate entities are shown in the history, but they dont allow the same precision. I realize this is a separate issue, and I will therefore create a separate ticket for it.

However, let me explain what I tried to do with the other PR: When attempting to update the opentherm_gw code to support another value for Precision, I stumbled upon the "floor temperature" option in opentherm_gw, which is basically a duplicate of the code in display_temp, but with the floor function instead of the round function.

Since I am allergic to code duplication, I tried to mitigate this by allowing the climate entity to have a round_mode, with default value nearest, and which propagates to display_temp, so that the opentherm_gw integration can instead just set the round_mode to floor instead of having duplicated code.'

The earlier PR is the first out of 3 or 4 PRs that I would be trying to make, in the spirit of "small PRs get reviewed & merged more easily".

Anyway, I will open a new ticket for this and include the above reasoning in there, and then link the earlier PR to the new ticket.

[emontnemery]

Thanks for the explanation 👍

I think it would make sense to allow users to customize the display precision of current_temperature for climate entities, just as we allow for sensors. You can propose this in an architecture issue.

I'm not sure we need to support floor + ceil rounding modes though. Before moving forward with that, I think @mvn23 should explain why that option even exists in opentherm_gw; the solution to avoid code duplication could be to remove it from opentherm_gw instead of adding it to the helper.

[mvn23]

I wrote a small explanation here, from my side this feature can be removed without issue.

[FlorianOosterhof]

I have no objection to removing the option in principle (as I am not using it), but it does constitute a breaking change to the integration, which I have learned to avoid.

Is adding a "rounding mode" option really such a bad idea?

[emontnemery]

Is adding a "rounding mode" option really such a bad idea

The suggestion is to add new complexity to the helper, we should not add it just because it's not a bad idea, we should only add if it's actually a good idea.

Searching the code base it seems opentherm_gw is the only integration which allows the user to floor the current_temperature.

Hence, if adding a configurable display precision to climate entities makes the rounding code in opentherm_gw more complicated because of the floor option it would probably be better to remove that option from openthem_gw than to add it to the helper.

I think that change, although breaking, would be OK.

[FlorianOosterhof]

Is adding a "rounding mode" option really such a bad idea

The suggestion is to add new complexity to the helper, we should not add it just because it's not a bad idea, we should only add if it's actually a good idea.

Fair enough.

Searching the code base it seems opentherm_gw is the only integration which allows the user to floor the current_temperature.

Hence, if adding a configurable display precision to climate entities makes the rounding code in opentherm_gw more complicated because of the floor option it would probably be better to remove that option from openthem_gw than to add it to the helper.

I think that change, although breaking, would be OK.

Should this be in a separate ticket? Or as one of N PRs for the "add more precision to climate entity" ticket?

[emontnemery]

Should this be in a separate ticket? Or as one of N PRs for the "add more precision to climate entity" ticket?

I suggest this should be a feature for all climate entities, not just opentherm_gw. If you agree, start by opening a new discussion here https://github.com/home-assistant/architecture/discussions, where you propose adding more precision to climate entities and explain why it's needed.

My suggestion would be to add:

• User configuration from the UI, just like we have for numerical sensors, to override the precision to use for display of current_temperature
• Maybe also add a new property current_temperature_precision, I'm not sure this is needed though

[emontnemery]

@FlorianOosterhof can we close this issue now, or do you want to proceed with the more general climate domain changes?

[FlorianOosterhof]

@FlorianOosterhof can we close this issue now, or do you want to proceed with the more general climate domain changes?

I would say both: The problem of this ticket is resolved, so the ticket should be closed. Rhe climate problem, regardless of how generic it will be solved, should be solved under a different ticket.