Skip to content

config: boards: pocketbeagle2: Enable better USB Gadget #8382

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

config: boards: pocketbeagle2: Enable better USB Gadget #8382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions config/boards/pocketbeagle2.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,55 @@ TIBOOT3_FILE="tiboot3-am62x-hs-fs-evm.bin"
DEFAULT_CONSOLE="serial"
KERNEL_TARGET="edge"
KERNEL_TEST_TARGET="edge"
SERIALCON="ttyS2,ttyGS0"
SERIALCON="ttyS2"
Copy link

Choose a reason for hiding this comment

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

Why would you remove the gadget serial console?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, currently, enabling gadget serial console causes armbian first run to silently die. Or well, to be more accurate, the gadget serial console ends up being run after armbian-first run is already started. And armbian-first-run does not like a new serial console showing up all of a sudden.

ATF_BOARD="lite"
SRC_EXTLINUX="yes"
SRC_CMDLINE="root=/dev/mmcblk1p2 rootwait console=ttyS2,115200n8 console=ttyGS0,115200n8 modules-load=dwc2,g_cdc"
SRC_CMDLINE="root=/dev/mmcblk1p2 rootwait console=ttyS2,115200n8"
BOOT_FDT_FILE="ti/k3-am6232-pocketbeagle2.dtb"
OPTEE_PLATFORM="k3-am62x"

# USB Gadget
function post_family_tweaks_bsp__pocketbeagle2_firmware() {
Copy link

Choose a reason for hiding this comment

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

Assuming, based on our discussion, that you won't do this here, but instead make this into a package? This just seems wrong.

display_alert "Setup USB Gadget for ${BOARD}" "${RELEASE}" "warn"

# USB Gadget Network service
mkdir -p $destination/usr/local/bin/
Copy link

Choose a reason for hiding this comment

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

I don't think any distro-installed software should be in /usr/local.

mkdir -p $destination/usr/lib/systemd/system/
mkdir -p $destination/etc/initramfs-tools/scripts/init-bottom/
# The service expects the name setup-usbgadget-network.sh. Depending on the board vendor, the
# usbgadget configuration they would like can be wildly different. However, the same service
# will work fine for almost all cases. So better to rename file rather than modifying the service.
install -Dm755 $SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh $destination/usr/local/bin/setup-usbgadget-network.sh ||
exit_with_error "Failed to install USB gadget setup script"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ ||
Copy link

Choose a reason for hiding this comment

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

/usr/local is just wrong in a distro-installed component.

exit_with_error "Failed to install USB gadget remove script"
install -Dm744 $SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service $destination/usr/lib/systemd/system/ ||
exit_with_error "Failed to install USB gadget service"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook $destination/etc/initramfs-tools/hooks/usb-gadget ||
exit_with_error "Failed to install USB gadget initramfs hook"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount $destination/etc/initramfs-tools/scripts/init-premount/usb-gadget ||
exit_with_error "Failed to install USB gadget initramfs premount script"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ ||
exit_with_error "Failed to install USB gadget initramfs premount dropbear"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/ ||
exit_with_error "Failed to install USB gadget initramfs premount kill dropbear"
Comment on lines +25 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix unit file mode (0644) and quote paths to harden installs

  • Systemd unit should be 0644, not 0744.
  • Quote $SRC and $destination to avoid pathname splitting and globbing.

Apply:

-	mkdir -p $destination/usr/local/bin/
-	mkdir -p $destination/usr/lib/systemd/system/
-	mkdir -p $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-multi.sh $destination/usr/local/bin/setup-usbgadget-network.sh ||
+	install -Dm755 "$SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh" "$destination/usr/local/bin/setup-usbgadget-network.sh" ||
 		exit_with_error "Failed to install USB gadget setup script"
-	install -Dm755 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ ||
+	install -Dm755 "$SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh" "$destination/usr/local/bin/" ||
 		exit_with_error "Failed to install USB gadget remove script"
-	install -Dm744 $SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service $destination/usr/lib/systemd/system/ ||
+	install -Dm644 "$SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service" "$destination/usr/lib/systemd/system/" ||
 		exit_with_error "Failed to install USB gadget service"
-	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-hook" "$destination/etc/initramfs-tools/hooks/usb-gadget" ||
 		exit_with_error "Failed to install USB gadget initramfs hook"
-	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/usb-gadget-initramfs-premount" "$destination/etc/initramfs-tools/scripts/init-premount/usb-gadget" ||
 		exit_with_error "Failed to install USB gadget initramfs premount script"
-	install -Dm755 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ ||
+	install -Dm755 "$SRC/packages/bsp/usb-gadget-network/dropbear" "$destination/etc/initramfs-tools/scripts/init-premount/" ||
 		exit_with_error "Failed to install USB gadget initramfs premount dropbear"
-	install -Dm755 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/ ||
+	install -Dm755 "$SRC/packages/bsp/usb-gadget-network/kill-dropbear" "$destination/etc/initramfs-tools/scripts/init-bottom/" ||
 		exit_with_error "Failed to install USB gadget initramfs premount kill dropbear"
📝 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.

Suggested change
mkdir -p $destination/usr/local/bin/
mkdir -p $destination/usr/lib/systemd/system/
mkdir -p $destination/etc/initramfs-tools/scripts/init-bottom/
# The service expects the name setup-usbgadget-network.sh. Depending on the board vendor, the
# usbgadget configuration they would like can be wildly different. However, the same service
# will work fine for almost all cases. So better to rename file rather than modifying the service.
install -Dm755 $SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh $destination/usr/local/bin/setup-usbgadget-network.sh ||
exit_with_error "Failed to install USB gadget setup script"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ ||
exit_with_error "Failed to install USB gadget remove script"
install -Dm744 $SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service $destination/usr/lib/systemd/system/ ||
exit_with_error "Failed to install USB gadget service"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook $destination/etc/initramfs-tools/hooks/usb-gadget ||
exit_with_error "Failed to install USB gadget initramfs hook"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount $destination/etc/initramfs-tools/scripts/init-premount/usb-gadget ||
exit_with_error "Failed to install USB gadget initramfs premount script"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ ||
exit_with_error "Failed to install USB gadget initramfs premount dropbear"
install -Dm755 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/ ||
exit_with_error "Failed to install USB gadget initramfs premount kill dropbear"
mkdir -p "$destination/usr/local/bin"
mkdir -p "$destination/usr/lib/systemd/system"
mkdir -p "$destination/etc/initramfs-tools/scripts/init-bottom"
# The service expects the name setup-usbgadget-network.sh. Depending on the board vendor, the
# usbgadget configuration they would like can be wildly different. However, the same service
# will work fine for almost all cases. So better to rename file rather than modifying the service.
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh" "$destination/usr/local/bin/setup-usbgadget-network.sh" ||
exit_with_error "Failed to install USB gadget setup script"
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh" "$destination/usr/local/bin/" ||
exit_with_error "Failed to install USB gadget remove script"
install -Dm644 "$SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service" "$destination/usr/lib/systemd/system/" ||
exit_with_error "Failed to install USB gadget service"
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook" "$destination/etc/initramfs-tools/hooks/usb-gadget" ||
exit_with_error "Failed to install USB gadget initramfs hook"
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount" "$destination/etc/initramfs-tools/scripts/init-premount/usb-gadget" ||
exit_with_error "Failed to install USB gadget initramfs premount script"
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/dropbear" "$destination/etc/initramfs-tools/scripts/init-premount/" ||
exit_with_error "Failed to install USB gadget initramfs premount dropbear"
install -Dm755 "$SRC/packages/bsp/usb-gadget-network/kill-dropbear" "$destination/etc/initramfs-tools/scripts/init-bottom/" ||
exit_with_error "Failed to install USB gadget initramfs premount kill dropbear"
🤖 Prompt for AI Agents 
In config/boards/pocketbeagle2.conf around lines 25 to 44, the systemd unit is
being installed with mode 0744 and several install commands use unquoted $SRC
and $destination which can cause pathname splitting/globbing; change the
usbgadget-rndis.service install to mode 0644 and wrap all occurrences of $SRC
and $destination in double quotes (e.g., "$SRC/..." and "$destination/...") so
paths are safe and the unit file permissions are corrected.

}

function post_family_tweaks__pocketbeagle2_enable_services() {
display_alert "Enable Services for ${BOARD}" "${RELEASE}" "warn"

# We need unudhcpd from armbian repo, so enable it
mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources
Copy link

Choose a reason for hiding this comment

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

This feels wrong.

do_with_retries 3 chroot_sdcard_apt_get_install unudhcpd
# disable armbian repo back
mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled
Copy link

Choose a reason for hiding this comment

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

Odd.


chroot_sdcard systemctl enable usbgadget-rndis.service
Copy link

Choose a reason for hiding this comment

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

This feels like it should be in a package postinst.

# We want custom IPs. systemctl edit does not seem to work in chroot.
mkdir -p "${SDCARD}"/etc/systemd/system/usbgadget-rndis.service.d
echo -e "[Service]\nEnvironment=\"unudhcpd_host_ip=192.168.7.2\"\nEnvironment=\"unudhcpd_client_ip=192.168.7.3\"" > "${SDCARD}"/etc/systemd/system/usbgadget-rndis.service.d/override.conf
Copy link

Choose a reason for hiding this comment

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

This feels like it should be in a package install.

}

function current_beagle_kernel_uboot() {
declare -g KERNELSOURCE="https://github.com/beagleboard/linux" # BeagleBoard kernel
declare -g KERNEL_MAJOR_MINOR="6.12"
Expand Down
105 changes: 105 additions & 0 deletions packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

I think it makes sense to put this in a package outside the BSP.

Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#!/bin/bash
set -euo pipefail

deviceinfo_name="USB Gadget Network"
deviceinfo_manufacturer="Armbian"
#deviceinfo_usb_idVendor=
#deviceinfo_usb_idProduct=
#deviceinfo_usb_serialnumber=

exit_with_error() {
echo "$1"
exit 1
}

setup_usb_network_configfs() {
# See: https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
CONFIGFS=/sys/kernel/config/usb_gadget

if ! [ -e "$CONFIGFS" ]; then
exit_with_error "$CONFIGFS does not exist, skipping configfs usb gadget"
fi

if [ -e "$CONFIGFS/g1" ]; then
echo "$CONFIGFS/g1 already exists, skipping configfs usb gadget"
return
fi

# Default values for USB-related deviceinfo variables
usb_idVendor="${deviceinfo_usb_idVendor:-0x1D6B}" # Linux Foundation
usb_idProduct="${deviceinfo_usb_idProduct:-0x0104}" # Multifunction Composite Gadget
usb_serialnumber="${deviceinfo_usb_serialnumber:-0123456789}"
usb_network_function="ncm.usb0"
usb_serial_function="acm.usb0"

echo "Setting up an USB gadget through configfs"
# Create an usb gadet configuration
mkdir $CONFIGFS/g1 || exit_with_error "Couldn't create $CONFIGFS/g1"
echo "$usb_idVendor" > "$CONFIGFS/g1/idVendor"
echo "$usb_idProduct" > "$CONFIGFS/g1/idProduct"
echo 0x0404 > "$CONFIGFS/g1/bcdDevice"
echo 0x0200 > "$CONFIGFS/g1/bcdUSB"

# Create english (0x409) strings
mkdir $CONFIGFS/g1/strings/0x409 || echo "Couldn't create $CONFIGFS/g1/strings/0x409"

# shellcheck disable=SC2154
echo "$deviceinfo_manufacturer" > "$CONFIGFS/g1/strings/0x409/manufacturer"
echo "$usb_serialnumber" > "$CONFIGFS/g1/strings/0x409/serialnumber"
# shellcheck disable=SC2154
echo "$deviceinfo_name" > "$CONFIGFS/g1/strings/0x409/product"

# Create network function.
mkdir $CONFIGFS/g1/functions/"$usb_network_function" ||
echo "Couldn't create $CONFIGFS/g1/functions/$usb_network_function"

# Create configuration instance for the gadget
mkdir $CONFIGFS/g1/configs/c.1 ||
echo "Couldn't create $CONFIGFS/g1/configs/c.1"
echo 250 > $CONFIGFS/g1/configs/c.1/MaxPower
mkdir $CONFIGFS/g1/configs/c.1/strings/0x409 ||
echo "Couldn't create $CONFIGFS/g1/configs/c.1/strings/0x409"
echo "NCM Configuration" > $CONFIGFS/g1/configs/c.1/strings/0x409/configuration ||
echo "Couldn't write configration name"

# Link the network instance to the configuration
ln -s $CONFIGFS/g1/functions/"$usb_network_function" $CONFIGFS/g1/configs/c.1 ||
echo "Couldn't symlink $usb_network_function"

mkdir -p $CONFIGFS/g1/functions/"$usb_serial_function" ||
echo "Couldn't create $CONFIGFS/g1/functions/$usb_serial_function"
ln -s $CONFIGFS/g1/functions/"$usb_serial_function" $CONFIGFS/g1/configs/c.1 ||
echo "Couldn't symlink $usb_serial_function"

# Check if there's an USB Device Controller
if [ ! -d /sys/class/udc ] || [ -z "$(ls /sys/class/udc 2>/dev/null)" ]; then
exit_with_error "No USB Device Controller available"
fi

# Link the gadget instance to an USB Device Controller. This activates the gadget.
# See also: https://github.com/postmarketOS/pmbootstrap/issues/338
# shellcheck disable=SC2005
ls /sys/class/udc | head -1 > $CONFIGFS/g1/UDC || exit_with_error "Couldn't write UDC"
}

set_usbgadget_ipaddress() {
local host_ip="${unudhcpd_host_ip:-172.16.42.1}"
local client_ip="${unudhcpd_client_ip:-172.16.42.2}"
echo "Starting dnsmasq with server ip $host_ip, client ip: $client_ip"
# Get usb interface
INTERFACE=""
ip addr replace "${host_ip}/16" dev usb0 2>/dev/null && ip link set usb0 up && INTERFACE=usb0
if [ -z "$INTERFACE" ]; then
echo "Interfaces:"
ip link
exit_with_error "Could not find an interface to run a dhcp server on"
fi

echo "Using interface $INTERFACE"
echo "Starting the DHCP daemon"
ip a show $INTERFACE > /var/log/unudhcpd.log
nohup /usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
Comment on lines +85 to +101
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix message (dnsmasq vs unudhcpd) and avoid double-starting DHCP

  • Log mentions dnsmasq but you run unudhcpd.
  • Guard against multiple instances; prefer no nohup in systemd contexts.
 set_usbgadget_ipaddress() {
 	local host_ip="${unudhcpd_host_ip:-172.16.42.1}"
 	local client_ip="${unudhcpd_client_ip:-172.16.42.2}"
-	echo "Starting dnsmasq with server ip $host_ip, client ip: $client_ip"
+	echo "Starting DHCP (unudhcpd) with server ip $host_ip, client ip: $client_ip"
 	# Get usb interface
-	INTERFACE=""
+	local INTERFACE=""
 	ip addr replace "${host_ip}/16" dev usb0 2>/dev/null && ip link set usb0 up && INTERFACE=usb0
 	if [ -z "$INTERFACE" ]; then
 		echo "Interfaces:"
 		ip link
 		exit_with_error "Could not find an interface to run a dhcp server on"
 	fi
 
 	echo "Using interface $INTERFACE"
 	echo "Starting the DHCP daemon"
-	ip a show $INTERFACE > /var/log/unudhcpd.log
-	nohup /usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
+	ip a show "$INTERFACE" > /var/log/unudhcpd.log
+	# Skip if already running (systemd unit or prior invocation)
+	if pgrep -x unudhcpd >/dev/null 2>&1; then
+		echo "unudhcpd already running; skip"
+		return
+	fi
+	/usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
 	return
 }
📝 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.

Suggested change
set_usbgadget_ipaddress() {
local host_ip="${unudhcpd_host_ip:-172.16.42.1}"
local client_ip="${unudhcpd_client_ip:-172.16.42.2}"
echo "Starting dnsmasq with server ip $host_ip, client ip: $client_ip"
# Get usb interface
INTERFACE=""
ip addr replace "${host_ip}/16" dev usb0 2>/dev/null && ip link set usb0 up && INTERFACE=usb0
if [ -z "$INTERFACE" ]; then
echo "Interfaces:"
ip link
exit_with_error "Could not find an interface to run a dhcp server on"
fi
echo "Using interface $INTERFACE"
echo "Starting the DHCP daemon"
ip a show $INTERFACE > /var/log/unudhcpd.log
nohup /usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
set_usbgadget_ipaddress() {
local host_ip="${unudhcpd_host_ip:-172.16.42.1}"
local client_ip="${unudhcpd_client_ip:-172.16.42.2}"
echo "Starting DHCP (unudhcpd) with server ip $host_ip, client ip: $client_ip"
# Get usb interface
local INTERFACE=""
ip addr replace "${host_ip}/16" dev usb0 2>/dev/null && ip link set usb0 up && INTERFACE=usb0
if [ -z "$INTERFACE" ]; then
echo "Interfaces:"
ip link
exit_with_error "Could not find an interface to run a dhcp server on"
fi
echo "Using interface $INTERFACE"
echo "Starting the DHCP daemon"
ip a show "$INTERFACE" > /var/log/unudhcpd.log
# Skip if already running (systemd unit or prior invocation)
if pgrep -x unudhcpd >/dev/null 2>&1; then
echo "unudhcpd already running; skip"
return
fi
/usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
return
}

Copy link
Contributor

@Grippy98 Grippy98 Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is probably a valid comment - doesn't regular beagle Debian image use dnsmasq already for out of box web UI anyway? Why use unudhcpd on top? - correction nvm then unudhcpd only

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@Grippy98 That's an excellent point! Let me investigate the current DHCP service usage in the Armbian BeagleBone ecosystem to understand if there's potential redundancy or conflict.

🏁 Script executed:

#!/bin/bash
# Check for existing dnsmasq usage in BeagleBone configurations
echo "=== Searching for dnsmasq usage in BeagleBone/PocketBeagle configs ==="
rg -i "dnsmasq" --type=conf --type=sh -A3 -B3

echo -e "\n=== Searching for existing DHCP server configurations ==="
rg -i "dhcp.*server|udhcpd|dnsmasq.*dhcp" --type=conf --type=sh -A2 -B2

echo -e "\n=== Checking other BeagleBone board configurations ==="
fd "beagle" -t f -e conf | head -10 | xargs cat

echo -e "\n=== Looking for existing USB gadget network implementations ==="
fd "usb.*gadget|gadget.*usb" -t f -e sh | head -5 | xargs grep -l "dhcp\|dnsmasq\|udhcpd" 2>/dev/null || true

Length of output: 6725

🏁 Script executed:

#!/bin/bash
# Let's examine the existing USB gadget network implementation and search more broadly
echo "=== Examining existing USB gadget network setup ==="
cat packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\n=== Checking for dnsmasq usage across the entire codebase ==="
rg -i "dnsmasq" -A2 -B2

echo -e "\n=== Looking for any DHCP-related configurations ==="
rg -i "dhcp|udhcpd" -A2 -B2 --type=sh

echo -e "\n=== Examining the systemd service file ==="
cat packages/bsp/usb-gadget-network/usbgadget-rndis.service 2>/dev/null || echo "Service file not found"

Length of output: 11217

You've raised an excellent architectural question! After investigating the codebase, here's what I found:

Current Armbian Architecture:

  • Armbian's existing USB gadget implementation (setup-usbgladget-network.sh) already uses unudhcpd, not dnsmasq
  • There's no actual dnsmasq usage found in the current Armbian BeagleBone configurations
  • The unudhcpd package is specifically installed from the Armbian repository for USB gadget networking

The Confusion:
Both the existing script and the new one have misleading log messages saying "Starting dnsmasq" while actually using unudhcpd. This is exactly what CodeRabbit flagged - the log message doesn't match the actual implementation.

Why unudhcpd vs dnsmasq:

  • unudhcpd is a lightweight, minimal DHCP server ideal for simple USB gadget scenarios
  • It has a smaller footprint and fewer dependencies than dnsmasq
  • The existing Armbian ecosystem already standardized on this approach

Regarding upstream BeagleBoard images:
You're likely thinking of the official BeagleBoard.org Debian images, which may indeed use dnsmasq. However, Armbian maintains its own approach with unudhcpd for USB gadget networking.

SystemD Ordering:
The service file shows Before=dnsmasq.service, which suggests the developers were aware of potential conflicts but designed the service to run before any dnsmasq instance that might be installed later.

Recommendation:
The original CodeRabbit suggestion to fix the log message is valid - it should say "Starting DHCP (unudhcpd)" instead of "Starting dnsmasq" to avoid confusion. The architecture choice itself appears sound and consistent with existing Armbian practices.

🐰 thoughtfully considering the DHCP ecosystem

Copy link
Member Author

@Ayush1325 Ayush1325 Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is probably a valid comment - doesn't regular beagle Debian image use dnsmasq already for out of box web UI anyway? Why use unudhcpd on top? - correction nvm then unudhcpd only

Well, the above setup is mostly a modified version of already existing armbian infra for doing this: https://github.com/armbian/build/blob/main/packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

To address this particular commit, we would need to dissociate things from the existing infra (since I do not think it's the best idea to change the setups in all the boards that are already using the current method). And at that point, it would be much better to have it as a debian package instead of copying scripts to the image.

Copy link
Contributor

Choose a reason for hiding this comment

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

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!

Copy link

Choose a reason for hiding this comment

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

Armbian probably copied what we were doing already, but made some tweaks. Rather than add a new script, why not just use settings available to that script or modified to extend for our needs? Seems like we are needlessly re-inventing a re-invented wheel. Feel free to comment @coderabbitai.

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@jadonk You raise an excellent architectural point about avoiding unnecessary duplication. Let me analyze both scripts to understand the differences and assess whether consolidation is feasible.

🏁 Script executed:

#!/bin/bash
echo "=== Examining the existing USB gadget network script ==="
cat packages/bsp/usb-gadget-network/setup-usbgladget-network.sh

echo -e "\n=== Examining the new multi-function script ==="
cat packages/bsp/usb-gadget-network/setup-usbgladget-network-multi.sh

echo -e "\n=== Key differences analysis ==="
echo "Comparing function names and structure..."
grep -n "^[[:space:]]*[a-zA-Z_][a-zA-Z0-9_]*[[:space:]]*(" packages/bsp/usb-gadget-network/setup-usbgladget-network.sh || echo "No functions found in original"
grep -n "^[[:space:]]*[a-zA-Z_][a-zA-Z0-9_]*[[:space:]]*(" packages/bsp/usb-gadget-network/setup-usbgladget-network-multi.sh || echo "No functions found in new script"

echo -e "\n=== Examining USB gadget configuration differences ==="
echo "Original script USB function configuration:"
grep -A5 -B5 "usb_network_function\|mkdir.*functions" packages/bsp/usb-gadget-network/setup-usbgladget-network.sh || echo "No USB function config found"

echo -e "\nNew script USB function configuration:"
grep -A5 -B5 "usb_network_function\|usb_serial_function\|mkdir.*functions" packages/bsp/usb-gadget-network/setup-usbgladget-network-multi.sh || echo "No USB function config found"

Length of output: 1017

Let me first locate the existing USB gadget scripts and ensure I'm examining the right files:

🏁 Script executed:

#!/bin/bash
echo "=== Finding USB gadget related scripts ==="
fd -t f -e sh | xargs grep -l "usb.*gadget\|gadget.*usb" 2>/dev/null | head -10

echo -e "\n=== Searching for existing setup scripts ==="
fd "setup.*usb" -t f

echo -e "\n=== Looking for the exact script mentioned in comments ==="
fd "setup-usbgladget-network.sh" -t f

echo -e "\n=== Current repository state ==="
pwd
git branch --show-current
git log --oneline -5

echo -e "\n=== Looking for USB-related files in packages/bsp ==="
find . -name "*usb*" -type f 2>/dev/null | head -10

echo -e "\n=== Searching for any USB gadget scripts ==="
rg -l "USB.*gadget|gadget.*USB" --type=sh | head -10

Length of output: 1702

Perfect! Now I can see both scripts exist. Let me examine them to analyze the differences and assess whether consolidation is viable:

Now let me analyze the structural differences and identify consolidation opportunities:

Based on my analysis of both scripts, here's my technical assessment:

Key Differences Between Scripts

Existing script (setup-usbgadget-network.sh):

  • Uses RNDIS function (rndis.usb0) - older, Windows-compatible protocol
  • Single function gadget (network only)
  • Simpler configuration structure

New script (setup-usbgladget-network-multi.sh):

  • Uses NCM function (ncm.usb0) - modern, cross-platform protocol
  • Multi-function gadget: Network + Serial (acm.usb0)
  • More complex configuration with multiple function links

Consolidation Assessment

You're absolutely right about the duplication concern. Here are the consolidation options:

Option 1: Parameterize the Existing Script ✅ Recommended

Option 2: Template-based Approach

Create a shared function library and board-specific wrapper scripts.

Option 3: Configuration File Driven

Use a simple config file to define gadget characteristics.

Recommendation

Extend the existing script rather than creating a new one. The differences are primarily:

  1. Network function type (RNDIS vs NCM)
  2. Addition of serial function
  3. Slightly different USB IDs

These can easily be parameterized with environment variables or function arguments, maintaining backward compatibility while enabling the multi-function capability needed for PocketBeagle 2.

Benefits of consolidation:

  • Reduces maintenance burden
  • Eliminates code duplication
  • Provides consistent behavior across boards
  • Easier to fix bugs in one place
  • Better aligns with DRY principles

Implementation suggestion:
Modify the existing script to accept parameters like USB_GADGET_TYPE=multi and USB_ENABLE_SERIAL=true, then update the PocketBeagle 2 board config to export these variables before calling the consolidated script.

This approach respects the existing Armbian architecture while avoiding unnecessary reinvention of the wheel, as you astutely pointed out.

🏁 Script executed:

#!/bin/bash
echo "=== Examining the existing USB gadget network script ==="
cat packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\n=== Examining the new multi-function script ==="
cat packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

echo -e "\n=== Side-by-side function comparison ==="
echo "EXISTING SCRIPT FUNCTIONS:"
grep -n "^[[:space:]]*[a-zA-Z_][a-zA-Z0-9_]*[[:space:]]*(" packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\nNEW SCRIPT FUNCTIONS:"
grep -n "^[[:space:]]*[a-zA-Z_][a-zA-Z0-9_]*[[:space:]]*(" packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

echo -e "\n=== Key configuration differences ==="
echo "EXISTING - USB Functions:"
grep -A2 -B2 "usb_network_function=" packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\nNEW - USB Functions:"
grep -A3 -B1 "usb_network_function=\|usb_serial_function=" packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

Length of output: 8561

🏁 Script executed:

#!/bin/bash
echo "=== Analyzing gadget creation logic differences ==="
echo "EXISTING - Gadget creation:"
grep -A10 -B2 "mkdir.*CONFIGFS.*g1" packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\nNEW - Gadget creation:"  
grep -A10 -B2 "mkdir.*CONFIGFS.*g1" packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

echo -e "\n=== Function linking differences ==="
echo "EXISTING - Function links:"
grep -A3 -B1 "ln -s.*functions" packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\nNEW - Function links:"
grep -A3 -B1 "ln -s.*functions" packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

echo -e "\n=== Checking for parameterization opportunities ==="
echo "EXISTING - Hardcoded values:"
grep -E "rndis\.usb0|ncm\.usb0|acm\.usb0" packages/bsp/usb-gadget-network/setup-usbgadget-network.sh

echo -e "\nNEW - Hardcoded values:"
grep -E "rndis\.usb0|ncm\.usb0|acm\.usb0" packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh

Length of output: 4684

🏁 Script executed:

# Add parameters to control gadget type
usb_gadget_type="${USB_GADGET_TYPE:-rndis}"  # rndis, ncm, multi
enable_serial="${USB_ENABLE_SERIAL:-false}"

case "$usb_gadget_type" in
  "rndis") usb_network_function="rndis.usb0" ;;
  "ncm") usb_network_function="ncm.usb0" ;;
  "multi") usb_network_function="ncm.usb0"; enable_serial="true" ;;
esac

Length of output: 0

Perfect! Now I have the complete analysis. The scripts are remarkably similar - this is indeed a case of reinventing a reinvented wheel.

Detailed Technical Analysis

The scripts are 99% identical. Here are the only meaningful differences:

1. USB Product ID

  • Existing: 0x0103 (NCM Ethernet Gadget)
  • New: 0x0104 (Multifunction Composite Gadget)

2. Serial Function Addition

  • New script adds: usb_serial_function="acm.usb0"
  • New script links: Serial function to configuration

3. Minor Version Differences

  • Existing: bcdDevice = 0x0100
  • New: bcdDevice = 0x0404

4. Error Handling Style

  • Existing: Uses echo and continues gracefully
  • New: Uses exit_with_error() function (stricter)

Everything else is identical: DHCP logic, configfs setup, NCM network function, IP configuration, etc.

Consolidation Implementation

Here's exactly how to consolidate them:

#!/bin/bash
# Consolidated USB Gadget Network Setup

# Configurable parameters
USB_ENABLE_SERIAL="${USB_ENABLE_SERIAL:-false}"
USB_GADGET_PRODUCT_ID="${USB_GADGET_PRODUCT_ID:-0x0103}"  # Default: NCM only
USB_GADGET_BCD_DEVICE="${USB_GADGET_BCD_DEVICE:-0x0100}"

setup_usb_network_configfs() {
    # ... existing setup code ...
    
    # Dynamic Product ID based on configuration
    if [ "$USB_ENABLE_SERIAL" = "true" ]; then
        usb_idProduct="${deviceinfo_usb_idProduct:-${USB_GADGET_PRODUCT_ID:-0x0104}}"
        usb_serial_function="acm.usb0"
    else
        usb_idProduct="${deviceinfo_usb_idProduct:-${USB_GADGET_PRODUCT_ID:-0x0103}}"
    fi
    
    usb_network_function="ncm.usb0"
    
    # ... standard gadget creation ...
    
    # Link network function
    ln -s $CONFIGFS/g1/functions/"$usb_network_function" $CONFIGFS/g1/configs/c.1 ||
        echo "Couldn't symlink $usb_network_function"
    
    # Conditionally add serial function
    if [ "$USB_ENABLE_SERIAL" = "true" ]; then
        mkdir -p $CONFIGFS/g1/functions/"$usb_serial_function" ||
            echo "Couldn't create $CONFIGFS/g1/functions/$usb_serial_function"
        ln -s $CONFIGFS/g1/functions/"$usb_serial_function" $CONFIGFS/g1/configs/c.1 ||
            echo "Couldn't symlink $usb_serial_function"
    fi
    
    # ... rest of setup ...
}

Board Configuration Update

For PocketBeagle 2:

# In config/boards/pocketbeagle2.conf
post_family_tweaks_bsp__pocketbeagle2_firmware() {
    display_alert "Installing USB gadget networking assets" "PocketBeagle 2" "info"
    
    # Enable multi-function USB gadget
    export USB_ENABLE_SERIAL=true
    export USB_GADGET_PRODUCT_ID=0x0104
    export USB_GADGET_BCD_DEVICE=0x0404
    
    # Use the consolidated script
    run_host_command_logged cp -pv "${SRC}"/packages/bsp/usb-gadget-network/setup-usbgladget-network.sh "${destination}"/usr/bin/
    # ... rest of installation ...
}

Benefits of This Approach

✅ Eliminates duplication - One script, multiple configurations
✅ Maintains backward compatibility - Existing boards unchanged
✅ Reduces maintenance - Bug fixes in one place
✅ Follows DRY principle - Don't repeat yourself
✅ Clear parameterization - Intent obvious from variable names
✅ Future extensible - Easy to add RNDIS support later

Migration Path

  1. Phase 1: Update existing script with parameterization
  2. Phase 2: Update PocketBeagle 2 to use parameters
  3. Phase 3: Remove the duplicate script
  4. Phase 4: Update other boards to use parameters if desired

This approach exactly addresses your concern about needless reinvention while providing a clean, maintainable solution that respects the existing Armbian architecture.

The 15+ lines of actual difference don't justify a completely separate 105-line script when simple parameterization achieves the same goal.

🧠 Learnings used 
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.

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.

return
Comment on lines +98 to +102
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid double-starting DHCP daemon; don’t background with nohup

Starting unudhcpd blindly can spawn multiple daemons across restarts and hides the process from systemd.

Minimal containment:

-	echo "Starting the DHCP daemon"
-	ip a show $INTERFACE > /var/log/unudhcpd.log
-	nohup /usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
+	echo "Starting the DHCP daemon"
+	ip a show "$INTERFACE" > /var/log/unudhcpd.log
+	# Skip if already running
+	if pgrep -x unudhcpd >/dev/null 2>&1; then
+		echo "unudhcpd already running; skip"
+		return
+	fi
+	unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &

Better: move unudhcpd into its own systemd unit (Type=simple), started/managed by systemd instead of “nohup &”. I can draft the unit if desired.

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

Suggested change
echo "Using interface $INTERFACE"
echo "Starting the DHCP daemon"
ip a show $INTERFACE > /var/log/unudhcpd.log
nohup /usr/bin/unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
return
echo "Using interface $INTERFACE"
echo "Starting the DHCP daemon"
ip a show "$INTERFACE" > /var/log/unudhcpd.log
# Skip if already running
if pgrep -x unudhcpd >/dev/null 2>&1; then
echo "unudhcpd already running; skip"
return
fi
unudhcpd -i "$INTERFACE" -s "$host_ip" -c "$client_ip" >> /var/log/unudhcpd.log 2>&1 &
return
🤖 Prompt for AI Agents 
In packages/bsp/usb-gadget-network/setup-usbgadget-network-multi.sh around lines
98-102, the script launches unudhcpd directly with nohup and & which can spawn
multiple daemons across restarts and hides the process from systemd; instead
remove the nohup/ backgrounding and the return, create a dedicated systemd unit
(Type=simple) that runs /usr/bin/unudhcpd with the -i/-s/-c args (or
environment-derived values), have the script enable/start that unit (or check
systemctl is-active before starting) and rely on the journal for logs rather
than redirecting to /var/log, so systemd manages lifecycle and prevents
double-starting.

Copy link

Choose a reason for hiding this comment

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

I agree with @coderabbitai here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jadonk Thank you for confirming the approach! The suggestion to move away from nohup & backgrounding and implement proper systemd service management will indeed provide better process lifecycle management and prevent the multiple daemon issue.

If @Ayush1325 would like me to draft the systemd unit file for the unudhcpd service, I'd be happy to help with that implementation.

}
setup_usb_network_configfs
set_usbgadget_ipaddress