otbr-rcp: allow a named RCP without an OpenThread product string - #57
Merged
ksperling-apple merged 3 commits intoAug 18, 2026
Merged
Conversation
This was referenced Aug 13, 2026
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 <c.glombek@cosa.systems>
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 <c.glombek@cosa.systems>
LorbusChris
force-pushed
the
otbr-rcp-selector
branch
from
August 17, 2026 19:00
cf02c9b to
158e5e4
Compare
ksperling-apple
approved these changes
Aug 18, 2026
ksperling-apple
added
the
sdk-maintainer-approved
|
Tick the box to add this pull request to the merge queue (same as
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #53 as suggested. Two commits, since the two changes were reviewed separately there; happy to squash.
All ten
[ x -a y ]tests become[ x ] && [ y ](first commit). POSIX marks-a/-oobsolescent and shellcheck flags them (SC2166); none of the ten was actually ambiguous, so this is lint and readability, not a bug fix.A dongle named by bus position no longer needs to say "OpenThread" (second commit). Naming a bus position is the operator saying "this is the RCP", so it waives the product-string requirement — several good dongles (Home Assistant's ZBT-2 for one) don't carry the string.
What it must not waive is the boot-loader check: a dongle in its UF2/DFU boot loader also exposes a cdc_acm interface, and a naively relaxed match would hand otbr-agent the boot loader's serial port and report a failed flash as success. The relaxed match is therefore deferred until the device is disqualified as a boot loader, by two independent means: a mass-storage interface on the same device (works even when the firmware-handler package is not installed), and a firmware handler recognizing it (the probe now runs even when the caller did not ask for installable devices, purely to disqualify). Accepting a device on the relaxed rule is logged at notice level.