luci-proto-openthread: add protocol support for the border router - #8942
luci-proto-openthread: add protocol support for the border router#8942LorbusChris wants to merge 1 commit into
Conversation
d0e820b to
4fd96e0
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; commit message matches the diff. Three inline comments — the option set, tab layout, hexstring datatype and optional/rmempty flags all line up with what proto_openthread_init_config()/proto_openthread_setup() in openthread-br consume, and the _net_device selector in interfaces.js covers the device option the handler requires, so the missing pieces are the unregistered handler error codes and the unmasked dataset field.
Generated by Claude Code
4fd96e0 to
1a90b72
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the range 4fd96e0..1a90b72. The two code findings from the previous round are addressed correctly: the six registerErrorCode() calls match the codes proto_openthread_setup() actually raises one-for-one and land at module scope like map.js/qmi.js do, and o.password = true on the dataset field is the right knob (it reaches ui.Textfield via form.Value.renderWidget()). The Makefile/LUCI_DEPENDS point is resolved by the updated PR description. Commit message still matches the diff.
One inline comment on the remaining gap: openthread is a no_device protocol, which makes LuCI hide the device picker and — on save — actively remove the option device the handler requires.
Generated by Claude Code
1a90b72 to
341e507
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the range 1a90b72..341e507. The only change is the new device option in renderFormOptions(), which addresses the previous round's finding — but not completely: without an explicit o.ucioption, the sibling-alias guard in AbstractValue.remove() skips the option, so _net_device still unsets option device on save. One inline comment with the fix; the correction is on my previous suggestion, which omitted it.
Commit message still matches the diff. Formality CI is green on 341e507.
Generated by Claude Code
A netifd protocol handler for the openthread protocol that openthread-br registers, so an interface running a Thread border router can be configured from the network pages rather than only from uci. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
341e507 to
0bc0731
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; no new issues found.
Re-reviewed the range 341e507..0bc0731. The only change is the device form option being renamed to _thread_device with an explicit o.ucioption = 'device', which closes the previous round's finding: remove() now skips the continue at form.js:2228-2229 for this sibling, matches on this_opt/sibling_opt both being device, and returns early instead of reaching unset() — so the inactive _net_device no longer deletes option device on save. Both options live on the same section s, and load()/write() resolve through this.ucioption ?? this.option at form.js:1878-1881, so the field still round-trips the same uci key. getDevice() is unaffected by the rename, as expected.
Commit message still matches the diff (the series was squashed to a single commit; subject scope luci-proto-openthread: matches the two added files). Formality CI is green on 0bc0731.
Generated by Claude Code
Adds a netifd protocol handler for the
openthreadprotocol, so an interface running a Threadborder router can be configured from the network pages instead of only from uci.
The protocol handler itself ships with openthread-br, which already installs /lib/netifd/proto/openthread.sh at packages master.
Options, matching what the proto handler consumes:
otbr-agentcreates for the Thread interface; declared here because the stock device picker is hidden for virtual protocolsotbr-agentreaches the 802.15.4 radioOne thing worth calling out, since it is the only non-obvious part: the Thread radio is a tun-like
netdev with no
DEVTYPEin sysfs, so the genericDeviceclass falls back to the ethernet icon.The protocol therefore owns its device instances and reports
wifi/ "Thread Radio", the samepattern
luci-proto-relayuses. Without it the interface list shows a wired icon for a radio.Testing
Running on a Turris Omnia with an 802.15.4 USB dongle: the interface appears with the Thread type
and icon, the backbone network and radio URL round-trip through uci, and the advanced options are
written only when set.
Related
Independent of #8871 (
luci-app-openthread) — different package, no shared files — and of theopenthread-br changes in the packages feed. It can be reviewed and merged on its own.