[sendspin] Clear metadata and controller state on disconnect - #18289
Merged
Conversation
…t value A metadata field the server has cleared arrives as nullopt, the same as one it never sent, and both were skipped rather than published. A sensor therefore kept showing the previous track's value indefinitely and never fired on_value, so a track with no album name left the album sensor stuck on the last album, with no way for a display to tell "this track has no album" from "the album has not changed". Publish the empty string for a cleared text field and NAN for a cleared numeric one, and stop the track progress poller when progress goes away instead of leaving a frozen position on the frontend. Both sensor types skip the publish while has_state() is false, so a field the server never provides does not fire on_value with an empty value on the first metadata of every connection; that also avoids reading Sensor::raw_state before it is initialized. Handle MetadataRoleListener::on_metadata_clear(), which was left unimplemented: the hub now fans out a default-constructed state object on disconnect, so every metadata sensor blanks through the same path the per-field clears use rather than holding the previous server's metadata.
…nect Follow-ups to the metadata clear handling: - Stop the track progress poller in setup(). PollingComponent starts it before setup() runs, so the sensor published a position of 0 on every tick from boot until the first metadata arrived, rather than unknown. - Handle on_controller_state_clear() in the hub. The media player now returns to idle when the connection drops, since the server never sends a final stopped group update. This goes through its own callback because volume and muted are plain values, so a default constructed state object would read as a real 0% volume instead of no value. - Correct the comment claiming the sensor's raw state is uninitialized before the first publish. Sensor's constructor sets it to NAN. - Document that an empty text sensor state is not the same as unknown. missing_state follows has_state(), which is never cleared once set.
Contributor
|
To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file: external_components:
- source: github://pr#18289
components: [sendspin]
refresh: 1h(Added by the PR bot) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #18289 +/- ##
=======================================
Coverage 88.02% 88.02%
=======================================
Files 65 65
Lines 14902 14902
Branches 2241 2241
=======================================
Hits 13117 13117
Misses 1473 1473
Partials 312 312 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes stale Sendspin entity state after a server disconnect by propagating the sendspin-cpp library’s “clear” events to ESPHome child entities, ensuring metadata, progress, and playback state no longer remain frozen in the frontend.
Changes:
- Fan out cleared metadata as an “empty” metadata state so child sensors/text_sensors publish unknown/empty values instead of keeping old values.
- Add controller-state “clear” fan-out and make the Sendspin media player return to idle on disconnect.
- Improve dedup/initial-state handling for metadata and progress so “unknown” doesn’t churn or incorrectly become a real state.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| esphome/components/sendspin/text_sensor/sendspin_text_sensor.cpp | Publish empty string on cleared/absent metadata (with initial-state guard) so old text fields don’t linger. |
| esphome/components/sendspin/sensor/sendspin_sensor.cpp | Publish NAN for cleared numeric fields; stop progress polling and report unknown when progress is cleared/absent; add NAN-aware dedup. |
| esphome/components/sendspin/sendspin_hub.h | Add controller-state-clear callback registration and declare on_*_clear() overrides for controller/metadata. |
| esphome/components/sendspin/sendspin_hub.cpp | Implement on_controller_state_clear() and on_metadata_clear() fan-out behavior. |
| esphome/components/sendspin/media_player/sendspin_media_player.cpp | Subscribe to controller-state-clear and force media player state back to idle on disconnect. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Memory Impact AnalysisComponents:
This analysis runs automatically when components change. Memory usage is measured from a representative test configuration. |
Contributor
|
|
esphbot
approved these changes
Aug 11, 2026
Contributor
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
bdraco
previously approved these changes
Aug 11, 2026
The !has_state() branches in the two publish_if_changed_ helpers repeated what the dedup below them already did: the raw state starts as NAN for a sensor and empty for a text sensor, so a field that is already cleared when the first update arrives is suppressed either way. Keep the dedup only. Also note on add_metadata_update_callback() that a disconnect fans out an all-empty state object, since that is where a subscriber reads the contract.
Contributor
Author
|
Implemented the 3 suggested items esphbot suggested. |
bdraco
previously approved these changes
Aug 11, 2026
Contributor
Confirmed, all three landed. Both One leftover: same redundant |
Contributor
|
|
esphbot
approved these changes
Aug 11, 2026
Contributor
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
Drop the has_state() term from the track progress clear guard. The raw state starts as NAN, so the isnan() check already covers the first update, and the two publish guards in the file now read the same way. Make controller_state_callbacks_ lazy as well. The switch action and the media source both enable the controller role without a media_player, so neither controller callback has a subscriber in those builds. The old comment claimed only the clear callback was rarely subscribed, which was not true. Pull the media player state change and log out of the two callbacks into set_playback_state_(), so they cannot drift apart.
Contributor
PR Review — [sendspin] Clear metadata and controller state on disconnectAll three prior suggestions are resolved and the disconnect-clear semantics check out against the sendspin-cpp 0.7.1 sources. Merge-ready with no findings. What this revision gets right, verified rather than assumed:
Prior review items, all confirmed fixed in cfc3e40:
No blocking issues. ✅ Resolved since last review (2)Previously-flagged issues verified fixed
Checklist
Automated review by Kōan (Claude) |
esphbot
approved these changes
Aug 11, 2026
Contributor
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
Contributor
|
Note No code changes since the last review ( |
bdraco
approved these changes
Aug 11, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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?
When the connection to a Sendspin server drops, the library clears its cached metadata and controller state, but ESPHome kept displaying whatever the last server had sent. Track title, artist, album, year, and position all stayed frozen on the frontend, and the media player kept reporting that it was playing.
This makes the child entities follow the library's clear events.
Metadata
SendspinHubnow implementson_metadata_clear()and fans out an empty state object, so every field reaches the children asnullopt. The children publish an absent field instead of skipping it:NAN, which reads as unknown.missing_stateon the API followshas_state(), andhas_state()is never cleared once set, so an empty state is the closest a text sensor can get after it has published a real value. This is documented in the code.Both paths keep their dedup so a field that stays cleared does not republish on every metadata update.
NANnever compares equal to itself, so that case needed an explicit check.The same handling applies to a field the server clears mid-session, not just to disconnects. Previously an absent field was skipped entirely, so a track with no album would still show 8000 the previous track's album.
Track progress
The progress sensor stops its poller and reports unknown when the position is no longer known, rather than leaving the last position frozen.
It also stops the poller in
setup().PollingComponentstarts the poller beforesetup()runs, so from boot until the first metadata arrived the sensor published a position of 0 on every tick instead of unknown. The metadata callback starts it once playback is running.Controller state
on_controller_state_clear()is now handled, and the media player returns to idle when the connection drops. The server never gets to send a final stopped group update, so without this the entity keeps reporting playing indefinitely.This goes through its own callback rather than reusing the controller state fan-out.
ServerStateControllerObjectstores volume and muted as plain values rather than optionals, so a default constructed object would arrive at the children as a real 0% volume rather than as no value. Volume and mute therefore keep their last values, sincemedia_playerhas no way to express an unknown volume.Types of changes
Related issue or feature (if applicable): not applicable
Pull request in esphome.io with documentation (if applicable): not applicable
Pull request in developers.esphome.io with developer documentation (if applicable): not applicable
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: