Update opentherm_gw.sensor to use entity_description #124283
Conversation
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_MASTER_MEMBERID, | ||
| friendly_name_format="Thermostat Member ID {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_SLAVE_MEMBERID, | ||
| friendly_name_format="Boiler Member ID {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Those 2 sensors return the member IDs as reported by the thermostat and the boiler, respectively. From what I gather so far it seems to be an ID that the OpenTherm Association assigns to its members (equipment manufacturers).
In my setup, the thermostat reports member ID 13 (99% sure it's a Honeywell) and the boiler returns 24(Vaillant I believe), I can't verify the manufacturers right now as I won't be home for another while.
It is a value between 0 and 255, where ID 0 is reserved for a customer non-specific device. I have not been able to find a complete list of IDs and manufacturers as the OpenTherm Association is remarkably closed for an entity with a name like that...
There was a problem hiding this comment.
I think these sensors can be deprecated in the future to be replaced with a device info manufacturer. You could use logging in the library to identify new member IDs and complete the list
There was a problem hiding this comment.
What do you mean by logging? Like notifying users through HA and asking them to report their values and manufacturers?
There was a problem hiding this comment.
Yep, I do this in Withings, since they have some undocumented values, so if the value is not in the enum, I will log a message asking people to open an issue in the lib repo
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_SLAVE_OEM_FAULT, | ||
| friendly_name_format="Boiler OEM Fault Code {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Boiler manufacturers can return a manufacturer-specific fault code between 0 and 255, which will be reported in this sensor.
There was a problem hiding this comment.
Do we have a list of them? Can we do an attempt to collect all the possible values from users and then complete the manufacturer specific list and add names to them so they can be contextualized?
There was a problem hiding this comment.
As they are manufacturer-specific and hopefully rare I doubt that's feasible. It would have to be combined with the manufacturer info, basically requiring a separate list of codes to be populated for every manufacturer out there.
I haven't found a list of them for any manufacturer so far.
There was a problem hiding this comment.
Dont they have a manual with these codes?
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_OEM_DIAG, | ||
| friendly_name_format="OEM Diagnostic Code {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
This sensor reports a manufacturer-specific diagnostic/service code from the boiler. Possible values are between 0 and 65535.
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_TOTAL_BURNER_HOURS, | ||
| friendly_name_format="Total Burner Hours {}", | ||
| device_class=SensorDeviceClass.DURATION, | ||
| state_class=SensorStateClass.TOTAL, | ||
| native_unit_of_measurement=UnitOfTime.HOURS, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Does this only increase? Should this use total_increasing?
There was a problem hiding this comment.
It normally only increases, but it wraps after 65535. Some boilers allow resetting the value to 0, but support for this is not mandatory (and we don't provide a mechanism for it in HA apart from sending a raw protocol command through SERVICE_SEND_TRANSP_CMD). I chose SensorStateClass.TOTAL as per the dev docs for a value that only increases.
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_MASTER_OT_VERSION, | ||
| friendly_name_format="Thermostat OpenTherm Version {}", | ||
| suggested_display_precision=SENSOR_FLOAT_SUGGESTED_DISPLAY_PRECISION, | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_SLAVE_OT_VERSION, | ||
| friendly_name_format="Boiler OpenTherm Version {}", | ||
| suggested_display_precision=SENSOR_FLOAT_SUGGESTED_DISPLAY_PRECISION, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Why does a version have a display precision?
Are the thermostat and the boiler 2 separate devices? Should we split them up and use the device_info sw_version for this?
There was a problem hiding this comment.
The display precision is there because the version is reported in the protocol as a float. OpenTherm floats are only accurate to 1/256th, so the precision is there to make sure a value of 0.1 doesn't show as 0,1015625 or 0,09765625.
The thermostat and boiler are indeed 2 different devices, the OpenTherm Gateway sits in between them.
I could separate them into 3 devices, but as support for a lot of features is optional and differ hugely between different manufacturers and models, most of them would end up with loads of unknown values. Neither my thermostat nor my boiler report their OpenTherm Version, for example.
If it is still considered a good idea to split these devices, then I suggest to make it a separate PR as that operation will be a bit more involved.
There was a problem hiding this comment.
Oh yea definitely, I just picked this PR to leave comments because I can leave comments for every sensor. I think it would be nice to create separate devices for them, also because in the end we want to move to _attr_has_entity_name = True, which means that naming is going to change to {device} {entity name}. So if we could have a device for a boiler, thermostat and otgw, we can make it nicer and also allow the user to place the device in the right area. (as I can imagine you probably have your thermostat in the living room, but your boiler in the attic)
There was a problem hiding this comment.
Like, every comment I leave here is not a blocker, it's just asking the right questions to give hints and a bit of direction to how we can make it even better
There was a problem hiding this comment.
Sounds like the way to go is different devices. So far I have used the source info in the friendly name and entity_id to differentiate between values known by the thermostat and boiler respectively (they are not always the same), but different devices could make that separation clearer. There has actually already been some confusion about this that lead to a bug report in the past.
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_MODE, | ||
| friendly_name_format="Gateway/Monitor Mode {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
0 (Monitor mode), 1 (Gateway mode) or R (reset), the latter being a transient mode that is used to reset the OpenTherm Gateway.
I would have to dig deeper to see if this sensor could possibly have value R.
There was a problem hiding this comment.
I think this would be a good example for a refactor. So we have the ENUM device class. This sets up the sensor with a predefined list of options, which can be translated. Because the sensor has predefined options, we can use this in automations. So if you want to create an automation based on the state of an entity, we can just show a dropdown because we already know every possible state for example.
It would require a breaking change, as the value would go from 0 to monitor_mode (or monitor, whatever you prefer) and then we add the state translations in the strings.json. But since this would be breaking, please do this in a separate PR (I would recommend adding this after we sorted out device translations and potentially the split of devices)
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_MASTER_PRODUCT_TYPE, | ||
| friendly_name_format="Thermostat Product Type {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_MASTER_PRODUCT_VERSION, | ||
| friendly_name_format="Thermostat Product Version {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_SLAVE_PRODUCT_TYPE, | ||
| friendly_name_format="Boiler Product Type {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.BOILER, gw_vars.THERMOSTAT], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.DATA_SLAVE_PRODUCT_VERSION, | ||
| friendly_name_format="Boiler Product Version {}", | ||
| ), | ||
| ), | ||
| ( |
There was a problem hiding this comment.
These look static and don't belong in a sensor tbh
There was a problem hiding this comment.
They could be used to populate the device info when splitting off the thermostat and boiler into separate devices, but in my case all of these are unknown as support is not mandatory for manufacturers.
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_DHW_OVRD, | ||
| friendly_name_format="Gateway Hot Water Override Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_ABOUT, | ||
| friendly_name_format="Gateway Firmware Version {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_BUILD, | ||
| friendly_name_format="Gateway Firmware Build {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_CLOCKMHZ, | ||
| friendly_name_format="Gateway Clock Speed {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_A, | ||
| friendly_name_format="Gateway LED A Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_B, | ||
| friendly_name_format="Gateway LED B Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_C, | ||
| friendly_name_format="Gateway LED C Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_D, | ||
| friendly_name_format="Gateway LED D Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_E, | ||
| friendly_name_format="Gateway LED E Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_LED_F, | ||
| friendly_name_format="Gateway LED F Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_GPIO_A, | ||
| friendly_name_format="Gateway GPIO A Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_GPIO_B, | ||
| friendly_name_format="Gateway GPIO B Mode {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Hot Water Override Mode: 0/1 (override active in off/on mode) or A (override inactive)
Firmware version/build/clock speed: Info about the OpenTherm Gateway firmware/hardware. Could possibly be integrated in device info?
LED/GPIO modes: configured modes for the different LEDs and GPIO pins on the gateway. LED mode is one of RXTBOFHWCEMP and GPIO mode is in the range 0 through 8.
There was a problem hiding this comment.
Sounds like good examples for enum sensors as well
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_SETP_OVRD_MODE, | ||
| friendly_name_format="Gateway Room Setpoint Override Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_SMART_PWR, | ||
| friendly_name_format="Gateway Smart Power Mode {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_THRM_DETECT, | ||
| friendly_name_format="Gateway Thermostat Detection {}", | ||
| ), | ||
| ), | ||
| ( | ||
| [gw_vars.OTGW], | ||
| OpenThermSensorEntityDescription( | ||
| key=gw_vars.OTGW_VREF, | ||
| friendly_name_format="Gateway Reference Voltage Setting {}", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Room Setpoint Override Mode: One of T (Temporary Override), C (Constant/Permanent override) or N (Override disabled)
Smart Power Mode: The active smart power mode (low/medium/high)
Thermostat Detection: The thermostat detection mode in use by the gateway. Some thermostats have quirks that need to be handled, but detection is not always reliable. The active mode can be changed between C (force Remeha Celsia 20), I (force Remeha iSense), S (force standard thermostat) or D (auto-detect).
Reference voltage setting: The gateway has an adjustable reference voltage that is used to receive protocol messages. This reports that setting in a range between 0 and 9, where actual voltages depend on the PIC used in the gateway.
Proposed change
Modernize the opentherm_gw integration by using entity_description for the sensor platform
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: