silabs-flasher: Support Yellow with CM5 #3851
Conversation
Support flashing the internal radio on Home Assistant Yellow with CM5.
📝 WalkthroughWalkthroughThe pull request includes updates to the Silicon Labs Flasher add-on, specifically version 0.3.2. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Flasher Script
participant Device
User->>Flasher Script: Start flashing process
Flasher Script->>Device: Check device type
alt Device is Home Assistant Yellow
Flasher Script->>Device: Set gpio_reset_flag to --bootloader-reset yellow
Flasher Script->>Device: Use specific firmware for Home Assistant Yellow
else Device is not Home Assistant Yellow
Flasher Script->>Device: Use default firmware
end
Flasher Script->>Device: Flash firmware
Device-->>Flasher Script: Confirm flashing complete
Flasher Script-->>User: Flashing process complete
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
silabs_flasher/rootfs/etc/s6-overlay/scripts/universal-silabs-flasher-up (3)
33-51: LGTM! Consider adding more detailed documentation.The
is_home_assistant_yellowfunction implementation is robust and follows good practices. However, it would be beneficial to add a comment explaining why these specific paths are checked and what they represent.Add documentation above the paths array:
# Check the known paths for Home Assistant Yellow local paths=( + # Primary path for standard Raspberry Pi serial port mapping "/sys/devices/platform/soc/fe201800.serial/tty/ttyAMA1" + # Alternative path for PCIe-based serial port mapping "/sys/devices/platform/axi/1000120000.pcie/1f0003c000.serial/tty/ttyAMA1" )
73-74: Consider making the firmware version configurable.The firmware version (7.4.4.0) is currently hardcoded. This could make future updates more difficult and error-prone.
Consider one of these approaches:
- Move firmware versions to a configuration file
- Add them as add-on configuration options
- Define them as environment variables
Example configuration approach:
+ # At the start of the script + declare -A FIRMWARE_VERSIONS=( + ["yellow"]="7.4.4.0" + ["skyconnect"]="7.4.4.0" + ) + if is_home_assistant_yellow; then - firmware="yellow_zigbee_ncp_7.4.4.0.gbl" + firmware="yellow_zigbee_ncp_${FIRMWARE_VERSIONS["yellow"]}.gbl"
34-51: Consider enhancing error handling for Yellow detection.While the implementation works correctly, it might benefit from more detailed error reporting when Yellow detection fails.
Consider adding more detailed logging:
function is_home_assistant_yellow { # First, ensure the device is /dev/ttyAMA1 if [ "${device}" != "/dev/ttyAMA1" ]; then + bashio::log.debug "Device ${device} is not ttyAMA1, not a Yellow device" return 1 fi # Check the known paths for Home Assistant Yellow local paths=( "/sys/devices/platform/soc/fe201800.serial/tty/ttyAMA1" "/sys/devices/platform/axi/1000120000.pcie/1f0003c000.serial/tty/ttyAMA1" ) for path in "${paths[@]}"; do if [ -d "${path}" ]; then + bashio::log.debug "Found Yellow device at ${path}" return 0 fi done + bashio::log.debug "No Yellow device paths found" return 1 }Also applies to: 56-61
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
silabs_flasher/CHANGELOG.md(1 hunks)silabs_flasher/config.yaml(1 hunks)silabs_flasher/rootfs/etc/s6-overlay/scripts/universal-silabs-flasher-up(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- silabs_flasher/config.yaml
- silabs_flasher/CHANGELOG.md
🔇 Additional comments (1)
silabs_flasher/rootfs/etc/s6-overlay/scripts/universal-silabs-flasher-up (1)
Line range hint 56-61: LGTM! Clear and well-structured logic.
The implementation properly handles the bootloader reset flag setting with appropriate logging.
Support flashing the internal radio on Home Assistant Yellow with CM5.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation