From 811439704e31f8f983c8788c4e575b1d1ab48a06 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Sun, 9 Aug 2026 07:23:42 +0200 Subject: [PATCH 1/2] openthread-br: replace the deprecated -a tests in otbr-rcp POSIX marks the -a and -o binary operators of test(1) obsolescent, and shellcheck flags them (SC2166): with unconstrained operands the parse is ambiguous, since test cannot tell an operand apart from an operator. Nothing in this script hits that ambiguity today -- the operands are all comparisons or file tests -- so this is a readability and lint fix, not a bug fix. All ten occurrences, so the script is consistent and stays quiet under shellcheck. `[ A -a B ] || x` becomes `[ A ] && [ B ] || x`, which keeps the same short circuit: A false skips B and runs x, A true and B false runs x, both true skips it. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek --- third_party/openthread-br/files/otbr-rcp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/third_party/openthread-br/files/otbr-rcp b/third_party/openthread-br/files/otbr-rcp index a0e50f3..3b29289 100644 --- a/third_party/openthread-br/files/otbr-rcp +++ b/third_party/openthread-br/files/otbr-rcp @@ -56,7 +56,7 @@ main() { done local rcp=any - if [ $# -gt 0 -a "$1" != "--" ]; then + if [ $# -gt 0 ] && [ "$1" != "--" ]; then rcp="$1" validate_rcp_selector "$rcp" || usage shift @@ -64,7 +64,7 @@ main() { local launch= if [ $# -gt 0 ]; then - [ "$1" = "--" -a $# -gt 1 ] || usage + [ "$1" = "--" ] && [ $# -gt 1 ] || usage launch=1 shift fi @@ -81,7 +81,7 @@ main() { local rcpdev="${REPLY% *}" installhid="${REPLY#* }" # Install firmware if necessary - if [ -n "$update" -a "$installhid" != "-" ]; then + if [ -n "$update" ] && [ "$installhid" != "-" ]; then install_rcp "$rcpdev" "$installhid" || return $? rcpdev="${REPLY% *}" update= fi @@ -126,12 +126,12 @@ validate_rcp_selector() { # selector find_rcp() { # selector [include-installable] => $REPLY local rcpdev= installdev= installhid= usb_device_foreach _find_rcp_cb "$@" - if [ $? -eq "$RC_BREAK" -a -n "$rcpdev" ]; then + if [ $? -eq "$RC_BREAK" ] && [ -n "$rcpdev" ]; then log debug "Found RCP interface $rcpdev for selector '$1'" REPLY="$rcpdev -" return 0 fi - if [ -n "$2" -a -n "$installdev" -a -n "$installhid" ]; then + if [ -n "$2" ] && [ -n "$installdev" ] && [ -n "$installhid" ]; then log debug "Found installable $installhid device $installdev for selector '$1'" REPLY="$installdev $installhid" return 0 @@ -146,12 +146,12 @@ _find_rcp_cb() { # selector [include-installable] # Check for a usable cdc_acm interface and OpenThread product string usb_interface_foreach "" _find_rcp_if_cb - [ $? -eq "$RC_BREAK" -a -n "$rcpdev" ] \ + [ $? -eq "$RC_BREAK" ] && [ -n "$rcpdev" ] \ && usb_read_property '' product && REPLY=" $REPLY " && [ "${REPLY/ OpenThread /}" != "$REPLY" ] \ && return "$RC_BREAK" # If requested, check if we could install RCP firmware on this device - if [ -n "$2" -a -n "$RCPFWHANDLERS" -a -z "$installdev" ]; then + if [ -n "$2" ] && [ -n "$RCPFWHANDLERS" ] && [ -z "$installdev" ]; then local hid for hid in $RCPFWHANDLERS; do call_firmware_handler "$hid" installable "$DEVICENAME" && installdev="$DEVICENAME" installhid="$hid" && break @@ -211,7 +211,7 @@ usb_device_foreach() { # callback ... (with $DEVICENAME) local _dev DEVICENAME for _dev in /sys/bus/usb/devices/*; do DEVICENAME="${_dev##*/}" # e.g. "1-1" - [ "${DEVICENAME%:*}" = "$DEVICENAME" -a -f "$_dev/bDeviceClass" ] || continue + [ "${DEVICENAME%:*}" = "$DEVICENAME" ] && [ -f "$_dev/bDeviceClass" ] || continue "$@"; [ $? -ne "$RC_BREAK" ] || return "$RC_BREAK" done } @@ -219,7 +219,7 @@ usb_device_foreach() { # callback ... (with $DEVICENAME) usb_interface_foreach() { # devicename callback ... (with $DEVICENAME) local _dn="${1:-$DEVICENAME}" _if DEVICENAME; shift for _if in "/sys/bus/usb/devices/$_dn/$_dn:"*; do - [ -d "$_if" -a -f "$_if/bInterfaceClass" ] || continue + [ -d "$_if" ] && [ -f "$_if/bInterfaceClass" ] || continue DEVICENAME="${_if##*/}" # e.g. "1-1:1.0" "$@"; [ $? -ne "$RC_BREAK" ] || return "$RC_BREAK" done @@ -296,7 +296,7 @@ load_firmware_handlers() { # => $RCPFWHANDLERS local _script _hname _hid _type RCPFWHANDLERS= for _script in /usr/share/openthread-rcp/*.sh; do - [ -f "$_script" -a -r "$_script" ] || continue + [ -f "$_script" ] && [ -r "$_script" ] || continue _hname="${_script##*/}" _hid="${_hname%.sh}"; _hid="${_hid//-/_}" if [ -n "${_hid//[a-z0-9_]}" ]; then From 158e5e483131c7a4bd4d1616b27f5737c5e48068 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Sun, 9 Aug 2026 07:26:06 +0200 Subject: [PATCH 2/2] openthread-br: let a named RCP still be checked before it is used A dongle is only accepted as an RCP if its USB product string contains "OpenThread". Several perfectly good ones do not say that -- Home Assistant's ZBT-2 among them -- so they cannot be used even when the operator points at the device explicitly with a bus/port selector. Naming a bus position is the operator saying "this is the RCP", so treat it as waiving the product-string requirement. What it must not waive is the check that the device is not sitting in its boot loader: on a UF2 or DFU boot loader the dongle exposes a cdc_acm interface next to the drive it takes firmware on, so a relaxed match would hand otbr-agent a boot loader's serial port. It would then fail to read a firmware version and respawn-loop, and install_rcp's post-flash check -- which calls find_rcp with an explicit selector -- would report a failed flash as success. So the relaxed match is deferred until after the device has been disqualified as a boot loader, by two independent means: - A device that also presents a mass-storage interface is in a boot loader. This does not depend on a firmware handler being installed at all, which matters because the handlers ship in a separate package. - A firmware handler recognizing the device says the same thing. The handler probe therefore runs even when the caller did not ask for installable devices, purely to disqualify; whether installdev/installhid are recorded still depends on what the caller asked for. A handler that recognizes the device but fails for another reason -- its firmware image is missing, say -- also disqualifies it, rather than falling through to the next handler. Accepting a device on the relaxed rule is logged, since it is worth seeing. Checked against a synthetic sysfs covering: a real RCP; a dongle with no OpenThread string and no handler; that same dongle with a handler that does not recognize it; a boot-loader dongle with the handler present, with it absent, and with its firmware image missing; a boot loader exposing mass storage before and after the serial interface; and a boot loader and a real RCP plugged in at once. In every case a boot loader is refused as an RCP whether or not it was named, and offered for flashing only when an update was asked for. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek --- third_party/openthread-br/files/otbr-rcp | 68 ++++++++++++++++++++---- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/third_party/openthread-br/files/otbr-rcp b/third_party/openthread-br/files/otbr-rcp index 3b29289..36ba7a5 100644 --- a/third_party/openthread-br/files/otbr-rcp +++ b/third_party/openthread-br/files/otbr-rcp @@ -22,6 +22,10 @@ # Additional an RCP selector string can be specified. This can be # - "any" (the default) matches any eligible device # - -

, e.g. "1-1.2.3" matches a specific bus/port path +# Naming a bus/port path waives the OpenThread product string requirement, so +# that dongles which do not identify themselves as OpenThread devices can be +# used. Such a device is never flashed: firmware is only installed on, or +# updated for, devices a firmware handler recognizes. # If the --wait option is given, the script will wait for a # suitable device to be hot-plugged if necessary. # @@ -69,8 +73,10 @@ main() { shift fi - # Find and/or wait for the RCP device - [ -z "$update" ] || load_firmware_handlers + # Find and/or wait for the RCP device. The firmware handlers are loaded + # even when no update was asked for: recognizing a device is what tells + # a boot loader apart from an RCP, so selection needs them too. + load_firmware_handlers if [ -n "$wait" ]; then log debug "Looking for RCP devices matching selector '$rcp'" hotplug_wait find_rcp "$rcp" "$update" @@ -140,23 +146,61 @@ find_rcp() { # selector [include-installable] => $REPLY } _find_rcp_cb() { # selector [include-installable] # Must match the selector (if any) + local explicit= case "$1" in - *-*) [ "$1" = "$DEVICENAME" ] || return 0 + *-*) [ "$1" = "$DEVICENAME" ] || return 0; explicit=1 esac # Check for a usable cdc_acm interface and OpenThread product string usb_interface_foreach "" _find_rcp_if_cb - [ $? -eq "$RC_BREAK" ] && [ -n "$rcpdev" ] \ - && usb_read_property '' product && REPLY=" $REPLY " && [ "${REPLY/ OpenThread /}" != "$REPLY" ] \ - && return "$RC_BREAK" + if [ $? -eq "$RC_BREAK" ] && [ -n "$rcpdev" ]; then + # A device that also presents mass storage is sitting in a UF2 or + # DFU boot loader, which exposes a serial port next to the drive it + # takes firmware on. Never an RCP, and this does not depend on a + # firmware handler recognizing it. + usb_interface_foreach "" _find_rcp_msc_cb + if [ $? -eq "$RC_BREAK" ]; then + log debug "USB device ${DEVICENAME} also presents mass storage, treating it as a boot loader" + rcpdev= + fi + fi + if [ -n "$rcpdev" ]; then + usb_read_property '' product && REPLY=" $REPLY " && [ "${REPLY/ OpenThread /}" != "$REPLY" ] \ + && return "$RC_BREAK" + # Without an explicit selector the product string is all there is + # to go on, so this device is not a candidate. + [ -n "$explicit" ] || rcpdev= + fi - # If requested, check if we could install RCP firmware on this device - if [ -n "$2" ] && [ -n "$RCPFWHANDLERS" ] && [ -z "$installdev" ]; then - local hid + # Check whether RCP firmware could be installed on this device. This runs + # even when the caller did not ask for installable devices, because a + # device a handler recognizes is one in its boot loader: it exposes a + # cdc_acm interface too, and wants flashing rather than to be handed to + # otbr-agent as a serial port. + if [ -n "$RCPFWHANDLERS" ] && [ -z "$installdev" ]; then + local hid rc for hid in $RCPFWHANDLERS; do - call_firmware_handler "$hid" installable "$DEVICENAME" && installdev="$DEVICENAME" installhid="$hid" && break + call_firmware_handler "$hid" installable "$DEVICENAME" + rc=$? + # Only "not my device" lets the next handler have a look. Any + # other failure means the handler recognized the device and + # could not act on it, which still rules it out as an RCP. + [ "$rc" != "$RC_NOT_SUPPORTED" ] || continue + [ "$rc" != 0 ] && log warning "Firmware handler '$hid' failed on USB device $DEVICENAME (rc=$rc)" + [ -z "$2" ] || [ "$rc" != 0 ] || { installdev="$DEVICENAME"; installhid="$hid"; } + rcpdev= + break done fi + + # An explicit bus position is the operator saying this device is the RCP, + # so a dongle that does not carry the word OpenThread in its product + # string (Home Assistant's ZBT-2 for one) can still be used -- but only + # once the handlers above have had their say. + if [ -n "$explicit" ] && [ -n "$rcpdev" ]; then + log notice "Using USB device ${rcpdev%:*} as RCP for selector '$1' without an OpenThread product string" + return "$RC_BREAK" + fi } _find_rcp_if_cb() { usb_read_property '' INTERFACE && [ "$REPLY" = 2/2/0 ] || return 0 @@ -164,6 +208,10 @@ _find_rcp_if_cb() { rcpdev="$DEVICENAME" return "$RC_BREAK" } +_find_rcp_msc_cb() { + usb_read_property '' INTERFACE && [ "${REPLY%%/*}" = 8 ] || return 0 + return "$RC_BREAK" +} update_rcp() { # rcpdev rcptty rcpurl local rcpdev="$1" rcptty="$2" rcpurl="$3"