-
Notifications
You must be signed in to change notification settings - Fork 2
properly propagate configuration changes #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughLOADED now fires before initialization in the scan node, and MOTION_CONFIG is always published after awake tasks. SED battery-setting setters short-circuit no-ops, mark in-memory BatteryConfig dirty on real changes and await propagation. Tests updated to expect awake_duration 20 instead of 15. Changes
Sequence Diagram(s)sequenceDiagram
participant Cache as Cache
participant Scan as Scan Node
participant Subs as Subscribers
Cache->>Scan: _load_from_cache()
Scan->>Scan: call LOADED callback
Scan->>Scan: perform initialization (moved after LOADED)
Scan->>Scan: _run_awake_tasks()
Scan->>Subs: publish NodeFeature.MOTION_CONFIG (current motion_config)
sequenceDiagram
participant Cache as Cache
participant SED as SED Node
participant Device as Device
participant Subs as Subscribers
Cache->>SED: _load_from_cache()
alt battery config dirty
SED->>SED: _sed_configure_update() (apply to Device, update cache, publish)
else battery config clean
SED->>Subs: publish NodeFeature.BATTERY (battery_config)
end
Note over SED: Setter flow (example: set_awake_duration)
SED->>SED: if new == current -> return False
SED->>SED: update in-memory BatteryConfig (dirty=True)
SED->>SED: await _sed_configure_update()
SED->>Device: apply config
SED->>Subs: publish NodeFeature.BATTERY (updated config)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial 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 (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #312 +/- ##
=======================================
Coverage 80.58% 80.59%
=======================================
Files 36 36
Lines 7547 7555 +8
=======================================
+ Hits 6082 6089 +7
- Misses 1465 1466 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/nodes/sed.py (1)
233-253: Avoid redundant writes in set_awake_duration by short-circuiting unchanged valuesUnlike the other setters, set_awake_duration lacks an equality check. Now that it immediately persists and publishes, calls with identical values will trigger unnecessary cache writes and notifications.
Apply this diff to align with the other setters and reduce redundant work:
async def set_awake_duration(self, seconds: int) -> bool: @@ if seconds < 1 or seconds > MAX_UINT_2: raise ValueError( f"Invalid awake duration ({seconds}). It must be between 1 and 255 seconds." ) + if self._battery_config.awake_duration == seconds: + return False @@ self._battery_config = replace( self._battery_config, awake_duration=seconds, dirty=True, ) await self._sed_configure_update() return True
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugwise_usb/nodes/scan.py(1 hunks)plugwise_usb/nodes/sed.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/sed.pyplugwise_usb/nodes/scan.py
🧬 Code Graph Analysis (2)
plugwise_usb/nodes/sed.py (2)
plugwise_usb/nodes/helpers/subscription.py (1)
publish_feature_update_to_subscribers(51-66)plugwise_usb/api.py (1)
NodeFeature(39-58)
plugwise_usb/nodes/scan.py (2)
plugwise_usb/nodes/helpers/subscription.py (1)
publish_feature_update_to_subscribers(51-66)plugwise_usb/api.py (1)
NodeFeature(39-58)
🔇 Additional comments (3)
plugwise_usb/nodes/scan.py (1)
173-177: Publish MOTION_CONFIG on clean cache loads — good propagationThis ensures subscribers receive the current motion configuration even when the cache is clean, aligning behavior with SED battery config publishing. Nice consistency and avoids stale UI state after cold starts.
plugwise_usb/nodes/sed.py (2)
178-182: Publish BATTERY on clean cache loads — good consistencyPublishing the current BatteryConfig when the cache isn’t dirty aligns with the Scan node behavior and prevents subscribers from missing initial state on cold start.
251-251: Immediate cache + subscriber updates in setters — aligns with objectivesAwaiting _sed_configure_update() right after mutating BatteryConfig makes updates propagate immediately (optimistic with dirty=True) while device application remains deferred to awake. This is the right trade-off for responsiveness.
Also applies to: 273-273, 290-290, 312-312, 337-337
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugwise_usb/nodes/sed.py (1)
253-253: Optionally coalesce multiple setter calls to reduce duplicate save/publish churn.If clients may call several setters in quick succession (e.g., UI workflows), each call runs
save_cache()and publishes. Consider a simple batching mechanism (context manager or a small queue/debounce) to coalesce adjacent updates and publish once.Example approach:
- Add a short debounce (e.g., 20–50 ms) when scheduling
_sed_configure_update().- Or provide an
apply_battery_config(...)that sets multiple fields then calls_sed_configure_update()once.This is an optimization; current behavior is correct.
Also applies to: 275-275, 292-292, 314-314, 339-339
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/nodes/sed.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/sed.py
🧬 Code Graph Analysis (1)
plugwise_usb/nodes/sed.py (3)
plugwise_usb/nodes/helpers/subscription.py (1)
publish_feature_update_to_subscribers(51-66)plugwise_usb/api.py (2)
NodeFeature(39-58)battery_config(484-495)plugwise_usb/nodes/node.py (1)
battery_config(121-127)
⏰ 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)
- GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (2)
plugwise_usb/nodes/sed.py (2)
178-182: Publishing clean cached battery config on load is correct and consistent with scan-node changes.Good call to publish the current BatteryConfig to subscribers when the cache is clean. This ensures observers always receive an initial state after load without forcing a device reconfigure.
253-253: Awaiting _sed_configure_update() in setters ensures immediate propagation to cache and subscribers.This is the right move to make updates observable immediately while keeping the device reconfiguration deferred to the next awake cycle. Nicely aligns with the PR objective.
Also applies to: 275-275, 292-292, 314-314, 339-339
1dac925 to
bd052d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_usb.py (1)
2000-2011: Clarify intent and add a no-op assertion for completeness.Setting 20 right before handling the “accepted” awake response while still asserting a final value of 15 can be confusing to future readers. Add a brief comment to explain the rationale and also assert the no-op path explicitly after the config becomes clean.
Apply this diff:
@@ awake_response2.timestamp = awake_response1.timestamp + td( seconds=pw_sed.AWAKE_RETRY ) - assert await test_sed.set_awake_duration(20) + # Intentionally use a value different from the currently staged 15 to + # avoid the setter's early-exit (which would return False) and validate it returns True. + # The accepted configuration below should still reflect the previously staged 15. + assert await test_sed.set_awake_duration(20) assert test_sed.battery_config.dirty mock_stick_controller.send_response = sed_config_accepted await test_sed._awake_response(awake_response2) # pylint: disable=protected-access await asyncio.sleep(0.001) # Ensure time for task to be executed assert not test_sed.battery_config.dirty assert test_sed.battery_config.awake_duration == 15 assert test_sed.awake_duration == 15 + # Verify no-op behavior explicitly for awake duration as well. + assert not await test_sed.set_awake_duration(15)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_usb.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/test_usb.py (3)
plugwise_usb/api.py (1)
set_awake_duration(526-549)plugwise_usb/nodes/sed.py (1)
set_awake_duration(233-254)plugwise_usb/nodes/node.py (1)
set_awake_duration(736-742)
🔇 Additional comments (1)
tests/test_usb.py (1)
2003-2003: Good change to avoid setter early-exit no-op.Switching to set_awake_duration(20) is correct given the new early-exit in NodeSED.set_awake_duration (returns False when the value is unchanged). Using 15 here would make the assertion fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/codeql-analysis.yml (1)
39-39: Consider using the latest version v5 instead of the specific patch version.While pinning to
v5.0.0provides version stability, usingv5(without the patch version) is generally preferred in GitHub workflows. This allows you to automatically receive security patches and bug fixes while maintaining compatibility with the major version.- uses: actions/checkout@v5.0.0 + uses: actions/checkout@v5plugwise_usb/nodes/circle.py (1)
719-768: Consider optimizing the cache update logic to reduce redundant string operations.The current implementation splits and rejoins the entire cache string for each update. For large cache strings, this could be inefficient. Consider maintaining a set or dict structure for faster lookups and updates.
While the current implementation works correctly, you could optimize it by maintaining a more efficient data structure:
async def _energy_log_record_update_state( self, address: int, slot: int, timestamp: datetime, pulses: int, import_only: bool = False, ) -> bool: """Process new energy log record. Returns true if record is new or changed.""" self._energy_counters.add_pulse_log( address, slot, timestamp, pulses, import_only=import_only ) if not self._cache_enabled: return False log_cache_record = ( f"{address}:{slot}:{timestamp.strftime('%Y-%m-%d-%H-%M-%S')}:{pulses}" ) # Consider caching the parsed entries in a class variable to avoid repeated parsing if (cached_logs := self._get_cache(CACHE_ENERGY_COLLECTION)) is not None: # Only parse if we haven't already entries = set(cached_logs.split("|")) if cached_logs else set() if log_cache_record not in entries: _LOGGER.debug( "Adding logrecord (%s, %s) to cache of %s", str(address), str(slot), self._mac_in_str, ) new_cache = ( f"{log_cache_record}|{cached_logs}" if cached_logs else log_cache_record ) self._set_cache(CACHE_ENERGY_COLLECTION, new_cache) await self.save_cache(trigger_only=True) return True _LOGGER.debug( "Energy logrecord already present for %s, ignoring", self._mac_in_str ) return False _LOGGER.debug( "Cache is empty, adding new logrecord (%s, %s) for %s", str(address), str(slot), self._mac_in_str, ) self._set_cache(CACHE_ENERGY_COLLECTION, log_cache_record) return TrueHowever, this is a minor optimization and the current implementation is functionally correct.
📜 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 (8)
.github/workflows/codeql-analysis.yml(1 hunks).github/workflows/merge.yml(1 hunks).github/workflows/verify.yml(13 hunks).pre-commit-config.yaml(1 hunks)CHANGELOG.md(1 hunks)plugwise_usb/nodes/circle.py(11 hunks)pyproject.toml(1 hunks)tests/test_usb.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- CHANGELOG.md
- pyproject.toml
- .pre-commit-config.yaml
🧰 Additional context used
🧠 Learnings (3)
📚 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-13T17:29:32.324Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#314
File: plugwise_usb/nodes/circle.py:695-705
Timestamp: 2025-08-13T17:29:32.324Z
Learning: In plugwise_usb/nodes/circle.py, the method self._energy_counters.get_pulse_logs() already returns data in reverse-sorted order (newest-first), so no additional sorting is required when serializing energy log records to cache.
Applied to files:
plugwise_usb/nodes/circle.py
📚 Learning: 2025-08-14T12:26:12.760Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#314
File: plugwise_usb/nodes/circle.py:570-573
Timestamp: 2025-08-14T12:26:12.760Z
Learning: In plugwise_usb/nodes/circle.py, the energy_log_update method's return value of True indicates successful processing and storing of energy log records in memory (via _energy_counters), regardless of whether the records were new, duplicates, or empty. The docstring "return True only when at least one recent, non-empty record was stored" refers to successful processing/storage in the energy counters system, not necessarily new cache entries.
Applied to files:
plugwise_usb/nodes/circle.py
🧬 Code Graph Analysis (2)
plugwise_usb/nodes/circle.py (6)
plugwise_usb/nodes/helpers/pulses.py (2)
logs(110-124)calc_log_address(20-36)plugwise_usb/messages/responses.py (2)
log_data(775-794)log_address(770-772)plugwise_usb/nodes/helpers/counter.py (4)
timestamp(179-187)update(147-176)update(286-328)add_pulse_log(74-89)plugwise_usb/api.py (1)
save_cache(368-371)plugwise_usb/nodes/node.py (1)
save_cache(705-719)plugwise_usb/messages/requests.py (18)
CircleEnergyLogsRequest(1131-1165)response(124-128)send(366-375)send(388-397)send(439-441)send(477-486)send(513-522)send(543-554)send(567-576)send(589-598)send(622-631)send(668-677)send(720-729)send(776-785)send(809-818)send(849-858)send(881-890)send(913-922)
tests/test_usb.py (3)
plugwise_usb/nodes/sed.py (4)
set_awake_duration(233-254)battery_config(357-366)dirty(345-347)awake_duration(350-354)plugwise_usb/api.py (2)
set_awake_duration(526-549)battery_config(484-495)plugwise_usb/nodes/node.py (2)
set_awake_duration(736-742)battery_config(121-127)
⏰ 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)
- GitHub Check: Check commit
🔇 Additional comments (12)
tests/test_usb.py (1)
2003-2010: Test value updated to match new awake_duration value.The test correctly updates the awake_duration from 15 to 20 seconds, and the assertions properly verify this new value in both
battery_config.awake_durationandawake_duration. The change aligns with the battery configuration handling improvements in the PR..github/workflows/merge.yml (1)
29-29: Good workflow consistency improvement!Updating the checkout action to v5 aligns with the other workflow files in this PR, maintaining consistency across the CI/CD pipeline. This is a sensible upgrade that ensures all workflows use the same tooling versions.
.github/workflows/verify.yml (2)
26-26: Excellent addition of Python version propagation!Adding the
python-versionoutput ensures all jobs in the workflow use the exact same Python minor version. This is a robust pattern that prevents version mismatches across different job runners.
257-261: Good artifact handling upgrade!The update to
download-artifact@v5with the new inputs (pattern,merge-multiple,path) provides better control over artifact collection. The change fromcoverage*/.coverage*toartifacts/.coverage*simplifies the directory structure.plugwise_usb/nodes/circle.py (8)
77-77: Well-designed constant for energy log retention.The
MAX_LOG_HOURSconstant provides a clear, configurable boundary for energy log retention. UsingDAY_IN_HOURSas its value (24 hours) is a sensible default that balances data retention with memory usage.
83-117: Robust cache record parsing with proper error handling.The
_collect_recordsfunction effectively parses the pipe-delimited cache format with good defensive programming:
- Handles both strptime and manual date parsing as fallback
- Validates field count before processing
- Prevents duplicate slot entries by keeping only the first occurrence
The function correctly maintains the newest-first semantics mentioned in the learnings.
571-627: Improved method signature with boolean return and cache control.The updated
energy_log_updatemethod signature provides better control and clearer semantics:
- Boolean return value clearly indicates success/failure
save_cacheparameter allows callers to control caching behavior- Early return for None address prevents unnecessary processing
The implementation correctly updates cache only when needed and logs appropriate debug messages.
629-646: Good addition of timestamp validation for energy logs.The
_check_timestamp_is_recentmethod properly filters out stale energy logs older thanMAX_LOG_HOURS. The informative log message helps with debugging when old records are ignored.
647-696: Well-structured cache loading with proper pruning.The cache loading implementation correctly:
- Filters out entries older than MAX_LOG_HOURS using the cutoff timestamp
- Processes entries in reverse-sorted order (newest first)
- Creates background tasks for missing logs when needed
- Uses the
import_onlyflag to prevent triggering updates during cache load
697-718: Efficient cache persistence with immediate disk write.The new cache saving implementation:
- Serializes logs in a compact, timestamp-based format
- Maintains newest-first ordering (as per the learnings)
- Triggers immediate cache write with
trigger_only=Trueto avoid blockingThe format
address:slot:YYYY-MM-DD-HH-MM-SS:pulsesis clear and parseable.
496-531: Smart initial energy log collection with early termination.The refactored
_get_initial_energy_logsmethod efficiently:
- Calculates the appropriate number of addresses based on consumption/production intervals
- Limits collection to MAX_LOG_HOURS worth of data
- Stops early when encountering empty or outdated logs (avoiding unnecessary requests)
- Uses
save_cache=Falseduring collection, then saves once at the end
535-570: Improved error handling in missing log collection.The updated
get_missing_energy_logsmethod now properly handles failures:
- Creates tasks for all missing addresses
- Cancels remaining tasks if one fails (avoiding wasted requests)
- Properly drains cancelled tasks with
gather(..., return_exceptions=True)- Saves cache only after all collection is complete
This is a significant improvement in error handling and efficiency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
plugwise_usb/nodes/sed.py (1)
5-5: Fix timeout handling: catch asyncio.TimeoutError from wait_for.asyncio.wait_for raises asyncio.TimeoutError, not the built-in TimeoutError. The current except TimeoutError block won’t run, so nodes may never be marked unavailable on missed awake.
Apply this diff:
-from asyncio import CancelledError, Future, Task, gather, get_running_loop, wait_for +from asyncio import CancelledError, Future, Task, TimeoutError, gather, get_running_loop, wait_forNo other code changes are needed since the except TimeoutError now refers to asyncio’s TimeoutError due to the import.
Also applies to: 543-557
♻️ Duplicate comments (1)
plugwise_usb/nodes/sed.py (1)
245-246: Standardize early-exit equality checks to use effective values (battery_config).Using self._battery_config.* can be None pre-init and triggers unnecessary updates when the requested value equals the default/effective value. Compare against self.battery_config.* for consistent no-op semantics across setters.
Apply this diff:
@@ - if self._battery_config.awake_duration == seconds: + if self.battery_config.awake_duration == seconds: return False @@ - if self._battery_config.clock_sync == sync: + if self.battery_config.clock_sync == sync: return False @@ - if self._battery_config.sleep_duration == minutes: + if self.battery_config.sleep_duration == minutes: return FalseAlso applies to: 286-287, 333-334
🧹 Nitpick comments (1)
plugwise_usb/nodes/sed.py (1)
248-254: DRY the repeated 'replace + dirty=True + publish' pattern via a small helper.Each setter repeats the same sequence. A private helper improves readability and reduces future drift.
Apply this diff within the setters:
@@ - self._battery_config = replace( - self._battery_config, - awake_duration=seconds, - dirty=True, - ) - await self._sed_configure_update() + await self._apply_battery_update(awake_duration=seconds) @@ - self._battery_config = replace( - self._battery_config, clock_interval=minutes, dirty=True - ) - await self._sed_configure_update() + await self._apply_battery_update(clock_interval=minutes) @@ - self._battery_config = replace( - self._battery_config, clock_sync=sync, dirty=True - ) - await self._sed_configure_update() + await self._apply_battery_update(clock_sync=sync) @@ - self._battery_config = replace( - self._battery_config, maintenance_interval=minutes, dirty=True - ) - await self._sed_configure_update() + await self._apply_battery_update(maintenance_interval=minutes) @@ - self._battery_config = replace( - self._battery_config, sleep_duration=minutes, dirty=True - ) - await self._sed_configure_update() + await self._apply_battery_update(sleep_duration=minutes)And add this helper (outside the ranges, e.g., near other config helpers):
async def _apply_battery_update(self, **changes: Any) -> None: """Replace battery config with given changes, mark dirty, persist and publish.""" self._battery_config = replace(self._battery_config, **changes, dirty=True) await self._sed_configure_update()Also applies to: 272-276, 289-293, 311-315, 336-340
📜 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 (3)
plugwise_usb/nodes/scan.py(1 hunks)plugwise_usb/nodes/sed.py(6 hunks)tests/test_usb.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugwise_usb/nodes/scan.py
- tests/test_usb.py
🧰 Additional context used
🧠 Learnings (1)
📚 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/sed.py
🧬 Code Graph Analysis (1)
plugwise_usb/nodes/sed.py (2)
plugwise_usb/nodes/helpers/subscription.py (1)
publish_feature_update_to_subscribers(51-66)plugwise_usb/api.py (1)
NodeFeature(39-58)
⏰ 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)
- GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (1)
plugwise_usb/nodes/sed.py (1)
178-182: Good addition: publish non-dirty cached battery config on load.Publishing BATTERY state in the non-dirty path ensures subscribers receive the current config after startup without waiting for the next awake. This aligns with the PR objective.
bouwew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|



Summary by CodeRabbit
Bug Fixes
Tests
Documentation