Enable direct flashing method in armbian-install menu #9022
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
github-actions
bot
added
02
![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
🧹 Nitpick comments (2)
packages/bsp/common/usr/bin/armbian-install (2)
932-936: Consider verifying command existence before execution.While the current implementation safely falls back if
armbian-configis missing (the option simply won't appear), explicitly checking for the command's existence would make the intent clearer and provide better diagnostics if needed.Apply this diff to add explicit command verification:
# show flasher if there are available block devices - armbian-config --api module_images status >/dev/null - if [[ $? -eq 0 ]]; then + if command -v armbian-config >/dev/null 2>&1 && \ + armbian-config --api module_images status >/dev/null 2>&1; then options+=("8" "Download and flash clean image") fiNote: The comment on line 932 states "if there are available block devices" but the code checks
module_images status. Consider updating the comment to accurately reflect what's being checked, such as "show flasher if module is available."
1021-1024: Add error handling and user feedback for the flash operation.The current implementation calls
armbian-config --cmd FLASH1without checking its exit status or providing user feedback. If the command fails, the user won't know what went wrong.Apply this diff to add error handling:
8) - armbian-config --cmd FLASH1 - return + if armbian-config --cmd FLASH1; then + dialog --backtitle "$backtitle" --title 'Flash operation' --msgbox '\n Done.' 7 30 + else + dialog --backtitle "$backtitle" --title 'Flash operation' --msgbox '\n Failed. Check logs for details.' 7 40 + fi + return ;;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/bsp/common/usr/bin/armbian-install(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
📚 Learning: 2025-09-27T21:50:04.845Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-edge.config:80-82
Timestamp: 2025-09-27T21:50:04.845Z
Learning: In the Armbian build system, kernel configuration files are generated through this automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bsp/common/usr/bin/armbian-install (1)
932-937: Pre-check for flasher option is sound; consider inlining command for robustness.The gating on
armbian-config --api module_images statusis a good way to only show option 8 when supported. You can simplify and harden this a bit by inlining the command in theifand silencing stderr, which avoids depending on$?and keeps the dialog output clean ifarmbian-configemits warnings:- # show flasher if there are available block devices - armbian-config --api module_images status >/dev/null - if [[ $? -eq 0 ]]; then - options+=("8" "Download and flash clean image") - fi + # show flasher if armbian-config reports module_images as available + if armbian-config --api module_images status >/dev/null 2>&1; then + options+=("8" "Download and flash clean image") + fiPlease double-check against the current
armbian-configAPI thatmodule_images statusis the right health check for enabling direct flashing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/bsp/common/usr/bin/armbian-install(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-02T05:46:10.664Z
Learning: In the Armbian build system, the modern recommended approach for kernel configuration is to use the kernel-config command via "./compile.sh BOARD=boardname BRANCH=branchname kernel-config" instead of the deprecated KERNEL_CONFIGURE=yes flag. This provides a two-step workflow: configure using menuconfig, then build, with better transparency and control over configuration changes.
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.
Applied to files:
packages/bsp/common/usr/bin/armbian-install
🔇 Additional comments (1)
packages/bsp/common/usr/bin/armbian-install (1)
1021-1024: New option 8 wiring is consistent; ensureFLASH1flow is safe in this context.The integration of option 8 into the menu and dispatcher looks correct, and using
returnhere matches the pattern of other early-exit actions (5–7) while propagatingarmbian-config’s status.One thing to verify is that
armbian-config --cmd FLASH1is safe to invoke from here (no recursive call back intoarmbian-install, and its TUI flow doesn’t assume it was started “from scratch” in a conflicting way).Please confirm that the
FLASH1command in the correspondingarmbian-config/configng version does not call back intoarmbian-installand behaves correctly when launched from within this script.
| fi | ||
|
|
||
| # show flasher if there are available block devices | ||
| armbian-config --api module_images status >/dev/null |
There was a problem hiding this comment.
kindly preface this with a test for the executability of armbian-config
Description
Enable block device flasher in armbian-install. Related to armbian/configng#705
Documentation summary for feature / change
armbian/configng#705
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.