server: track UI-created device lifetime cancel in sync.Map#29007
Draft
DerAndereAndi wants to merge 1 commit intoevcc-io:masterfrom
Draft
server: track UI-created device lifetime cancel in sync.Map#29007DerAndereAndi wants to merge 1 commit intoevcc-io:masterfrom
DerAndereAndi wants to merge 1 commit intoevcc-io:masterfrom
Conversation
Devices created or updated via the config UI now register their cancel function in a package-level sync.Map keyed by config ID. deleteDevice loads and fires the cancel; updateDevice swaps old for new so the previous instance's cancel is invoked after the new one is installed. This gives resource-holding devices (for example EEBus chargers and meters) a clean hook to release external state via their existing <-ctx.Done() goroutine — no new public interface, no changes to configurableDevice. Known limitation: devices loaded at startup from the database (via cmd/setup.go:configurableInstance) are not registered in the map. Deleting such a device through the UI will not trigger its lifetime cancel, so any external registration will linger until evcc restart. The common create-via-UI → delete-via-UI flow is fully covered. Closing this gap requires the per-device lifecycle work that is not yet implemented.
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The package-level
deviceCancelssync.Mapwith rawanyvalues and repeated.(context.CancelFunc)assertions would be easier to reason about and safer if you wrapped it in small helper functions (e.g.setDeviceCancel,swapDeviceCancel,cancelAndDeleteDevice) so the casting and lifecycle semantics live in one place. - The long limitation comment above
deviceCancelsmixes behavioural rationale with future plans; consider shortening it and moving the detailed explanation into a higher-level design note or the PR description to keep this handler file focused on implementation details.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The package-level `deviceCancels` `sync.Map` with raw `any` values and repeated `.(context.CancelFunc)` assertions would be easier to reason about and safer if you wrapped it in small helper functions (e.g. `setDeviceCancel`, `swapDeviceCancel`, `cancelAndDeleteDevice`) so the casting and lifecycle semantics live in one place.
- The long limitation comment above `deviceCancels` mixes behavioural rationale with future plans; consider shortening it and moving the detailed explanation into a higher-level design note or the PR description to keep this handler file focused on implementation details.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Devices created or updated via the config UI now register their cancel function in a package-level sync.Map keyed by config ID. deleteDevice loads and fires the cancel; updateDevice swaps old for new so the previous instance's cancel is invoked after the new one is installed. This gives resource-holding devices (for example EEBus chargers and meters) a clean hook to release external state via their existing <-ctx.Done() goroutine — no new public interface, no changes to configurableDevice.
Known limitation: devices loaded at startup from the database (via cmd/setup.go:configurableInstance) are not registered in the map. Deleting such a device through the UI will not trigger its lifetime cancel, so any external registration will linger until evcc restart. The common create-via-UI → delete-via-UI flow is fully covered. Closing this gap requires the per-device lifecycle work that is not yet implemented.