Skip to content

Fix multiple loading of nodes#652

Merged
wollew merged 6 commits intoJulius2342:masterfrom
wollew:fix_load_all_nodes
Apr 4, 2026
Merged

Fix multiple loading of nodes#652
wollew merged 6 commits intoJulius2342:masterfrom
wollew:fix_load_all_nodes

Conversation

@wollew
Copy link
Copy Markdown
Collaborator

@wollew wollew commented Mar 26, 2026

The current code replaces instances of Node with new instances every time PyVLX.load_nodes() is called, even when the actual the device in the gateway didn't change. This leaves dangling callbacks and potentially leaks memory. This PR changes the code that gateway devices which are considered "physically the same device" (=either identical serial number or no serial number available but identical node id) do not generate new node instances.
Multiple calls to PyVLX.load_nodes() will be necessary in order to fulfill HA's "dynamic devices" quality scale rule.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/pyvlx
  node.py 40, 57, 73, 84
  nodes.py 63-67, 109, 116, 129, 132, 138, 150
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 prevents repeated PyVLX.load_nodes() calls from unnecessarily replacing Node instances (which can leave dangling callbacks), by introducing node identity matching and explicit disposal/unregistration for nodes that are replaced or removed.

Changes:

  • Add Node.dispose() and Node.represents_same_node() to support safe lifecycle management and identity matching.
  • Change Nodes._load_all_nodes() / _load_node() to merge snapshots by physical identity (serial, or node_id fallback when both serials are missing) instead of always recreating nodes.
  • Expand unit tests to assert callback unregistration on clear/replace and identity-preserving reload behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/pyvlx/node.py Adds disposal and identity helper used to preserve node instances across reloads.
src/pyvlx/nodes.py Disposes replaced/cleared nodes and merges loaded snapshots to keep existing instances when appropriate.
test/nodes_test.py Adds tests for disposal behavior and for identity-preserving _load_all_nodes() / _load_node() behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wollew wollew marked this pull request as draft March 26, 2026 12:27
wollew and others added 2 commits March 29, 2026 09:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@wollew wollew marked this pull request as ready for review March 29, 2026 07:56
@wollew wollew merged commit 274bd2b into Julius2342:master Apr 4, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants