Fix yaml changes updates to dashboard device#4
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates device-loading behavior so the dashboard reflects user edits to esphome: metadata in the YAML immediately, rather than waiting for the next compile to refresh StorageJSON.
Changes:
- Add a lightweight parser to extract
esphome.name,esphome.friendly_name, andesphome.commentfrom YAML content. - Prefer YAML-provided
name/friendly_name/commentoverStorageJSONvalues when building theDevicemodel. - When creating a device, persist a parsed platform into
StorageJSON.target_platformif no board match is found.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fallback_name = filename.removesuffix(".yml").removesuffix(".yaml") | ||
| storage_name = storage.name if storage else None | ||
| name = yaml_name or storage_name or fallback_name | ||
|
|
||
| storage_friendly = storage.friendly_name if storage else None | ||
| friendly_name = yaml_friendly if yaml_friendly is not None else (storage_friendly or name) | ||
|
|
There was a problem hiding this comment.
_parse_esphome_meta is documented to return None only when a key is absent (so callers can distinguish an explicitly empty string). But _load_device_from_storage uses name = yaml_name or storage_name or fallback_name, which treats an empty string as falsy and incorrectly falls back to storage/filename. Use an is not None check (like the friendly_name logic below) so an explicit empty YAML value is handled consistently, or adjust the docstring if empty names should never be honored.
| # Strip inline `# comment` and matching quotes. | ||
| if "#" in value and not (value.startswith('"') or value.startswith("'")): | ||
| value = value.split("#", 1)[0].rstrip() | ||
| if (value.startswith('"') and value.endswith('"')) or ( | ||
| value.startswith("'") and value.endswith("'") | ||
| ): | ||
| value = value[1:-1] |
There was a problem hiding this comment.
Inline comment stripping is currently skipped whenever the value starts with a quote. In YAML, quoted scalars can still have a trailing inline comment (e.g. friendly_name: "My Device" # living room), and the current logic will keep the # ... as part of the value and also prevent the surrounding-quote stripping from working. Consider stripping #... only when the # is outside quotes (or parse this block with the project’s YAML loader) so quoted values with trailing comments are handled correctly.
* On-disk validation of cleanup_ttl_seconds (Copilot). The WS validator on set_settings gates write-through-WS, but the on-disk decode path didn't apply the same not_bool / range check. A sidecar with cleanup_ttl_seconds: true would deserialise as 1 (bool is an int subclass) and the sweep would treat anything older than 1s as cold. Other wrong types (string, float, None) would propagate to the sweep's now - ttl_seconds arithmetic and raise TypeError every cycle. New __post_init__ on RemoteBuildSettings coerces non-int / bool back to DEFAULT_CLEANUP_TTL_SECONDS and clamps the result to [MIN, MAX]. Doesn't reject the row (no ValueError) — the load path stays robust against partially-corrupt sidecars; the operator's last-good enabled toggle survives even if the TTL field is broken. Six new tests in test_config_controller.py pin the coerce-to-default branches, the clamp-to-MIN / clamp-to-MAX branches, and the pass-through-on-valid branch. * Public seam on FirmwareController for in-flight remote-peer jobs (reviewer concern #1). Reaching into firmware._jobs from the cleanup loop coupled the two controllers to the private dict shape — a future refactor (lock-wrapped jobs, QUEUED + RUNNING split into two dicts, indexed view) would silently break this loop. New FirmwareController.active_remote_peer_jobs generator filters to QUEUED / RUNNING + non-empty remote_peer. The cleanup loop now consumes through that seam. Two new tests pin every filter branch (local skip, remote queued / running yielded, remote completed skipped, empty case). * Named constant for the parse_from_configuration tail-segment count (reviewer nit). The +3 magic was folklore; replaced with _TAIL_SEGMENT_COUNT and a docstring explaining the three required tail parts (dashboard_id, device_name, YAML filename under the device subtree). Reviewer concern #2 (RAM-canonical settings) and #4 (rate-limit per cycle) deferred per their "worth a small follow-up" framing. Concern #3 (first-sweep latency) was considered but YAGNI for this PR: the receiver-side cleanup ships ahead of any production user (alpha, ~10 beta testers), so no accumulated subtrees exist before this code's first cycle — and the default 24h TTL means even a fresh receiver has nothing to reclaim for the first day regardless.
No description provided.