iaqualink: Add basic DHCP discovery for iAquaLink devices#168256
iaqualink: Add basic DHCP discovery for iAquaLink devices#168256flz wants to merge 1 commit intohome-assistant:devfrom
Conversation
flz
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds DHCP-based discovery for the iaqualink integration by matching iAquaLink-style hostnames and starting the existing user config flow with discovery context.
Changes:
- Add DHCP matchers for
iaqualink(manifest + generated DHCP matcher list). - Implement
async_step_dhcpin the iAquaLink config flow and surface discovery details in the user step description. - Add tests covering DHCP discovery behavior and single-instance unique ID behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/iaqualink/config_flow.py |
Adds DHCP entrypoint, single-instance unique-id helper, and discovery text injection into the user step. |
homeassistant/components/iaqualink/manifest.json |
Declares DHCP discovery pattern (iaqualink-*). |
homeassistant/generated/dhcp.py |
Updates generated DHCP matcher registry for the new iaqualink hostname matcher. |
homeassistant/components/iaqualink/strings.json |
Updates the user-step description to include a {discovery} placeholder prefix. |
tests/components/iaqualink/test_config_flow.py |
Adds test coverage for DHCP-initiated flows and unique-id behavior. |
| async def _async_set_single_instance_unique_id(self) -> None: | ||
| """Assign the unique ID used by this single-instance integration.""" | ||
| await self.async_set_unique_id(DOMAIN, raise_on_progress=False) | ||
| self._abort_if_unique_id_configured(error="single_instance_allowed") |
There was a problem hiding this comment.
Add the missing single_instance_allowed abort translation key so the new abort reason can be shown to users.
_abort_if_unique_id_configured(error="single_instance_allowed") requires a corresponding entry in homeassistant/components/iaqualink/strings.json (typically mapping to [%key:common::config_flow::abort::single_instance_allowed%]), otherwise the abort reason will be untranslated / fail translation validation.
| self._abort_if_unique_id_configured(error="single_instance_allowed") | |
| self._abort_if_unique_id_configured() |
| discovery = "" | ||
| if ( | ||
| self.source == SOURCE_DHCP | ||
| and self._discovered_hostname is not None | ||
| and self._discovered_ip is not None | ||
| ): | ||
| discovery = ( | ||
| "A likely iAquaLink device was discovered on your network at " | ||
| f"{self._discovered_ip} ({self._discovered_hostname}). " | ||
| ) | ||
|
|
||
| return self.async_show_form( | ||
| step_id="user", | ||
| data_schema=CREDENTIALS_DATA_SCHEMA, | ||
| description_placeholders={"discovery": discovery}, |
There was a problem hiding this comment.
Make the DHCP discovery message translatable by moving the user-facing sentence into strings.json and only passing raw values as placeholders.
Right now the discovery text is hard-coded in English in the config flow and injected via {discovery}. Consider adding a dedicated discovery/confirm step (with description placeholders like {ip} and {hostname}) or a separate translated description key, so translations can render the message properly.
| discovery = "" | |
| if ( | |
| self.source == SOURCE_DHCP | |
| and self._discovered_hostname is not None | |
| and self._discovered_ip is not None | |
| ): | |
| discovery = ( | |
| "A likely iAquaLink device was discovered on your network at " | |
| f"{self._discovered_ip} ({self._discovered_hostname}). " | |
| ) | |
| return self.async_show_form( | |
| step_id="user", | |
| data_schema=CREDENTIALS_DATA_SCHEMA, | |
| description_placeholders={"discovery": discovery}, | |
| description_placeholders: dict[str, str] | None = None | |
| if ( | |
| self.source == SOURCE_DHCP | |
| and self._discovered_hostname is not None | |
| and self._discovered_ip is not None | |
| ): | |
| description_placeholders = { | |
| "ip": self._discovered_ip, | |
| "hostname": self._discovered_hostname, | |
| } | |
| return self.async_show_form( | |
| step_id="user", | |
| data_schema=CREDENTIALS_DATA_SCHEMA, | |
| description_placeholders=description_placeholders, |
| "name": "Jandy iAqualink", | ||
| "codeowners": ["@flz"], | ||
| "config_flow": true, | ||
| "dhcp": [{ "hostname": "iaqualink-*" }], |
There was a problem hiding this comment.
Update the PR type-of-change section to match the actual change (new DHCP discovery feature).
The PR description/checklist currently marks this as “Code quality improvements”, but the manifest adds DHCP discovery (new user-facing functionality) which should be reflected in the PR metadata for correct release note categorization.
flz
force-pushed
the
iaqualink-discovery
branch
from
baa43dd to
91ed25e
Compare
|
Unclear to me at this point why some integrations have a dhcp step defined and some don't. |
Breaking change
Proposed change
Add hostname-based DHCP discovery.
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: