Surface MAC + derived ethernet/bluetooth MACs from mDNS#338
Merged
Conversation
Adds a new ``mac_address`` field on the ``Device`` model populated
from the ``mac`` TXT record on the device's
``_esphomelib._tcp.local.`` mDNS service. Mirrors the existing
``version`` / ``config_hash`` / ``api_encryption`` TXT pipelines:
- ``DeviceStateMonitor`` gains an ``apply_mac_address`` method and
an ``on_mac_address_change`` callback. Values are normalized at
ingest (``_normalize_mac`` strips ``:`` / ``-`` / ``.`` separators
and lowercases) so the dedupe + sidecar stay canonical even if a
future firmware switches case or separator style. Non-hex /
wrong-length inputs are dropped — same shape as the empty-TXT
no-op other apply-* methods use.
- The TXT extraction site at ``_apply_service_info_to_device``
picks up ``props.get("mac")`` alongside ``version`` / ``config_hash``.
- ``DevicesController._on_mac_address_change`` writes through to
the matching device(s), fires ``DEVICE_UPDATED``, and schedules
a sidecar ``set_device_metadata(..., mac_address=mac)`` write —
but only after the existing ``device.mac_address == mac`` early-
return, so a steady-state mDNS announce of the same value is a
no-op (no in-memory write, no executor-bound I/O). Persistence
matters because ESPHome devices are mDNS-silent until probed:
without the sidecar, the dashboard shows a blank MAC for the
bootstrap window after every restart.
- ``mac_address`` joins ``ip`` and ``expected_config_hash`` in
``_VOLATILE_DEVICE_METADATA_FIELDS`` so the archive flow scrubs
it (the YAML may be redeployed to a different physical board on
unarchive; a stale persisted MAC would render until the next
announce overwrote it).
13 new tests cover the monitor's first-observation / dedupe /
unknown-name / refire-after-rebuild / normalization / rejection
paths, the controller's persist-on-change + skip-on-unchanged /
skip-on-unknown branches, the sidecar round-trip + clear-on-empty
contract, and the archive volatility extension.
The dashboard already normalized at ingest, but the canonical form was lowercase 12-hex-char. Switching to ``XX:XX:XX:XX:XX:XX`` (uppercase, colon-separated) so the in-memory model, sidecar, and frontend wire all share the human-friendly form — no frontend-side pretty-printer needed, callers reading the sidecar or the WS event get a value that's already in the shape every router admin / OUI lookup expects. ``_normalize_mac`` now strips ``:`` / ``-`` / ``.`` separators, uppercases, validates 12 hex chars, then re-inserts ``:`` between each octet. Idempotent: an already-canonical re-broadcast passes through unchanged. ``derive_interface_macs`` likewise operates on canonical form end-to-end (input + output), so ``ethernet_mac`` / ``bluetooth_mac`` share the wire shape. The 17-char length check rejects the older compact form a caller might pass by mistake. Test coverage extended with idempotence, separator-style and already-canonical input branches, the "new-primary-clears-stale-derived" branch, and the helper's "reject uncanonical input" branch.
Merged
9 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end surfacing of device MAC addresses from the _esphomelib._tcp.local. mDNS mac TXT record into the backend Device model, persists the primary MAC to the device metadata sidecar for fast display after restart, and deterministically derives Ethernet/Bluetooth MACs for certain platforms based on ESP-IDF allocation rules.
Changes:
- Add MAC ingestion/normalization in
DeviceStateMonitorand propagate updates throughDevicesController(including sidecar persistence). - Introduce
derive_interface_macs()helper to computeethernet_mac/bluetooth_macfrom primary MAC + platform + loaded integrations. - Expand device loading/scanning + metadata persistence/tests to include
mac_addressand archive-scrubbing behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_mdns_mac_address.py | New tests for mDNS MAC ingestion, normalization, dedupe, controller persistence, and derivations. |
| tests/test_mac_address_derivation.py | New unit tests covering derivation rules and edge cases. |
| tests/test_config_controller.py | Adds sidecar persistence/clear tests for mac_address. |
| tests/controllers/devices/test_archive.py | Ensures mac_address is treated as volatile and scrubbed on archive. |
| tests/conftest.py | Adds callback plumbing for on_mac_address_change. |
| esphome_device_builder/models/devices.py | Extends Device with mac_address, ethernet_mac, bluetooth_mac. |
| esphome_device_builder/helpers/mac_addresses.py | New helper to derive interface MACs from the primary MAC. |
| esphome_device_builder/helpers/device_yaml.py | Recomputes derived MACs during device load based on stored primary MAC + integrations. |
| esphome_device_builder/controllers/devices/controller.py | Wires MAC callback, updates device fields, derives interface MACs, persists primary MAC. |
| esphome_device_builder/controllers/config.py | Persists/clears mac_address and marks it volatile for archive scrub. |
| esphome_device_builder/controllers/_device_state_monitor.py | Adds MAC normalization + apply path from mDNS TXT props. |
| esphome_device_builder/controllers/_device_scanner.py | Threads mac_address metadata into device loading. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
+ Coverage 98.60% 98.62% +0.01%
==========================================
Files 46 47 +1
Lines 5303 5373 +70
==========================================
+ Hits 5229 5299 +70
Misses 74 74
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- ``MacAddressChangeCallback`` doc now describes the canonical form the callback receives (``XX:XX:XX:XX:XX:XX``, normalized by ``_normalize_mac``) instead of the pre-normalization "lowercase 12-hex-char" lie left over from the earlier shape. - ``apply_mac_address`` doc spells out that ``_normalize_mac`` uppercases + re-inserts colons, matching what the function actually does after the canonical-form flip in 9cdac68. - ``_MAC_SEPARATORS`` comment no longer claims ``:`` is the firmware's wire format — today's broadcast is the compact 12-hex-char form; ``:`` is the dashboard's *canonical* form applied at ingest. The strip-and-re-insert logic is independent of which form ESPHome happens to broadcast. - ``derive_interface_macs`` validates hex on the trailing octet in addition to length, so a corrupt / hand-edited 17-char sidecar value with non-hex chars (``94:C9:60:1F:8C:ZZ``) returns empty derived MACs instead of raising ``ValueError`` deep inside ``_offset_last_octet``. Regression test added.
When a ``.local`` device responds to ping but isn't broadcasting ``_esphomelib._tcp.local.`` (non-API ESPHome firmware, the ``zwave-proxy-seeedw5500.local`` case), the resolved address from ``dns_cache.async_resolve`` was being thrown away — the ``is_local_hostname`` guard in ``_ping_sweep`` skipped ``apply_ip`` "to keep mDNS in charge of IP tracking", but for devices without an ``_esphomelib._tcp`` broadcast the mDNS path never populates ``device.ip`` at all, leaving the drawer / table to render an em-dash even after successful pings. The guard's stated reason (avoid stale-DNS clobbering live mDNS values) is moot because ``apply_ip`` already preserves an existing multi-IP set when the incoming target is one of the already-known addresses — the typical case for a ``.local`` device with an active broadcast. So the guard is unconditionally removed, and ``test_ping_sweep_applies_ip_for_local_hosts`` inverts the previous "doesn't apply" assertion to pin the new behaviour.
Three branches the diff added but the existing test set didn't
reach:
- ``_normalize_mac``'s ``except ValueError`` arm: covered by a
12-char-after-separator-strip but non-hex input
(``"94c9601f8cZZ"``). The length check passes, ``int(_, 16)``
raises, and the helper returns empty so a corrupt TXT can't
pollute the sidecar with a value that looks the right shape
but doesn't decode.
- ``_apply_service_info``'s ``if mac := props.get("mac"):`` plumb
to ``apply_mac_address``. Mirrors the existing
``test_apply_service_info_claims_online`` shape but populates
``decoded_properties = {"mac": "94c9601f8cf1"}`` and asserts
the recording callback receives the normalized canonical form.
- ``_on_mac_address_change``'s "primary changed → derived
re-derives" branch on a device that still has both ethernet
and bluetooth integrations loaded. The previous
``clears_stale_derived`` test only exercised the
integrations-removed path; this pins the inverse — when the
integrations stay loaded, ``ethernet_mac`` and
``bluetooth_mac`` follow the new base via their respective
offsets (a factory-replacement scenario where the same YAML
gets pointed at a new physical board).
Codecov patch coverage on this PR was flagged at 94.44%
(_device_state_monitor.py with 4 missing lines); these tests
take it to 100%.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- ``derive_interface_macs`` docstring fully qualifies the ``_normalize_mac`` reference as ``controllers._device_state_monitor._normalize_mac``. ``mac_addresses.py`` doesn't import the function (only its output), so the bare-name cross-reference would dead-link in rendered docs. - ``_resolve_device_metadata`` docstring updated to cover the new ``mac_address`` sidecar field and to note that ``ethernet_mac`` / ``bluetooth_mac`` are *not* persisted — they're recomputed at ``Device`` construction so a YAML edit toggling integrations picks up the new derivation on the next reload, no stale-cache window.
A monitor built without ``on_mac_address_change`` (older in-process callers, narrower test fixtures) hits the ``return False`` branch at the top of ``apply_mac_address`` before the normalize / dedupe / dispatch path. Codecov was the only thing flagging this; the new ``test_apply_mac_address_no_op_when_callback_unwired`` walks the branch by constructing a monitor with only the required ``on_state_change`` / ``on_ip_change`` callbacks. Patch coverage on PR #338 was at 98.61% with this single line flagged; this takes it to 100%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Adds MAC-address surfacing to
DevicesController/DeviceStateMonitor: the broadcastmacTXT on_esphomelib._tcp.local.lands onDevice.mac_address(canonicalXX:XX:XX:XX:XX:XXform, normalized at ingest), and devices with theethernetoresp32_ble*/bluetooth_*integrations loaded also get a derivedethernet_mac/bluetooth_macper Espressif's MAC allocation table (Ethernet = base + 3, Bluetooth = base + 2). RP2040 / RP2350 share a single MAC across interfaces, soethernet_mac == mac_addressthere.Same monitor → controller pipeline as the existing
version/config_hash/api_encryptionTXT records. The primary MAC is persisted to the per-device metadata sidecar (ip/expected_config_hashneighbour) so a freshly-restarted dashboard renders the value immediately — ESPHome devices stay mDNS-silent until probed, otherwise the column renders blank for the discovery sweep's bootstrap window. Persistence is gated on a real change (the existingdevice.mac_address == macearly-return covers both the in-memory write and the executor-boundset_device_metadatacall) so the steady-state "same MAC every announce" cycle stays off-disk.mac_addressjoinsip/expected_config_hashin_VOLATILE_DEVICE_METADATA_FIELDSso archive scrubs it (the YAML may be redeployed to a different physical board on unarchive).The derived MACs are deterministic from primary +
loaded_integrations, so we recompute on every primary-MAC observation and on everyload_device_from_storagerather than persisting — a YAML edit that toggles bluetooth picks up the new derived MAC on the next reload, no stale-cache window._normalize_macstrips:/-/.separators, validates 12 hex chars, then re-inserts:between each octet so the form is idempotent regardless of which case / separator style the firmware happens to broadcast.Espressif allocation reference: https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/misc_system_api.html#mac-address-allocation
Related issue or feature (if applicable):
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesFrontend coordination
Checklist
ruff,codespell, yaml/json/python checks).tests/where applicable.components.jsonhas not been hand-edited (regenerate viascript/sync_components.pyif a sync is needed).docs/ARCHITECTURE.mdand/ordocs/API.md.