Carry api_encryption_active across scanner reloads#365
Merged
Conversation
mDNS announces fire on the device's TTL refresh — typically a couple of minutes between Added/Updated callbacks for a healthy device. Anything that triggers a scanner.reload between announces (a successful flash, an --only-generate run, an unrelated YAML edit on a sibling, an atomic-save remove/re-add cycle) was rebuilding the in-memory Device with default fields, wiping a previously-truthy api_encryption_active back to None. The frontend's getEncryptionState reads api_encryption_active together with has_pending_changes and renders the lock icon's four-state indicator. With api_encryption_active=null and has_pending_changes=true (which the post-flash refresh sets even on an OTA that succeeded), the indicator flips into the 'pending install' chip on a freshly-flashed encrypted device that's still actively broadcasting its noise PSK on mDNS. The user reported it as 'pending encryption install on every encryption device that mDNS could see' — a restart of the backend cleared it because the next mDNS announce after the fresh subscription repopulated the field. Mirror the existing carry-forward shape used for state / deployed_config_hash / ip_addresses: pull api_encryption_active off previous when one is provided, default to None on first load. apply_api_encryption follows the 'Device is the source of truth' rule from PR #75 and dedupes against device.api_encryption_active rather than a monitor-side cache, so seeding the value from previous doesn't break the 'next mDNS announce corrects mismatched state' guarantee — the next genuine observation still hits the callback path.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #365 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 52 52
Lines 5911 5912 +1
=======================================
+ Hits 5831 5832 +1
Misses 80 80
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
16 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a device-state regression where api_encryption_active (observed via mDNS and used by the dashboard’s lock/encryption indicator) was being dropped back to None whenever the device scanner rebuilt Device objects during reloads, causing transient but confusing UI state.
Changes:
- Carry forward
api_encryption_activefrom the prior in-memoryDevicewhenload_device_from_storage(..., previous=...)is used (matching existing carry-forward behavior for other monitor-derived fields). - Update the
load_device_from_storagedocstring to includeip_addressesandapi_encryption_activein the carry-forward contract. - Add regression tests covering truthy encryption values, empty-string plaintext confirmation, and the no-previous default (
None).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
esphome_device_builder/helpers/device_yaml.py |
Preserves api_encryption_active across reloads by copying it from previous into the new Device. |
tests/test_device_yaml.py |
Adds regression tests ensuring encryption-active state is carried over correctly (including "") and defaults to None on first load. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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?
load_device_from_storagewas building the newDevicefromdefaults whenever
scanner.reload(...)fired, dropping anyapi_encryption_activevalue the mDNS browser had alreadywritten. mDNS announces only fire on the service-record TTL —
typically a couple of minutes between
Added/Updatedcallbacks — so any reload triggered between announces (a
successful flash, an
--only-generaterun, an unrelated YAMLedit on a sibling, an atomic-save remove/re-add cycle) wiped
the value back to
Nonefor the gap window.The frontend's
getEncryptionStatereadsapi_encryption_activetogether withhas_pending_changesandrenders the lock icon's four-state indicator. With
api_encryption_active=nullandhas_pending_changes=true(which the post-flash refresh sets even on an OTA that
succeeded), the indicator flips into the Pending install
chip on a freshly-flashed encrypted device that's still
actively broadcasting its noise PSK on the wire. Reported as
"every encryption device that mDNS could see still says pending
install"; a backend restart cleared it because the next mDNS
announce after the fresh subscription repopulated the field.
Mirror the existing carry-forward shape used for
state/deployed_config_hash/ip_addresses: pullapi_encryption_activeoffpreviouswhen one is provided,default to
Noneon first load.apply_api_encryptionfollows the "Device is the source of truth" rule from #75 and
dedupes against
device.api_encryption_activerather than amonitor-side cache, so seeding the value from
previousdoesn't break the "next mDNS announce corrects mismatched
state" guarantee — the next genuine observation still hits the
callback path. Adds three regression tests covering the
truthy /
""/ no-previouscases.Related issue or feature (if applicable):
install on every encryption device" report addressed by
Resolve build_info.json through ext_storage_path #364 (build_info.json path fix). Even with that fix in
place, the brief reload window kept tripping the indicator
on healthy devices.
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.