Skip to content

Send the mDNS PTR record lifetime, not its remaining TTL#1685

Merged
bdraco merged 1 commit into
mainfrom
mdns-expiry-from-last-seen
Jun 24, 2026
Merged

Send the mDNS PTR record lifetime, not its remaining TTL#1685
bdraco merged 1 commit into
mainfrom
mdns-expiry-from-last-seen

Conversation

@bdraco

@bdraco bdraco commented Jun 24, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

Follow-up to #1682. That PR shipped mdns_ptr_ttl_remaining_seconds plus a devices/get_reachability one-shot command so the drawer could poll the PTR's remaining TTL. Testing the live drawer showed the remaining TTL reads out of sync with "last seen": the drawer actively probes the A record (keeping last-seen fresh) while the PTR's remaining TTL is only refreshed by the zeroconf browser at ~80% of the lifetime, so the countdown drifted against last-seen.

This sends the PTR's full announced lifetime instead, mdns_ptr_ttl_seconds (= ptr.ttl, the device's own record lifetime), and the frontend derives the offline countdown as that lifetime minus the last-seen age, so the two move in lockstep on every subscription push. That makes the poll unnecessary, so devices/get_reachability is removed; no extra mDNS traffic is generated.

Related issue or feature (if applicable):

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix
  • New feature (non-breaking change which adds functionality) — new-feature
  • Enhancement to an existing feature — enhancement
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — docs
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Frontend coordination

Checklist

  • The code change is tested and works locally.
  • Pre-commit hooks pass (ruff, codespell, yaml/json/python checks).
  • Tests have been added or updated under tests/ where applicable.
  • components.index.json / definitions/components/*.json have not been hand-edited (regenerate via script/sync_components.py if a sync is needed).
  • Architecture-level changes are reflected in docs/ARCHITECTURE.md and/or docs/API.md.

@github-actions github-actions Bot added the enhancement Improvement to an existing feature label Jun 24, 2026
@bdraco bdraco marked this pull request as ready for review June 24, 2026 21:38
Copilot AI review requested due to automatic review settings June 24, 2026 21:38
@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks


Comparing mdns-expiry-from-last-seen (e27bda7) with main (584d722)

Open in CodSpeed

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (584d722) to head (e27bda7).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1685      +/-   ##
==========================================
- Coverage   99.54%   99.54%   -0.01%     
==========================================
  Files         227      227              
  Lines       18142    18134       -8     
==========================================
- Hits        18060    18052       -8     
  Misses         82       82              
Flag Coverage Δ
py3.12 99.52% <100.00%> (-0.01%) ⬇️
py3.14 99.54% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
..._builder/controllers/_device_state_monitor/mdns.py 99.36% <100.00%> (ø)
...evice_builder/controllers/_reachability_tracker.py 100.00% <100.00%> (ø)
...e_device_builder/controllers/devices/controller.py 99.73% <ø> (-0.01%) ⬇️
esphome_device_builder/models/devices.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the device reachability payload to send the mDNS PTR record’s full announced lifetime (mdns_ptr_ttl_seconds) rather than its remaining TTL, so the frontend can derive an “offline in N” countdown from the PTR lifetime minus “last seen” age (removing the need for polling).

Changes:

  • Replace mdns_ptr_ttl_remaining_seconds with mdns_ptr_ttl_seconds throughout the reachability snapshot/event payload.
  • Remove the devices/get_reachability one-shot WS command (polling no longer needed).
  • Update API docs and adjust/remove tests accordingly.

Reviewed changes

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

Show a summary per file
File Description
esphome_device_builder/controllers/_device_state_monitor/mdns.py Emit PTR announced TTL (ptr.ttl) instead of remaining TTL in get_mdns_cache_info.
esphome_device_builder/controllers/_reachability_tracker.py Rename snapshot field to mdns_ptr_ttl_seconds and propagate into the wire snapshot.
esphome_device_builder/models/devices.py Update DeviceReachabilityData TypedDict field name to mdns_ptr_ttl_seconds.
esphome_device_builder/controllers/devices/controller.py Remove the devices/get_reachability API command.
docs/API.md Update devices/subscribe_reachability documentation and remove the devices/get_reachability entry.
tests/test_state_monitor_reachability.py Update assertions to validate PTR lifetime instead of remaining TTL.
tests/test_reachability_tracker.py Update snapshot expectations for renamed PTR TTL field.
tests/controllers/devices/test_subscribe_reachability.py Remove coverage for the deleted devices/get_reachability command.

Comment thread esphome_device_builder/controllers/_reachability_tracker.py Outdated
@esphbot

esphbot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

The drawer derives its offline countdown as the record lifetime minus
the last-seen age, so the value re-anchors in lockstep with last seen
instead of tracking the PTR's remaining TTL; the browser refreshes that
remaining TTL at ~80% of the lifetime, which drifts against the
actively-probed A record and reads out of sync with last seen.

Replaces the reachability snapshot's mdns_ptr_ttl_remaining_seconds with
mdns_ptr_ttl_seconds (the device's own announced PTR lifetime) and drops
devices/get_reachability; the countdown is computed client-side from data
the subscription already pushes, so no poll is needed.
@bdraco bdraco force-pushed the mdns-expiry-from-last-seen branch from f6dd282 to e27bda7 Compare June 24, 2026 21:53
@esphbot

esphbot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

PR Review — Send the mDNS PTR record lifetime, not its remaining TTL

Clean, well-tested follow-up. Merge-ready; only a minor docstring-style nit.

Strengths:

  • The mdns_ptr_ttl_remaining_secondsmdns_ptr_ttl_seconds rename is applied consistently across the TypedDict, dataclass, snapshot builder, mdns reader, docs, and tests — no dangling references. Verified get_reachability_snapshot (controller.py:1103) is a separate, retained method still wired into reachability.py; only the get_reachability command is removed.

  • The value change is correct: ptr.ttl is the constant announced lifetime, so the test rightly drops the flaky pytest.approx(..., abs=1.0) timing tolerance for an exact == 4500.0.

  • docs/API.md is updated in lockstep, including corrected field semantics.

  • Suggestion: the MdnsCacheInfo docstring (and matching mdns.py comment) carry drift rationale that repo style wants out of docstrings — non-blocking, partly pre-existing.

  • Note: dropping a WS command shipped in Expose the mDNS PTR record TTL on the reachability snapshot #1682 is a public-API removal, but it's coordinated with frontend PR Enable SLOT (flake8-slots) ruff rule #969 and the feature is early beta, so acceptable.



Checklist

  • Field rename applied consistently, no dangling references
  • PTR lifetime value semantics correct (ptr.ttl vs get_remaining_ttl)
  • Docs (API.md) updated to match
  • Test coverage updated for renamed field and removed command
  • Docstring/comment style (contract-focused, no rationale)
  • Public-API removal coordinated with frontend

Automated review by Kōan (Claude) HEAD=e27bda7 1 min 11s

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

@bdraco bdraco merged commit aa63eb4 into main Jun 24, 2026
21 checks passed
@bdraco bdraco deleted the mdns-expiry-from-last-seen branch June 24, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants