Skip to content

feat(config_flow): discover robots via zeroconf and DHCP (from #35, @StratoGh0st99) - #78

Merged
sjmotew merged 5 commits into
masterfrom
feat/zeroconf-dhcp-discovery
Aug 21, 2026
Merged

feat(config_flow): discover robots via zeroconf and DHCP (from #35, @StratoGh0st99)#78
sjmotew merged 5 commits into
masterfrom
feat/zeroconf-dhcp-discovery

Conversation

@sjmotew

@sjmotew sjmotew commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Extracts the zeroconf + DHCP discovery from #35 onto current master, authored to @StratoGh0st99.

This is the cherry-pick I offered on #35 rather than asking for another rebase. #35 bundled discovery with 3,806 lines across 27 files, most of which has since landed through #49, #50, #52, #53, #54, #61, #62, #63 and #67 — discovery was the part with no equivalent on master, and the biggest outstanding setup win in the tracker. #40 shows setup failing outright when the 15 s wake timeout expires before a sleeping robot answers; a pre-filled host sidesteps that whole class of "which IP is it" failures.

@StratoGh0st99 — you're the commit author. If you'd rather carry this yourself, say so and I'll close this in favour of your branch.

What it does

Declares _narwal_sweeper._tcp.local. and the NARWAL_* / narwal_* DHCP hostnames in the manifest, and routes both into the existing user step with the address filled in. The model is still chosen by hand — it isn't in the mDNS payload.

Verified on hardware

Browsed from the LAN against a Flow (AX12):

_app_wss_server_7bb53c._narwal_sweeper._tcp.local.
    server = NARWAL_7bb53c.local.
    addrs  = ['10.0.0.112']   port = 9002

So the service type, the instance-name shape and the NARWAL_* hostname pattern in #35 are all confirmed, not taken on trust.

That capture also settled a design question. A configured entry's unique_id is the full device_id, read over the WebSocket — something neither mDNS nor DHCP can see. Without a link between the two, a robot added by hand reappears as a "Discovered" card forever. This robot's device_id is 71c53f01c14f49088338863e147bb53c and it advertises as NARWAL_7bb53c: the last six hex characters. Matching on that suffix identifies the device itself, and a match at a new address repoints the existing entry instead of orphaning it.

Two changes from #35's implementation

  1. Host-only matching → device_id suffix matching. The original matched on host, and its "IP drifted (DHCP renewal, robot moved subnets)" branch rewrote the entry with the same host it had just matched on — a no-op under a comment describing real behaviour. Suffix matching does what that comment intended.
  2. The user-step schema keeps the host in hand, so a failed connect no longer clears what you typed.

Everything else is @StratoGh0st99's design, including the fallback-to-DHCP rationale (multicast is routinely dropped across VLANs and under wireless client isolation).

Tests

12 new config-flow tests plus the service_info stubs they need: both discovery paths, the trailing-dot strip on mDNS hostnames, DHCP leases with no hostname, device-suffix matching, the repoint-on-new-address case, a second robot not being suppressed by the first, and the manual flow being unaffected. 251 passing.

Not included from #35

The diagnostic entities and map work, all superseded by what has landed since. tools/narwal_capture.py and coverage_probe.py still can't land — tools/ is gitignored here — but the standing offer holds: I'll link and credit them from docs/PROTOCOL.md §12 if you want them findable.

Closes the discovery half of #35.

Extracted from #35, which bundled discovery with 3,806 lines of work that
has since landed via other PRs. Discovery was the part with no equivalent
on master and the biggest outstanding setup win: #40 showed setup failing
outright because the 15s wake timeout expires before a sleeping robot
answers, and a pre-filled host sidesteps the whole class of "which IP is
it" failures.

Declares `_narwal_sweeper._tcp.local.` in the manifest plus `NARWAL_*` /
`narwal_*` DHCP hostnames, and routes both into the existing user step
with the address pre-filled. The model still has to be picked by hand —
it isn't in the mDNS payload.

Verified against a Flow (AX12) on the local network:

    _app_wss_server_7bb53c._narwal_sweeper._tcp.local.
    server=NARWAL_7bb53c.local.  addrs=['10.0.0.112']  port=9002

That capture also settled how to match a discovery against an entry the
user added by hand. A configured entry's unique_id is the full device_id,
read over the WebSocket, which discovery cannot see — so a manual entry
would otherwise reappear as a "Discovered" card forever. The robot's
device_id is 71c53f01c14f49088338863e147bb53c and it advertises as
NARWAL_7bb53c: the last six hex characters. Matching on that suffix
identifies the device itself, and a match at a new address repoints the
existing entry instead of orphaning it.

Two changes from the original #35 implementation:

- It matched on host only, and its "IP drifted" branch rewrote the entry
  with the same host it had just matched on — a no-op under a comment
  claiming otherwise. Suffix matching does what that comment intended.
- The user-step schema now keeps whatever host is in hand, so a failed
  connect no longer clears what you typed.

Adds 12 config-flow tests and the service-info stubs they need.

Co-authored-by: Steve Motew <sjmotew@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAo9szBPifvDJrsag2oW6Y

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

Hassfest rejects `NARWAL_*`: "Value needs to be lowercase for dictionary
value @ data['dhcp'][1]['hostname']". Home Assistant lowercases a
discovered hostname before matching, so `narwal_*` already catches the
robot's announced `NARWAL_<6hex>` — the second entry was invalid and
redundant at the same time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAo9szBPifvDJrsag2oW6Y

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

#76 restructured async_step_user (device-ID step, _async_create_entry),
which collided with the discovery block and with the test file where both
sides appended.

Resolution: kept the whole discovery block plus master's one-line
async_step_user signature, and kept both sets of tests — master's
non-broadcast device-ID cases inside TestNarwalConfigFlow, TestDiscovery
after them. 262 passing = master's 250 plus this branch's 12, which is the
check that neither side was silently dropped.

The two features compose: discovery pre-fills the host, and a CX7 picked on
that form still routes to the device-ID step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAo9szBPifvDJrsag2oW6Y

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

…discovery

# Conflicts:
#	custom_components/narwal/manifest.json
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds zeroconf and DHCP discovery for Narwal robots and pre-fills the existing setup flow with the discovered address.

  • Declares Narwal mDNS and DHCP matchers in the integration manifest.
  • Matches discoveries to configured entries by the advertised device-ID suffix and updates addresses after network changes.
  • Adds localized discovery titles, setup documentation, service-info stubs, and config-flow coverage.

Confidence Score: 1/5

The PR does not appear safe to merge because discovery can rebind a configured robot to an unauthenticated endpoint, leave a loaded client using a stale address, and create duplicate flows across discovery transports.

The existing findings remain present: suffix-only matching can persist a discovery-controlled host without endpoint authentication, the live coordinator does not adopt that host update until reload, and zeroconf and DHCP assign different provisional identities to the same unconfigured robot.

Files Needing Attention: custom_components/narwal/config_flow.py

Important Files Changed

Filename Overview
custom_components/narwal/config_flow.py Adds zeroconf/DHCP routing, discovery matching, host pre-filling, and address updates for existing entries.
custom_components/narwal/manifest.json Registers the Narwal DHCP hostname pattern and mDNS service type.
tests/test_config_flow.py Expands config-flow coverage across both discovery transports, known-device matching, address changes, and manual setup.
tests/ha_stubs.py Adds lightweight Home Assistant service-info payload stubs required by the discovery tests.
README.md Documents automatic discovery, manual fallback, hostname matching, and cross-VLAN connectivity considerations.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Zeroconf or DHCP discovery] --> B[Extract host and provisional identity]
    B --> C{Existing entry matches host or device suffix?}
    C -->|Yes| D[Update stored host if changed]
    D --> E[Abort as already configured]
    C -->|No| F[Set provisional unique ID]
    F --> G[Open user step with host pre-filled]
    G --> H[Connect and obtain full device ID]
    H --> I[Create config entry]
Loading

Reviews (2): Last reviewed commit: "docs(readme): document the VLAN-segmente..." | Re-trigger Greptile

Comment thread custom_components/narwal/config_flow.py
Comment on lines +93 to +103
same_device = bool(suffix) and str(
entry.data.get("device_id", "")
).lower().endswith(suffix)
if not same_device and entry.data.get("host") != host:
continue
if same_device and entry.data.get("host") != host:
# Same robot, new address — a DHCP renewal or a subnet move.
# Repoint the entry instead of leaving it pointing at nothing.
self.hass.config_entries.async_update_entry(
entry, data={**entry.data, "host": host}
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Discovery suffix permits endpoint rebinding

A LAN device advertising the same observable six-hex suffix as a configured robot causes this branch to persist its address without authenticating the endpoint. After an entry reload, Home Assistant connects over plaintext WebSocket, sends the configured device ID and robot commands, and accepts fabricated responses as the configured robot.

How this was verified: The discovery-controlled suffix reaches async_update_entry, while the runtime client performs no endpoint authentication before sending commands or accepting protocol messages.

Comment on lines +123 to +125
return await self._async_discovered(
str(discovery_info.host), discovery_info.hostname.rstrip(".")
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Discovery IDs diverge across transports

Zeroconf retains .local in the provisional unique ID while DHCP uses the hostname without it, so the same unconfigured robot receives two flow identities. This permits duplicate discovery cards and setup attempts before final device-ID validation rejects the duplicate.

Robots have been reported not to answer connections sourced from outside
their own subnet even with 9002/TCP permitted, which presents as a plain
handshake timeout with no integration-level error. SNAT is confirmed
working; the root cause (source filtering vs. an ignored DHCP gateway) is
stated as open rather than guessed at, along with the capture that would
settle it.

Placed beside the discovery notes because mDNS is affected by the same
network topology for a different reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sjmotew
sjmotew merged commit 16be16e into master Aug 21, 2026
4 checks passed
@sjmotew
sjmotew deleted the feat/zeroconf-dhcp-discovery branch August 21, 2026 18:39
@sjmotew

sjmotew commented Aug 21, 2026

Copy link
Copy Markdown
Owner Author

Merged as 16be16e.

@StratoGh0st99 — I held this for your review from 2026-08-16 and merged it today without hearing back. Explaining why rather than leaving you to find it merged:

  • It had been sitting five days, and v1.0.4 had broken it in the meantime — the conflict was only manifest.json (the version bump against your zeroconf key), but a PR that goes stale twice tends to stay stale.
  • @DeNo64 field-confirmed it on #81 without being asked to: they installed this branch and reported it detected their Flow 2 with the correct IP. That is a second network, one I have no access to, and it is better evidence than my own testing was.

The authorship is yours — the discovery commit is --author'd to you in git and stays that way in master's history. Nothing about merging it changes that, and if you'd rather have carried this yourself I'm sorry to have taken it out of your hands.

Two things I added on top, both flagged so you can object to either:

  1. fix(manifest): DHCP hostname patterns must be lowercase — HA lowercases hostnames before matching, so NARWAL_* never fired. The declared pattern is narwal_*.
  2. A README note on VLAN-segmented networks, prompted by [Bug]: Accessible over VLANs #81. It sits beside your discovery notes because mDNS is affected by the same topology for a different reason.

The service type and hostname pattern were verified by a live mDNS browse against my own Flow before this landed; the device-ID-suffix match (the six hex characters are the tail of the robot's device ID) is what lets a discovery re-point a robot someone already added by hand, which your original host-only match couldn't do.

267 tests, all checks green. Thank you for the work — it's shipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants