Skip to content

Comments

Add an OpenThread RCP firmware package based on openthread/ot-nrf528xx#39

Open
ksperling-apple wants to merge 3 commits intoproject-chip:mainfrom
ksperling-apple:ot-rcp
Open

Add an OpenThread RCP firmware package based on openthread/ot-nrf528xx#39
ksperling-apple wants to merge 3 commits intoproject-chip:mainfrom
ksperling-apple:ot-rcp

Conversation

@ksperling-apple
Copy link
Contributor

This initial version targets the NRF528408 MDK dongle because the UF2
bootloader makes it very easy to flash with minimal tooling.

Also add an otbr-rcp script for finding / managing USB RCPs and use
it to add hot plugging support to the otbr-agent service.
Miscellaneous improvements:

  • Avoid clobbering /etc/config/otbr-agent
  • Add default configuration for a 'thread' firewall zone
  • Minor patches to otbr-agent to improve RCP device handling
  • Mark host-only packages as build-only

This initial version targets the NRF528408 MDK dongle because the UF2
bootloader makes it very easy to flash with minimal tooling.

Also add an otbr-rcp script for finding / managing USB RCPs and use
it to add hot plugging support to the otbr-agent service.
Miscellaneous improvements:
- Avoid clobbering /etc/config/otbr-agent
- Add default configuration for a 'thread' firewall zone
- Minor patches to otbr-agent to improve RCP device handling
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an OpenThread RCP firmware package for the NRF52840 MDK dongle, which is a significant addition. The changes include a new prebuilt toolchain package, utilities for UF2 flashing, and substantial enhancements to the otbr-agent service for improved RCP device management, such as hot-plugging and automatic firmware updates. The overall implementation is well-structured and robust. I have identified one high-severity bug related to device property parsing in a shell script and a medium-severity issue concerning code clarity in the new UF2 utility. The rest of the changes look good.

}
uint32_t family, base;
if ((argc == 5 || argc == 6) && !strcmp(argv[1], "convert") &&
!parse_uint32(argv[2], &family) &&
Copy link

@ForemanZack-CableLabs ForemanZack-CableLabs Feb 23, 2026

Choose a reason for hiding this comment

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

There should be some kind of extra checks against this parse_uint32. Particularly if this is ran on a 64 bit system vs 32. The strtoul would silently truncate the values in the case of 64 bit machine.

If the program is called with a argument of value of:
4294967296, the function would return value of 0, but family would have value of 0
8589934591, the function would return value of 0, but family would have value of 4294967295
-1, the function would return value of 0, but family would have value of 4294967295
99999999999999999999999, function would return 0, but family would have value of 4294967295

(potential octal interpretation with leading 0)
like: 010, 017, family has value of 8, and 15 respectively;

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants