From 11047b5fa95625bc80cdaf2bef12d7de871fe782 Mon Sep 17 00:00:00 2001 From: gdluck Date: Sat, 24 Oct 2026 16:24:06 +0300 Subject: [PATCH 1/8] feat(config_flow): identify a manually added gateway after authentication After the password is accepted ask the gateway who it is in the #420 order: `*#13**15##` first, `*#1013**1##` only on the shared code 200, and the MAC (`*#13**12##`) only from a model known to answer it, since real H4684 and MH200 daemons crash on that frame. A typed placeholder MAC or model is corrected from the reply and the unique_id re-set; the `password_required` probe answer is logged at DEBUG. Confirmed live on an F461 2.0.11. Co-Authored-By: Claude Fable 5.1 --- .github/release-notes/2.0.0b16.md | 1 + custom_components/myhome/config_flow.py | 6 + .../myhome/config_flow_discovery.py | 119 +++++++++++- tests/test_config_flow.py | 8 + tests/test_config_flow_identify.py | 170 ++++++++++++++++++ 5 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 tests/test_config_flow_identify.py diff --git a/.github/release-notes/2.0.0b16.md b/.github/release-notes/2.0.0b16.md index 196af1a42..1a8dce774 100644 --- a/.github/release-notes/2.0.0b16.md +++ b/.github/release-notes/2.0.0b16.md @@ -27,6 +27,7 @@ See the cover docs for the details and the known limits. - **Diagnostics**: the gateway profile block reports the OWNd profile's real `model_name`, `display_name`, `event_keepalive_interval` and `keepalive_summary` instead of `Generic` / 90 s for every gateway. - **Setup**: when `myhome.yaml` has gateway sections but none under this gateway's MAC, a warning names the entry MAC and the section keys that match no configured gateway (#106). +- **Setup**: a gateway added manually is asked who it is once the password is accepted (`*#13**15##`, then `*#1013**1##` on the shared code 200, the MAC only from models known to answer `*#13**12##`); a typed placeholder MAC or model is corrected from the reply. - **Expose Home Assistant to the bus**: a declarative `expose` engine projects HA entities and the clock onto free SCS addresses: virtual WHO 1 / WHO 2 actuators that a wall control can drive (mirroring HA state back to its LEDs), WHO 4 virtual temperature probes, WHO 18 virtual energy meters and the WHO 13 bus clock. Custom actions run as Home Assistant scripts (#750). - **Covers**: `class:` in `myhome.yaml` now reaches WHO 2 covers (a `curtain` is no longer shown as a shutter; #39); Venetian blind slat tilt as timed slat pulses (`slat_tilt`, `slat_time`) (#508); impulse garage and gate covers on WHO 1 relays with optional safety approval, door-entry strike locks and doorbell events (#667); native broadcast for general covers behind an interface (#679); a cover stop overtakes other queued frames (#694); cover kinematics split into pluggable strategies (#739). - **Lighting**: 2-digit Area 0 point addresses `01`–`09` (#713); ON dimmers on a fresh install are probed for brightness support (#742); timed turn-on uses the OWNd builders (#659); lights the startup sweep already reported are not polled again (#701); area and group re-syncs only skip once every member has echoed, counting only the members' own echoes in the commanded state (#740, #749). diff --git a/custom_components/myhome/config_flow.py b/custom_components/myhome/config_flow.py index 44496c4d5..9aa3bd29e 100644 --- a/custom_components/myhome/config_flow.py +++ b/custom_components/myhome/config_flow.py @@ -31,6 +31,7 @@ ConfigFlowDiscoveryMixin, MACAddress, _get_serial_ports, + identify_gateway, ) from .config_flow_expose import ExposeSubentryFlowHandler from .config_flow_options import MyhomeOptionsFlowHandler @@ -73,6 +74,8 @@ def __init__(self): # type: ignore self.gateway_handler: Optional[OWNGateway] = None self.discovered_gateways: Optional[Dict[str, dict[str, typing.Any]]] = None self._existing_entry: ConfigEntry | None = None + # Set by the manual step: the MAC and model were typed, not discovered. + self._identify_after_auth = False # (title, data, options) of an entry held back by the bus topology step self._pending_entry: ( tuple[str, dict[str, typing.Any], dict[str, typing.Any]] | None @@ -304,6 +307,8 @@ async def async_step_custom_manual(self, user_input=None, errors=None): # type: self.gateway_handler = OWNGateway(user_input) await self.async_set_unique_id(user_input["serialNumber"], raise_on_progress=False) self._abort_if_unique_id_configured() + # Typed by hand: once authenticated, ask the gateway for its MAC and model. + self._identify_after_auth = True return await self.async_step_test_connection() address_val = getattr(self, "_custom_address", "192.168.1.100") @@ -353,4 +358,5 @@ async def async_step_custom_manual(self, user_input=None, errors=None): # type: "command_session_default", "find_gateways", "get_gateway", + "identify_gateway", ] diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index b05eca504..32a314f16 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -1,6 +1,8 @@ """SSDP discovery, reconfigure, reauth, connection test, and authentication steps for MyHome config flow (#732).""" from __future__ import annotations +import asyncio +import contextlib import ipaddress import re import typing @@ -17,6 +19,9 @@ ) from homeassistant.helpers import config_validation as cv from homeassistant.helpers import device_registry as dr +from OWNd.connection import OWNCommandSession +from OWNd.message import OWNGatewayCommand, OWNGatewayDiagnosticCommand +from OWNd.profiles import get_gateway_profile from voluptuous import All, Coerce, In, Range, Required from .const import ( @@ -33,6 +38,7 @@ LOGGER, ) from .gateway import command_session_default +from .identity import read_who13, read_who1013 from .typing_compat import flow_schema TEST_CONNECTION_ABORT_REASONS = frozenset( @@ -48,6 +54,85 @@ } ) TEST_CONNECTION_RETRY_DELAY: float = 1.5 +IDENTIFY_TIMEOUT: float = 15.0 +# Models a physical unit answered *#13**12## with its real MAC (F461 2.0.11, +# MyHomeServer1 3.87.13, both 2026-10-24). Real H4684 020054 and MH200 020102 +# daemons crash on the request, and the absence of a crash on an emulated daemon +# is no proof of safety, so an unknown or unlisted model is never asked. +MAC_REQUEST_SAFE_MODELS: frozenset[str] = frozenset({"F461", "MYHOMESERVER1"}) + + +# WHO=13 dimension 15 answer shared by the F454 / F461 / MyHomeServer1 class, the +# only case in which the runtime asks WHO=1013 as well (#420). +WHO13_SHARED_CODE = "200" + + +def mac_request_is_safe(who13_code: str | None, who1013_code: str | None = None) -> bool: + """Whether every model the reported codes can name is known to answer *#13**12##. + + The WHO=1013 code decides when present (it is only asked on the shared code 200); + otherwise the WHO=13 device type does. + """ + if who1013_code: + models = read_who1013(who1013_code).models + elif who13_code and who13_code != WHO13_SHARED_CODE: + models = read_who13(who13_code).models + else: + return False + normalised = {m.strip().upper().replace(" ", "").replace("-", "").replace("_", "") for m in models} + return bool(normalised) and normalised <= MAC_REQUEST_SAFE_MODELS + + +async def identify_gateway(gateway: typing.Any) -> tuple[str | None, str | None, str | None]: + """Ask an authenticated gateway who it is: ``(mac, who13_code, who1013_code)``. + + The order follows the runtime identification (#420): ``*#13**15##`` first, which + every gateway answers; ``*#1013**1##`` only when that answer is the shared code + ``200`` (F454 / F461 / MyHomeServer1 class), keeping older gateways away from a + diagnostic frame they may not support; ``*#13**12##`` (MAC) only from a model known + to answer it (``MAC_REQUEST_SAFE_MODELS``): it crashes a daemon on the H4684 and the + MH200 in the firmware oracle. Best effort: any failure or silence yields ``None``. + """ + mac: str | None = None + code13: str | None = None + code1013: str | None = None + session = OWNCommandSession(gateway=gateway, logger=LOGGER) + + async def _ask(request: typing.Any) -> list[typing.Any]: + replies = await session.send(request, is_status_request=True) + return replies if isinstance(replies, list) else [] + + try: + async with asyncio.timeout(IDENTIFY_TIMEOUT): + res = await session.connect() + if not (isinstance(res, dict) and res.get("Success")): + return None, None, None + for reply in await _ask(OWNGatewayCommand.request_device_type()): + value = getattr(reply, "device_type_code", None) + if getattr(reply, "who", None) == 13 and value: + code13 = code13 or str(value) + if code13 == WHO13_SHARED_CODE: + for reply in await _ask(OWNGatewayDiagnosticCommand.request_diagnostic_object_model()): + if getattr(reply, "who", None) == 1013 and getattr(reply, "object_model", None): + code1013 = code1013 or str(reply.object_model) + if mac_request_is_safe(code13, code1013): + for reply in await _ask(OWNGatewayCommand.request_mac_address()): + mac = mac or getattr(reply, "mac_address", None) + else: + LOGGER.debug( + "Gateway %s reports WHO=13 device type %s / WHO=1013 model %s; not asking its MAC " + "(*#13**12##), which is only known to be answered by %s", + getattr(gateway, "address", None), + code13, + code1013, + ", ".join(sorted(MAC_REQUEST_SAFE_MODELS)), + ) + except Exception as err: # pylint: disable=broad-except + LOGGER.debug("Gateway %s identification failed: %r", getattr(gateway, "address", None), err) + finally: + with contextlib.suppress(Exception): + await session.close() + return mac, code13, code1013 class MACAddress: @@ -368,6 +453,10 @@ async def _run_test_connection() -> dict[str, typing.Any]: reason="reauth_successful", ) + if getattr(self, "_identify_after_auth", False): + self._identify_after_auth = False + await self._async_apply_gateway_identity(gateway) + _new_entry_data = { CONF_ID: dr.format_mac(gateway.serial), CONF_HOST: gateway.address, @@ -403,7 +492,8 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) else: msg = test_result.get("Message") - LOGGER.warning( + # password_required is the normal first answer of a password-protected gateway. + (LOGGER.debug if msg == "password_required" else LOGGER.warning)( "Gateway %s (%s:%s) test connection failed: %s", gateway.model_name, gateway.address, @@ -423,6 +513,33 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) return self.async_abort(reason=abort_reason) # type: ignore[attr-defined] + async def _async_apply_gateway_identity(self, gateway: typing.Any) -> None: + """Correct the MAC and model typed in the manual step from what the gateway reports.""" + from . import config_flow as _cf + + mac, code13, code = await _cf.identify_gateway(gateway) + reported_mac = dr.format_mac(mac) if mac else None + if reported_mac and reported_mac != dr.format_mac(str(gateway.serial)): + LOGGER.info( + "Gateway at %s reports MAC %s; using it instead of the entered %s", + gateway.address, + reported_mac, + gateway.serial, + ) + gateway.serial = reported_mac + await self.async_set_unique_id(reported_mac, raise_on_progress=False) # type: ignore[attr-defined] + self._abort_if_unique_id_configured() # type: ignore[attr-defined] + reading = read_who1013(code) if code else (read_who13(code13) if code13 and code13 != WHO13_SHARED_CODE else None) + if reading is not None and reading.known and reading.canonical != gateway.model_name: + LOGGER.info( + "Gateway at %s reports %s; using it instead of the selected model %s", + gateway.address, + reading.describe(), + gateway.model_name, + ) + gateway.model_name = gateway.model = reading.canonical + gateway.profile = get_gateway_profile(gateway.model_name, gateway.model_number) + async def async_step_port(self, user_input=None, errors=None): # type: ignore """Port information for the gateway is missing.""" if errors is None: diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index c92f51f89..dc96ba8af 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -512,6 +512,10 @@ async def test_password_required_and_error(hass: HomeAssistant) -> None: {"Success": True}, ], ), + patch( + "custom_components.myhome.config_flow.identify_gateway", + AsyncMock(return_value=(None, None, None)), + ), patch( "custom_components.myhome.async_setup_entry", return_value=True, @@ -906,6 +910,10 @@ async def test_custom_manual_entry_manufacturer_type(hass: HomeAssistant) -> Non "custom_components.myhome.config_flow.OWNSession.test_connection", return_value={"Success": True}, ), + patch( + "custom_components.myhome.config_flow.identify_gateway", + AsyncMock(return_value=(None, None, None)), + ), patch( "custom_components.myhome.async_setup_entry", return_value=True, diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py new file mode 100644 index 000000000..26b2bc8e0 --- /dev/null +++ b/tests/test_config_flow_identify.py @@ -0,0 +1,170 @@ +"""The manual config flow asks the authenticated gateway for its MAC and model (live F461 replies).""" +import logging +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from homeassistant import config_entries +from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType +from OWNd.message import OWNMessage + +from custom_components.myhome.const import DOMAIN + +F461_MAC_REPLY = "*#13**12*0*3*80*0*7*119##" +F461_MODEL_REPLY = "*#1013**1*134*15*5*0##" + + +async def _manual_flow(hass: HomeAssistant, test_connection: AsyncMock, identify: AsyncMock): + with ( + patch("custom_components.myhome.config_flow.find_gateways", return_value=[]), + patch("custom_components.myhome.config_flow.get_gateway", return_value=None), + patch("custom_components.myhome.config_flow.OWNSession.test_connection", test_connection), + patch("custom_components.myhome.config_flow.identify_gateway", identify), + patch("custom_components.myhome.async_setup_entry", return_value=True), + ): + 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"], {"serial": "00:00:00:00:00:00"}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {"address": "192.0.2.92", "port": 20000} + ) + assert result["step_id"] == "custom_manual" + # The defaults the form offers when the user does not know better + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {"serialNumber": "00:03:50:00:00:00", "modelName": "MyHomeServer1"} + ) + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "password" + result = await hass.config_entries.flow.async_configure(result["flow_id"], {"password": "12345"}) + await hass.async_block_till_done() + return result + + +async def test_manual_flow_corrects_mac_and_model_from_the_gateway(hass: HomeAssistant, caplog) -> None: + test_connection = AsyncMock( + side_effect=[{"Success": False, "Message": "password_required"}, {"Success": True}] + ) + identify = AsyncMock(return_value=("00:03:50:00:07:77", "200", "134")) + + with caplog.at_level(logging.DEBUG, logger="custom_components.myhome"): + result = await _manual_flow(hass, test_connection, identify) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["mac"] == "00:03:50:00:07:77" + assert result["data"]["name"] == "F461" + assert result["title"] == "F461 Gateway" + assert result["result"].unique_id == "00:03:50:00:07:77" + identify.assert_awaited_once() + # The password probe is the normal path, not a failure worth a WARNING + assert not [ + r for r in caplog.records if r.levelno >= logging.WARNING and "password_required" in r.getMessage() + ] + + +async def test_manual_flow_keeps_the_answers_when_the_gateway_says_nothing(hass: HomeAssistant) -> None: + test_connection = AsyncMock( + side_effect=[{"Success": False, "Message": "password_required"}, {"Success": True}] + ) + identify = AsyncMock(return_value=(None, None, None)) + + result = await _manual_flow(hass, test_connection, identify) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["mac"] == "00:03:50:00:00:00" + assert result["data"]["name"] == "MyHomeServer1" + + +async def test_manual_flow_aborts_when_the_real_mac_is_already_configured(hass: HomeAssistant) -> None: + from pytest_homeassistant_custom_component.common import MockConfigEntry + + MockConfigEntry( + domain=DOMAIN, data={"host": "192.0.2.92", "mac": "00:03:50:00:07:77"}, unique_id="00:03:50:00:07:77" + ).add_to_hass(hass) + test_connection = AsyncMock( + side_effect=[{"Success": False, "Message": "password_required"}, {"Success": True}] + ) + identify = AsyncMock(return_value=("00:03:50:00:07:77", "200", "134")) + + result = await _manual_flow(hass, test_connection, identify) + + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "already_configured" + + +async def test_identify_gateway_reads_the_f461_replies() -> None: + """identify_gateway asks the model first, then the MAC (an F461 answers both), and decodes the live replies.""" + from custom_components.myhome import config_flow_discovery as cfd + + session = MagicMock() + session.connect = AsyncMock(return_value={"Success": True}) + replies = { + "*#13**15##": [OWNMessage.parse("*#13**15*200##")], + "*#13**12##": [OWNMessage.parse(F461_MAC_REPLY)], + "*#1013**1##": [OWNMessage.parse(F461_MODEL_REPLY)], + } + session.send = AsyncMock(side_effect=lambda message, is_status_request=False: replies[str(message)]) + session.close = AsyncMock() + + with patch.object(cfd, "OWNCommandSession", return_value=session): + mac, code13, code = await cfd.identify_gateway(MagicMock()) + + assert (mac, code13, code) == ("00:03:50:00:07:77", "200", "134") + assert [str(c.args[0]) for c in session.send.await_args_list] == ["*#13**15##", "*#1013**1##", "*#13**12##"] + session.close.assert_awaited() + + +@pytest.mark.parametrize(("model", "who13"), [("H4684", "13"), ("MH200", "4")]) +async def test_identify_gateway_never_asks_the_mac_from_a_gateway_that_crashes_on_it(model, who13) -> None: + """*#13**12## crashes a daemon on the H4684 and MH200 (real-binary oracle run): only *#13**15## goes out. + + Their WHO=13 device type is not the shared 200, so *#1013**1## is not asked either (#420). + """ + from custom_components.myhome import config_flow_discovery as cfd + + session = MagicMock() + session.connect = AsyncMock(return_value={"Success": True}) + replies = {"*#13**15##": [OWNMessage.parse(f"*#13**15*{who13}##")]} + session.send = AsyncMock(side_effect=lambda message, is_status_request=False: replies[str(message)]) + session.close = AsyncMock() + + with patch.object(cfd, "OWNCommandSession", return_value=session): + mac, code13, code = await cfd.identify_gateway(MagicMock()) + + assert (mac, code13, code) == (None, who13, None) + assert [str(c.args[0]) for c in session.send.await_args_list] == ["*#13**15##"] + + +def test_mac_request_is_safe_only_for_models_that_answered_it() -> None: + """Only models a physical unit answered with a real MAC (F461 2.0.11, MyHomeServer1 3.87.13).""" + from custom_components.myhome import config_flow_discovery as cfd + + assert cfd.mac_request_is_safe("200", "134") is True # F461 + assert cfd.mac_request_is_safe("200", "67") is True # MyHomeServer1 + assert cfd.mac_request_is_safe("200", "51") is False # F454: emulated answer only, no live MAC capture + assert cfd.mac_request_is_safe("200", None) is False # shared code, model unknown + assert cfd.mac_request_is_safe(None, None) is False + assert cfd.mac_request_is_safe("13", None) is False # H4684 by its WHO=13 device type + assert cfd.mac_request_is_safe("999999", None) is False # unknown code: not asked + + +async def test_identify_gateway_survives_a_refused_session() -> None: + from custom_components.myhome import config_flow_discovery as cfd + + session = MagicMock() + session.connect = AsyncMock(side_effect=OSError("refused")) + session.close = AsyncMock() + with patch.object(cfd, "OWNCommandSession", return_value=session): + assert await cfd.identify_gateway(MagicMock()) == (None, None, None) + + +async def test_identify_gateway_gives_up_when_the_session_is_not_accepted() -> None: + """A connect that does not report Success asks nothing and yields no identity.""" + from custom_components.myhome import config_flow_discovery as cfd + + session = MagicMock() + session.connect = AsyncMock(return_value={"Success": False, "Message": "password_error"}) + session.send = AsyncMock() + session.close = AsyncMock() + with patch.object(cfd, "OWNCommandSession", return_value=session): + assert await cfd.identify_gateway(MagicMock()) == (None, None, None) + session.send.assert_not_awaited() + session.close.assert_awaited() From 7864ba6e8e09cb75639f56b5ee65951bd2516cdd Mon Sep 17 00:00:00 2001 From: gdluck Date: Sat, 24 Oct 2026 23:50:10 +0300 Subject: [PATCH 2/8] fix(config_flow): resolve the identified model through resolve_gateway_identity The manual flow re-implemented the model choice inline and so broke three invariants the runtime resolver keeps: a brand variant of the reported model (H4890 / LN4890 on WHO 1013 code 30, 003598 on 51) was overwritten by the canonical name, an uncertain Nmap WHO 13 code could override an explicit choice, and the entry never recorded `model_source`, so diagnostics reported `manual` for a corrected model. The flow now hands the typed model (none for Generic / Gateway / Unknown), the WHO 13 code and the WHO 1013 code to `resolve_gateway_identity()`, persists `resolution.source` as `model_source` and refreshes the OWNd profile and log id with the model. Review follow-up (review 5639869140, point 1). Co-Authored-By: Claude Fable 5.1 --- .../myhome/config_flow_discovery.py | 52 +++++++++-- custom_components/myhome/identity.py | 5 +- tests/test_config_flow_identify.py | 88 ++++++++++++++++++- 3 files changed, 132 insertions(+), 13 deletions(-) diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index 32a314f16..9b54b2afb 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -38,7 +38,14 @@ LOGGER, ) from .gateway import command_session_default -from .identity import read_who13, read_who1013 +from .identity import ( + UNTRUSTED_MODEL_LABELS, + CodeReading, + GatewayIdentityEvidence, + read_who13, + read_who1013, + resolve_gateway_identity, +) from .typing_compat import flow_schema TEST_CONNECTION_ABORT_REASONS = frozenset( @@ -83,6 +90,13 @@ def mac_request_is_safe(who13_code: str | None, who1013_code: str | None = None) return bool(normalised) and normalised <= MAC_REQUEST_SAFE_MODELS +def _inband_reading(who13_code: str | None, who1013_code: str | None) -> CodeReading: + """The code reading the resolver labelled the gateway from: WHO=1013 when known, else WHO=13.""" + if who1013_code and read_who1013(who1013_code).known: + return read_who1013(who1013_code) + return read_who13(who13_code or "") + + async def identify_gateway(gateway: typing.Any) -> tuple[str | None, str | None, str | None]: """Ask an authenticated gateway who it is: ``(mac, who13_code, who1013_code)``. @@ -453,9 +467,10 @@ async def _run_test_connection() -> dict[str, typing.Any]: reason="reauth_successful", ) + model_source: str | None = None if getattr(self, "_identify_after_auth", False): self._identify_after_auth = False - await self._async_apply_gateway_identity(gateway) + model_source = await self._async_apply_gateway_identity(gateway) _new_entry_data = { CONF_ID: dr.format_mac(gateway.serial), @@ -473,6 +488,8 @@ async def _run_test_connection() -> dict[str, typing.Any]: CONF_MAC: dr.format_mac(gateway.serial), CONF_UDN: gateway.udn, } + if model_source: + _new_entry_data["model_source"] = model_source _new_entry_options = { CONF_WORKER_COUNT: command_session_default(gateway.model_name), } @@ -513,8 +530,14 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) return self.async_abort(reason=abort_reason) # type: ignore[attr-defined] - async def _async_apply_gateway_identity(self, gateway: typing.Any) -> None: - """Correct the MAC and model typed in the manual step from what the gateway reports.""" + async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str: + """Correct the MAC and model typed in the manual step from what the gateway reports. + + The model follows ``resolve_gateway_identity``, the same verdict the runtime + reaches: a brand variant of the reported model is kept, an uncertain WHO=13 code + may label a ``Generic`` entry but never overrides a chosen model, a certain code + corrects it. Returns the source the model rests on, for the entry's ``model_source``. + """ from . import config_flow as _cf mac, code13, code = await _cf.identify_gateway(gateway) @@ -529,16 +552,27 @@ async def _async_apply_gateway_identity(self, gateway: typing.Any) -> None: gateway.serial = reported_mac await self.async_set_unique_id(reported_mac, raise_on_progress=False) # type: ignore[attr-defined] self._abort_if_unique_id_configured() # type: ignore[attr-defined] - reading = read_who1013(code) if code else (read_who13(code13) if code13 and code13 != WHO13_SHARED_CODE else None) - if reading is not None and reading.known and reading.canonical != gateway.model_name: + configured = str(gateway.model_name or "").strip() + resolution = resolve_gateway_identity( + GatewayIdentityEvidence( + manual=None if configured.lower() in UNTRUSTED_MODEL_LABELS else configured, + who13_code=code13, + who1013_code=code, + ) + ) + model = resolution.model + if model and model != configured: + reading = resolution.corrected_reading or _inband_reading(code13, code) LOGGER.info( "Gateway at %s reports %s; using it instead of the selected model %s", gateway.address, reading.describe(), - gateway.model_name, + configured, ) - gateway.model_name = gateway.model = reading.canonical - gateway.profile = get_gateway_profile(gateway.model_name, gateway.model_number) + gateway.model_name = gateway.model = model + gateway.profile = get_gateway_profile(model, gateway.model_number) + gateway._log_id = f"[{model} gateway - {gateway.host}]" # pylint: disable=protected-access + return resolution.source async def async_step_port(self, user_input=None, errors=None): # type: ignore """Port information for the gateway is missing.""" diff --git a/custom_components/myhome/identity.py b/custom_components/myhome/identity.py index 6a7b9a859..eb5cc9576 100644 --- a/custom_components/myhome/identity.py +++ b/custom_components/myhome/identity.py @@ -54,6 +54,9 @@ gateway_model_family, ) +# Labels that name no model: an entry carrying one is labelled from the in-band evidence. +UNTRUSTED_MODEL_LABELS: frozenset[str] = frozenset({"", "generic", "gateway", "unknown"}) + def _normalized(model: str | None) -> str: """Compare model names without case, spaces, dashes or underscores.""" @@ -327,7 +330,7 @@ def identification_source(self) -> str: if model_source == IDENTIFICATION_MANUAL: return IDENTIFICATION_MANUAL model = data.get(CONF_NAME) - if model and str(model).strip().lower() not in ("", "generic", "gateway", "unknown"): + if model and str(model).strip().lower() not in UNTRUSTED_MODEL_LABELS: return IDENTIFICATION_MANUAL return IDENTIFICATION_UNKNOWN diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index 26b2bc8e0..ea825cb73 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -14,7 +14,18 @@ F461_MODEL_REPLY = "*#1013**1*134*15*5*0##" -async def _manual_flow(hass: HomeAssistant, test_connection: AsyncMock, identify: AsyncMock): +def _password_then_ok() -> AsyncMock: + """A password-protected gateway: the first probe asks for the password, the second accepts it.""" + return AsyncMock(side_effect=[{"Success": False, "Message": "password_required"}, {"Success": True}]) + + +async def _manual_flow( + hass: HomeAssistant, + test_connection: AsyncMock, + identify: AsyncMock, + mac: str = "00:03:50:00:00:00", + model: str = "MyHomeServer1", +): with ( patch("custom_components.myhome.config_flow.find_gateways", return_value=[]), patch("custom_components.myhome.config_flow.get_gateway", return_value=None), @@ -28,9 +39,9 @@ async def _manual_flow(hass: HomeAssistant, test_connection: AsyncMock, identify result["flow_id"], {"address": "192.0.2.92", "port": 20000} ) assert result["step_id"] == "custom_manual" - # The defaults the form offers when the user does not know better + # By default the values the form offers when the user does not know better result = await hass.config_entries.flow.async_configure( - result["flow_id"], {"serialNumber": "00:03:50:00:00:00", "modelName": "MyHomeServer1"} + result["flow_id"], {"serialNumber": mac, "modelName": model} ) assert result["type"] == FlowResultType.FORM assert result["step_id"] == "password" @@ -53,6 +64,7 @@ async def test_manual_flow_corrects_mac_and_model_from_the_gateway(hass: HomeAss assert result["data"]["name"] == "F461" assert result["title"] == "F461 Gateway" assert result["result"].unique_id == "00:03:50:00:07:77" + assert result["data"]["model_source"] == "who13" identify.assert_awaited_once() # The password probe is the normal path, not a failure worth a WARNING assert not [ @@ -71,6 +83,76 @@ async def test_manual_flow_keeps_the_answers_when_the_gateway_says_nothing(hass: assert result["type"] == FlowResultType.CREATE_ENTRY assert result["data"]["mac"] == "00:03:50:00:00:00" assert result["data"]["name"] == "MyHomeServer1" + assert result["data"]["model_source"] == "manual" + + +@pytest.mark.parametrize("model", ["H4890", "LN4890"]) +async def test_manual_flow_keeps_a_compatible_brand_variant(hass: HomeAssistant, model: str) -> None: + """WHO=1013 code 30 names the AM4890 and its brand variants: the chosen variant stays (#466).""" + identify = AsyncMock(return_value=(None, "200", "30")) + + result = await _manual_flow(hass, _password_then_ok(), identify, model=model) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["name"] == model + assert result["title"] == f"{model} Gateway" + assert result["data"]["model_source"] == "manual" + + +async def test_manual_flow_keeps_a_legrand_name_of_the_reported_model(hass: HomeAssistant) -> None: + """003598 is the Legrand name of the F454 (WHO=1013 code 51): not a correction.""" + identify = AsyncMock(return_value=(None, "200", "51")) + + result = await _manual_flow(hass, _password_then_ok(), identify, model="003598") + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["name"] == "003598" + assert result["data"]["model_source"] == "manual" + + +@pytest.mark.parametrize( + ("model", "codes", "expected_name", "expected_source"), + [ + ("Generic", ("44", None), "MH200N", "who13"), + ("MH202", ("44", None), "MH202", "manual"), + ("Generic", ("200", "134"), "F461", "who13"), + ], +) +async def test_manual_flow_uncertain_who13_code_labels_generic_but_never_overrides_a_choice( + hass: HomeAssistant, model: str, codes: tuple[str | None, str | None], expected_name: str, expected_source: str +) -> None: + """Code 44 is known from Nmap only (certain=False): it may label a Generic entry, not contradict a chosen model; + a Generic entry is labelled from WHO=1013 as well.""" + identify = AsyncMock(return_value=(None, *codes)) + + result = await _manual_flow(hass, _password_then_ok(), identify, model=model) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["name"] == expected_name + assert result["title"] == f"{expected_name} Gateway" + assert result["data"]["model_source"] == expected_source + + +async def test_apply_gateway_identity_refreshes_the_profile_and_log_id(hass: HomeAssistant) -> None: + """The corrected model drives the OWNd profile and the log prefix, as the runtime's _apply_model does.""" + from OWNd.connection import OWNGateway + + from custom_components.myhome.config_flow import MyhomeFlowHandler + + flow = MyhomeFlowHandler() + flow.hass = hass + gateway = OWNGateway( + {"address": "192.0.2.92", "port": 20000, "serialNumber": "00:03:50:00:00:00", "modelName": "MyHomeServer1"} + ) + with patch( + "custom_components.myhome.config_flow.identify_gateway", AsyncMock(return_value=(None, "200", "134")) + ): + source = await flow._async_apply_gateway_identity(gateway) + + assert source == "who13" + assert gateway.model_name == "F461" + assert type(gateway.profile).__name__ == "F461Profile" + assert gateway.log_id == "[F461 gateway - 192.0.2.92]" async def test_manual_flow_aborts_when_the_real_mac_is_already_configured(hass: HomeAssistant) -> None: From 6a8dfde371fa4ad23908e6557a19a1c54ce6d24f Mon Sep 17 00:00:00 2001 From: gdluck Date: Sat, 24 Oct 2026 23:51:18 +0300 Subject: [PATCH 3/8] fix(config_flow): validate a reported MAC before it replaces the typed one OWNd formats any six integers of `*#13**12*a*b*c*d*e*f##` without a range check, so an octet above 255 yields a 13-digit string, a factory configuration answers six zeros (`00:00:00:00:00:00`, also the sentinel of the first form's "Custom (IP Gateway)" choice) and nothing stops a broadcast address. Such a value replaced a real MAC the user typed and became the unique_id. The reply now has to pass the form's `MACAddress` validation and must not be the all-zero or broadcast address; otherwise the typed MAC stays and the reply is logged at DEBUG. Review follow-up (review 5639869140, point 2). Co-Authored-By: Claude Fable 5.1 --- .../myhome/config_flow_discovery.py | 28 +++++++++++++- tests/test_config_flow_identify.py | 38 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index 9b54b2afb..0e2426d80 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -67,6 +67,9 @@ # daemons crash on the request, and the absence of a crash on an emulated daemon # is no proof of safety, so an unknown or unlisted model is never asked. MAC_REQUEST_SAFE_MODELS: frozenset[str] = frozenset({"F461", "MYHOMESERVER1"}) +# A reported MAC that names no interface: the all-zero factory value (also the +# "Custom (IP Gateway)" sentinel of the first form) and the broadcast address. +UNUSABLE_MACS: frozenset[str] = frozenset({"00:00:00:00:00:00", "ff:ff:ff:ff:ff:ff"}) # WHO=13 dimension 15 answer shared by the F454 / F461 / MyHomeServer1 class, the @@ -90,6 +93,22 @@ def mac_request_is_safe(who13_code: str | None, who1013_code: str | None = None) return bool(normalised) and normalised <= MAC_REQUEST_SAFE_MODELS +def validated_reported_mac(mac: typing.Any) -> str | None: + """The MAC a gateway reported, normalised, or ``None`` when it cannot name the gateway. + + OWNd formats any six integers of ``*#13**12*a*b*c*d*e*f##`` without range checks, + so an octet above 255 yields a 13-digit string, and a factory configuration answers + six zeros; neither may replace the MAC the user typed. + """ + if not mac: + return None + try: + formatted = dr.format_mac(str(MACAddress(str(mac)))) + except ValueError: + return None + return None if formatted in UNUSABLE_MACS else formatted + + def _inband_reading(who13_code: str | None, who1013_code: str | None) -> CodeReading: """The code reading the resolver labelled the gateway from: WHO=1013 when known, else WHO=13.""" if who1013_code and read_who1013(who1013_code).known: @@ -541,7 +560,14 @@ async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str: from . import config_flow as _cf mac, code13, code = await _cf.identify_gateway(gateway) - reported_mac = dr.format_mac(mac) if mac else None + reported_mac = validated_reported_mac(mac) + if mac and not reported_mac: + LOGGER.debug( + "Gateway at %s reports the MAC %r, which names no interface; keeping the entered %s", + gateway.address, + mac, + gateway.serial, + ) if reported_mac and reported_mac != dr.format_mac(str(gateway.serial)): LOGGER.info( "Gateway at %s reports MAC %s; using it instead of the entered %s", diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index ea825cb73..272b2c83d 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -155,6 +155,44 @@ async def test_apply_gateway_identity_refreshes_the_profile_and_log_id(hass: Hom assert gateway.log_id == "[F461 gateway - 192.0.2.92]" +@pytest.mark.parametrize( + "frame", + [ + "*#13**12*0*0*0*0*0*0##", # six zeros: the factory value, and the UI sentinel of the first form + "*#13**12*255*255*255*255*255*255##", # broadcast + "*#13**12*256*3*80*0*7*119##", # an octet outside 0..255 (OWNd formats it as "100") + ], +) +async def test_manual_flow_keeps_the_typed_mac_when_the_reported_one_names_no_interface( + hass: HomeAssistant, frame: str +) -> None: + reported = OWNMessage.parse(frame).mac_address + assert reported # OWNd decodes all three; the flow is what must refuse them + identify = AsyncMock(return_value=(reported, "200", "134")) + + result = await _manual_flow(hass, _password_then_ok(), identify, mac="00:03:50:00:12:34") + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["mac"] == "00:03:50:00:12:34" + assert result["result"].unique_id == "00:03:50:00:12:34" + assert result["data"]["name"] == "F461" + + +def test_validated_reported_mac() -> None: + from custom_components.myhome.config_flow_discovery import validated_reported_mac + + assert validated_reported_mac("00:03:50:00:07:77") == "00:03:50:00:07:77" + assert validated_reported_mac("00:03:50:00:07:77".upper()) == "00:03:50:00:07:77" + assert validated_reported_mac("00:00:00:00:00:00") is None + assert validated_reported_mac("ff:ff:ff:ff:ff:ff") is None + assert validated_reported_mac("FF:FF:FF:FF:FF:FF") is None + assert validated_reported_mac("100:00:00:00:00:00") is None + assert validated_reported_mac("00:03:50:00:07") is None + assert validated_reported_mac("not a mac") is None + assert validated_reported_mac(None) is None + assert validated_reported_mac("") is None + + async def test_manual_flow_aborts_when_the_real_mac_is_already_configured(hass: HomeAssistant) -> None: from pytest_homeassistant_custom_component.common import MockConfigEntry From 27ed5353ed81b0faa3b1e01992a5d0a41f961ab5 Mon Sep 17 00:00:00 2001 From: gdluck Date: Sat, 24 Oct 2026 23:53:52 +0300 Subject: [PATCH 4/8] fix(config_flow): set the manual unique_id after the gateway reported its MAC The manual step set the unique_id from the typed MAC and aborted `already_configured` before the connection test, so a second gateway typed with the form's placeholder `00:03:50:00:00:00` was refused whenever an earlier manual entry (an F454 or MH202, whose MAC is never asked) kept that placeholder, although the new gateway would have reported its real MAC right after authentication. The unique_id is now set once, in the connection test after `_async_apply_gateway_identity`, from the final `dr.format_mac(gateway.serial)`; a true duplicate still aborts there. Review follow-up (review 5639869140, point 3). Co-Authored-By: Claude Fable 5.1 --- custom_components/myhome/config_flow.py | 5 +- .../myhome/config_flow_discovery.py | 8 ++-- tests/test_config_flow.py | 6 +++ tests/test_config_flow_identify.py | 46 +++++++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/custom_components/myhome/config_flow.py b/custom_components/myhome/config_flow.py index 9aa3bd29e..c11c10cb2 100644 --- a/custom_components/myhome/config_flow.py +++ b/custom_components/myhome/config_flow.py @@ -305,9 +305,10 @@ async def async_step_custom_manual(self, user_input=None, errors=None): # type: user_input["modelNumber"] = None user_input["UDN"] = None self.gateway_handler = OWNGateway(user_input) - await self.async_set_unique_id(user_input["serialNumber"], raise_on_progress=False) - self._abort_if_unique_id_configured() # Typed by hand: once authenticated, ask the gateway for its MAC and model. + # The unique_id is set from the MAC the gateway then reports (or the typed + # one), so a second gateway entered with the form's placeholder MAC is not + # refused before it could say who it is. self._identify_after_auth = True return await self.async_step_test_connection() diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index 0e2426d80..afd5c251e 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -490,6 +490,10 @@ async def _run_test_connection() -> dict[str, typing.Any]: if getattr(self, "_identify_after_auth", False): self._identify_after_auth = False model_source = await self._async_apply_gateway_identity(gateway) + # The manual step left the unique_id to this point: the MAC the gateway + # reports, or the typed one, decides whether this gateway already exists. + await self.async_set_unique_id(dr.format_mac(gateway.serial), raise_on_progress=False) # type: ignore[attr-defined] + self._abort_if_unique_id_configured() # type: ignore[attr-defined] _new_entry_data = { CONF_ID: dr.format_mac(gateway.serial), @@ -552,7 +556,7 @@ async def _run_test_connection() -> dict[str, typing.Any]: async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str: """Correct the MAC and model typed in the manual step from what the gateway reports. - The model follows ``resolve_gateway_identity``, the same verdict the runtime + The caller sets the unique_id from the final MAC afterwards. The model follows ``resolve_gateway_identity``, the same verdict the runtime reaches: a brand variant of the reported model is kept, an uncertain WHO=13 code may label a ``Generic`` entry but never overrides a chosen model, a certain code corrects it. Returns the source the model rests on, for the entry's ``model_source``. @@ -576,8 +580,6 @@ async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str: gateway.serial, ) gateway.serial = reported_mac - await self.async_set_unique_id(reported_mac, raise_on_progress=False) # type: ignore[attr-defined] - self._abort_if_unique_id_configured() # type: ignore[attr-defined] configured = str(gateway.model_name or "").strip() resolution = resolve_gateway_identity( GatewayIdentityEvidence( diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index dc96ba8af..a7461b7d6 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -150,6 +150,10 @@ async def test_form_already_configured(hass: HomeAssistant) -> None: "custom_components.myhome.config_flow.OWNSession.test_connection", return_value={"Success": True}, ), + patch( + "custom_components.myhome.config_flow.identify_gateway", + AsyncMock(return_value=(None, None, None)), + ) as identify, ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -179,8 +183,10 @@ async def test_form_already_configured(hass: HomeAssistant) -> None: }, ) + # The duplicate is refused once the gateway has had the chance to report its MAC assert result4["type"] == FlowResultType.ABORT assert result4["reason"] == "already_configured" + identify.assert_awaited_once() async def test_form_discovery(hass: HomeAssistant) -> None: diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index 272b2c83d..0af84c0f9 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -46,6 +46,11 @@ async def _manual_flow( assert result["type"] == FlowResultType.FORM assert result["step_id"] == "password" result = await hass.config_entries.flow.async_configure(result["flow_id"], {"password": "12345"}) + if result["type"] == FlowResultType.FORM and result["step_id"] == "bus_topology": + # Another IP gateway exists: the topology question comes before the entry (#524) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {"bus_topology": "standalone"} + ) await hass.async_block_till_done() return result @@ -210,6 +215,47 @@ async def test_manual_flow_aborts_when_the_real_mac_is_already_configured(hass: assert result["reason"] == "already_configured" +async def test_manual_flow_adds_a_second_gateway_typed_with_the_placeholder_mac(hass: HomeAssistant) -> None: + """An F454 kept the placeholder as unique_id (its MAC is never asked); an F461 typed with the same + placeholder is not refused before it can report its real MAC.""" + from pytest_homeassistant_custom_component.common import MockConfigEntry + + MockConfigEntry( + domain=DOMAIN, + data={"host": "192.0.2.54", "mac": "00:03:50:00:00:00", "name": "F454"}, + unique_id="00:03:50:00:00:00", + ).add_to_hass(hass) + identify = AsyncMock(return_value=("00:03:50:00:07:77", "200", "134")) + + result = await _manual_flow(hass, _password_then_ok(), identify, model="F461") + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["mac"] == "00:03:50:00:07:77" + assert result["result"].unique_id == "00:03:50:00:07:77" + assert result["data"]["name"] == "F461" + assert len(hass.config_entries.async_entries(DOMAIN)) == 2 + + +async def test_manual_flow_still_refuses_a_duplicate_placeholder_the_gateway_cannot_resolve( + hass: HomeAssistant, +) -> None: + """Two gateways whose MAC is never learned cannot share the placeholder: the second aborts after auth.""" + from pytest_homeassistant_custom_component.common import MockConfigEntry + + MockConfigEntry( + domain=DOMAIN, + data={"host": "192.0.2.54", "mac": "00:03:50:00:00:00", "name": "F454"}, + unique_id="00:03:50:00:00:00", + ).add_to_hass(hass) + identify = AsyncMock(return_value=(None, "200", "5")) # an MH202: not in MAC_REQUEST_SAFE_MODELS + + result = await _manual_flow(hass, _password_then_ok(), identify, model="MH202") + + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "already_configured" + identify.assert_awaited_once() + + async def test_identify_gateway_reads_the_f461_replies() -> None: """identify_gateway asks the model first, then the MAC (an F461 answers both), and decodes the live replies.""" from custom_components.myhome import config_flow_discovery as cfd From 83cb5d48ebf418b2f711e4e159d38747410cedc7 Mon Sep 17 00:00:00 2001 From: gdluck Date: Sat, 24 Oct 2026 23:58:11 +0300 Subject: [PATCH 5/8] fix(config_flow): ask the password a split-auth gateway needs for commands A gateway with an open monitor session (`*99*1##`, IP whitelist) and a password-protected command session (`*99*0##`) passed the connection test without a password, and the identification probe then answered `password_required` while OWNd logged a WARNING and an ERROR in the middle of a successful setup. `identify_gateway` now returns a `GatewayIdentity` named tuple whose `password_required` flag routes the manual flow to the password step (the runtime command workers need that password anyway) and probes again with it; a second refusal is treated as no answer, so the flow cannot loop. The probe runs on `PROBE_LOGGER`, which records every OWNd line at DEBUG: the probe is best effort and its refusals are expected. Review follow-up (review 5639869140, point 4). Co-Authored-By: Claude Fable 5.1 --- custom_components/myhome/config_flow.py | 2 + .../myhome/config_flow_discovery.py | 69 +++++++++++--- tests/test_config_flow_identify.py | 90 +++++++++++++++++-- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/custom_components/myhome/config_flow.py b/custom_components/myhome/config_flow.py index c11c10cb2..29c553360 100644 --- a/custom_components/myhome/config_flow.py +++ b/custom_components/myhome/config_flow.py @@ -29,6 +29,7 @@ TEST_CONNECTION_ABORT_REASONS, TEST_CONNECTION_RETRY_DELAY, ConfigFlowDiscoveryMixin, + GatewayIdentity, MACAddress, _get_serial_ports, identify_gateway, @@ -359,5 +360,6 @@ async def async_step_custom_manual(self, user_input=None, errors=None): # type: "command_session_default", "find_gateways", "get_gateway", + "GatewayIdentity", "identify_gateway", ] diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index afd5c251e..f89698171 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -4,6 +4,7 @@ import asyncio import contextlib import ipaddress +import logging import re import typing @@ -77,6 +78,32 @@ WHO13_SHARED_CODE = "200" +class _DebugOnlyLogger(logging.Logger): + """A logger that records everything at DEBUG. + + The identification probe is best effort and its refusals are expected: a gateway + with an open monitor session (`*99*1##`) and a password-protected command session + (`*99*0##`) answers the probe with `password_required`, which OWNd would otherwise + log at WARNING and ERROR in the middle of a successful setup. + """ + + def _log(self, level: int, msg: object, args: typing.Any, *extra: typing.Any, **kwargs: typing.Any) -> None: + super()._log(logging.DEBUG, msg, args, *extra, **kwargs) + + +PROBE_LOGGER: logging.Logger = _DebugOnlyLogger(f"{LOGGER.name}.identify") +PROBE_LOGGER.parent = LOGGER + + +class GatewayIdentity(typing.NamedTuple): + """What ``identify_gateway`` learned: the three answers, and whether the command session wants a password.""" + + mac: str | None + who13_code: str | None + who1013_code: str | None + password_required: bool = False + + def mac_request_is_safe(who13_code: str | None, who1013_code: str | None = None) -> bool: """Whether every model the reported codes can name is known to answer *#13**12##. @@ -116,9 +143,13 @@ def _inband_reading(who13_code: str | None, who1013_code: str | None) -> CodeRea return read_who13(who13_code or "") -async def identify_gateway(gateway: typing.Any) -> tuple[str | None, str | None, str | None]: +async def identify_gateway(gateway: typing.Any) -> GatewayIdentity: """Ask an authenticated gateway who it is: ``(mac, who13_code, who1013_code)``. + A command session refused with ``password_required`` yields ``password_required=True`` + and nothing else: the gateway protects ``*99*0##`` although its monitor session was + open, and the runtime command workers will need that password anyway. + The order follows the runtime identification (#420): ``*#13**15##`` first, which every gateway answers; ``*#1013**1##`` only when that answer is the shared code ``200`` (F454 / F461 / MyHomeServer1 class), keeping older gateways away from a @@ -129,7 +160,7 @@ async def identify_gateway(gateway: typing.Any) -> tuple[str | None, str | None, mac: str | None = None code13: str | None = None code1013: str | None = None - session = OWNCommandSession(gateway=gateway, logger=LOGGER) + session = OWNCommandSession(gateway=gateway, logger=PROBE_LOGGER) async def _ask(request: typing.Any) -> list[typing.Any]: replies = await session.send(request, is_status_request=True) @@ -139,7 +170,8 @@ async def _ask(request: typing.Any) -> list[typing.Any]: async with asyncio.timeout(IDENTIFY_TIMEOUT): res = await session.connect() if not (isinstance(res, dict) and res.get("Success")): - return None, None, None + refused = isinstance(res, dict) and res.get("Message") == "password_required" + return GatewayIdentity(None, None, None, password_required=bool(refused)) for reply in await _ask(OWNGatewayCommand.request_device_type()): value = getattr(reply, "device_type_code", None) if getattr(reply, "who", None) == 13 and value: @@ -165,7 +197,7 @@ async def _ask(request: typing.Any) -> list[typing.Any]: finally: with contextlib.suppress(Exception): await session.close() - return mac, code13, code1013 + return GatewayIdentity(mac, code13, code1013) class MACAddress: @@ -488,8 +520,19 @@ async def _run_test_connection() -> dict[str, typing.Any]: model_source: str | None = None if getattr(self, "_identify_after_auth", False): - self._identify_after_auth = False model_source = await self._async_apply_gateway_identity(gateway) + if model_source is None: + # Split authentication: the monitor session is open, the command + # session is password-protected. Ask once; the command workers need it. + LOGGER.debug( + "Gateway %s (%s:%s) accepts the monitor session without a password " + "but protects the command session; asking for it", + gateway.model_name, + gateway.address, + gateway.port, + ) + return await self.async_step_password() # type: ignore + self._identify_after_auth = False # The manual step left the unique_id to this point: the MAC the gateway # reports, or the typed one, decides whether this gateway already exists. await self.async_set_unique_id(dr.format_mac(gateway.serial), raise_on_progress=False) # type: ignore[attr-defined] @@ -553,17 +596,21 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) return self.async_abort(reason=abort_reason) # type: ignore[attr-defined] - async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str: + async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str | None: """Correct the MAC and model typed in the manual step from what the gateway reports. - The caller sets the unique_id from the final MAC afterwards. The model follows ``resolve_gateway_identity``, the same verdict the runtime - reaches: a brand variant of the reported model is kept, an uncertain WHO=13 code - may label a ``Generic`` entry but never overrides a chosen model, a certain code - corrects it. Returns the source the model rests on, for the entry's ``model_source``. + The caller sets the unique_id from the final MAC afterwards. The model follows + ``resolve_gateway_identity``, the same verdict the runtime reaches: a brand + variant of the reported model is kept, an uncertain WHO=13 code may label a + ``Generic`` entry but never overrides a chosen model, a certain code corrects + it. Returns the source the model rests on, for the entry's ``model_source``, or + ``None`` when the command session wants a password the monitor session did not. """ from . import config_flow as _cf - mac, code13, code = await _cf.identify_gateway(gateway) + mac, code13, code, password_required = GatewayIdentity(*await _cf.identify_gateway(gateway)) + if password_required and not gateway.password: + return None reported_mac = validated_reported_mac(mac) if mac and not reported_mac: LOGGER.debug( diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index 0af84c0f9..69bf158d6 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -270,10 +270,11 @@ async def test_identify_gateway_reads_the_f461_replies() -> None: session.send = AsyncMock(side_effect=lambda message, is_status_request=False: replies[str(message)]) session.close = AsyncMock() - with patch.object(cfd, "OWNCommandSession", return_value=session): - mac, code13, code = await cfd.identify_gateway(MagicMock()) + with patch.object(cfd, "OWNCommandSession", return_value=session) as session_class: + identity = await cfd.identify_gateway(MagicMock()) - assert (mac, code13, code) == ("00:03:50:00:07:77", "200", "134") + assert identity == cfd.GatewayIdentity("00:03:50:00:07:77", "200", "134") + assert session_class.call_args.kwargs["logger"] is cfd.PROBE_LOGGER assert [str(c.args[0]) for c in session.send.await_args_list] == ["*#13**15##", "*#1013**1##", "*#13**12##"] session.close.assert_awaited() @@ -293,9 +294,9 @@ async def test_identify_gateway_never_asks_the_mac_from_a_gateway_that_crashes_o session.close = AsyncMock() with patch.object(cfd, "OWNCommandSession", return_value=session): - mac, code13, code = await cfd.identify_gateway(MagicMock()) + identity = await cfd.identify_gateway(MagicMock()) - assert (mac, code13, code) == (None, who13, None) + assert identity == cfd.GatewayIdentity(None, who13, None) assert [str(c.args[0]) for c in session.send.await_args_list] == ["*#13**15##"] @@ -319,7 +320,7 @@ async def test_identify_gateway_survives_a_refused_session() -> None: session.connect = AsyncMock(side_effect=OSError("refused")) session.close = AsyncMock() with patch.object(cfd, "OWNCommandSession", return_value=session): - assert await cfd.identify_gateway(MagicMock()) == (None, None, None) + assert await cfd.identify_gateway(MagicMock()) == cfd.GatewayIdentity(None, None, None) async def test_identify_gateway_gives_up_when_the_session_is_not_accepted() -> None: @@ -331,6 +332,81 @@ async def test_identify_gateway_gives_up_when_the_session_is_not_accepted() -> N session.send = AsyncMock() session.close = AsyncMock() with patch.object(cfd, "OWNCommandSession", return_value=session): - assert await cfd.identify_gateway(MagicMock()) == (None, None, None) + assert await cfd.identify_gateway(MagicMock()) == cfd.GatewayIdentity(None, None, None) session.send.assert_not_awaited() session.close.assert_awaited() + + +async def test_identify_gateway_reports_a_password_protected_command_session(caplog) -> None: + """Open monitor session, protected command session: the probe says so and asks nothing, at DEBUG only.""" + from custom_components.myhome import config_flow_discovery as cfd + + session = MagicMock() + + async def _refuse(): + # What OWNd logs on the logger it was given, at WARNING and ERROR + cfd.PROBE_LOGGER.warning("Connection requires a password but none was provided.") + cfd.PROBE_LOGGER.error("Command session negotiation failed (password_required); giving up.") + return {"Success": False, "Message": "password_required"} + + session.connect = AsyncMock(side_effect=_refuse) + session.send = AsyncMock() + session.close = AsyncMock() + with ( + caplog.at_level(logging.DEBUG, logger="custom_components.myhome"), + patch.object(cfd, "OWNCommandSession", return_value=session), + ): + identity = await cfd.identify_gateway(MagicMock()) + + assert identity == cfd.GatewayIdentity(None, None, None, password_required=True) + assert identity.password_required is True + session.send.assert_not_awaited() + probe_records = [r for r in caplog.records if "password" in r.getMessage()] + assert len(probe_records) == 2 + assert all(r.levelno == logging.DEBUG for r in probe_records) + assert all(r.name == "custom_components.myhome.identify" for r in probe_records) + + +async def test_manual_flow_asks_the_password_a_split_auth_gateway_needs_for_commands( + hass: HomeAssistant, caplog +) -> None: + """Monitor session open without a password, command session protected: the flow asks for it once, + then identifies the gateway with it (the runtime command workers need that password anyway).""" + from custom_components.myhome.config_flow_discovery import GatewayIdentity + + test_connection = AsyncMock(return_value={"Success": True}) + identify = AsyncMock( + side_effect=[ + GatewayIdentity(None, None, None, password_required=True), + GatewayIdentity("00:03:50:00:07:77", "200", "134"), + ] + ) + + with caplog.at_level(logging.DEBUG, logger="custom_components.myhome"): + result = await _manual_flow(hass, test_connection, identify) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["password"] == "12345" + assert result["data"]["mac"] == "00:03:50:00:07:77" + assert result["data"]["name"] == "F461" + assert identify.await_count == 2 + assert test_connection.await_count == 2 + assert not [ + r for r in caplog.records if r.levelno >= logging.WARNING and "password" in r.getMessage().lower() + ] + + +async def test_manual_flow_does_not_loop_when_the_command_session_still_wants_a_password( + hass: HomeAssistant, +) -> None: + """With a password already given, a second password_required is treated as no answer: the entry is created.""" + from custom_components.myhome.config_flow_discovery import GatewayIdentity + + identify = AsyncMock(return_value=GatewayIdentity(None, None, None, password_required=True)) + + result = await _manual_flow(hass, _password_then_ok(), identify) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["mac"] == "00:03:50:00:00:00" + assert result["data"]["name"] == "MyHomeServer1" + identify.assert_awaited_once() From ac7b2a5e1c409549cedc87116be5990d31818636 Mon Sep 17 00:00:00 2001 From: gdluck Date: Sun, 25 Oct 2026 00:00:44 +0300 Subject: [PATCH 6/8] refactor(config_flow): refresh the flow context after identification, reuse the shared-code table The flow context and its title placeholders were filled before the gateway reported its MAC and model, so the bus topology step that follows when another IP gateway exists showed the typed placeholder and the uncorrected model; they are refreshed after `_async_apply_gateway_identity`. The duplicated `WHO13_SHARED_CODE = "200"` goes: `read_who13(code).shared` (`WHO13_SHARED_DEVICE_TYPES`) is the single source of truth. A test pins that a MyHomeServer1 corrected to an MH200 (`*#13**15*4##`) or an H4684 (`*#13**15*13##`) starts one command worker. Review follow-up (review 5639869140, point 5). Co-Authored-By: Claude Fable 5.1 --- .../myhome/config_flow_discovery.py | 39 ++++++++------- tests/test_config_flow_identify.py | 49 +++++++++++++++++++ 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index f89698171..4c8ee2bb4 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -73,10 +73,6 @@ UNUSABLE_MACS: frozenset[str] = frozenset({"00:00:00:00:00:00", "ff:ff:ff:ff:ff:ff"}) -# WHO=13 dimension 15 answer shared by the F454 / F461 / MyHomeServer1 class, the -# only case in which the runtime asks WHO=1013 as well (#420). -WHO13_SHARED_CODE = "200" - class _DebugOnlyLogger(logging.Logger): """A logger that records everything at DEBUG. @@ -112,7 +108,7 @@ def mac_request_is_safe(who13_code: str | None, who1013_code: str | None = None) """ if who1013_code: models = read_who1013(who1013_code).models - elif who13_code and who13_code != WHO13_SHARED_CODE: + elif who13_code and not read_who13(who13_code).shared: models = read_who13(who13_code).models else: return False @@ -176,7 +172,8 @@ async def _ask(request: typing.Any) -> list[typing.Any]: value = getattr(reply, "device_type_code", None) if getattr(reply, "who", None) == 13 and value: code13 = code13 or str(value) - if code13 == WHO13_SHARED_CODE: + # A shared code (WHO13_SHARED_DEVICE_TYPES) names no model: ask WHO=1013 (#420) + if code13 and read_who13(code13).shared: for reply in await _ask(OWNGatewayDiagnosticCommand.request_diagnostic_object_model()): if getattr(reply, "who", None) == 1013 and getattr(reply, "object_model", None): code1013 = code1013 or str(reply.object_model) @@ -453,18 +450,7 @@ async def async_step_test_connection( # pylint: disable=unused-argument gateway = self.gateway_handler assert gateway is not None - self.context.update( # type: ignore[attr-defined] - { - CONF_HOST: gateway.host, - CONF_NAME: gateway.model_name, - CONF_MAC: gateway.serial, - "title_placeholders": { - CONF_HOST: str(gateway.host or ""), - CONF_NAME: str(gateway.model_name or ""), - CONF_MAC: str(gateway.serial or ""), - }, - } - ) + self._update_flow_context(gateway) async def _run_test_connection() -> dict[str, typing.Any]: try: @@ -533,6 +519,8 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) return await self.async_step_password() # type: ignore self._identify_after_auth = False + # The MAC and model may have changed: the flow title must say so + self._update_flow_context(gateway) # The manual step left the unique_id to this point: the MAC the gateway # reports, or the typed one, decides whether this gateway already exists. await self.async_set_unique_id(dr.format_mac(gateway.serial), raise_on_progress=False) # type: ignore[attr-defined] @@ -596,6 +584,21 @@ async def _run_test_connection() -> dict[str, typing.Any]: ) return self.async_abort(reason=abort_reason) # type: ignore[attr-defined] + def _update_flow_context(self, gateway: typing.Any) -> None: + """Keep the flow context and its title placeholders in step with the gateway's MAC and model.""" + self.context.update( # type: ignore[attr-defined] + { + CONF_HOST: gateway.host, + CONF_NAME: gateway.model_name, + CONF_MAC: gateway.serial, + "title_placeholders": { + CONF_HOST: str(gateway.host or ""), + CONF_NAME: str(gateway.model_name or ""), + CONF_MAC: str(gateway.serial or ""), + }, + } + ) + async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str | None: """Correct the MAC and model typed in the manual step from what the gateway reports. diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index 69bf158d6..c81f30ecd 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -256,6 +256,55 @@ async def test_manual_flow_still_refuses_a_duplicate_placeholder_the_gateway_can identify.assert_awaited_once() +async def test_test_connection_refreshes_the_flow_context_after_identification(hass: HomeAssistant) -> None: + """With another IP gateway configured the flow goes on to the topology question: the context and the + title placeholders it shows must carry the corrected MAC and model, not the typed ones.""" + from OWNd.connection import OWNGateway + from pytest_homeassistant_custom_component.common import MockConfigEntry + + from custom_components.myhome.config_flow import MyhomeFlowHandler + + MockConfigEntry( + domain=DOMAIN, data={"host": "192.0.2.54", "mac": "00:03:50:00:00:54"}, unique_id="00:03:50:00:00:54" + ).add_to_hass(hass) + flow = MyhomeFlowHandler() + flow.hass = hass + flow.context = {"source": config_entries.SOURCE_USER} + flow.gateway_handler = OWNGateway( + {"address": "192.0.2.92", "port": 20000, "serialNumber": "00:03:50:00:00:00", "modelName": "MyHomeServer1"} + ) + flow._identify_after_auth = True + with ( + patch("custom_components.myhome.config_flow.OWNSession.test_connection", AsyncMock(return_value={"Success": True})), + patch("custom_components.myhome.config_flow.identify_gateway", AsyncMock(return_value=("00:03:50:00:07:77", "200", "134"))), + ): + result = await flow.async_step_test_connection() + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "bus_topology" + assert flow.context["name"] == "F461" + assert flow.context["mac"] == "00:03:50:00:07:77" + assert flow.context["title_placeholders"] == {"host": "192.0.2.92", "name": "F461", "mac": "00:03:50:00:07:77"} + assert flow._pending_entry is not None + assert flow._pending_entry[0] == "F461 Gateway" + + +@pytest.mark.parametrize(("who13", "model"), [("4", "MH200"), ("13", "H4684")]) +async def test_manual_flow_worker_count_follows_the_corrected_model(hass: HomeAssistant, who13: str, model: str) -> None: + """A MyHomeServer1 (two command sessions by default) corrected to a one-session gateway starts one worker.""" + from custom_components.myhome.gateway import command_session_default + + assert command_session_default("MyHomeServer1") == 2 + identify = AsyncMock(return_value=(None, who13, None)) + + result = await _manual_flow(hass, _password_then_ok(), identify) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"]["name"] == model + assert result["data"]["model_source"] == "who13" + assert result["options"]["command_worker_count"] == 1 + + async def test_identify_gateway_reads_the_f461_replies() -> None: """identify_gateway asks the model first, then the MAC (an F461 answers both), and decodes the live replies.""" from custom_components.myhome import config_flow_discovery as cfd From e73e8a1e4eb3691cca317d21f2f9e52f10f75c51 Mon Sep 17 00:00:00 2001 From: gdluck Date: Sun, 25 Oct 2026 00:02:29 +0300 Subject: [PATCH 7/8] docs(identification): document the setup-time probe and the MAC request gate `gateway-identification.md` gains a section on what the manual config flow asks once the password is accepted (`*#13**15##`, `*#1013**1##` on a shared code, `*#13**12##` only behind the fail-closed `MAC_REQUEST_SAFE_MODELS` gate, since real H4684 and MH200 daemons crash on it), how the MAC reply is validated, how the model goes through `resolve_gateway_identity`, and the split-authentication case. The `*#1013*0*1##` example now reads `*#1013**1##`, the frame the integration sends. Review follow-up (review 5639869140, point 7). Co-Authored-By: Claude Fable 5.1 --- docs/configuration/gateway-identification.md | 26 +++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/configuration/gateway-identification.md b/docs/configuration/gateway-identification.md index 77b5153b2..4b5f23771 100644 --- a/docs/configuration/gateway-identification.md +++ b/docs/configuration/gateway-identification.md @@ -81,7 +81,7 @@ If your gateway reports a code that is in none of these tables, the integration A shared code such as `200` cannot label an unconfigured gateway, and it cannot check an SSDP or manual label either. So whenever `WHO=13` answers a shared code — **whatever the source of the configured model** — the integration sends one status request on the command session: ```text -*#1013*0*1## → *#1013**1*## +*#1013**1## → *#1013**1*## ``` `WHO=1013` is the *Gateway Diagnostic* family (Nmap's table of WHO values calls it *Device Diagnostic*); dimension 1 is the model code, and its catalogue has one code per model (`4` MH200, `5` MH202, `19` H4684, `26` LGRH4684, `44` MH200N, `51` F454, `67` MyHOMEServer1, `134` F461, … — the full list is `WHO1013_OBJECT_MODELS` in `const.py`, taken from the OpenWebNet device database as listed in #370, with H4684 and LGRH4684 confirmed via firmware reverse-engineering). A real reply is four values, not one — `OBJECT_MODEL`, `N_CONF`, `BRAND`, `LINE` (per the OpenWebNet Encyclopedia's work on `MHCatalogue.db`, via @anotherjulien in [#420](https://github.com/OpenWebNet-HA/MyHOME/pull/420)): @@ -127,6 +127,30 @@ Each source is recorded separately — what you picked, what the gateway announc --- +## Setup time: a manually added gateway is asked who it is + +The manual form (`Custom (IP Gateway)` → address → MAC and model) offers defaults that are easy to leave in place: MAC `00:03:50:00:00:00` and model `MyHomeServer1`. The runtime resolver above corrects the **model** later, but the **MAC** is the entry's `unique_id` and is fixed when the entry is created. So, once the gateway has accepted the password, the config flow opens one command session and asks, in the order the runtime uses: + +| step | request | asked when | what it settles | +| :--- | :--- | :--- | :--- | +| 1 | `*#13**15##` | always | the WHO=13 device type; every known gateway answers it | +| 2 | `*#1013**1##` | only when step 1 returned a **shared** code (`200`) | the OBJECT_MODEL, one code per model | +| 3 | `*#13**12##` (MAC) | only when **every** model the codes can name is in `MAC_REQUEST_SAFE_MODELS` | the gateway's real MAC | + +The whole probe is best effort with a 15 s cap: a gateway that refuses the session, answers nothing or times out leaves the typed values in place. + +**The MAC request is gated, and the gate fails closed.** A real H4684 (`020054`) and a real MH200 (`020102`) daemon crash on `*#13**12##` (own-firmware-oracle, real-binary runs); the F459 / F460 / F461 / MyHomeServer1 factory images answer `*#13**12*0*0*0*0##`, and the F450 / F454 / MH202 images answer with the interface's bytes. Only a model a **physical unit** answered with its real MAC is in `MAC_REQUEST_SAFE_MODELS` (`config_flow_discovery.py`): the F461 (firmware 2.0.11) and the MyHomeServer1 (3.87.13), both captured in the `tests/fixtures/traces` live runs. An unknown, unlisted or merely emulated-without-crash model is never asked, and when the gate stays closed the typed MAC stands. The list is meant to move into an OWNd profile flag once the oracle rows are published. + +What the answers do: + +- **MAC** — the reply must pass the same validation as the form (six octets, `0..255`) and must not be the all-zero factory value `00:00:00:00:00:00` (also the first form's *Custom* sentinel) or the broadcast address; otherwise the typed MAC is kept and the reply is logged at DEBUG. A usable MAC that differs from the typed one replaces it and becomes the `unique_id`. The `unique_id` is set only at this point, so a second gateway typed with the placeholder MAC is not refused before it could report its real one; a true duplicate still aborts with `already_configured`. +- **Model** — the codes go through the same `resolve_gateway_identity` as at runtime: a certain code (2006 table or `WHO=1013`) corrects the chosen model, a brand variant of the reported model (`H4890` for OBJECT_MODEL `30`, `003598` for `51`) is kept, an uncertain code may label a `Generic` entry but never overrides a chosen model. A corrected model also changes the OWNd profile, the entry title and the default number of command workers, and the entry records `model_source: who13` so the diagnostics block below reports it. +- **Split authentication** — a gateway whose monitor session (`*99*1##`) is open but whose command session (`*99*0##`) wants a password answers the probe with `password_required`; the flow then asks for the password (the runtime command workers need it anyway) and probes again with it. The probe's own OWNd log lines are recorded at DEBUG. + +A gateway discovered over SSDP or added through the serial transport is not probed: its identity is authoritative. + +--- + ## The rule the integration applies When a dimension-15 reply arrives, the handler compares the reported model with the configured one. A configured model that one of the tables lists by name has a code of its own, so only that name or a brand variant of it agrees: `MH200N` (`44`) is contradicted by `4` (MH200), and `F452V` (`7`) by `6` (F452), even though each pair shares a family. A name no table lists is compared **by family** (`F452X` → `F452`), so an unlisted variant suffix is never downgraded. Then: From 652995b99507d010dc621385397c8d96f498beac Mon Sep 17 00:00:00 2001 From: gdluck Date: Sun, 25 Oct 2026 00:47:32 +0300 Subject: [PATCH 8/8] fix(config_flow): ask the command password once, log the probe through a registered logger Two points from an independent review of the follow-up diff. The password form accepts an empty string, and `_async_apply_gateway_identity` returned "ask for a password" whenever the command session said `password_required` with no password set, so a blank answer brought the form back forever; the flow now asks once (`_command_password_asked`) and treats a second refusal as no answer. `PROBE_LOGGER` was a `logging.Logger` subclass built outside the manager, so `logging.getLogger(PROBE_LOGGER.name)` (and `caplog.set_level(..., logger=...)`) addressed a different object; it is now `LOGGER.getChild("identify")` with a filter that lowers every record to DEBUG. Review follow-up (review 5639869140, points 4 and 5, second opinion). Co-Authored-By: Claude Fable 5.1 --- .../myhome/config_flow_discovery.py | 30 +++++++++++-------- tests/test_config_flow_identify.py | 22 ++++++++++++-- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/custom_components/myhome/config_flow_discovery.py b/custom_components/myhome/config_flow_discovery.py index 4c8ee2bb4..3a3ac84c5 100644 --- a/custom_components/myhome/config_flow_discovery.py +++ b/custom_components/myhome/config_flow_discovery.py @@ -74,21 +74,24 @@ -class _DebugOnlyLogger(logging.Logger): - """A logger that records everything at DEBUG. +class _RecordAtDebug(logging.Filter): + """Lower every record of the identification probe to DEBUG. - The identification probe is best effort and its refusals are expected: a gateway - with an open monitor session (`*99*1##`) and a password-protected command session - (`*99*0##`) answers the probe with `password_required`, which OWNd would otherwise - log at WARNING and ERROR in the middle of a successful setup. + The probe is best effort and its refusals are expected: a gateway with an open + monitor session (`*99*1##`) and a password-protected command session (`*99*0##`) + answers it with `password_required`, which OWNd would otherwise log at WARNING and + ERROR in the middle of a successful setup. """ - def _log(self, level: int, msg: object, args: typing.Any, *extra: typing.Any, **kwargs: typing.Any) -> None: - super()._log(logging.DEBUG, msg, args, *extra, **kwargs) + def filter(self, record: logging.LogRecord) -> bool: + record.levelno = logging.DEBUG + record.levelname = logging.getLevelName(logging.DEBUG) + return True -PROBE_LOGGER: logging.Logger = _DebugOnlyLogger(f"{LOGGER.name}.identify") -PROBE_LOGGER.parent = LOGGER +# A registered child logger, so logging.getLogger(PROBE_LOGGER.name) is this object. +PROBE_LOGGER: logging.Logger = LOGGER.getChild("identify") +PROBE_LOGGER.addFilter(_RecordAtDebug()) class GatewayIdentity(typing.NamedTuple): @@ -507,9 +510,11 @@ async def _run_test_connection() -> dict[str, typing.Any]: model_source: str | None = None if getattr(self, "_identify_after_auth", False): model_source = await self._async_apply_gateway_identity(gateway) - if model_source is None: + if model_source is None and not getattr(self, "_command_password_asked", False): # Split authentication: the monitor session is open, the command - # session is password-protected. Ask once; the command workers need it. + # session is password-protected. Ask once (the command workers need + # it); a second refusal, e.g. after a blank password, is not asked again. + self._command_password_asked = True LOGGER.debug( "Gateway %s (%s:%s) accepts the monitor session without a password " "but protects the command session; asking for it", @@ -614,6 +619,7 @@ async def _async_apply_gateway_identity(self, gateway: typing.Any) -> str | None mac, code13, code, password_required = GatewayIdentity(*await _cf.identify_gateway(gateway)) if password_required and not gateway.password: return None + # A refusal after a password was given is a refusal, not a question: nothing reported reported_mac = validated_reported_mac(mac) if mac and not reported_mac: LOGGER.debug( diff --git a/tests/test_config_flow_identify.py b/tests/test_config_flow_identify.py index c81f30ecd..064aaa30f 100644 --- a/tests/test_config_flow_identify.py +++ b/tests/test_config_flow_identify.py @@ -25,6 +25,7 @@ async def _manual_flow( identify: AsyncMock, mac: str = "00:03:50:00:00:00", model: str = "MyHomeServer1", + password: str = "12345", ): with ( patch("custom_components.myhome.config_flow.find_gateways", return_value=[]), @@ -45,7 +46,7 @@ async def _manual_flow( ) assert result["type"] == FlowResultType.FORM assert result["step_id"] == "password" - result = await hass.config_entries.flow.async_configure(result["flow_id"], {"password": "12345"}) + result = await hass.config_entries.flow.async_configure(result["flow_id"], {"password": password}) if result["type"] == FlowResultType.FORM and result["step_id"] == "bus_topology": # Another IP gateway exists: the topology question comes before the entry (#524) result = await hass.config_entries.flow.async_configure( @@ -412,8 +413,9 @@ async def _refuse(): session.send.assert_not_awaited() probe_records = [r for r in caplog.records if "password" in r.getMessage()] assert len(probe_records) == 2 - assert all(r.levelno == logging.DEBUG for r in probe_records) + assert all(r.levelno == logging.DEBUG and r.levelname == "DEBUG" for r in probe_records) assert all(r.name == "custom_components.myhome.identify" for r in probe_records) + assert logging.getLogger("custom_components.myhome.identify") is cfd.PROBE_LOGGER async def test_manual_flow_asks_the_password_a_split_auth_gateway_needs_for_commands( @@ -445,6 +447,22 @@ async def test_manual_flow_asks_the_password_a_split_auth_gateway_needs_for_comm ] +async def test_manual_flow_asks_the_command_password_only_once(hass: HomeAssistant) -> None: + """The password form accepts an empty string: a blank answer must not bring the form back forever.""" + from custom_components.myhome.config_flow_discovery import GatewayIdentity + + test_connection = AsyncMock(return_value={"Success": True}) + identify = AsyncMock(return_value=GatewayIdentity(None, None, None, password_required=True)) + + result = await _manual_flow(hass, test_connection, identify, password="") + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert not result["data"]["password"] # the blank answer is stored as no password + assert result["data"]["mac"] == "00:03:50:00:00:00" + assert identify.await_count == 2 + assert test_connection.await_count == 2 + + async def test_manual_flow_does_not_loop_when_the_command_session_still_wants_a_password( hass: HomeAssistant, ) -> None: