Skip to content

Added guntamatic heater integration#167419

Open
JensTimmerman wants to merge 49 commits intohome-assistant:devfrom
JensTimmerman:add_guntamatic_integration
Open

Added guntamatic heater integration#167419
JensTimmerman wants to merge 49 commits intohome-assistant:devfrom
JensTimmerman:add_guntamatic_integration

Conversation

@JensTimmerman
Copy link
Copy Markdown

@JensTimmerman JensTimmerman commented Apr 5, 2026

Proposed change

Add a new integration: Guntamatic heater, this allows one to see the data of their guntamatic heater (and connected pumps, buffer vats, underfloor heating/radiators, temp sensors...) it's very usefull for other automations to be able to use this data.
It uses the guntamatic python library to talk to it, https://pypi.org/project/guntamatic/

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Bronze

  • action-setup - Service actions are registered in async_setup
  • appropriate-polling - If it's a polling integration, set an appropriate polling interval
  • brands - Has branding assets available for the integration
  • common-modules - Place common patterns in common modules
  • config-flow-test-coverage - Full test coverage for the config flow
  • config-flow - Integration needs to be able to be set up via the UI
    • Uses data_description to give context to fields
    • Uses ConfigEntry.data and ConfigEntry.options correctly
  • dependency-transparency - Dependency transparency
  • docs-actions - The documentation describes the provided service actions that can be used
  • docs-high-level-description - The documentation includes a high-level description of the integration brand, product, or service
  • docs-installation-instructions - The documentation provides step-by-step installation instructions for the integration, including, if needed, prerequisites
  • docs-removal-instructions - The documentation provides removal instructions
  • entity-event-setup - Entity events are subscribed in the correct lifecycle methods
  • entity-unique-id - Entities have a unique ID
  • has-entity-name - Entities use has_entity_name = True
  • runtime-data - Use ConfigEntry.runtime_data to store runtime data
  • test-before-configure - Test a connection in the config flow
  • test-before-setup - Check during integration initialization if we are able to set it up correctly
  • unique-config-entry - Don't allow the same device or service to be able to be set up twice

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies a diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

Copilot AI review requested due to automatic review settings April 5, 2026 00:22
Copy link
Copy Markdown
Contributor

@home-assistant home-assistant Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @JensTimmerman,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

  1. If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

  2. If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

    • If you only made a single commit you should be able to run
      git commit --amend --author="Author Name <email@address.com>"
      
      (substituting "Author Name" and "email@address.com" for your actual information) to set the authorship information.
    • If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
      1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
      2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
    • Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Apr 5, 2026

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request introduces the Guntamatic Sensor integration, which allows Home Assistant to monitor sensors from Guntamatic heaters (wood/pellet boilers). The integration uses the guntamatic Python library to fetch sensor data such as boiler temperature, buffer status, and pump states through a config flow that only requires the heater's hostname or IP address.

Changes:

  • New Guntamatic sensor integration with support for dynamic sensor discovery from the device
  • Config flow for easy setup via UI with connection validation
  • Full test coverage for the config flow including error handling
  • Proper integration setup with coordinator-based data updates
  • Branding and configuration files aligned with Home Assistant standards

Reviewed changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
homeassistant/components/guntamatic_sensor/ New integration with config flow, sensor entity definitions, and constants
tests/components/guntamatic_sensor/ Test files for config flow validation and error handling
mypy.ini Type checking configuration for the new integration
homeassistant/generated/ Auto-generated integration registry and config flow files
requirements_all.txt, requirements_test_all.txt Dependency declarations for guntamatic library
CODEOWNERS Added codeowner for the new integration
.strict-typing Added to strict typing list for the integration

Comment thread homeassistant/components/guntamatic_sensor/config_flow.py Outdated
Comment thread tests/components/guntamatic/conftest.py
Comment thread homeassistant/components/guntamatic_sensor/config_flow.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/strings.json Outdated
Comment thread homeassistant/components/guntamatic_sensor/sensor.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/sensor.py Outdated
Copilot AI review requested due to automatic review settings April 5, 2026 08:56
@JensTimmerman JensTimmerman marked this pull request as ready for review April 5, 2026 08:56
@home-assistant home-assistant Bot dismissed their stale review April 5, 2026 08:56

Stale

@JensTimmerman
Copy link
Copy Markdown
Author

I added the email adres to github, not sure why the cla-error label isn't removed, since sla-signed has been added I see?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated 4 comments.

Comment thread homeassistant/components/guntamatic_sensor/README.md Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/README.md Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
Comment thread homeassistant/components/guntamatic_sensor/__init__.py Outdated
return await hass.config_entries.async_unload_platforms(entry, _PLATFORMS)


async def async_remove_config_entry_device(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's omit this from this PR

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

)


async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str, Any]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be inlined IMO

Copy link
Copy Markdown
Author

@JensTimmerman JensTimmerman Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I try ruff always complains and wants me to create an inner function:
Is there an example of what pattern to use to do this config_flow correctly?
I looked at https://developers.home-assistant.io/docs/data_entry_flow_index/#validation and this uses an external function

  TRY301 Abstract `raise` to an inner function
    --> homeassistant/components/guntamatic_sensor/config_flow.py:55:21
     |
  53 |                 heater = Heater(user_input[CONF_HOST])
  54 |                 if not await self.hass.async_add_executor_job(heater.get_data):
  55 |                     raise ConnectionError("No data received")
     |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  56 |             except requests.exceptions.ConnectionError, ConnectionError:
  57 |                 errors["base"] = "cannot_connect"
     |


Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in a diferent way

Comment on lines +39 to +48
def _check_sensors() -> None:
current_sensors = set(coordinator.data)
new_sensors = current_sensors - known_sensors
if new_sensors:
known_sensors.update(new_sensors)
async_add_entities(
GuntamaticSensor(coordinator, name, heater.host) for name in new_sensors
)

_check_sensors()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep dynamic devices for a later PR

Copy link
Copy Markdown
Author

@JensTimmerman JensTimmerman Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire guntamatic python library is about dynamic sensors, there is no fixed list, since they could be different per device. I could remove the dynamic adding of new sensors, but there will be no difference in code lines

async def async_setup_entry(
    hass: HomeAssistant,
    entry: ConfigEntry,
    async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
    """Set up Guntamatic sensors from config entry."""

    data = entry.runtime_data
    coordinator = data.coordinator
    heater = data.heater

    # Create one entity per sensor
    sensors = [
        GuntamaticSensor(coordinator, name, heater.host) for name in coordinator.data
    ]

    async_add_entities(sensors)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on that? Like ideally we have a list of entity descriptions in HA we match with, so we can properly contextualise the entire, for example adding translations

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment I only have access to one guntamatic, it's model has 2 urls, one with pure data, and 1 with the description of the data
The data provided is stable for my specific unit, but I don't know what other values might be availabel, e.g. some models have support for automatic woodchip/pellet inpellers.
So my initial choice was not to hardcode any specific values, just query the api and return what it provides.

However it does provide some empty values for things that are not connected. e.g. the motherboard has support for 8 heating circuits pumps and 3 auxiliary pumps and always returns information for all of these (most which are always OFF since only 2 are connected in my unit)

I also recently learned from a technician that all models share the same motherboard, so they probably all return the same data.

If you say it's better to first hardcode now the most usefull values that are acutally usefull and then wait for feedback from people requesting more values I'm ok with hardcoding them all here.
For a full ist of values I get now, see https://pypi.org/project/guntamatic/

Copy link
Copy Markdown
Author

@JensTimmerman JensTimmerman Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A different problem, and initial instigator for the way I started doing this is that the 'api' also returns diferent names depending on the language the device is set in,
Mine supports ;Deutsch;English;Espaniol;Francais;Italiano;Czech;Slovenian;Hungarian;Nederlands;

So if I wanted to explicitly map these and not have them dynamic I would have to also map them in all these languages? And then again provide translation strings for all of them in all the other languages? or is there a system in place for this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will wrap around these issues in the pypi library and have a few fixed sensors here,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, made the api always return a fixed set of values, hardcoded EntityDescriptions for these values.
Translation is in place in the api now, works for english, I will add more languages later, this won't require a change in HA.

"""Initialize the sensor."""
super().__init__(coordinator)
self._name = name
self._attr_has_entity_name = True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be set outside of the constructor

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

serial = coordinator.data.get("Serial", [None])[0] or host

self._attr_unique_id = (
f"guntamatic_{serial.replace('.', '_')}_{name.replace(' ', '_')}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add the domain to the unique id

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment on lines +73 to +80
unit = coordinator.data[name][1]
self._attr_native_unit_of_measurement = unit
self._attr_device_class = SENSOR_DEVICE_CLASSES.get(name)
self._attr_state_class = SENSOR_STATE_CLASSES.get(name)

self._attr_entity_category = (
EntityCategory.DIAGNOSTIC if name in DIAGNOSTIC_SENSORS else None
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use entity descriptions instead?

Copy link
Copy Markdown
Author

@JensTimmerman JensTimmerman Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this, I would define a list of SensorEntityDescriptions and use the ones based on unit.
e.g.

UNIT_TO_DESCRIPTION: dict[str, SensorEntityDescription] = {
    "°C": SensorEntityDescription(
        key="temperature",
        device_class=SensorDeviceClass.TEMPERATURE,
        state_class=SensorStateClass.MEASUREMENT,
        native_unit_of_measurement="°C",
    ),
    "%": SensorEntityDescription(
        key="percentage",
        state_class=SensorStateClass.MEASUREMENT,
        native_unit_of_measurement="%",
    ),
    "h": SensorEntityDescription(
        key="duration_hours",
        device_class=SensorDeviceClass.DURATION,
        state_class=SensorStateClass.TOTAL_INCREASING,
        native_unit_of_measurement="h",
    ),
    "d": SensorEntityDescription(
        key="duration_days",
        device_class=SensorDeviceClass.DURATION,
        state_class=SensorStateClass.TOTAL_INCREASING,
        native_unit_of_measurement="d",
    ),
}

I noticed other integrations are doing the same; e.g. these integrations do something similar in mapping units to device_class and state_class or usint a unit to sensorEntityDescription: vicare, fronius (huge list of SensorEntityDescrpitions filling the sensors file) rainforest_eagle, rainforest_raven, swiss_hydrological_data, ...

So I was wondering, might I not try and iplement a helper function (Factory method) in SensorEntityDescription that generates the boilerplate SensorEntityDescriptons that would make most sense, e.g.
e.g.
SensorEntityDescription.from_unit("°C")
would return a SensorEntityDescipriton with device_class=SensorDeviceClass.TEMPERATURE,;
state_class=SensorStateClass.MEASUREMENT, native_unit_of_measurement="°C",

I believe lots of sensors might be able to easily use these then?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redone in a different way after making library return fixed values.

Comment on lines +66 to +67
# serial might not be set on all devices
serial = coordinator.data.get("Serial", [None])[0] or host
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A host is not a serial and a host should not be used for the device identifiers

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, made serial required in coordinator

@home-assistant home-assistant Bot marked this pull request as draft April 5, 2026 10:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated 2 comments.

Comment thread homeassistant/components/guntamatic/sensor.py
Comment thread tests/components/guntamatic/test_config_flow.py Outdated
Copy link
Copy Markdown
Member

@erwindouna erwindouna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making very nice progress here, @JensTimmerman! Some few quick points I saw. :)

Comment on lines +21 to +23
heater = Heater(entry.data[CONF_HOST])

coordinator = GuntamaticCoordinator(hass, heater, entry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
heater = Heater(entry.data[CONF_HOST])
coordinator = GuntamaticCoordinator(hass, heater, entry)
coordinator = GuntamaticCoordinator(hass, Heater(entry.data[CONF_HOST]), entry)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines +46 to +47
serial = data["serial"][0]
await self.async_set_unique_id(serial)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
serial = data["serial"][0]
await self.async_set_unique_id(serial)
await self.async_set_unique_id(data["serial"][0])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines +86 to +87
serial = data["serial"][0]
await self.async_set_unique_id(serial)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
serial = data["serial"][0]
await self.async_set_unique_id(serial)
await self.async_set_unique_id(data["serial"][0])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines +122 to +128
sensors = [
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data
]

async_add_entities(sensors)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sensors = [
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data
]
async_add_entities(sensors)
async_add_entities([
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data
])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@home-assistant home-assistant Bot marked this pull request as draft April 20, 2026 20:10
Copilot AI review requested due to automatic review settings April 20, 2026 21:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

class GuntamaticConfigFlow(ConfigFlow, domain=DOMAIN):
"""Handle a config flow for guntamatic."""

VERSION = 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VERSION = 1

Since it's new, it will default to 1. :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


if user_input is not None:
try:
heater = Heater(user_input[CONF_HOST])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be placed outside the try except, right before it. Only place parts that we're trying and might crash.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or I could inline it in the next line?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that could work, but I think keeping it out is a bit cleaner

native_unit_of_measurement=UnitOfTemperature.CELSIUS,
),
SensorEntityDescription(
key="room_0_temperature",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you explained it already, but just checking: the room 0, 1 and 2 are default property for each installation? What do they represent? :)

Copy link
Copy Markdown
Author

@JensTimmerman JensTimmerman Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are temperature sensors in different rooms in the building, typically 3 wire thermostats. They don't need to be in every installation, I also noticed mine starts counting at 1 and I only have 2.
But the guntamatic will always return these values yes. If they are not connected they show up as 60degrees C. I'm planning on filtering out sensors with these values in the library. (I already have a check that these don't get added if they are not in the output data of the library: key not in coordinator.data in the loop that creates the sensors.

There are a bunch more sensors the device returns (most of which are not relevant to me or diagnostic) I started with only these because they are relevant to me, but I will add more in the future in a separate pr (or do you want me to add them all now?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if we are going to add climate entities, I assume we are going to create one for every room, is there still a need for these entities? Again, we can split them off this PR to discuss later

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according tot he documentation A climate entity controls temperature, humidity, or fans, ...
Yes, the guntamatic controls temperature, but at the moment I have no way of adding control in Home Assistant to a guntamatic device. I would need to figure out how to do this, there is talks on the internet about getting a key from the manufacturer; but they do not respond to my requests.
So at the moment this is purely sensor readouts for me.
If in the future I find a way to control it, and have a way to be able to set values via the network I can make climate entities, but I was not planing on that in the short run.

Or would you be fine with adding climate entities that only report temperature in HA?
Same for water heater entity, just sensors at the moment.

result["flow_id"],
{CONF_HOST: "1.1.1.1"},
)
await hass.async_block_till_done()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await hass.async_block_till_done()

result["flow_id"],
{CONF_HOST: "1.1.1.1"},
)
await hass.async_block_till_done()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await hass.async_block_till_done()

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated no new comments.


async def async_setup_entry(hass: HomeAssistant, entry: GuntamaticConfigEntry) -> bool:
"""Set up guntamatic from a config entry."""
coordinator = GuntamaticCoordinator(hass, Heater(entry.data[CONF_HOST]), entry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well just make the Heater object in the coordinator

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

except NoSerialException:
return self.async_abort(reason="bad_data")

# set serial as unique id for deduplication, ip isn't a good match
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# set serial as unique id for deduplication, ip isn't a good match

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed


if user_input is not None:
try:
heater = Heater(user_input[CONF_HOST])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that could work, but I think keeping it out is a bit cleaner

Comment on lines +11 to +13
},
{
"registered_devices": true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
},
{
"registered_devices": true

Your devices don't set the mac address (yet)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I read the documentation on this wrong.

Comment on lines +22 to +25
SensorEntityDescription(
key="Status",
entity_category=EntityCategory.DIAGNOSTIC,
),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets try to snake_case them and make them an enum device class, we can also consider to remove it from the initial PR and we can discuss this in a later one

native_unit_of_measurement=UnitOfTemperature.CELSIUS,
),
SensorEntityDescription(
key="room_0_temperature",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if we are going to add climate entities, I assume we are going to create one for every room, is there still a need for these entities? Again, we can split them off this PR to discuss later

Comment on lines +122 to +126
[
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data
]
GuntamaticSensor(coordinator, description)
for description in GUNTAMATIC_SENSORS
if description.key in coordinator.data

Comment on lines +27 to +58
"name": "Boiler Temperature"
},
"buffer_bottom_temperature": {
"name": "Buffer Bottem Temperature"
},
"buffer_center_temperature": {
"name": "Buffer Center Temperature"
},
"buffer_load": {
"name": "Buffer Load"
},
"buffer_top_temperature": {
"name": "Buffer Top Temperature"
},
"domestic_home_water_temperature": {
"name": "Domestic Home Water Temperature"
},
"outdoor_temperature": {
"name": "Outdoor Temperature"
},
"program": {
"name": "Program",
"state": {
"dhw": "Domestic Hot Water",
"dhw_boost": "Domestic Hot Water Boost",
"heat": "Heat",
"hibernate": "Hibernate",
"hibernate_to": "Hibernate To",
"off": "[%key:common::state::off%]",
"timer": "Timer"
}
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Sentence case

Copy link
Copy Markdown
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to message me on Discord

Comment thread tests/components/guntamatic/conftest.py Outdated
Comment on lines +38 to +41
with patch(
"homeassistant.components.guntamatic.Heater",
autospec=True,
) as mock:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can combine this one with the config_flow one to make sure you only need a single mock, check mealie for an example

Comment on lines +51 to +54
return MockConfigEntry(
domain=DOMAIN,
data={CONF_HOST: "1.1.1.1"},
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a unique id is missing

Comment thread tests/components/guntamatic/conftest.py Outdated


@pytest.fixture
def mock_config_entry(hass: HomeAssistant) -> MockConfigEntry:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hass is unused

Comment thread tests/components/guntamatic/conftest.py Outdated
Comment on lines +42 to +45
mock.return_value.get_data = MagicMock(return_value=MOCK_DATA)
mock.return_value.parse_data = MagicMock(return_value=MOCK_PARSE_DATA)
mock.return_value.host = "1.1.1.1"
yield mock
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mock.return_value.get_data = MagicMock(return_value=MOCK_DATA)
mock.return_value.parse_data = MagicMock(return_value=MOCK_PARSE_DATA)
mock.return_value.host = "1.1.1.1"
yield mock
instance = mock.return_value
instance.get_data = MagicMock(return_value=MOCK_DATA)
instance.parse_data = MagicMock(return_value=MOCK_PARSE_DATA)
instance.host = "1.1.1.1"
yield instance

Comment on lines +38 to +40
assert result["type"] is FlowResultType.CREATE_ENTRY
assert result["title"] == "Guntamatic Heater"
assert result["data"] == {CONF_HOST: "1.1.1.1"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also assert unique id

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add a test where an entry is setup and the IP changes

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in test_dhcp_updates_ip

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider these sensor tests

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to sensor and added coordinator test

Comment on lines +39 to +48
await mock_config_entry.runtime_data.async_refresh()
await hass.async_block_till_done()

state = hass.states.get("sensor.mock_title_boiler_temperature")
assert state.state == STATE_UNAVAILABLE

# Recovery
mock_heater.return_value.parse_data.side_effect = None
mock_heater.return_value.parse_data.return_value = MOCK_PARSE_DATA
await mock_config_entry.runtime_data.async_refresh()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't touch the runtime_data to refresh, instead use the freezer to skip time and let it happen "naturally"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, I looked at other FreezeGun usages and tried to copy

Comment on lines +20 to +22
mock_config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(mock_config_entry.entry_id)
await hass.async_block_till_done()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use your method in __init__.py for this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines +50 to +60
async def test_unload_entry(
hass: HomeAssistant,
mock_config_entry: MockConfigEntry,
mock_heater: MagicMock,
) -> None:
"""Test unloading the integration."""
mock_config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(mock_config_entry.entry_id)
await hass.async_block_till_done()
assert await hass.config_entries.async_unload(mock_config_entry.entry_id)
assert mock_config_entry.state is ConfigEntryState.NOT_LOADED
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be combined with the first test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated 4 comments.

data: dict[str, list[str]] = await self.hass.async_add_executor_job(
self.heater.parse_data
)
except requests.exceptions.ConnectionError as err:
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle all request-related errors by raising UpdateFailed to avoid noisy stack traces on transient failures. _async_update_data currently only wraps requests.exceptions.ConnectionError, so other requests.exceptions.RequestException subclasses will bubble up and be logged as unexpected errors by the coordinator.

Suggested change
except requests.exceptions.ConnectionError as err:
except requests.exceptions.RequestException as err:

Copilot uses AI. Check for mistakes.
Comment thread tests/components/guntamatic/__init__.py Outdated
Comment thread tests/components/guntamatic/test_config_flow.py Outdated
Comment thread homeassistant/components/guntamatic/strings.json
Copy link
Copy Markdown
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing, can you take a look?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated 1 comment.

"name": "Boiler temperature"
},
"buffer_bottom_temperature": {
"name": "Buffer bottem temperature"
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the typo "bottem" to "bottom" in the entity name so the translated sensor name is spelled correctly.

Suggested change
"name": "Buffer bottem temperature"
"name": "Buffer bottom temperature"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated 6 comments.

Comment on lines +21 to +25
GUNTAMATIC_SENSORS: list[SensorEntityDescription] = [
SensorEntityDescription(
key="program",
translation_key="program",
device_class=SensorDeviceClass.ENUM,
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a sensor description for the heater "status" (or remove its translation key) so the integration’s declared translations match the entities actually created from coordinator data.

Copilot uses AI. Check for mistakes.
Comment thread homeassistant/components/guntamatic/coordinator.py
result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": SOURCE_USER}
)
mock_heater.parse_data.side_effect = (side_effect,)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set MagicMock.side_effect directly to an exception instance (not a 1-tuple) so parse_data actually raises and the config-flow error handling is exercised.

Suggested change
mock_heater.parse_data.side_effect = (side_effect,)
mock_heater.parse_data.side_effect = side_effect

Copilot uses AI. Check for mistakes.
mock_heater: MagicMock,
) -> None:
"""Test DHCP discovery shows confirmation form."""
mock_heater.parse_data.side_effect = (side_effect,)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set MagicMock.side_effect directly to an exception instance (not a 1-tuple) so parse_data actually raises and the DHCP error handling path is exercised.

Copilot uses AI. Check for mistakes.
Comment on lines +27 to +28
mock_heater.parse_data.side_effect = (
requests.exceptions.ConnectionError("Connection lost"),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set MagicMock.side_effect to the ConnectionError instance (not a 1-tuple) so the coordinator update actually fails and last_update_success is set correctly.

Suggested change
mock_heater.parse_data.side_effect = (
requests.exceptions.ConnectionError("Connection lost"),
mock_heater.parse_data.side_effect = requests.exceptions.ConnectionError(
"Connection lost"

Copilot uses AI. Check for mistakes.
"side_effect",
[
requests.exceptions.ConnectionError("Connection lost"),
NoSerialException,
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an exception instance for NoSerialException in the parametrized side_effects (passing the class is treated as callable and won’t be raised), otherwise this test may not cover the intended failure mode.

Suggested change
NoSerialException,
NoSerialException(),

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 23 changed files in this pull request and generated 1 comment.

Comment on lines +38 to +44
try:
data: dict[str, list[str]] = await self.hass.async_add_executor_job(
self.heater.parse_data
)
except requests.exceptions.ConnectionError as err:
raise UpdateFailed(f"Cannot connect to heater: {err}") from err
return data
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle all request/parse errors in the coordinator by raising UpdateFailed.

_async_update_data currently only wraps requests.exceptions.ConnectionError, but the same parse_data call is treated as potentially raising any requests.exceptions.RequestException (and NoSerialException) in the config flow. If those occur during polling, they will be logged as unexpected exceptions (with tracebacks) on every update. Catching requests.exceptions.RequestException (and any expected library exceptions) and re-raising as UpdateFailed will keep failures cleanly classified as communication/update problems.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants