Skip to content

Conversation

@schwar3kat
Copy link
Contributor

@schwar3kat schwar3kat commented Dec 21, 2025

Description

Modify led setup section of the radxa-e54c board config to use new led device names.

Modify the radxa-e54c board config to add a keyfile /etc/NetworkManager/conf.d/99-unmanaged-devices.conf on the board to set the internal ethernet, end1, used to connect to the internal switch chip to unmanaged. This interface doesn't accept DHCP IP addresses and Network Manager attempts to connect it via DHCP causing errors.

Modify the radxa-e54c board config to modify the boards /usr/lib/armbian/armbian-firstlogin file to set WaylandEnable = false if the build is for a vendor kernel with a Gnome desktop to prevent vendor kernel build with Gnome desktop attempting to use Wayland unsuccessfully causing the desktop to take over 10 minutes to initialize eventually using X11.

Jira reference number AR-2801

How Has This Been Tested?

  • Test on vendor kernel with trixie minimal build, noble minimal build, trixie with xfce desktop and noble with gnome desktop that wan, lan1, lan2, lan3 LEDs work when the network is connected to a matching Ethernet port.
  • Test on vendor kernel with trixie minimal build, noble minimal build, trixie with xfce desktop and noble with gnome desktop that NetworkManager does not attempt to manage the end1 network adapter.
  • Test that on a vendor kernel with Gnome desktop, the Gnome desktop initializes quickly.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Summary by CodeRabbit

  • Bug Fixes

    • Corrected network LED mappings so status indicators display accurately.
    • Ensured the internal Ethernet interface is left unmanaged to prevent connection conflicts.
  • Chores

    • Updated first-login desktop setup for vendor GNOME: disables Wayland and refines automatic login behavior.

These changes improve network status clarity and first‑login reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

Modify led setup section to use new led device names

Add a keyfile /etc/NetworkManager/conf.d/99-unmanaged-devices.conf to set the internal ethernet, end1, used to connect to the internal switch chip to unmanaged.  This interface doesn't accept dhcp IP addresses and Network Manager attempts to connect it via dhcp causing errors.

A vendor kernel build with Gnome desktop attempts to use Wayland unsuccessfully causing the desktop to take over 10 minutes to initialise eventually using X11.  Modify the board /usr/lib/armbian/armbian-firstlogin file to set WaylandEnable = false if the build is for a vendor kernel with a Gnome desktop.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 21, 2025

Walkthrough

Normalized Radxa E54C board config: removed @end1 suffixes from LED device names, added a post-config block that creates a NetworkManager unmanaged device entry for internal Ethernet end1, and conditionally disables Wayland in /usr/lib/armbian/armbian-firstlogin when DESKTOP_ENVIRONMENT=gnome and BRANCH=vendor.

Changes

Cohort / File(s) Summary
Radxa E54C board configuration
config/boards/radxa-e54c.conf
Removed @end1 suffix from LED device_name entries (lan1, lan2, lan3, wan); added post-config actions to create a NetworkManager unmanaged devices config entry for internal Ethernet end1; added conditional patch to update /usr/lib/armbian/armbian-firstlogin to set WaylandEnable = false when DESKTOP_ENVIRONMENT=gnome and BRANCH=vendor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Check LED name changes against vendor/kernel device names and sysfs paths.
  • Verify NetworkManager unmanaged config syntax and that it targets the correct interface (end1).
  • Ensure the conditional editing of the first-login script only applies for GNOME/vendor branches and uses safe patching.

Possibly related PRs

Suggested labels

BSP

Suggested reviewers

  • igorpecovnik
  • amazingfate

Poem

🐰 I hopped through configs, tidy and spry,

LEDs renamed so kernels comply.
I told end1 gently to rest and be grave,
Whispered to GNOME: "no Wayland, behave."
Cheers — the board boots quicker now, hip-hop hooray!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the three main changes: LED fixes, network configuration, and Gnome desktop initialization.
Linked Issues check ✅ Passed The code changes address all three objectives from AR-2801: LED device names updated, NetworkManager unmanaged config added for end1, and Wayland disabled for Gnome vendor builds.
Out of Scope Changes check ✅ Passed All changes in the radxa-e54c.conf file directly relate to the three objectives stated in AR-2801 with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added size/small PR with less then 50 lines 02 Milestone: First quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... labels Dec 21, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
config/boards/radxa-e54c.conf (1)

62-72: **Verify the NetworkManager plugin name.**Based on the web search results, the plugin name is 'ifcfg-rh' for NetworkManager. Line 67 contains "ifconfig-rh" which appears to be a typo - it should be "ifcfg-rh".

However, upon closer inspection of the configuration structure, the [keyfile] section with unmanaged-devices doesn't require the plugins line at all since the keyfile plugin is always enabled in NetworkManager. The unmanaged-devices directive will work regardless of the plugins specification.

The configuration creates /etc/NetworkManager/conf.d/99-unmanaged-devices.conf to mark the end1 interface as unmanaged, which correctly prevents NetworkManager from attempting DHCP on the internal switch interface.

Potential issue on Line 67: The plugin name "ifconfig-rh" appears to be a typo - the correct NetworkManager plugin name is "ifcfg-rh".

However, this may not cause actual issues because:

  • The [keyfile] section and unmanaged-devices directive are handled by the keyfile plugin, which is always enabled by default
  • The plugins line in this context may be redundant

Suggested improvement: Either correct the typo to "ifcfg-rh" or simplify by removing the plugins line entirely since the keyfile plugin handles the [keyfile] section by default.

🔎 Suggested fix

Option 1 - Correct the typo:

 	[main]
-	plugins=ifconfig-rh,keyfile
+	plugins=ifcfg-rh,keyfile

Option 2 - Simplify (recommended):

 	[main]
-	plugins=ifconfig-rh,keyfile
-
 	[keyfile]
 	unmanaged-devices=interface-name:end1
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between f6b90e5 and e3a5f60.

📒 Files selected for processing (1)
  • config/boards/radxa-e54c.conf (5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
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).

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
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/

Applied to files:

  • config/boards/radxa-e54c.conf
📚 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:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
Repo: armbian/build PR: 8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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/radxa-e54c.conf
🔇 Additional comments (1)
config/boards/radxa-e54c.conf (1)

24-24: LGTM! LED device names updated for new vendor kernel.

The device_name changes from lan1@end1 format to lan1 format correctly reflect the new LED device naming in the latest vendor kernel, which should restore LED functionality.

Also applies to: 33-33, 42-42, 51-51

Tested the suggestion and it works.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
config/boards/radxa-e54c.conf (1)

63-72: Consider removing the RedHat-specific plugin from NetworkManager config.

The NetworkManager configuration correctly marks end1 as unmanaged to prevent DHCP attempts on the internal switch interface. However, the plugins line includes ifconfig-rh, which is specific to RedHat/Fedora systems for reading ifcfg files. Since Armbian is Debian-based, this plugin is unnecessary.

🔎 Simplified NetworkManager config
 	cat <<- EOF > "${destination}"/etc/NetworkManager/conf.d/99-unmanaged-devices.conf
 	[main]
-	plugins=ifconfig-rh,keyfile
+	plugins=keyfile
 
 	[keyfile]
 	unmanaged-devices=interface-name:end1

Alternatively, you can omit the plugins line entirely, as NetworkManager loads the keyfile plugin by default on Debian-based systems.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e3a5f60 and 02b2bc3.

📒 Files selected for processing (1)
  • config/boards/radxa-e54c.conf (5 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.
Learnt from: djurny
Repo: armbian/build PR: 8235
File: packages/bsp/mvebu/helios4/helios4-wol.service:0-0
Timestamp: 2025-05-29T01:56:01.604Z
Learning: For ARM-based devices like Helios4 that use DeviceTree, the expected systemd network interface naming scheme is "end[0-9]+" (DeviceTree alias index), making the regex "^(eth|en[do])[0-9]+" appropriate as it covers traditional "eth", PCI on-board "eno", and DeviceTree "end" interfaces without unnecessary "enp" (PCI slot) support.
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:17-25
Timestamp: 2025-12-13T11:45:02.422Z
Learning: In the SpacemiT U-Boot patches for Armbian (patch/u-boot/legacy/u-boot-spacemit-k1/), the environment variable `devnum` is set to the device name string (e.g., "mmc", "nvme") rather than a numeric index, and `distro_bootpart` holds the partition number. This implementation aligns with mainline U-Boot conventions for the SpacemiT platform and has been verified to work correctly by the maintainer.
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
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).

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-12-01T02:34:37.451Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9000
File: config/desktop/questing/environments/xfce/debian/postinst:17-18
Timestamp: 2025-12-01T02:34:37.451Z
Learning: In the Armbian build system shell scripts, the sed pattern with -i flag placed after the script argument (e.g., `sed "s/.../.../" -i file`) is intentional and correct. While non-POSIX, this GNU sed extension is used throughout the codebase and works correctly in the Armbian build environment. This pattern should not be flagged as incorrect.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 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:

  • config/boards/radxa-e54c.conf
📚 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:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
Repo: armbian/build PR: 8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-11-09T22:30:27.163Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8720
File: lib/functions/rootfs/distro-specific.sh:38-47
Timestamp: 2025-11-09T22:30:27.163Z
Learning: In lib/functions/rootfs/distro-specific.sh, the systemd sleep.conf.d override that disables suspend/hibernation is intentionally applied system-wide to all Armbian images (desktop, CLI, and minimal), not gated to desktop-only builds, because suspend/resume is fragile on most boards.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 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:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-12-16T13:40:07.649Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-08-30T06:48:09.091Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-30T06:48:09.091Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the user prefers flexible grep patterns over anchored ones for BTRFS configuration checks, but agrees to use quiet grep (-q) to avoid polluting build logs.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-10-03T11:42:46.660Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8705
File: .github/workflows/rewrite-kernel-configs.yml:96-169
Timestamp: 2025-10-03T11:42:46.660Z
Learning: In the Armbian build repository's kernel config rewrite workflow (.github/workflows/rewrite-kernel-configs.yml), deletion of kernel config files does not need to be handled. The workflow intentionally only captures ACMR (added/copied/modified/renamed) changes and does not need to propagate deletions to the aggregator.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-06-24T10:08:40.313Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
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/

Applied to files:

  • config/boards/radxa-e54c.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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/radxa-e54c.conf
🔇 Additional comments (2)
config/boards/radxa-e54c.conf (2)

24-24: LED device names correctly updated for new kernel.

The removal of the @end1 suffix from the LED device_name entries aligns with the PR objective to support the latest vendor kernel's device naming. For boards with DSA (Distributed Switch Architecture), the interface names lan1, lan2, lan3, and wan are standard.

Also applies to: 33-33, 42-42, 51-51


74-76: Wayland disable logic is appropriate for the use case.

The conditional correctly gates the Wayland disable to vendor kernel + GNOME desktop builds only. As discussed in previous review comments, this modification is board-specific and addresses the PR objective to prevent long Gnome desktop initialization delays.

invert=0
EOF

# Internal Ethernet interface can't get an IP address and shouldn't be managed by NetworkManager so make it unmanaged.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if systemd-network is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if systemd-network is used?

It's only NetworkManager that tries to initialize end1 by DHCP and causes boot issues.
The keyfile has no effect on systemd-network, but the images boot okay without network, and all interfaces can then be configured normally. This seems okay for minimal images where we can expect slightly more savvy Linux users anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First quarter release Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

2 participants