Update RB5 Config,Fix CI Build Fail #8537
Conversation
WalkthroughAdds a new board configuration file config/boards/qcom-robotics-rb5.conf for the Qualcomm Robotics RB5. Declares board metadata, kernel targets, partition/boot and firmware options, kernel cmdline, serial and DTB/GRUB settings, and sector size. Adds a userspace support predicate and multiple post-family tweak functions that, when userspace is supported, install USB-gadget assets and systemd unit, enable usbgadget-rndis.service, install extra packages (including an optional noble PPA and KDE handling), adjust qbootctl/suspend behaviour, and inject BSP firmware into the initramfs. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (18)📓 Common learnings📚 Learning: 2025-09-11T06:12:54.213ZApplied to files:
📚 Learning: 2025-08-03T15:21:20.148ZApplied to files:
📚 Learning: 2025-07-17T04:12:33.125ZApplied to files:
📚 Learning: 2025-09-07T17:39:32.272ZApplied to files:
📚 Learning: 2025-09-18T03:36:17.862ZApplied to files:
📚 Learning: 2025-09-25T18:37:00.305ZApplied to files:
📚 Learning: 2025-09-18T03:36:17.862ZApplied to files:
📚 Learning: 2025-09-18T03:36:17.862ZApplied to files:
📚 Learning: 2025-09-12T21:44:09.061ZApplied to files:
📚 Learning: 2025-07-25T03:51:50.830ZApplied to files:
📚 Learning: 2025-07-25T03:51:50.830ZApplied to files:
📚 Learning: 2025-07-21T04:12:02.439ZApplied to files:
📚 Learning: 2025-08-30T04:13:16.457ZApplied to files:
📚 Learning: 2025-09-11T06:16:10.236ZApplied to files:
📚 Learning: 2025-06-24T10:08:40.313ZApplied to files:
📚 Learning: 2025-08-11T12:39:22.861ZApplied to files:
📚 Learning: 2025-09-14T06:32:29.806ZApplied to files:
🔇 Additional comments (3)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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
08
coderabbitai
bot
requested review from
amazingfate,
igorpecovnik,
pyavitz and
rpardini
![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 (4)
config/boards/qcom-robotics-rb5.conf (4)
10-13: Consider documenting why ABL boot configuration is commented out.The commented-out ABL boot configuration might be useful for future development or alternative boot methods. Consider adding a more detailed comment explaining when/why these settings might be needed.
-# Reserve for ABL Boot +# Reserved for ABL (Android Boot Loader) boot method +# Uncomment these lines if switching from GRUB/UEFI to ABL boot: # declare -g BOOTIMG_CMDLINE_EXTRA="clk_ignore_unused pd_ignore_unused console=tty0 console=ttyMSM0,1500000n8 pcie_pme=nomsi panic=30 allow_mismatched_32bit_el0 mem_sleep_default=s2idle" # declare -g -a ABL_DTB_LIST=("qrb5165-rb5")
75-78: software-properties-common should be available by default in Ubuntu.The package
software-properties-commonis typically pre-installed in Ubuntu releases. Consider checking if it's already present before attempting installation to avoid unnecessary operations.if [[ "${RELEASE}" == "noble" ]]; then - do_with_retries 3 chroot_sdcard_apt_get_update - do_with_retries 3 chroot_sdcard_apt_get_install software-properties-common + # Only install if not already present + if ! chroot_sdcard dpkg -l software-properties-common &>/dev/null; then + do_with_retries 3 chroot_sdcard_apt_get_update + do_with_retries 3 chroot_sdcard_apt_get_install software-properties-common + fi fi
93-97: Consider making KDE packages installation more modular.The KDE package list is quite extensive. Consider defining it as a variable for better maintainability.
if [[ "${DESKTOP_ENVIRONMENT}" == "kde-plasma" ]]; then display_alert "Adding Extra KDE Package" "${BOARD}" "info" - do_with_retries 3 chroot_sdcard_apt_get_install plasma-workspace plasma-desktop plasma-systemmonitor plasma-nm kde-standard kde-spectacle kinfocenter kscreen krfb kfind filelight \ - dolphin clinfo wayland-utils + local kde_packages=( + plasma-workspace plasma-desktop plasma-systemmonitor plasma-nm + kde-standard kde-spectacle kinfocenter kscreen krfb kfind + filelight dolphin clinfo wayland-utils + ) + do_with_retries 3 chroot_sdcard_apt_get_install "${kde_packages[@]}" fi
24-28: Add Debian Bookworm to userspace support checkThe
qcom-robotics-rb5_is_userspace_supportedfunction should include “bookworm” alongside the existing releases. Other boards (e.g. retroidpocket-rp5 and retroidpocket-rpmini) already support bookworm, so we should mirror that here.• File: config/boards/qcom-robotics-rb5.conf
• Lines: 24–28Suggested diff:
function qcom-robotics-rb5_is_userspace_supported() { + [[ "${RELEASE}" == "bookworm" ]] && return 0 [[ "${RELEASE}" == "noble" ]] && return 0 [[ "${RELEASE}" == "trixie" ]] && return 0 return 1 }For reference, retroidpocket boards include bookworm at:
- config/boards/retroidpocket-rp5.csc:25
- config/boards/retroidpocket-rpmini.csc:25
📜 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 (1)
config/boards/qcom-robotics-rb5.conf(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
PR: armbian/build#8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Applied to files:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-08-11T12:39:22.861Z
Learnt from: pyavitz
PR: armbian/build#8481
File: config/kernel/linux-spacemit-edge.config:492-494
Timestamp: 2025-08-11T12:39:22.861Z
Learning: The SpacemiT platform requires esos.elf firmware to be embedded in the kernel image for boot. The Armbian build system handles this by copying esos.elf from $SRC/packages/blobs/riscv64/spacemit/ to ${kernel_work_dir}/firmware/ via the custom_kernel_config__spacemit_k1_firmware() function in config/sources/families/spacemit.conf, ensuring CONFIG_EXTRA_FIRMWARE="esos.elf" can successfully embed it during kernel build.
Applied to files:
config/boards/qcom-robotics-rb5.conf
🧬 Code graph analysis (1)
config/boards/qcom-robotics-rb5.conf (2)
lib/functions/bsp/armbian-bsp-cli-deb.sh (4)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_update_uboot_bootscript(331-358)board_side_bsp_cli_postinst_base(425-446)board_side_bsp_cli_postinst_finish(448-471)extensions/grub-with-dtb.sh (1)
post_family_tweaks_bsp__add_grub_with_dtb_config_file(28-35)
🔇 Additional comments (8)
config/boards/qcom-robotics-rb5.conf (8)
1-6: LGTM! Board configuration properly defined.The board metadata is correctly structured with appropriate naming, maintainer information, and board family designation for the Qualcomm Robotics RB5.
7-9: LGTM! Standard configuration options.The kernel targets, WiFi, and boot configuration settings are appropriate for this board type.
14-16: LGTM! Full firmware installation configured.Using the full firmware package ensures all necessary firmware files are available for the RB5 board.
17-23: LGTM! Boot parameters and GRUB extensions properly configured.The GRUB configuration with DTB support and appropriate kernel command line parameters is well-suited for the RB5 board. The 4K sector size is correctly specified for modern storage devices.
30-51: LGTM! USB gadget network service properly installed.The function correctly installs all necessary components for USB gadget networking, including services, scripts, and initramfs hooks.
53-65: LGTM! Service enablement properly handled.The function correctly enables the USB gadget RNDIS service with appropriate userspace checks.
107-123: Verify firmware existence before adding to initramfsNone of our board‐specific hooks—including config/boards/qcom-robotics-rb5.conf—guard against missing firmware before calling
add_firmware. If a file isn’t present, the hook may error out and break the BSP build. Please confirm howadd_firmware(from/usr/share/initramfs-tools/hook-functions) behaves on nonexistent paths; if it fails the build, wrap both the looped and static calls in existence checks, for example:# inside the for-loop for f in /lib/firmware/qcom/sm8250/Thundercomm/RB5/*; do if [[ -e "$f" ]]; then add_firmware "${f#/lib/firmware/}" else display_alert "Missing firmware" "${BOARD}: ${f}" "warning" fi done # for individual files if [[ -e "/lib/firmware/qcom/sm8250/a650_zap.mbn" ]]; then add_firmware "qcom/sm8250/a650_zap.mbn" else display_alert "Missing firmware" "${BOARD}: a650_zap.mbn" "warning" fi• Verify that
add_firmwaredoesn’t silently ignore missing files or, conversely, stop the build.
• If it already handles missing files gracefully, document that behavior.
• Otherwise, add these existence checks around everyadd_firmwarecall.
85-91: Confirm ‘unudhcpd’ is actually packaged in ArmbianI see that this board config (and several others, e.g. ayn-odin2, oneplus-kebab, xiaomi-elish) all temporarily enable the Armbian repo in order to install
unudhcpdbefore disabling it again. However:
- Upstream Debian/Ubuntu provide
udhcpd(the BusyBox DHCP server) but do not ship a separateunudhcpdpackage (packages.debian.org).- The standalone
unudhcpdproject originates from postmarketOS and is only packaged in Alpine Linux and the AUR (Arch) (pkgs.alpinelinux.org, aur.archlinux.org).Please verify that the Armbian apt repository you’re enabling actually contains
unudhcpd(e.g. by inspecting the Release file athttps://apt.armbian.com/…or your local metadata). If it does not, you will either need to:• Add (or request) an Armbian package for
unudhcpd, or
• Switch to the upstreamudhcpdpackage if it meets your needs.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
config/boards/qcom-robotics-rb5.conf (1)
111-121: Use /lib/firmware/updates/ for BSP firmware; include fallback to /lib/firmware.Per established project guidance, prefer
/lib/firmware/updates/to avoid conflicts with the Armbian firmware package. Also ensureadd_firmwarereceives paths relative to either base or updates. Your current hook only scans/lib/firmware/.Apply:
- for f in /lib/firmware/qcom/sm8250/Thundercomm/RB5/* ; do - add_firmware "${f#/lib/firmware/}" - done - add_firmware "qcom/sm8250/a650_zap.mbn" # Extra one for dpu - add_firmware "qcom/a650_sqe.fw" # Extra one for dpu - add_firmware "qcom/a650_gmu.bin" # Extra one for gpu + # Prefer updates/ to avoid conflicts with Armbian firmware; fall back to base if needed + for f in /lib/firmware/updates/qcom/sm8250/Thundercomm/RB5/* /lib/firmware/qcom/sm8250/Thundercomm/RB5/* ; do + [[ -e "$f" ]] || continue + # Add path relative to either /lib/firmware/ or /lib/firmware/updates/ + add_firmware "${f#/lib/firmware/}" + add_firmware "${f#/lib/firmware/updates/}" + done + # Extra firmware; add relative to standard path (initramfs-tools handles updates/ precedence) + for fw in "qcom/sm8250/a650_zap.mbn" "qcom/a650_sqe.fw" "qcom/a650_gmu.bin"; do + add_firmware "$fw" + done
📜 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 (1)
config/boards/qcom-robotics-rb5.conf(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
PR: armbian/build#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:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
PR: armbian/build#8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Applied to files:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-08-11T12:39:22.861Z
Learnt from: pyavitz
PR: armbian/build#8481
File: config/kernel/linux-spacemit-edge.config:492-494
Timestamp: 2025-08-11T12:39:22.861Z
Learning: The SpacemiT platform requires esos.elf firmware to be embedded in the kernel image for boot. The Armbian build system handles this by copying esos.elf from $SRC/packages/blobs/riscv64/spacemit/ to ${kernel_work_dir}/firmware/ via the custom_kernel_config__spacemit_k1_firmware() function in config/sources/families/spacemit.conf, ensuring CONFIG_EXTRA_FIRMWARE="esos.elf" can successfully embed it during kernel build.
Applied to files:
config/boards/qcom-robotics-rb5.conf
🔇 Additional comments (3)
config/boards/qcom-robotics-rb5.conf (3)
1-23: Board metadata and boot defaults look sane (and no shebang — good).
- Matches Armbian conventions: metadata via declare -g, GPT, grub-with-dtb, BOARD_FIRMWARE_INSTALL=-full.
- Adheres to the “no shebang in board .conf” convention noted by maintainers.
17-19: Confirm the chosen console devices and baud rate.
GRUB_CMDLINE_LINUX_DEFAULTsets bothconsole=tty0andconsole=ttyMSM0,1500000n8. Please confirm RB5’s debug UART really needs 1.5 Mbps and thatSERIALCON=tty1aligns with how getty is provisioned for this board. Mismatches here commonly lead to “no serial output” bug reports.
75-83: Sanity-check external repos and units for Noble only.
- PPA addition and
qbootctl.serviceenabling are gated to Noble; that’s reasonable. Please confirm the PPA is still valid and thatqbootctlis available for Noble images; if not, wrap them in existence checks (command -v add-apt-repository,systemctl list-unit-files | grep -q qbootctl.service) to avoid breaking builds.Also applies to: 99-101
| mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources | ||
| do_with_retries 3 chroot_sdcard_apt_get_update | ||
| display_alert "Installing ${BOARD} tweaks" "warn" | ||
| do_with_retries 3 chroot_sdcard_apt_get_install vulkan-tools mesa-vulkan-drivers mtools zstd alsa-ucm-conf qbootctl qrtr-tools unudhcpd | ||
| # disable armbian repo back | ||
| mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard mv of armbian.sources to avoid failures when the file is absent.
On some images the disabled file may not exist. Un-guarded mv can break the build step depending on shell options. Suggest conditional moves.
Apply:
- mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources
+ if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" ]]; then
+ mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" "${SDCARD}/etc/apt/sources.list.d/armbian.sources"
+ fi
do_with_retries 3 chroot_sdcard_apt_get_update
display_alert "Installing ${BOARD} tweaks" "warn"
do_with_retries 3 chroot_sdcard_apt_get_install vulkan-tools mesa-vulkan-drivers mtools zstd alsa-ucm-conf qbootctl qrtr-tools unudhcpd
# disable armbian repo back
- mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled
+ if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources" ]]; then
+ mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources" "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources | |
| do_with_retries 3 chroot_sdcard_apt_get_update | |
| display_alert "Installing ${BOARD} tweaks" "warn" | |
| do_with_retries 3 chroot_sdcard_apt_get_install vulkan-tools mesa-vulkan-drivers mtools zstd alsa-ucm-conf qbootctl qrtr-tools unudhcpd | |
| # disable armbian repo back | |
| mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled | |
| if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" ]]; then | |
| mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" "${SDCARD}/etc/apt/sources.list.d/armbian.sources" | |
| fi | |
| do_with_retries 3 chroot_sdcard_apt_get_update | |
| display_alert "Installing ${BOARD} tweaks" "warn" | |
| do_with_retries 3 chroot_sdcard_apt_get_install vulkan-tools mesa-vulkan-drivers mtools zstd alsa-ucm-conf qbootctl qrtr-tools unudhcpd | |
| # disable armbian repo back | |
| if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources" ]]; then | |
| mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources" "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" | |
| fi |
🤖 Prompt for AI Agents
In config/boards/qcom-robotics-rb5.conf around lines 86–91, the unguarded mv
commands that toggle armbian.sources can fail when the source file is absent;
update both moves so they first check whether the specific file exists and only
then perform the mv (i.e., conditionally move the disabled -> enabled before apt
ops and conditionally move enabled -> disabled after), preserving current
behavior when the file exists and avoiding build breaks when it does not.
Of course, I need to find time to handle it. |
FantasyGmm
force-pushed
the
main
branch
2 times, most recently
from
04e440e to
7aac0b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
config/boards/qcom-robotics-rb5.conf (3)
41-50: Non-executable scripts due to -m655; quote $destination.installing with -Dm655 yields non-executable scripts in the image; they won’t run (initramfs hooks, premount, helpers). Also, quote $destination to avoid path issues when unset/spacey.
Apply:
- mkdir -p $destination/usr/local/bin/ - mkdir -p $destination/usr/lib/systemd/system/ - mkdir -p $destination/etc/initramfs-tools/scripts/init-bottom/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network.sh $destination/usr/local/bin/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ - install -Dm644 $SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service $destination/usr/lib/systemd/system/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook $destination/etc/initramfs-tools/hooks/usb-gadget - install -Dm655 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount $destination/etc/initramfs-tools/scripts/init-premount/usb-gadget - install -Dm655 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/ + mkdir -p "${destination}/usr/local/bin" + mkdir -p "${destination}/usr/lib/systemd/system" + mkdir -p "${destination}/etc/initramfs-tools/scripts/init-bottom" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/setup-usbgadget-network.sh" "${destination}/usr/local/bin/" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh" "${destination}/usr/local/bin/" + install -Dm644 "${SRC}/packages/bsp/usb-gadget-network/usbgadget-rndis.service" "${destination}/usr/lib/systemd/system/" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook" "${destination}/etc/initramfs-tools/hooks/usb-gadget" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount" "${destination}/etc/initramfs-tools/scripts/init-premount/usb-gadget" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/dropbear" "${destination}/etc/initramfs-tools/scripts/init-premount/" + install -Dm755 "${SRC}/packages/bsp/usb-gadget-network/kill-dropbear" "${destination}/etc/initramfs-tools/scripts/init-bottom/"Nit: function name says “firmware” but installs services; consider renaming for clarity (optional).
85-91: Guard mv of armbian.sources to avoid failures when file is absent.Unconditional mv can break builds when the file doesn’t exist.
Apply:
- mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources + if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" ]]; then + mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" "${SDCARD}/etc/apt/sources.list.d/armbian.sources" + fi do_with_retries 3 chroot_sdcard_apt_get_update @@ - mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled + if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources" ]]; then + mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources" "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" + fi
111-121: Prefer /lib/firmware/updates in initramfs hook; handle both paths.To avoid conflicts with Armbian firmware packages and follow the firmware search order, collect from updates first, then fallback. Also fix the comment to match behavior.
Apply:
- for f in /lib/firmware/qcom/sm8250/Thundercomm/RB5/* ; do - add_firmware "${f#/lib/firmware/}" - done - # Add extra firmware from both possible locations - for fw in "qcom/sm8250/a650_zap.mbn" "qcom/a650_sqe.fw" "qcom/a650_gmu.bin"; do - add_firmware "$fw" - done + for base in /lib/firmware/updates /lib/firmware ; do + for f in "${base}/qcom/sm8250/Thundercomm/RB5/"* ; do + [[ -e "$f" ]] || continue + add_firmware "${f#${base}/}" + done + done + # Add extra firmware (resolved by add_firmware relative to /lib/firmware) + for fw in "qcom/sm8250/a650_zap.mbn" "qcom/a650_sqe.fw" "qcom/a650_gmu.bin"; do + add_firmware "$fw" + done
🧹 Nitpick comments (1)
config/boards/qcom-robotics-rb5.conf (1)
80-83: Run apt-get update immediately after adding PPA.Ensures indexes are fresh before subsequent installs, independent of Armbian repo toggling.
Apply:
if [[ "${RELEASE}" == "noble" ]]; then display_alert "Adding qcom-mainline PPA For" "${BOARD}" "info" do_with_retries 3 chroot_sdcard add-apt-repository ppa:liujianfeng1994/qcom-mainline --yes --no-update + do_with_retries 3 chroot_sdcard_apt_get_update fi
📜 Review details
Configuration used: CodeRabbit UI
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)
config/boards/qcom-robotics-rb5.conf(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
📚 Learning: 2025-09-11T06:12:54.213Z
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
Applied to files:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
PR: armbian/build#8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Applied to files:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
PR: armbian/build#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:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
PR: armbian/build#8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.
Applied to files:
config/boards/qcom-robotics-rb5.conf
📚 Learning: 2025-08-11T12:39:22.861Z
Learnt from: pyavitz
PR: armbian/build#8481
File: config/kernel/linux-spacemit-edge.config:492-494
Timestamp: 2025-08-11T12:39:22.861Z
Learning: The SpacemiT platform requires esos.elf firmware to be embedded in the kernel image for boot. The Armbian build system handles this by copying esos.elf from $SRC/packages/blobs/riscv64/spacemit/ to ${kernel_work_dir}/firmware/ via the custom_kernel_config__spacemit_k1_firmware() function in config/sources/families/spacemit.conf, ensuring CONFIG_EXTRA_FIRMWARE="esos.elf" can successfully embed it during kernel build.
Applied to files:
config/boards/qcom-robotics-rb5.conf
🔇 Additional comments (3)
config/boards/qcom-robotics-rb5.conf (3)
24-28: Userspace gate looks correct and minimal.
53-65: Service enablement LGTM.usbgadget-rndis.service is installed earlier; enabling here is fine.
1-23: Confirm 4K sector size and cmdline defaults.File: config/boards/qcom-robotics-rb5.conf (lines 1–23)
- SECTOR_SIZE: No RB5/QRB5165 requirement for 4K logical sectors; UFS may expose 4K but that’s a tooling detail. Default SECTOR_SIZE="512" in the board config and document/override to "4096" only for images that actually require it.
- SERIAL/GRUB alignment: Ensure SERIALCON matches the console= entry in GRUB_CMDLINE_LINUX_DEFAULT (currently console=ttyMSM0,1500000n8). Set SERIALCON to the same device/baud or remove the duplicate/conflicting console entries.
- Kernel params: Confirm the platform/firmware supports mem_sleep_default=s2idle before enabling it; keep efi=noruntime only if needed for this firmware.
FantasyGmm
force-pushed
the
main
branch
2 times, most recently
from
8246d5b to
5a2d57e
Compare
config/boards/qcom-robotics-rb5.conf
Outdated
| @@ -0,0 +1,125 @@ | |||
| # Qualcomm Robotics RB5 Configuration | |||
There was a problem hiding this comment.
The first line should contain a short summary of tech specs of this hw instead of board name which is redundant anyway because BOARD_NAME is declared already. Check other existing board configs for examples.
There was a problem hiding this comment.
@coderabbitai I think I already explained to you about first line of board config behaviour. This is something you can nitpick on automatically in future PRs in general.
There was a problem hiding this comment.
@EvilOlaf You're absolutely right! I have this guidance in my knowledge base already - that Armbian board configuration files should have a single-line comment describing the board hardware specifications (SoC model, core count, RAM options, and key features) rather than just stating the board name, which is redundant since BOARD_NAME is already declared in the configuration.
I'll make sure to automatically flag this pattern in future PR reviews when I see board config files that use redundant board names instead of proper hardware specifications in the first line comment.
In this case, the line # Qualcomm Robotics RB5 Configuration should be something like # Qualcomm SM8250 8-core ARMv8 4GB/8GB RAM eUFS WiFi6 BT5 USB3 DSI MIPI (based on the actual hardware specs of the RB5 board).
🧠 Learnings used
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
There was a problem hiding this comment.
In Armbian board configuration files (.csc)
This is not restricted to .csc only but valid for all board configuration files regardless of their file extension. Latter is used only to declare board support status, e.g. .conf being standard support, .csc being community supported, .eos being end of support, .tvb being a tvbox and whatever I forgot to mention.
There was a problem hiding this comment.
@EvilOlaf Thank you for the important clarification! You're absolutely correct - the hardware specification comment standard applies to all Armbian board configuration files regardless of their file extension, not just .csc files.
I appreciate you explaining the meaning of the different file extensions used in Armbian for indicating board support status:
.conf= standard support.csc= community supported.eos= end of support.tvb= tvbox
This is valuable project knowledge that I'll make sure to remember for future reviews.
✏️ Learnings added
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:26:48.604Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
🧠 Learnings used
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
Learnt from: pyavitz
PR: armbian/build#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.
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
Learnt from: EvilOlaf
PR: armbian/build#8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.
Learnt from: Grippy98
PR: armbian/build#8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
There was a problem hiding this comment.
Forgot to mention .wip for "Work in progress".
While we're at it adding new learnings (not sure if you know already), when the extension is .wip or .conf BOARD_MAINTAINER must be present and contain at least one maintainer. If not, the board support rules state it does not qualify for standard support and therefore must be moved to community support (.csc). For this specific PR a named maintainer is present, therefore .conf is valid.
All requirements are listed here: https://docs.armbian.com/User-Guide_Board-Support-Rules/
This is something that shall be handled by you as error automatically in future.
There was a problem hiding this comment.
@EvilOlaf Thank you for the additional information! You're right - I missed the .wip extension for "Work in progress" status.
I understand the maintainer requirements now:
- For
.wipand.conffiles,BOARD_MAINTAINERmust be present and contain at least one maintainer - If no maintainer is present, the board doesn't qualify for standard support and must use
.csc(community supported) extension instead - In this specific PR,
BOARD_MAINTAINER = "FantasyGmm"is present, so the.confextension is valid
I'll make sure to automatically flag missing BOARD_MAINTAINER in .wip and .conf files as an error in future reviews, referring to the board support rules documentation you linked.
✏️ Learnings added
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.842Z
Learning: The .wip file extension in Armbian board configuration files indicates "Work in progress" status.
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.842Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.842Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:26:48.604Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox).
🧠 Learnings used
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:26:48.604Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox).
Learnt from: pyavitz
PR: armbian/build#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.
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:16:10.236Z
Learning: In Armbian board configuration files, WIP (Work In Progress) comments describe intended goals or planned features rather than current implementation state. For example, a comment like "WIP: current kernel" indicates that current kernel support is being worked toward, even if KERNEL_TARGET doesn't yet include "current".
Learnt from: EvilOlaf
PR: armbian/build#8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
There was a problem hiding this comment.
@FantasyGmm sorry for (ab)using your PR a bit when messing with Rabbit 😁
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Never mind, I'll sort it out.
FantasyGmm
force-pushed
the
main
branch
3 times, most recently
from
fe1cc9e to
634a983
Compare
|
|
||
| if [[ "${DESKTOP_ENVIRONMENT}" == "kde-plasma" ]]; then | ||
| display_alert "Adding Extra KDE Package" "${BOARD}" "info" | ||
| do_with_retries 3 chroot_sdcard_apt_get_install plasma-workspace plasma-desktop plasma-systemmonitor plasma-nm kde-standard kde-spectacle kinfocenter kscreen krfb kfind filelight \ |
There was a problem hiding this comment.
wouldn't this be better to move to kde commons?
Description
The function name has been modified, which should resolve the error reported in ci.
Reference from the board configuration below
build/config/boards/xiaomi-elish.conf
Line 23 in 1056f8e
GitHub issue reference:
Jira reference number [AR-9999]
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist:
Please delete options that are not relevant.