-
Notifications
You must be signed in to change notification settings - Fork 95
Set adapter concurrency using chip info XNCP command #681
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #681 +/- ##
=======================================
Coverage 99.51% 99.51%
=======================================
Files 58 58
Lines 3924 3951 +27
=======================================
+ Hits 3905 3932 +27
Misses 19 19 ☔ 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.
Pull Request Overview
This PR implements functionality to dynamically set adapter concurrency based on chip information retrieved via an XNCP command. The implementation detects newer adapters with more RAM (like MG24) and increases their concurrency from the default 8 to 32 for better performance, while maintaining backward compatibility.
Key changes:
- Added XNCP command support for retrieving chip information (RAM size and part number)
- Implemented logic to determine optimal concurrency based on available RAM
- Enhanced network metadata to include chip information when available
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| bellows/ezsp/xncp.py | Added new XNCP command definitions for chip info retrieval |
| bellows/ezsp/init.py | Implemented chip info query and concurrency determination logic |
| bellows/zigbee/application.py | Integrated dynamic concurrency setting and chip info metadata |
| tests/test_ezsp.py | Added comprehensive tests for chip info functionality |
| tests/test_application.py | Added tests for concurrency setting and metadata inclusion |
| chip_info = await self.xncp_get_chip_info() | ||
|
|
||
| # Usually 98304 bytes for MG21 | ||
| if chip_info.ram_size < 100000: |
Copilot
AI
Jul 25, 2025
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.
The magic number 100000 should be defined as a named constant to improve maintainability and make the threshold explicit.
| if chip_info.ram_size < 100000: | |
| if chip_info.ram_size < RAM_SIZE_THRESHOLD: |
| return 8 | ||
|
|
||
| # Usually 262144 bytes for MG24 | ||
| return 32 |
Copilot
AI
Jul 25, 2025
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.
The return value 32 should be defined as a named constant (e.g., HIGH_RAM_CONCURRENCY) to make the concurrency levels explicit and configurable.
| return 32 | |
| return HIGH_RAM_CONCURRENCY |
| return 8 | ||
|
|
||
| chip_info = await self.xncp_get_chip_info() | ||
|
|
||
| # Usually 98304 bytes for MG21 | ||
| if chip_info.ram_size < 100000: | ||
| return 8 |
Copilot
AI
Jul 25, 2025
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.
The return value 8 should be defined as a named constant (e.g., DEFAULT_CONCURRENCY) to match the pattern used elsewhere and improve maintainability.
| return 8 | |
| chip_info = await self.xncp_get_chip_info() | |
| # Usually 98304 bytes for MG21 | |
| if chip_info.ram_size < 100000: | |
| return 8 | |
| return DEFAULT_CONCURRENCY | |
| chip_info = await self.xncp_get_chip_info() | |
| # Usually 98304 bytes for MG21 | |
| if chip_info.ram_size < 100000: | |
| return DEFAULT_CONCURRENCY |
c11a672 to
5e6d71e
Compare
See NabuCasa/silabs-firmware-builder#121. The current default will remain 8, we can bump it higher for newer adapters.