Add Ouman EH-800 heating controller integration#169733
Add Ouman EH-800 heating controller integration#169733Markus98 wants to merge 18 commits intohome-assistant:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new ouman_eh_800 integration to Home Assistant to locally poll an Ouman EH-800 heating controller via the ouman-eh-800-api library, exposing device readings through the sensor platform and a UI config flow.
Changes:
- Added a new config entry integration with config flow, coordinator-based polling, and sensor entities.
- Added translation/icon metadata, quality scale metadata, strict typing configuration, and dependency pinning.
- Added initial test coverage for config flow and config entry setup/unload.
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/ouman_eh_800/__init__.py |
Sets up/unloads the config entry, constructs the API client, and wires the coordinator + sensor platform. |
homeassistant/components/ouman_eh_800/config_flow.py |
Implements the user config flow, including URL normalization and connectivity/auth validation via login. |
homeassistant/components/ouman_eh_800/const.py |
Defines domain constants, polling interval default, and endpoint categorization sets. |
homeassistant/components/ouman_eh_800/coordinator.py |
Adds a DataUpdateCoordinator to login once, discover endpoints, and poll values on an interval. |
homeassistant/components/ouman_eh_800/entity.py |
Introduces a base coordinator entity with device info, unique IDs, and translation keys per endpoint. |
homeassistant/components/ouman_eh_800/sensor.py |
Creates sensor entities for discovered non-controllable endpoints and maps units/device classes/categories. |
homeassistant/components/ouman_eh_800/strings.json |
Adds config flow strings plus sensor entity translation keys/names. |
homeassistant/components/ouman_eh_800/icons.json |
Adds icon overrides for selected sensor entities. |
homeassistant/components/ouman_eh_800/manifest.json |
Declares the new integration, requirements, documentation link, and quality scale. |
homeassistant/components/ouman_eh_800/quality_scale.yaml |
Declares quality-scale rule status tracking for the integration. |
tests/components/ouman_eh_800/__init__.py |
Adds the test package for the new integration. |
tests/components/ouman_eh_800/conftest.py |
Provides fixtures for a mock config entry and patched API client. |
tests/components/ouman_eh_800/test_config_flow.py |
Adds config flow tests for success, URL normalization, error handling, and duplicate prevention. |
tests/components/ouman_eh_800/test_init.py |
Adds config entry setup/unload and setup error handling tests. |
requirements_all.txt |
Pins ouman-eh-800-api==0.5.0 in the global requirements set. |
mypy.ini |
Enables strict-ish mypy settings for the new integration module. |
.strict-typing |
Adds the integration to the strict-typing list. |
CODEOWNERS |
Adds a codeowner entry for the integration code directory. |
homeassistant/generated/config_flows.py |
Registers the integration domain as having a config flow (generated). |
homeassistant/generated/integrations.json |
Registers the integration metadata (generated). |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
I am wondering what steps we can do to add more context on our side. As in, I generally am not a fan of keeping entity definitions in a library as it limits the amount of context we can add.
An example:
- In the const.py there are 2 sensors which represent the names of the zones that this device can control. In theory those could be different devices, and then the name becomes the device name (as we don't really need to store a zone name into the state machine as it has very little value for automating). This way you can also easily assign zones to a different area.
- Some entities have a string state, for example the configuration type. Ideally we make sure we give that an ENUM device class and provide the possible options in
options. this way users can easily automate with it and we can translate the values
There was a problem hiding this comment.
I am wondering what steps we can do to add more context on our side. As in, I generally am not a fan of keeping entity definitions in a library as it limits the amount of context we can add.
I am just using the things already defined in the API, since those properties made sense to also have in the API. It was easy to construct entities dynamically based on the available info.
So each endpoint which maps to an entity has these defined in the API and used by the integration:
- name
- unit
- enum definitions for controllable enum endpoints
- min/max values if there is numerical control
I wanted to avoid verbosity on the integration side since that made it easy to add things on the API side and have it serve as the source of truth. I do understand that this takes a lot of the context away from the integration side though.
We could have each entity defined with an explicit entity description object, but that will become a very verbose list (24 entity descriptions for the sensor platform). I am not opposed to doing that if that is deemed the best way to do it though. I would need some opinions on what the best approach would be.
One thing to note about the name property from the API is that there are endpoints which share a name but are actually a different endpoint object in the API. An example of this would be l1_curve_0_temperature which has a different endpoint in the 3-point and 5-point temperature curve modes. This is intentional, so the two different endpoints will get mapped to the same entity in home assistant even after switching modes. This would have to be taken into account when matching entity descriptions to endpoints but shouldn't be a problem.
I can see that using the name variable directly in the integration can also be problematic, but we could just not use it if we go with the verbose entity description mapping.
In the const.py there are 2 sensors which represent the names of the zones that this device can control. In theory those could be different devices, and then the name becomes the device name (as we don't really need to store a zone name into the state machine as it has very little value for automating). This way you can also easily assign zones to a different area.
This might be a good point. We could have one main device which contains the system sensors and two sub-devices which contain the L1 and L2 circuit related sensors. This way those circuits could be assigned to different areas. I think I read somewhere that this is technically possible. Need to check.
I would argue, however, that from a user perspective, having the entity ids not have the H1/H2 circuit prefix will be a pain when working with the entity ids in yaml. So instead of having sensor.h1_supply_water_temperature and sensor.h2_supply_water_temperature, we would just get sensor.supply_water_temperature and sensor.supply_water_temperature_2 or something similar. Or is the device name reflected in the entity id as well? I need to check this as well.
Edit: the device name is reflected in the entity ID, nvm.
Some entities have a string state, for example the configuration type. Ideally we make sure we give that an ENUM device class and provide the possible options in options. this way users can easily automate with it and we can translate the values
The entities with a string state in the sensor platform are only for diagnostic entities. These are mostly not meant to be automated with since they only change when the actual configuration is changed on the physical device (not via the API). The state of the entities is just the string that the API returns. All the return values for these have not been fully mapped, which is why it's hard to give them a solid options property, unless it is acceptable that it might not be complete.
I would categorize those string sensors as follows:
- User defined strings like the names of the heating circuits
- Mappable-ish string states which are quite inconsistent in the api
- Boolean values that return
1or0 - Formatted status strings
off,off,error,onon room temp sensor installed statuses
- Boolean values that return
- Status strings that are human readable and change language based on device language settings
I am honestly open to just dropping support for these if we do not want "not clean" diagnostic sensor states. They are not required for automation, they might just help with the initial setup. Would maybe like some opinions on this.
Below is a screenshot from a community member that was helping me test the various endpoints. It has the diagnostic sensors with the actual values that the device returns and how they get displayed in the UI.
There was a problem hiding this comment.
Update:
- I split the device into three
- main ouman eh-800 device
- H1 sub-device
- H2 sub-device
- I opted to use explicit entity descriptions for all of the endpoints that will be added as entities. This gives us full transparency on what can be added with the cost of more verbosity.
- I dropped the string based diagnostic sensors, since they are of questionable value anyways.
Replaces ad-hoc string suffix stripping with URL.origin(), and surfaces unparsable input as an invalid_url field error instead of letting the ValueError propagate.
Replaces dynamic entity-attribute computation with explicit per-endpoint SensorEntityDescription objects looked up in a dict keyed by OumanEndpoint; unmapped endpoints (the unmapped string-typed diagnostics) produce no entity. The single Ouman EH-800 device is split into a main controller plus L1/L2 sub-devices linked via via_device. The H1/H2 prefix moves from entity names to the (translated) sub-device names, so entity_ids become sensor.h1_*.
Parametrizes mock_ouman_client with named scenarios (room_sensors, no_room_sensors, l1_constant_temp_relay_summer_stop, plus four relay-only variants) so the sensor snapshot test exercises every endpoint the API can return — both curve types, both room-sensor variants on each channel, ConstantTempMode (present and absent), and all 5 mutually exclusive relay registries.
|
I've now applied pretty much all of the suggestions from the review. I also added snapshot tests for the sensor platform which increases the PR lines added. CI was complaining about test coverage so it was necessary. I've also refactored the code to provide more context for us on what will be added as entities and how. This change can be reverted if the old approach was preferred. The new approach gives us full transparency on what entities are being added and with which attributes. |
The fine adjustment effect, room sensor potentiometer, and delayed outdoor temperature effect represent temperature offsets/deltas rather than absolute temperatures.
| discovery-update-info: | ||
| status: exempt | ||
| comment: Integration is local polling only, no discovery. | ||
| discovery: | ||
| status: exempt | ||
| comment: Integration is local polling only, no discovery. |
There was a problem hiding this comment.
Why no discovery? Can we discover the device via mDNS or DHCP? What's the MAC address and hostname on the network?
There was a problem hiding this comment.
The device is quite outdated with how it works on the network. It only supports setting a static IP and network mask. I doesn't support acting as a DHCP client. It doesn't advertise a hostname or MAC address on the network by itself which is why I believe autodiscovery simply isn't possible unless some form of scanning is performed.
I can look into this a bit more to verify but I believe that autodiscovery won't be possible.
There was a problem hiding this comment.
So after checking:
- No DHCP
- No mDNS
- The MAC address has the manufacture OUI
00:22:A8, but I suppose that won't be useful without DHCP or other protocols that would advertise it.
So my conclusion is that autodiscovery with HA tools is not possible.
There was a problem hiding this comment.
The OUI seems to be of the company indeed. What is the hostname? I think we could be a bit more specific with the hostname, but this OUI would already be enough to see if the device that we encounter is a device we can work with :)
| "device": { | ||
| "l1": { "name": "H1" }, | ||
| "l2": { "name": "H2" } | ||
| }, |
There was a problem hiding this comment.
Why is this translatable? What should H1 and H2 translate to?
There was a problem hiding this comment.
It's H1 / H2 in English, but L1 / L2 in Finnish. The L1 / L2 terminology is also used in code. I think it's a bit stupid as well but that's what the manufacturer decided. This translation is reflected in the device UI and manuals.
| L1BaseEndpoints.WATER_OUT_MIN_TEMP: 12.0, | ||
| L1BaseEndpoints.WATER_OUT_MAX_TEMP: 75.0, | ||
| L1BaseEndpoints.TEMPERATURE_LEVEL_STATUS_TEXT: "L1 Normaalilämpö", | ||
| L1BaseEndpoints.CIRCUIT_NAME: "Patterilämmitys", |
There was a problem hiding this comment.
Oh, didn't we want to try to put the circuit name as device names for the circuits?
There was a problem hiding this comment.
Hmm, I didn't realize that you suggested it like that. It might not be a bad idea...
The circuit name is a string that the user can define in the device UI. The problem is that most people might just leave it as default, so I wonder if that will cause confusion. Maybe we could prefix or suffix the name with H1/H2. I'll take a look what makes sense.
| L1ConstantTempMode, | ||
| RelayPumpSummerStop, | ||
| ], | ||
| # Theoretical combinations for testing remaining endpoints |
There was a problem hiding this comment.
how come these are theoretical?
There was a problem hiding this comment.
With a real device, the SystemEndpoints, L1BaseEndpoints, and some set of L1* registries will always be returned like in the realistic combinations. The theoretical combinations are there to make sure we cover all the Relay* registries which are mutually exclusive. So the theoretical combinations are missing the L1 stuff that would always be there otherwise from the real API.
We could test them with the full realistic setups, but that inflated the snapshots quite a bit and it would just be testing the same entities for the L1/L2 again. There is no cross-communication between any entities in the integration besides a coordinated refresh, so I thought it would be wasteful to test the full sets just for the relay specific registries. Ofc we can do full sets as well if we do not need to worry about saving space or CI compute.
| @pytest.fixture | ||
| def mock_ouman_client(request: pytest.FixtureRequest) -> Generator[AsyncMock]: | ||
| """Mock the Ouman EH-800 client. | ||
|
|
||
| Indirectly parametrize with a key from ``SCENARIOS`` to choose which | ||
| registry set the mocked device exposes; defaults to ``room_sensors``. | ||
| """ | ||
| scenario_id = getattr(request, "param", _DEFAULT_SCENARIO) |
There was a problem hiding this comment.
I think I would do it like:
- Create a fixture that returns a str. This one should be
@pytest.fixture(params=[".."])and then have the fixture return the param. - Have a fixture that returns the OumanRegistrySet based on the value of the scenario fixture
This way you can just automatically test all scenarios at once, except for when you @pytest.mark.parametrize("scenario", [".."]) (you can also turn it around and only do it on request)
| client = AsyncMock() | ||
| client.get_active_registries.return_value = registry_set | ||
| client.get_values.return_value = values | ||
| with ( | ||
| patch( | ||
| "homeassistant.components.ouman_eh_800.coordinator.OumanEh800Client", | ||
| return_value=client, | ||
| ), | ||
| patch( | ||
| "homeassistant.components.ouman_eh_800.config_flow.OumanEh800Client", | ||
| return_value=client, | ||
| ), | ||
| ): |
There was a problem hiding this comment.
instead look at how we do this in mealie with autospec=True. this way ytou don't have to add all methods with AsyncMock
| ) -> None: | ||
| """Test successful user config flow.""" | ||
| result = await hass.config_entries.flow.async_init( | ||
| DOMAIN, context={"source": config_entries.SOURCE_USER} |
There was a problem hiding this comment.
| DOMAIN, context={"source": config_entries.SOURCE_USER} | |
| DOMAIN, context={"source": SOURCE_USER} |
Personal ick
| result = await hass.config_entries.flow.async_configure( | ||
| result["flow_id"], USER_INPUT | ||
| ) | ||
| await hass.async_block_till_done() |
There was a problem hiding this comment.
| await hass.async_block_till_done() |
It's not a blocking operation
| @pytest.mark.usefixtures("mock_ouman_client") | ||
| async def test_user_flow_normalizes_url(hass: HomeAssistant) -> None: | ||
| """Test that URL is normalized (trailing slashes and eh800.html removed).""" | ||
| result = await hass.config_entries.flow.async_init( | ||
| DOMAIN, context={"source": config_entries.SOURCE_USER} | ||
| ) | ||
|
|
||
| result = await hass.config_entries.flow.async_configure( | ||
| result["flow_id"], | ||
| { | ||
| CONF_URL: f"{TEST_URL}/eh800.html", | ||
| CONF_USERNAME: TEST_USERNAME, | ||
| CONF_PASSWORD: TEST_PASSWORD, | ||
| }, | ||
| ) | ||
| await hass.async_block_till_done() | ||
|
|
||
| assert result["type"] is FlowResultType.CREATE_ENTRY | ||
| assert result["data"][CONF_URL] == TEST_URL |
There was a problem hiding this comment.
You could probably parametrize this one with the first test
Proposed change
Add integration for the Ouman EH-800 heating control unit. This integration is meant only for this specific device, not other devices from the manufacturer Ouman.
This initial version adds support for the
sensorplatform. It lets us monitor the readings from the device (temperatures, valve position, diagnostics, etc.). Follow-up PRs will add platforms for device control.I own the ouman-eh-800-api library used in this integration and it was designed with the Home Assistant integration in mind.
This integration was originally a custom integration which has some users (8 installed according to analytics). The API and the custom integration have been tested to work with my device and a device of a community member.
Features that will be added in follow-up PRs
This integration contains the code from the original custom integration split into smaller PRs. Some functionality is already ready and tested but will be included follow-up PRs:
numberplatform for controlling device configurationselectplatform for controlling device modesvalveplatform for manual valve controlLinks
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: