allow get_state request for non-data NodeFeatures #328
Conversation
WalkthroughThis PR makes get_state more defensive across multiple node classes by skipping assignments when the superclass result lacks a feature key and changes PlugwiseBaseNode.get_state to log-and-skip unsupported features instead of raising. It also updates the changelog for release v0.44.13 and bumps the project version to 0.44.13. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant N as Node Subclass
participant B as BaseNode
participant A as Adapter
Note over C,N: Request states for one or more features
C->>N: get_state(features)
loop per feature
N->>B: super().get_state((feature,))
B->>A: fetch/collect raw state
A-->>B: { ... } or {}
alt Base returned contains feature
B-->>N: {feature: value}
N->>N: assign states[feature] = value
else Base returned missing feature
B-->>N: {} (logs debug for unsupported/missing)
N->>N: skip assignment
end
end
opt ensure AVAILABLE
N->>N: add NodeFeature.AVAILABLE if absent
end
N-->>C: aggregated states dict
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugwise_usb/nodes/circle.py (1)
1246-1246: Fix Ruff PLR0912 onget_stateThe function exceeds the allowed branch count (GitHub Actions failure). Either refactor or add a targeted noqa to unblock CI.
Minimal unblock diff:
- async def get_state(self, features: tuple[NodeFeature]) -> dict[NodeFeature, Any]: + async def get_state(self, features: tuple[NodeFeature]) -> dict[NodeFeature, Any]: # noqa: PLR0912Longer-term, consider extracting feature-handling cases into small helpers to reduce branching.
🧹 Nitpick comments (1)
plugwise_usb/nodes/scan.py (1)
510-512: Docstring nit: pluralize “feature”.The function accepts a tuple of features.
Apply:
- """Update latest state for given feature.""" + """Update latest state for given features."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
plugwise_usb/nodes/circle.py(1 hunks)plugwise_usb/nodes/node.py(1 hunks)plugwise_usb/nodes/scan.py(1 hunks)plugwise_usb/nodes/sed.py(1 hunks)plugwise_usb/nodes/sense.py(1 hunks)plugwise_usb/nodes/switch.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-19T06:38:04.702Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#255
File: plugwise_usb/nodes/circle.py:477-482
Timestamp: 2025-06-19T06:38:04.702Z
Learning: In plugwise_usb/nodes/circle.py, the timestamp comparison `prev_address_timestamp - self._last_collected_energy_timestamp` in the `get_missing_energy_logs` method is intentional. The code iterates backwards through time-ordered energy log addresses, where `prev_address_timestamp` contains the more recent timestamp from the previous iteration, and this subtraction correctly calculates the time gap to determine when to stop collecting outdated data.
Applied to files:
plugwise_usb/nodes/circle.py
📚 Learning: 2025-08-07T17:50:29.188Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#302
File: plugwise_usb/nodes/scan.py:132-142
Timestamp: 2025-08-07T17:50:29.188Z
Learning: In the plugwise_usb codebase, the `_load_defaults` method in node classes (scan.py, switch.py, sense.py) is only called once during node initialization, not during runtime. Therefore, flags like `_sed_node_info_update_task_scheduled` don't need to be reset at the start of `_load_defaults` as there's no risk of stale state from previous loads.
Applied to files:
plugwise_usb/nodes/node.pyplugwise_usb/nodes/scan.pyplugwise_usb/nodes/sense.py
🧬 Code graph analysis (2)
plugwise_usb/nodes/circle.py (1)
plugwise_usb/nodes/helpers/cache.py (1)
states(23-25)
plugwise_usb/nodes/scan.py (1)
plugwise_usb/nodes/helpers/cache.py (1)
states(23-25)
🪛 GitHub Actions: Latest commit
plugwise_usb/nodes/circle.py
[error] 1246-1246: Command: 'ruff check plugwise_usb/ tests/' failed: PLR0912 Too many branches (13 > 12).
plugwise_usb/nodes/node.py
[error] 638-638: Command: 'ruff check plugwise_usb/ tests/' reported G003 Logging statement uses +.
🔇 Additional comments (6)
plugwise_usb/nodes/switch.py (1)
156-159: Guarding superclass result prevents KeyError — good changeThe membership check before indexing
state_resultis correct and aligns with the new base-class behavior.plugwise_usb/nodes/sed.py (1)
649-652: Defensive access ofstate_resultlooks goodThe added membership check avoids errors when a requested feature intentionally returns no data.
plugwise_usb/nodes/circle.py (1)
1296-1299: Safe access pattern adopted — approvedGuarding against missing keys from
super().get_state()is correct and consistent across nodes.plugwise_usb/nodes/sense.py (1)
167-170: Membership check avoids KeyError — approvedMatches the new “non-data feature returns nothing” contract from the base class.
plugwise_usb/nodes/scan.py (2)
531-534: Good guard to avoid KeyError for non-data features.Conditionally assigning only when
featureis present instate_resultprevents KeyError and aligns with the PR objective to return no data (instead of raising) for non-data features.
519-524: Ignore inconsistent behavior alert: allget_stateimplementations—including the base class—still raiseNodeErrorfor unsupported features, and subclasses mirror that behavior.Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
==========================================
- Coverage 80.56% 80.47% -0.10%
==========================================
Files 36 36
Lines 7559 7564 +5
==========================================
- Hits 6090 6087 -3
- Misses 1469 1477 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Some NodeFeatures are not carrying any data, however they are used to generate entities such as buttons in HA. If polling requests data for these features, no data should be returned, and no exception generated.
Summary by CodeRabbit
Bug Fixes
Stability
Chores