Another Ruff update - update_missing_registrations #271
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Starter
participant Registry
Starter->>Registry: start()
Registry->>Registry: update_missing_registrations_quick()
loop For each address
Registry->>Registry: get_registration(address)
alt Found empty MAC
Registry->>Registry: update registry, break
end
Registry->>Registry: sleep(0.1s)
end
Registry->>Registry: schedule update_missing_registrations_full()
Registry->>Registry: call quick scan finished callback
Registry->>Registry: log quick scan completion
Registry->>Registry: update_missing_registrations_full()
loop For each address
Registry->>Registry: get_registration(address)
alt Found empty MAC
Registry->>Registry: update registry
end
Registry->>Registry: sleep(10s)
end
Registry->>Registry: mark scan completed, save cache, log, call full scan finished callback
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
==========================================
+ Coverage 81.19% 81.20% +0.01%
==========================================
Files 36 36
Lines 7528 7534 +6
==========================================
+ Hits 6112 6118 +6
Misses 1416 1416 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (1)
plugwise_usb/network/registry.py (1)
215-241: Efficient quick scan implementation with proper task scheduling.The quick scan method effectively:
- Performs rapid discovery with 0.1s delays
- Breaks early when finding the first empty address slot
- Schedules the full scan as a background task
- Properly manages callback execution
The task scheduling logic (lines 233-236) correctly checks if a registration task is already running before creating a new one, preventing race conditions.
Consider extracting the task scheduling logic into a helper method to improve readability:
+ def _schedule_full_scan_if_needed(self) -> None: + """Schedule full scan if no registration task is running.""" + if self._registration_task is None or self._registration_task.done(): + self._registration_task = create_task( + self.update_missing_registrations_full() + ) + async def update_missing_registrations_quick(self) -> None: """Quick retrieval of all unknown network registrations from network controller.""" for address in range(0, 64): # ... existing logic ... await sleep(0.1) - if self._registration_task is None or self._registration_task.done(): - self._registration_task = create_task( - self.update_missing_registrations_full() - ) + self._schedule_full_scan_if_needed()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)CHANGELOG.md(1 hunks)plugwise_usb/network/registry.py(3 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
plugwise_usb/network/registry.py
[warning] 204-213: plugwise_usb/network/registry.py#L204-L213
Added lines #L204 - L213 were not covered by tests
🔇 Additional comments (3)
.gitignore (1)
20-20: LGTM! Standard gitignore pattern for application data.The addition properly excludes application data that may be generated during registry scanning operations.
plugwise_usb/network/registry.py (2)
111-111: LGTM! Clean integration of the refactored quick scan.The start method now properly initiates the quick scan phase, which will handle scheduling the full scan as needed.
182-214: Well-implemented full scan with proper completion handling.The full scan method correctly:
- Skips already-registered addresses to avoid duplicate work
- Updates first_free_address when empty MACs are found
- Includes appropriate delays between requests
- Handles cache saving and completion callbacks
The 10-second delay between address checks is appropriate for a comprehensive background scan to avoid overwhelming the network.
The static analysis indicates lines 204-213 lack test coverage. Consider adding tests to verify:
- Cache saving functionality after full scan completion
- Full scan finished callback execution
- Proper scan completion flag setting
#!/bin/bash # Check existing test coverage for the registry module fd -e py test | xargs rg -l "update_missing_registrations|full_scan" -A 3
|
Reduce complexity by splitting update_missing_registrations into update_missing_registrations_quick and update_missing_registrations_full.
Summary by CodeRabbit
Chores
.gitignoreto exclude theappdata_folderdirectory or file from version control.Bug Fixes