Skip to content

openthread-br: drop luci-app-openthread, proposed for luci repo - #30096

Open
LorbusChris wants to merge 4 commits into
openwrt:masterfrom
LorbusChris:openthread-br-drop-luci
Open

openthread-br: drop luci-app-openthread, proposed for luci repo#30096
LorbusChris wants to merge 4 commits into
openwrt:masterfrom
LorbusChris:openthread-br-drop-luci

Conversation

@LorbusChris

@LorbusChris LorbusChris commented Jul 25, 2026

Copy link
Copy Markdown

📦 Package Details

Maintainer: @stintel

Should land after #30091

Description

Removes the luci-app-openthread subpackage from net/openthread-br. The LuCI
web interface is proposed for the openwrt/luci repository as
luci-app-openthread (openwrt/luci#8871), following the standard convention for
LuCI apps, as requested in #29791 and openthread/ot-br-posix#3431. Dropping it
here means the UI is no longer shipped from two places.

Split out of #30091 at @stintel's request so the removal can be reviewed on its
own. The branch sits on top of #30091: the first three commits here are #30091's
(the 2026.08.0 update, the REST listen-address option, the internal mDNS
switch), and only the last commit is what this PR proposes. It is meant to land
with or after #30091 — merging this merges that too.

PKG_RELEASE is bumped to 2 on top of #30091's version reset, since the
package set changes without a version change.

Note the app being removed is the old Lua/luci-lua-runtime implementation; the
openwrt/luci proposal is a rewrite using the modern client-side JS + rpcd ucode
conventions, so this is not a straight move of the same files.

openwrt/luci#8871 is still open, so this should ideally land together with (or
after) it, otherwise there is a window with no OpenThread web UI available at
all. The in-tree copy in ot-br-posix is proposed for removal separately in
openthread/ot-br-posix#3326 (draft).

🧪 Run Testing Details

The removal commit itself changes no code in the openthread-br package — the
remaining Package/openthread-br recipe is untouched. The src/openwrt/ tree
in ot-br-posix stays in use: src/openwrt/ubus/ builds the otbr ubus object
that the netifd proto handler waits on (ubus -t30 wait_for otbr, then
ubus call otbr threadstart), so -DOTBR_OPENWRT=ON is untouched here and must
stay.

The luci-app-openthread arms are dropped from test.sh and test-version.sh
(both added in #30091 underneath): those scripts refuse to pass a package they
don't know, so a case for one that is no longer built would defeat that.

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

@LorbusChris LorbusChris changed the title openthread-br: drop luci-app-openthread, moved to luci repo openthread-br: drop luci-app-openthread, proposed for luci repo Jul 25, 2026
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 2 times, most recently from 21142ed to dad9226 Compare July 25, 2026 23:00

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 2 new commits; both commit messages match their diffs. Three inline notes, none blocking.


Generated by Claude Code

Comment thread net/openthread-br/Makefile
Comment thread net/openthread-br/Makefile
Comment thread net/openthread-br/test.sh
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch from dad9226 to 300b53e Compare July 26, 2026 15:06

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 2 new commits; no new issues found.

The only new content since dad9226 is the README.md "### LuCI" hunk, which resolves the earlier nit — test.sh and the Makefile are byte-identical to what was already reviewed. Both commit messages still match their diffs, and the three FormalityCheck runs on 300b53e are green.


Generated by Claude Code

@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 2 times, most recently from 75b6565 to fabe4a6 Compare July 27, 2026 12:08

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit (300b53efabe4a6, the test.sh commit amended to also add test-version.sh). The amended commit message matches its diff.

The only new content since the last review is net/openthread-br/test-version.sh; test.sh, the Makefile and README.md are unchanged from what was already reviewed. One inline note on the new file. All three FormalityCheck runs on fabe4a6 are green.


Generated by Claude Code

Comment thread net/openthread-br/test-version.sh Outdated
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch from fabe4a6 to 03936f7 Compare July 29, 2026 20:26

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit (fabe4a603936f7, the test commit amended again); no new issues found.

The only change since the last review is net/openthread-br/test-version.sh switching case "$1" / '$1' to $PKG_NAME, which resolves the earlier note and now matches the documented contract in CONTRIBUTING.md:264-266 ("The script receives PKG_NAME and PKG_VERSION as environment variables") and the worked example below it. test.sh correctly keeps "$1", since that script is documented as receiving positional arguments. test.sh, the Makefile and README.md are byte-identical to what was already reviewed, and the amended commit message still matches its diff.

All three FormalityCheck runs on 03936f7 are green.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Removes the luci-app-openthread subpackage from net/openthread-br so the LuCI UI is no longer shipped from this feed and can instead live exclusively in the openwrt/luci repository, avoiding duplication and aligning with LuCI app conventions.

Changes:

  • Drop the luci-app-openthread package definition and install recipe from net/openthread-br/Makefile and bump PKG_RELEASE.
  • Add test.sh and test-version.sh to replace the generic version probe with functional smoke tests suitable for this package.
  • Update README.md to reflect that the LuCI UI is no longer shipped from this package.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
net/openthread-br/Makefile Removes the LuCI subpackage definition and bumps PKG_RELEASE due to content change.
net/openthread-br/README.md Updates documentation to point users to the LuCI-provided UI package.
net/openthread-br/test.sh Adds a functional smoke test exercising otbr-agent and ot-ctl.
net/openthread-br/test-version.sh Overrides generic version detection which cannot work for this package.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread net/openthread-br/README.md Outdated
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 2 times, most recently from 671e677 to b7e4ecc Compare August 2, 2026 17:12

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the force-push since the last review (03936f7b7e4ecc). The branch was rewritten rather than extended: the two previously reviewed commits are gone, and the head now carries #30091's three commits plus a README-only commit.

Commit checks

  • b7e4ecc "openthread-br: drop luci-app-openthread, proposed for luci repo" — the message says "Remove the luci-app-openthread subpackage (and its install recipe)", but the commit only touches README.md. The Makefile hunk that the earlier version of this commit (d9419b7, 27 lines removed) carried is no longer present, so the PR does not currently do what its title, body and commit message say. Details inline.

The other three commits (55c50a7, 39d8999, c4254e9) are the ones from #30091 and their messages do match their diffs. c4254e9 is #30091's current head, so the two PRs now hold the same three commits — worth deciding whether #30096 should still be the independent LuCI-only split described in its body, or whether the two have merged back together.

Non-blocking notes: v2026.07.0 exists upstream (ec16e39), the PKG_VERSION / PKG_RELEASE:=1 pairing is correct for a version bump, and all three FormalityCheck runs on b7e4ecc are green.


Generated by Claude Code

Comment thread net/openthread-br/Makefile
Comment thread net/openthread-br/README.md
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 2 times, most recently from 89804cb to 268dca4 Compare August 3, 2026 12:55

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the force-push since the last review (b7e4ecc268dca4). The previous blocker is resolved: the head commit now carries the Makefile hunks again — define Package/luci-app-openthread, its install recipe and the BuildPackage call are all gone — and the now-dead luci-app-openthread arms are dropped from test.sh and test-version.sh, so the README and the build agree again. Nothing else in the tree references the subpackage. The other three commits (55c50a7, 39d8999, c4254e9) are unchanged from what was already reviewed.

Commit checks

  • 268dca4 "openthread-br: drop luci-app-openthread, proposed for luci repo" — the message says "Split out of #30091 so it can be reviewed and land independently of the 2026.07.0 update", but the commit's parent is c4254e9, which is #30091's current head. The branch still carries all three of #30091's commits below this one, so as it stands this cannot land independently of the 2026.07.0 update — merging #30096 merges #30091 with it. Same mismatch in the PR body ("#30091 now contains the version update … and will be rebased on this"), which describes the opposite stacking from the one that exists. This was raised on the previous force-push and the Makefile half of it was fixed; the topology half is unchanged, so either the branch wants rebasing onto master or the body and commit message want updating to say #30096 sits on top of #30091.

    Secondary, non-blocking: the message describes only the Makefile removal, while the diff also rewrites README's ### LuCI section and edits both test scripts. Both are in scope for the change, just undocumented.

Non-blocking note inline on PKG_RELEASE. All three FormalityCheck runs on 268dca4 are green. For the record, luci-app-openthread is not yet present in openwrt/luci master (8b5f528), which is what the README's "a LuCI feed recent enough to carry it" caveat already covers.


Generated by Claude Code

Comment thread net/openthread-br/Makefile Outdated
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 4 times, most recently from db04eb3 to db20da6 Compare August 5, 2026 17:39
@openwrt

openwrt Bot commented Aug 5, 2026

Copy link
Copy Markdown

Formality Check: Suggestions Available

We checked this pull request against the contribution guidelines. Here is what needs your attention:

⚠️ STYLISTIC WARNINGS & SUGGESTIONS

Package Release Audit:

  • ⚠️ Package net/openthread-br version updated from '2f3c799c7463c8f674754e65c53f78bc0bbcbd58' to '2026.08.0', but PKG_RELEASE was not reset to 1 (currently: '2')

Something broken? Consider reporting an issue.
Running version 0e15d1c deployed on 2026-08-12 22:36:32 CEST

@LorbusChris
LorbusChris requested a review from openwrt-ai August 5, 2026 17:40
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch from db20da6 to d158d1e Compare August 5, 2026 19:28
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch from d158d1e to 27ab7f9 Compare August 5, 2026 19:47

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the force-push since the last review (268dca427ab7f9). The branch was rewritten rather than extended: #30091's three commits were replaced by their 2026.08.0 versions (df88201, 6875a57, 6e05e4c), and the drop commit was reapplied as 27ab7f9. Those three are byte-identical to #30091's current head, so the PR body's description of the stacking now matches the branch — the earlier topology mismatch is resolved.

All four commit messages match their diffs, so no ## Commit checks section this time.

Grounding for the new content, checked against v2026.08.0 (3377117) and the openthread submodule it pins (c7a3a19):

  • The tag exists and is the newest on openthread/ot-br-posix (git ls-remote --tags).
  • -DOTBR_VERSION=$(PKG_VERSION) works as the commit message describes: CMakeLists.txt:72-95 only synthesises a version when OTBR_VERSION is unset, and it feeds OTBR_PACKAGE_VERSION, which is what realmain.cpp:192-194 prints for --version (before the vendor/product check, so it exits 0).
  • Dropping test-version.sh is safe. The generic check only hard-fails when every executable misses the version — test_entrypoint.sh:245-256 fails on exec_checked -eq version_missing and only warns otherwise. otbr-agent now reports 2026.08.0, so ot-ctl not reporting it downgrades to a warning. The case "$PKG_NAME" in test.sh is fine too: the harness exports PKG_NAME/PKG_VERSION and passes them positionally (L327-L360).
  • ot-ctl -h is still the right probe: client.cpp:211-215 has no --version entry, and the terminating {nullptr, 0, nullptr, 0} is present, so the old segfault is indeed gone.
  • The CMAKE_OPTIONS rework checks out: options.cmake:104-121 is the Only one Advertising Proxy can be enabled. FATAL_ERROR, OTBR_DUA_ROUTING no longer exists, and OTBR_MDNS=openthread is a valid value that forces OT_MDNS/OT_SRP_ADV_PROXY on. OTBR_BORDER_AGENT_MESHCOP_SERVICE defaults OFF in that mode, so the OTBR DNSSD support must not be enabled with OT mDNS. guard is not tripped, and the MeshCoP instance name survives: OTBR hands its OTBR_VENDOR_NAME " " OTBR_PRODUCT_NAME base name to OT core via application.cpp:311, rather than falling back to OT's "OpenThread BR (unspecified vendor)" default.
  • Both dropped patches are genuinely upstream — third_party/CMakeLists.txt now carries the pkg_check_modules(CJSON libcjson) logic verbatim — and the remaining 100-fix-gcc14-build.patch still matches its context in the mbedtls revision this release pins.
  • Nothing in the tree references luci-app-openthread any more except the README paragraph that points at openwrt/luci.

Two inline notes, neither blocking. The first retracts an earlier openwrt-ai comment of mine on PKG_RELEASE that no longer holds now that the branch stayed stacked on #30091.


Generated by Claude Code

Comment thread net/openthread-br/Makefile
Comment thread net/openthread-br/README.md Outdated
Switch from a pinned commit to the 2026.08.0 release tag (the latest
release). Drop the two local patches whose fixes are upstream since
2026.07.0:
- 101-rest-Permit-to-externally-provide-cJSON-library.patch
- 102-openwrt-fix-LuCI-icons.patch

Adjust CMAKE_OPTIONS for the proxy rework that arrived with 2026.07.0.
That release added OT core Advertising and Discovery Proxies that default
on whenever OTBR_MDNS is set and are mutually exclusive with the legacy
POSIX proxies, so forcing OTBR_SRP_ADVERTISING_PROXY=ON and
OTBR_DNSSD_DISCOVERY_PROXY=ON now fails configure with "Only one
Advertising Proxy can be enabled." Drop both flags to adopt the OT core
proxies, matching upstream's default build. Also drop OTBR_DUA_ROUTING,
which is no longer a recognized option.

Set OTBR_VENDOR_NAME and OTBR_PRODUCT_NAME. 2026.07.0 removed the
built-in defaults that border_agent.hpp used to supply, so otbr-agent now
exits with "Vendor name must be set." unless they are given at compile time
or on the command line.

They are set to OpenThread and BorderRouter, the values those defaults had,
rather than to anything OpenWrt specific. The pair also forms the MeshCoP
service instance name, as "<vendor> <product>", which is the name shown when
adding the border router in a Thread client and the one already-paired
clients have recorded. Keeping the previous values means upgrading does not
rename anyone's border router. It also leaves OTBR_NAME at
OpenThread_BorderRouter, which is what it defaulted to.

Pass OTBR_VERSION=$(PKG_VERSION). Left alone, otbr-agent --version
reports the upstream CMake project version (0.3.0), which is unrelated
to the version this package is built from, so the generic version check
fails -- as it already does on master, independently of this update.
OTBR_VERSION overrides that string at configure time, so the binary
reports the version it was packaged as and the check passes with no
override script. luci-app-openthread installs no executables, so the
generic checks have nothing to probe there and need no override either.

Add a test.sh as the functional smoke test: it exercises both binaries,
which also covers their runtime library closure. ot-ctl is probed with -h
because it has no version option at this release
(openthread/openthread#13424 adds one, but the bundled openthread
predates it). The segfault on unrecognized long options that earlier made
-h the only safe probe is fixed in the openthread this release bundles
(openthread/openthread#13423).

The mDNS provider is left unchanged here; a follow-up commit switches it
to OpenThread's internal implementation.

Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
Assisted-By: Claude Fable 5
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch 2 times, most recently from bc8708d to 31e07e1 Compare August 8, 2026 15:22

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the force-push since the last review (27ab7f931e07e1); no new issues found.

The branch was rewritten but the only new content is in net/openthread-br/README.md (+17/-4). Per-commit, the patches are unchanged apart from these two hunks and rebase line offsets:

  • 07cd2dc "update to 2026.08.0" — gains the new ### Version string section.
  • 2586fcc "make the REST API listen address configurable" — rewrites the required/optional paragraph under "Create network".
  • 2c803bc "use OpenThread's internal mDNS" and 31e07e1 "drop luci-app-openthread" — byte-identical to 6e05e4c / 27ab7f9.

All four commit messages still match their diffs, so no ## Commit checks section. 2c803bc is also #30091's current head, so the two PRs still hold the same three commits below this one, matching the stacking the body describes.

Grounding for the two new paragraphs:

  • "Only backbone_network, device and radio_url are required" is accurate and fixes the previous wording: the proto handler hard-fails on exactly those three plus the resolved backbone ifname (openthread-proto.sh:55-60), and dataset is applied only when set (L81-L83). "or if backbone_network names an interface that has no device" is the MISSING_BACKBONE_IFNAME path. This resolves my earlier nit on that line.
  • "the repacked source tree has no git directory for git describe to read" holds for this package: PKG_SOURCE_URL is a github.com URL, so dl_method_git selects github_archive, and both that path and its rawgit fallback end with rm -rf .git .gitmodules before packing (download.mk:232-234).
  • The [REST Server](#rest-server) link still resolves to the ### REST Server heading, and nothing outside the README references luci-app-openthread any more.

All three FormalityCheck runs on 31e07e1 are green.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Correction to the download.mk citation in my review just above: the commit SHA in that permalink is wrong. The correct link is include/download.mk:233-234 (openwrt/openwrt at 83e93e0) — the rm -rf .git .gitmodules in DownloadMethod/rawgit. The point stands unchanged; only the link was broken.


Generated by Claude Code

As of the 2026.07.0 update, otbr-agent's REST API defaults to listening
on 127.0.0.1, so remote clients (e.g. Home Assistant on another host) can
no longer reach it. Add rest_listen_address and rest_listen_port proto
options and pass them through to otbr-agent when set; leaving them unset
keeps the secure loopback-only default.

Also drop the leftover unused 'foobar' proto config option.

Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
Assisted-By: Claude Fable 5
Switch OTBR_MDNS from mDNSResponder to openthread, drop the mdnsd and
mDNSResponder dependencies, and remove the proto handler's mdnsd service
check, which would otherwise refuse to start otbr-agent now that mdnsd is
no longer installed.

This follows upstream, which made the internal implementation the default
in openthread/ot-br-posix#2996 and uses it in its own OpenWrt example
package, and it lets otbr-agent share a router with umdns, which is what
advertises the router's procd-registered services (ssh, samba and the
like) and resolves <hostname>.local.

The two can coexist because they never contend for a name: OpenThread's
mDNS names its host after the Thread extended address, while umdns keeps
<hostname>.local. Both bind UDP 5353 with SO_REUSEADDR and each answers
only for its own records. mDNSResponder instead takes its name from
gethostname(), so it and umdns race for the same name, and the loser
either stops announcing without logging anywhere procd captures or, if
neither notices, both keep claiming it. That is the conflict with a
running mdnsd noted in openwrt#29784.

Border router discovery is unaffected: in this mode OpenThread's own
border agent registers the _meshcop._udp service (its
OPENTHREAD_CONFIG_BORDER_AGENT_MESHCOP_SERVICE_ENABLE follows
OPENTHREAD_CONFIG_MULTICAST_DNS_ENABLE), and OTBR's publisher is disabled
by design, so clients such as Home Assistant still find the border router.

Note that OpenThread's mDNS serves a single interface, the one given by
the backbone_network option, so Thread service records are announced on
that network only.

Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
Assisted-By: Claude Opus 4.8
The LuCI web interface for OpenThread Border Router is proposed for the
openwrt/luci repository as luci-app-openthread (openwrt/luci#8871),
following the standard convention for LuCI apps, as requested in
openwrt#29791 and
openthread/ot-br-posix#3431.

Remove the luci-app-openthread subpackage and its install recipe so the
UI is no longer shipped from two places. The README's LuCI section now
points at the openwrt/luci app instead of documenting the dropped one,
and the luci-app-openthread arm is removed from test.sh -- that script
refuses to pass a package it doesn't know, so keeping a case for one
that is no longer built would defeat that.

PKG_RELEASE is bumped: the package set changes without a version change.

Split out of openwrt#30091 at the maintainer's request. This sits on top of
openwrt#30091's update to 2026.08.0 and is meant to land with or after it.

Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
Assisted-By: Claude Fable 5
@LorbusChris
LorbusChris force-pushed the openthread-br-drop-luci branch from 31e07e1 to f8a2e50 Compare August 13, 2026 17:54

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the force-push since the last review (31e07e1f8a2e50); no new issues found.

The branch was rewritten but the tree delta is four lines in net/openthread-br/README.md and nothing else (git diff 31e07e1..f8a2e50 --stat). Per-commit:

  • 07cd2dc "update to 2026.08.0" — unchanged, same SHA as last review.
  • caacf17 (was 2586fcc) "make the REST API listen address configurable" — carries the whole delta: the two -- in the required/optional paragraph become em dashes and the "check ifstatus for the OpenThread interface" line is rewrapped. This matches the em dashes the README already uses at README.md:90 and :178, so it makes the file internally consistent. No wording changed, so the accuracy of that paragraph (verified against the proto handler last time) is unaffected.
  • 233cfaa (was 2c803bc) "use OpenThread's internal mDNS" and f8a2e50 (was 31e07e1) "drop luci-app-openthread" — content-identical, only rebase blob hashes differ.

All four commit messages are byte-identical to the ones already reviewed and still match their diffs, so no ## Commit checks section.

233cfaa is also #30091's current head, so the stacking still matches what this PR's body describes: the three commits below the drop commit are #30091's, and merging this merges that. All three FormalityCheck runs on f8a2e50 are green.


Generated by Claude Code

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.

3 participants