Skip to content

luci-app-wwand + luci-proto-wwand: add LuCI support for the wwand connection manager - #8917

Open
ddimension wants to merge 2 commits into
openwrt:masterfrom
ddimension:wwand-luci
Open

luci-app-wwand + luci-proto-wwand: add LuCI support for the wwand connection manager#8917
ddimension wants to merge 2 commits into
openwrt:masterfrom
ddimension:wwand-luci

Conversation

@ddimension

@ddimension ddimension commented Aug 7, 2026

Copy link
Copy Markdown

LuCI companion packages for the wwand cellular connection manager (submitted to openwrt/packages as net/wwand, upstream repo https://github.com/ddimension/wwand).

  • luci-app-wwand — Status → Modem pages: live signal + cell environment (serving/neighbour cells, LTE & NR5G, peak-hold for antenna alignment), connection details, SIM/eSIM management (slots, PIN/PUK unlock, SGP.22 profiles), SMS, operator scan/selection and a modem settings editor (modes/bands, guarded modem-reset). Includes a "Migratable interfaces" list that converts an existing proto qmi/mbim/ncm interface in place to proto wwand (user-triggered). Ships the shared wwand.bands band/frequency resource.
  • luci-proto-wwand — protocol handler for proto 'wwand' (Network → Interfaces): modem selection, APN / PDP type / auth, per-interface options.

Good-citizen coexistence. luci-proto-wwand ships only protocol/wwand.js — it does not install a protocol/qmi.js alias and does not conflict with luci-proto-qmi. proto qmi/mbim/ncm interfaces stay with the stock LuCI proto handlers; wwand manages only proto wwand, and moving an interface across is the explicit "Migrate" action above. Pairs with openwrt/packages#30185 (wwand 1.2.0).

🤖 Generated with Claude Code

@openwrt openwrt Bot added the add package Introduces a new package Makefile build script label Aug 7, 2026

@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 messages match their diffs). 12 inline comments, the main ones being an rpc expect unwrapping bug in wwand.modemopts, two ACL gaps (modem_scan_start/modem_scan_status, file list /dev/), the undeclared protocol/qmi.js file collision with luci-proto-qmi, and a poller that outlives the interface-edit modal.


Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/modemopts.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/modemopts.js Outdated
Comment thread applications/luci-app-wwand/root/usr/share/rpcd/acl.d/luci-app-wwand.json Outdated
Comment thread protocols/luci-proto-wwand/Makefile
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/bands.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread protocols/luci-proto-wwand/htdocs/luci-static/resources/protocol/wwand.js Outdated
@openwrt openwrt Bot added the not following guidelines Pull request does not follow formatting guidelines label Aug 8, 2026
@ddimension

Copy link
Copy Markdown
Author

Updated to the current sources, addressing the inline review:

  • Dropped the legacy qmi proto alias (protocol/qmi.js) — it collided with luci-proto-qmi (same path) and presented proto qmi as wwand-managed even when it isn't. proto qmi now stays with luci-proto-qmi/uqmi; interfaces are moved to proto wwand from a new Migratable interfaces list in the modem page (in line with wwand becoming opt-in / coexisting with the stock stack — see wwand: add cellular connection manager packages#30185).
  • SIM-slot dropdown: unwrap the statusRaw reply's .modems (was Object.keys on the whole reply) and read sl.physical (was sl.slot).
  • Live-field poller is removed when its modal closes (was a permanent 5 s poll issuing status/signal/cells forever).
  • status.js cell headers use literal _() for the i18n scanner; settings.js uses dom.content(); ACL grants for the async scan; callGpioList/callRepower dead decls dropped.

Still on the list (follow-ups, not blockers): a modem selector on the settings page instead of hard-binding names[0].

@ddimension
ddimension force-pushed the wwand-luci branch 2 times, most recently from e9a073a to f52b465 Compare August 8, 2026 21:23
@openwrt openwrt Bot removed the not following guidelines Pull request does not follow formatting guidelines label Aug 8, 2026

@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 (343ab75, f52b465). The earlier findings are addressed: the protocol/qmi.js path collision is gone, the statusRaw/.modems unwrap and sl.physical are fixed, the async-scan ACL grants are in, status.js uses literal _() headers, and the interface-edit poller now unregisters itself. 4 new inline comments — the notable one is a modem_reset ACL gap plus a section_id/modem-name mixup, both fallout from the modem_repowermodem_reset rename in the shared wwand.modemopts module.

Commit checks

  • 343ab75 "luci-app-wwand: sync modem UI to current sources" — the two bullets cover the Modems-page additions and the review fixes, but not the largest parts of the diff. Undocumented: the Status → Modem detail page is deleted outright (view/wwand/detail.js, −306 lines, plus its admin/status/wwand-detail menu entry); view/wwand/settings.js is split into five new shared resources (wwand.rpc / format / esim / netsel / modemsid, −729 lines there); the _repower action and its wwand.modem_repower ubus call become _reset / wwand.modem_reset; the Modem Tools menu node loses its title; and a new fcc_auth modem option is added. Removing a user-visible page and swapping a ubus method are the kind of thing worth naming in the message — the ACL fallout of that swap is the subject of one of the inline comments.

Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/modemopts.js Outdated
Comment thread protocols/luci-proto-wwand/root/usr/share/rpcd/acl.d/luci-proto-wwand.json Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/netsel.js Outdated
ddimension pushed a commit to ddimension/wwand that referenced this pull request Aug 9, 2026
Package-G artefacts for the openwrt/packages#30185 + openwrt/luci#8917 push:
- upstream/dossier-packages-30185.md — maturity/coexistence dossier to post as a
  PR comment (ModemManager precedent, independent-verification story, HW/test
  matrix, audit changelog, device-ownership answer, re-review request).
- upstream/rfc-openwrt-devel.md — the architecture RFC (improve-existing-stack
  vs. a separate opt-in proto) for openwrt-devel.
- upstream/checklist.md — the push/tag/PR/feed action sequence.

Drafts for author review; sending/posting is manual.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@openwrt openwrt Bot added the not following guidelines Pull request does not follow formatting guidelines label Aug 14, 2026
@ddimension

Copy link
Copy Markdown
Author

Synced to the wwand 1.2.0 sources (paired with openwrt/packages#30185).

  • SIM unlock dialog on the modem list — PUK entry (modem_sim_puk, with remaining-attempts + a wrong-PUK safeguard) and the manual PIN release for PIN-class blocks; ACL grants for both.
  • "Configure modem" persists immediately (fixes a lost path binding); Save-SIM + Reattach actions; SMS-send ACL.
  • Status/format/netsel updates for the unified telemetry (mode / RAT / caps), an MCC/MNC operator-name resource, per-operator scan RAT.
  • Proto: hard_reconnect_on_ip_change flag.

All JS node --check-clean.

@openwrt openwrt Bot removed the not following guidelines Pull request does not follow formatting guidelines label Aug 14, 2026

@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 (5da1788). 5 inline comments — the blocking one is a var plmn redeclaration in status.js that both kills the new NAS panel and throws on an unregistered modem; the rest are an unreachable plmnEditor() in settings.js and three items in the reworked ACL file.

Checked the new UCI surface (wwand_plmnlist / plmn_list / hard_reconnect_on_ip_change) against the backend-coupling rule: wwand is not in the packages feed yet, but the PR body and the follow-up comment already name openwrt/packages#30185 as the pairing, so not flagged. CI on the head SHA is green.

Commit checks

  • 5da1788 "luci-app-wwand, luci-proto-wwand: sync to wwand 1.2.0 sources" — the four bullets describe real changes, but they leave out the largest one. Undocumented: the whole preferred-PLMN manager (renderPlmnManager, ~200 new lines in settings.js), the new modem_plmn_set / modem_plmn_restore ubus methods, a new UCI section type wwand_plmnlist (type + plmn list) and the plmn_list options it adds to both wwand.modemopts and wwand.simlist. Also unmentioned: the new Datapath & muxing status panel (modem_datapath), the Preferred networks (NAS) panel, the cachedCall/slowCache rework that moves cells/slots/datapath/PLMN off the 1 s poll onto per-call TTLs, the fmt.term() tooltip pass across status.js, adding modemmanager to the migration list, and the tab→space reindent of the ACL file. Introducing a new config section type and a new poll-caching model are the kind of thing worth naming in the message.

Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/status.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread applications/luci-app-wwand/root/usr/share/rpcd/acl.d/luci-app-wwand.json Outdated
Comment thread applications/luci-app-wwand/root/usr/share/rpcd/acl.d/luci-app-wwand.json Outdated
Comment thread applications/luci-app-wwand/root/usr/share/rpcd/acl.d/luci-app-wwand.json Outdated

@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 (12ef4b1, d7bb9ee). These replace the single 5da1788 I reviewed last round — git diff 5da1788..d7bb9ee is empty, so this is a pure re-split of that commit into an app half and a proto half (plus the dropped Co-Authored-By trailer). The split itself is clean: 12ef4b1 touches only applications/luci-app-wwand/, d7bb9ee only protocols/luci-proto-wwand/.

Because the tree is unchanged, the five findings from the previous round are all still open, including the blocking one: the var plmn redeclaration in status.js, which leaves the new "Preferred networks (NAS)" panel permanently unrendered and throws on an unregistered modem. The modem_repower leftover grant and its mirror in luci-proto-wwand.json (missing modem_reset) are likewise untouched. One new nit inline, on the only hunk in this round's diff that I had not already commented on. CI on the head SHA is green (all three FormalityCheck runs pass).

Commit checks

  • 12ef4b1 "luci-app-wwand: sync to wwand 1.2.0 sources" — the message is 5da1788's verbatim minus the proto bullet, so the omissions I flagged there carry over unchanged. Still undocumented in a 913-line diff: the whole preferred-PLMN manager (renderPlmnManager, ~200 lines in settings.js), the new modem_plmn_set / modem_plmn_restore ubus methods, a new UCI section type wwand_plmnlist (type + plmn list) with the plmn_list options it adds to both wwand.modemopts and wwand.simlist, the Datapath & muxing panel (modem_datapath), the Preferred networks (NAS) panel, the cachedCall/slowCache rework that moves cells/slots/datapath/PLMN off the 1 s poll onto per-call TTLs, the fmt.term() tooltip pass over status.js, modemmanager joining the migration list, and the tab→space reindent of the ACL file. A new config section type and a new poll-caching model are worth naming in the message — and the ACL reindent is what hid the two grant bugs.
  • d7bb9ee "luci-proto-wwand: sync to wwand 1.2.0 sources" — "add the hard_reconnect_on_ip_change option" covers 4 of the 10 added lines; the other three hunks add help text to pre-existing password, metric and peerdns options. Minor, but a "- plus help text for the PAP/CHAP password, gateway metric and peer-DNS options" bullet would make the diff match the body.

Generated by Claude Code

Comment thread protocols/luci-proto-wwand/htdocs/luci-static/resources/protocol/wwand.js Outdated
ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Aug 14, 2026
- status.js: fix the `var plmn` redeclaration. `plmn` was declared three
  times in one function scope (`res[5]` for the PLMN lists, then `reg.plmn`
  for the serving PLMN); by the NAS panel line `plmn` was the serving object,
  so `plmn.nas` was always undefined — the "Preferred networks (NAS)" panel
  never rendered and threw on an unregistered modem (swallowed by the status
  poller's error handler → the page silently stopped refreshing). Rename the
  lists reply to `plmnLists`.
- settings.js: drop the unreachable `plmnEditor()` (~62 dead lines; the only
  render path is `renderPlmnManager`). It also carried a latent `list.map`
  index-as-noRat bug.
- ACL: remove the dead `modem_repower` grant (the ubus method was renamed to
  `modem_reset`) and the `modem_sms_send` grant (no caller in the app — the
  SMS UI only lists/deletes). Restore tab indentation + inline single-element
  arrays to match the tree convention (100/104 acl.d files use tabs), which
  keeps the ACL delta reviewable.
- format/esim/netsel: move the shared `.wwe-*` stylesheet into wwand.format
  and inject it once from the settings view. Previously esim.js and netsel.js
  each emitted their own <style>, so the settings page (rendering both) got it
  twice, and netsel depended on esim purely for that string.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ddimension pushed a commit to ddimension/luci-proto-wwand that referenced this pull request Aug 14, 2026
- ACL: the write grant still named the removed `modem_repower` and did not
  grant `modem_reset`, so the Reset button rendered from the shared
  wwand.modemopts on the interface-edit page hit access-denied. Grant
  `modem_reset` instead.
- protocol/wwand.js: drop the redundant `o.default = '0'` on the
  hard_reconnect_on_ip_change flag — form.Flag already defaults to disabled,
  and it was the only literal '0'/'1' default in the file (the flags around it
  use the symbolic `o.enabled`/`o.disabled`).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@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 (7e1c0ec, 442ba27). Both messages match their diffs, so no commit checks this round — and this pair closes every finding from the last two rounds. One nit inline; CI on the head SHA is green (all three FormalityCheck runs).

What I verified:

  • status.js scope bug — fixed properly. plmnLists is now the only name bound to res[5], and var plmn = reg.plmn at :405 is the sole plmn left in renderLive (used only at :427, guarded by plmn ? …). renderNasList() already returns null for a missing/empty list, so an unregistered modem no longer throws into the poller's error handler.
  • plmnEditor() removal — no dangling references. plmnEditRow survives because renderPlmnManager still uses it (:358, :437) with an explicit wrapper rather than a bare list.map, and callPlmnSet is still reached at :407. No require in settings.js became unused by the deletion.
  • ACL — both files now match the actual call surface exactly. Every method wwand.rpc declares is granted and has at least one caller; conversely nothing is granted without a caller. modem_sms_send is correctly dropped (sms_send appears nowhere in either package), and luci-proto-wwand.json's modem_reset closes the access-denied on the Reset button that wwand.modemopts renders on the interface-edit Modem & SIM tab. modemopts' other calls from that page (status, modem_sim_slots, modem_probe, file list /sys/class/gpio/) are all covered by the proto grant too. Tab indentation matches the tree convention. Both files still parse as JSON.
  • CSS move — safe. wwand.esim and wwand.netsel are required only by view/wwand/settings.js, which is exactly where fmt.injectStyle() now runs, and it sits in the same returned tree as netsel.render() (:806) and esim.render() (:807), so neither panel loses its .wwe-* rules. No ESIM_CSS / esim.CSS references remain anywhere, and dropping the wwand.esim require from netsel.js leaves all of its remaining requires in use.
  • o.default = '0' — confirmed a no-op: form.js:5036-5037 sets this.disabled = '0'; this.default = this.disabled;.

Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/esim.js Outdated

@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 (eba976f). git diff 442ba27..eba976f is a single blank line removed from wwand/esim.js — the message matches the diff exactly, so no commit checks this round. Everything closed in the previous two rounds stays closed; nothing in this diff reopens a prior finding. CI on the head SHA is green (all three FormalityCheck runs).

The one nit inline is about where the commit sits in the series rather than what it does — the blank line it removes was introduced by 7e1c0ec earlier in this same series, so it reads as squash material.


Generated by Claude Code

@ddimension

Copy link
Copy Markdown
Author

Rebased onto current master and squashed from nine commits to two — one per package — and the sources are re-synced from upstream, which they had drifted from (the branch was still at the "wwand 1.2.0 sources" state).

Six files changed in the re-sync, the two substantive ones both being fixes found in the field since:

  • view/wwand/settings.js — the RAT mode picker rebuilt mode_preference from the four checkboxes it renders (GSM/UMTS/LTE/NR5G), so every save silently cleared CDMA (0x01), HDR/EVDO (0x02) and TD-SCDMA (0x20). Seen on a Quectel RG650E, where one save turned 0x7F into 0x5C. The mask now starts from the bits of the reported value that the picker does not render.
  • view/wwand/settings.js — a failing modem_set_settings printed only error ("qmi") and dropped detail, which made a firmware-level rejection undiagnosable from the GUI. It now appends the qmi result/code.

The rest are the status/modems/format/rpc/ACL updates that go with them. All JS passes node --check.

No takeover references remain: wwand no longer registers the qmi proto alias at all, so these packages only ever address proto wwand. That change is in the companion packages PR (openwrt/packages#30185), which now points at wwand v1.4.1.

@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 rebase: the nine-commit series is now two commits, one per package, and both messages match their diffs exactly (d41c76c touches only protocols/luci-proto-wwand/, b0f6e4e only applications/luci-app-wwand/) — so no commit checks this round.

The content delta against the previous head (eba976f) is the six files the PR comment names: git diff eba976f..b0f6e4e -- applications/luci-app-wwand protocols/luci-proto-wwand is +124/−6. The two fixes called out in that comment both check out — MODE_BITS_MASK correctly preserves the 0x23 (CDMA/EVDO/TD-SCDMA) bits the picker does not render, and describeError() recovers the detail.result/detail.code that a QMI-level rejection carries. Everything closed in the previous rounds stays closed; nothing here reopens a prior finding. CI on the head SHA is green (all three FormalityCheck runs).

Six inline comments, all on the new hunks. The two worth a second look are the control-protocol switch, whose comment claims a "supports both" gate that the code does not implement (and which the daemon documents as a Quectel-only AT command, broken on the RG650E for MBIM), and "Reset to defaults", which still sends DEFAULTS.mode_preference = 0x50 verbatim and so re-clobbers exactly the unrendered bits collect() now preserves.

I also re-checked the backend coupling for the two new ubus methods rather than assuming: modem_repower and modem_set_protocol both exist in the daemon (src-ucode/ubus.uc, src-ucode/hwops.uc, src-ucode/daemon.uc), both are called only from luci-app-wwand pages, and both are granted under write in luci-app-wwand.json — so no proto-side ACL gap this time, and no frontend-only option.


Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/settings.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/view/wwand/status.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/format.js Outdated
@ddimension

Copy link
Copy Markdown
Author

All six addressed. Five changed the code; one did not, and I want to be explicit about which.

Control-protocol switch. You were right that the comment promised a gate the code did not implement — but the underlying capability was already there and simply unexposed, so the fix is a real gate rather than a reworded comment. protocol_switch.uc in the daemon carries per-vendor AT recipes (Quectel AT+QCFG="usbnet", Fibocom, Sierra) and deliberately withholds the hardware-unverified ones, because a bad composition write can cost access to the modem. protocol_switch_supported() existed on the modem object but was never surfaced, so no UI could gate on it. The daemon now reports it as proto_switch in status(), and this section gates on that.

One correction to the review: the switch is not Quectel-specific. recipe_for() matches ^(RG|EG|EM|EC|BG|AG)[0-9] (Quectel), ^(FM|FG|NL)[0-9] (Fibocom) and ^MC[0-9] (Sierra).

Your RG650E point survives the gate, though, and it is the sharper half: that model has a recipe, so proto_switch is true for it, yet its firmware rejects MBIM_OPEN. A capability flag cannot express "the switch works but the destination does not". The MBIM direction now says so in the confirmation instead of promising the connections come back on their own.

"Reset to defaults". Correct, and it was worse than the bug it sat next to: 0x7F → 0x50 versus the 0x5C the Apply path produced. A reset now means "defaults for what is on screen" and keeps the bits the picker cannot render.

Protocol-switch failure path. Fixed — it now uses describeError(), which was indeed added in the same diff for exactly this shape.

Duplicated lock formatting. Moved to fmt.fmtLocks(). While it moved I took your second point: it renders the daemon shape ({ lte: { enabled, values: [earfcn, pci, …] } }) in the colon spelling the lock editor accepts, instead of JSON.stringify output. One behaviour change worth naming — a lock that is present but disarmed is no longer listed under "Locked to", which both call sites previously did.

in a block comment. Fixed.

modem_repower — not changed, checked. Every success path in hwops.uc returns { ok: true, action: … }: the reset-GPIO branch, the power-cycle branch, both. Failures return { error: … } with no action. So requiring res.ok is correct here and the Reboot buttons extra res.resetting tolerance is not needed — that field exists because modem_reset genuinely sets it. If the asymmetry is worth removing it belongs on the Reboot side, not here.

Note the gate needs the wwand release carrying the proto_switch status field; the packages PR (openwrt/packages#30185) will point at it. Until then the section simply does not render, which is the safe direction.

@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 amended head. d41c76c is byte-identical to the previous round; ad0894b replaces b0f6e4e, so the new content is git diff b0f6e4e..ad0894b — +88/−28 across five files, still scoped to applications/luci-app-wwand/ only, and the message matches the diff. No commit checks this round. CI on the head SHA is green (all three FormalityCheck runs).

All six findings from the last round are closed, and the follow-up comment's account of them checks out:

  • Duplicated lock formattingfmt.fmtLocks() is now the single implementation, and both call sites use it (status.js:409, settings.js:616). It renders the earfcn:pci / pci:arfcn:scs:band spelling the lock editor accepts, matching the placeholders in renderCellLock(), instead of JSON.stringify output. The disarmed-lock omission you called out is visible in the code (l.enabled === false skips the group), and unknown keys still fall through to the JSON branch, so nothing the daemon grows later is silently dropped.
  • "Reset to defaults" — now seeds from ~MODE_BITS_MASK exactly like collect(), and Object.assign keeps DEFAULTS itself unmutated across repeated presses. One question inline about which bits "defaults" should mean.
  • Control-protocol switch — gated on data.info.proto_switch, which comes off the same per-modem status object status.js reads caps.rats from, and fails closed when the daemon predates the field. The MBIM direction now warns about the MBIM_OPEN rejection; the QMI direction keeps the old wording. Failure path uses describeError(), so a QMI-level rejection reports result/code instead of bare "qmi".
  • NR band ordering — the narrower-before-broader invariant now holds at every overlap I could check: n14 before n28, n18 and n5 before the superset n26, n12 before n85, n261 before the superset n257. One nit inline on the new tie-break paragraph.
  • Em dash in the format.js block comment — literal now.
  • admin/network/wwand-tools — title and order restored, and load() falls back to the first modem when no ?modem= is present, so the menu entry lands on a usable page rather than an empty one.

Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/bands.js Outdated

@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 amended head (9a6dc44 replaces b0f6e4e; d41c76c is unchanged). The commit message is byte-identical to the one I reviewed last round and still matches the package it adds, so no commit checks this round. git diff b0f6e4e..9a6dc44 is +100/−31 across five files, all in luci-app-wwand, and it is exactly the follow-up the previous round's comments asked for. CI on the head SHA is green (all three FormalityCheck runs).

What I verified closed:

  • Reset to defaults re-clobbering the unrendered RAT bits — fixed on both halves. DEFAULTS.mode_preference is now derived from MODE_BITS_MASK instead of the hardcoded 0x50, and the button ORs in the & ~MODE_BITS_MASK remainder of the reported value, so a reset can no longer drop CDMA/EVDO/TD-SCDMA. Object.assign({}, DEFAULTS) keeps the shared band arrays unmutated.
  • Control-protocol switch gate — the section now actually gates on a capability (data.info.proto_switch) rather than on "the modem currently speaks QMI or MBIM", and the MBIM direction no longer promises the connections come back on their own. The dependency on a not-yet-released daemon field is called out in your comment, and the failure direction is safe (section simply does not render), so not flagged.
  • Protocol-switch failure path — now routed through describeError(), which is in scope at :90.
  • Duplicated lock formattingfmt.fmtLocks() is a genuine single source: the widths it groups by (2 for LTE, 4 for NR5G) match the spellings the cell-lock editor accepts (earfcn:pci, pci:arfcn:scs:band, placeholders at settings.js :585/:587), and both former call sites are gone. No lockRows/liveLocks leftovers, and fmt is required in both files.
  • Modem Tools menu title — restored, with order: 60 after Modems at 55. No collision with the core admin/network/* orders (10–50). The JSON parses and all four changed JS files pass node --check.

Three inline comments. The one worth a look is in bands.js: the reorder pass fixed n14, n18/n5 and n261, but n74 is still listed behind n50/n75, which leaves it reachable only for a 1 MHz sliver — the same class of bug this hunk was written to remove.


Generated by Claude Code

Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/bands.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/format.js Outdated
Comment thread applications/luci-app-wwand/htdocs/luci-static/resources/wwand/format.js Outdated

@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 amended app commit. d41c76c is unchanged since last round; c14c503 replaces b0f6e4e with the same message and +115/−33 of content, and since this is the single "add the package" commit the message still describes the whole package accurately — no commit checks this round. CI on the head SHA is green (all three FormalityCheck runs).

The six findings from the previous round all land, and the two you argued rather than changed hold up:

  • MODE_BITS_MASK as DEFAULTS.mode_preference — correct on both halves. DEFAULTS is now 0x5C instead of the hardcoded 0x50, so a reset no longer drops the 2G/3G fallback, and the Reset handler re-applies the same & ~MODE_BITS_MASK preservation collect() does, so the 0x01/0x02/0x20 bits survive the button that used to clobber them. s is in scope at the handler (settings.js:728),`` and Object.assign({}, DEFAULTS) is safe here — `apply()` only forwards the band arrays to `callSet`, it never mutates them.
  • Protocol-switch gatedata.info.proto_switch is checked before the qmi/mbim test, so an older daemon hides the section entirely rather than offering a switch it cannot perform. The MBIM-direction confirmation no longer promises the connections come back on their own, which was the RG650E half of the concern, and the failure path now goes through describeError() (defined at settings.js:89,`` in scope at the call site).
  • fmt.fmtLocks() — both call sites collapse onto it and both files already require wwand.format as fmt. The colon spelling matches what the editors accept in each direction: LTE width 2 renders earfcn:pci against the 1300:246 5230:118 placeholder, NR5G width 4 renders pci:arfcn:scs:band against 242:431070:1:78, and save() splits on /[\s,]+/ so the ', ' join round-trips. The l.enabled != null && !l.enabled guard correctly skips a disarmed lock while still rendering the bare { lte: true } shape.
  • Menu titleadmin/network/wwand-tools regains the title it lost several rounds ago, at order 60 behind admin/network/wwand (55), and its depends.ubus names modem_get_settings, which wrpc.getSettings really declares and the ACL really grants.

No new UCI option or ubus method in this diff, so nothing new to check against the backend-coupling rule. All nine app resources plus the proto handler pass node --check; all three JSON files parse.

Two inline comments, both on wwand/ shared resources: a question about which band the NR table should hand 1475–1517 MHz to now that n74 moved ahead of n50/n75, and a nit on the fmtLocks() fallback loop skipping its own enabled check.


Generated by Claude Code

Comment on lines +48 to +50
/* n74 (1475-1518) before n50/n75 (1432-1517): same narrower-before-broader
rule — behind them only the 1 MHz sliver above 1517 would ever reach it. */
['n74',1475,1518],['n50',1432,1517],['n75',1432,1517],

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.

Moving n74 ahead of n50/n75 does not just recover a dead entry — it hands 1475–1517 MHz to n74, and that is where the European L-band SDL carriers actually sit. Resolving the two tables against nrArfcn()'s first-match rule:

1455 MHz: before=n50  after=n50
1477 MHz: before=n50  after=n74
1490 MHz: before=n50  after=n74
1510 MHz: before=n50  after=n74

n74 is 1427–1470 UL / 1475–1518 DL, a Japanese FDD band; n50/n75 (1432–1517 SDL) are what the 1452–1492 MHz SDL deployments in Europe use, and the upper half of that window is exactly the range that flips. So the narrower-before-broader rule buys n74 reachability by mislabelling the more commonly deployed band — the opposite of the deployment argument the block four lines below applies to n66 vs n65`` ("n65 … is essentially undeployed there"). Was that trade intended here, or should n50/`n75` keep the overlap and `n74` stay behind them?

Same question, smaller stakes, for the sub-1 GHz reorder on line 45:`` putting n18/`n5` before `n26` makes both reachable but leaves `n26` (859–894) with only the 859–860 sliver, so 865/880/890 MHz now report `n18`/`n5` where they previously reported `n26`.

For what it's worth, I ran the reachability check the new header comment describes over the whole table — nothing else regressed. Only n51, n75, n65 and n90 are fully unreachable, and those are the four identical-range tie-breaks the comment already names.


Generated by Claude Code

Comment on lines +251 to +253
for (var k in locks)
if (k != 'lte' && k != 'nr5g' && locks[k] != null && locks[k] !== false)
out.push('%s: %s'.format(k, JSON.stringify(locks[k])));

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.

nit: this fallback bypasses the enabled check that group() applies, so it contradicts the rule stated in the doc comment above ("A lock that is present but DISARMED is omitted"). locks[k] !== false only catches a literal false; a future RAT reported in the same shape the daemon already uses for lte/nr5g{ enabled: false, values: [...] } — passes both guards and gets listed under "Locked to" as raw JSON, disarmed and all.

Routing unknown keys through group() with no width fixes that and drops the JSON.stringify output at the same time ({ n8: { enabled: true, values: [1, 2] } }n8 1, 2, { n8: true }n8 armed):

Suggested change
for (var k in locks)
if (k != 'lte' && k != 'nr5g' && locks[k] != null && locks[k] !== false)
out.push('%s: %s'.format(k, JSON.stringify(locks[k])));
for (var k in locks)
if (k != 'lte' && k != 'nr5g')
group(locks[k], 0, k);

Generated by Claude Code

@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 amended head. Both commits are new objects (d406f26 replaces d41c76c, 466fd78 replaces c14c503), but both messages are byte-identical to the ones I reviewed last round and still match what they add — so no commit checks this round.

The content delta against my last review is three lines: git diff c14c503..466fd78 is only the LUCI_DEPENDS comment in protocols/luci-proto-wwand/Makefile (+3/−3). The app half is unchanged. CI on the head SHA is green (all three FormalityCheck runs).

One inline comment, on that hunk: the new comment names six shared modules, and wwand.simlist is required by the handler but never used — the interface modal deliberately punts SIM overrides to the Modems page, so that require is dead.

Still open from the previous round, since neither file changed:

Backend coupling: no new UCI option or ubus method in this diff, so nothing new to check against the rule. The wwand.rpc method list and both ACL files are unchanged and still match exactly — every method the app declares is granted in luci-app-wwand.json, and the five read + two write methods the proto handler and the shared wwand.modemopts reach (status, modem_signal, modem_cells, modem_sim_slots, modem_probe, modem_reset, modem_sim_pin_verify, plus file list /sys/class/gpio/) are all covered by luci-proto-wwand.json.


Generated by Claude Code

Comment thread protocols/luci-proto-wwand/Makefile Outdated
Comment on lines +10 to +12
# This handler requires six resource modules that luci-app-wwand ships —
# wwand.bands, .format, .modemopts, .modemsid, .rpc and .simlist — so it
# depends on the app rather than carrying a second copy of them.

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.

Five of the six are used; wwand.simlist is not. 'require wwand.simlist as simlist' at wwand.js:9 is the only reference to the module in the whole handler — there is no simlist. call anywhere in the file, and the _sims_hint option that would have used it deliberately punts to the Modems page instead`` ("can't be embedded as a section list in the interface config modal"). So this comment, which is the only change in this round, documents a dependency that is really a dead require.

The other five check out: bands (3 uses), modemopts (4), wrpc (4), fmt (15), modemsid (2).

Drop the unused require at wwand.js:9 as well, and the count here matches what the file actually pulls in:

Suggested change
# This handler requires six resource modules that luci-app-wwand ships —
# wwand.bands, .format, .modemopts, .modemsid, .rpc and .simlist — so it
# depends on the app rather than carrying a second copy of them.
# This handler requires five resource modules that luci-app-wwand ships —
# wwand.bands, .format, .modemopts, .modemsid and .rpc — so it depends on
# the app rather than carrying a second copy of them.

Generated by Claude Code

ddimension pushed a commit to ddimension/luci-proto-wwand that referenced this pull request Aug 24, 2026
Found in review on openwrt/luci#8917. The handler requires wwand.simlist and
never touches it: SIM overrides moved to the dedicated Network → Modems page,
which the comment further down already says. Only the require stayed behind.

Five shared modules are actually used — wwand.bands, .modemopts, .rpc, .format
and .modemsid.
Netifd protocol handler for `proto wwand`, the cellular connection manager
proposed in openwrt/packages#30185. It renders the connection options the
daemon consumes — APN, PDP type, authentication, MUX id, MTU handling — and
resolves the modem an interface is bound to.

wwand registers `proto wwand` only; the historical `qmi` proto name stays
uqmi's, so this handler never competes with the stock one for an interface.

Signed-off-by: André Valentin <avalentin@marcant.net>
ddimension pushed a commit to ddimension/openwrt-repo that referenced this pull request Aug 24, 2026
wwand -> v1.4.6 (373e18e). A maintainer on openwrt/packages#30185 pointed out
that the device blocklist had been checked against qmi/mbim/ncm/wwan but not
against comgt's 3g and directip or against modemmanager. Checked all three
against their handlers: 3g and directip declare `device:device` (a real device
node) and were already covered; modemmanager declares a plain `device` and puts
a SYSFS PATH in it, which could never match a /dev node or a netdev name — so
such an interface claimed its modem invisibly. A `device` under /sys/ is now a
path-shaped claim too, resolved like uqmi's devpath and wwan.sh's bus.

luci-proto-wwand -> 680692f. Drops a dead `require wwand.simlist`: SIM
overrides live on the Modems page, only the require stayed behind. Found in
review on openwrt/luci#8917. The Makefile comment named the shared modules
wrongly in both directions — it claimed one (bands) when five are used, and
after a first correction claimed six by counting the dead one.
@ddimension

Copy link
Copy Markdown
Author

@patrakov Fixed in v1.4.7, which this PR now pins. Thank you for the screenshot
— the serving cell and the signal block being visibly inconsistent with each
other is what made it findable.

Cause. fill_signal_from_serving() builds the new signal block from a copy
of the previous one, then writes sig.lte / sig.nr5g only for the branches
the serving-cell read reports. A branch that is simply not written on a tick
therefore survived untouched — so when NR stopped serving, the last NR reading
stayed. The cells block does not have the problem because it is rebuilt from
scratch, which is exactly why your screenshot shows LTE/B3 above two frozen 5G
bars: two views of the same tick disagreeing.

Fix. A branch the serving read does not report is deleted. That is safe
specifically here: assemble_cells() returns before reaching this point unless
the read produced at least one branch, so an empty or failed read can never
blank a live reading — only a read that positively says "NR is gone" does. The
regression test replays your case (NR + LTE, then LTE alone, with the values
from the screenshot) and fails without the fix.

The QMI backend never had this: it replaces the whole signal snapshot per poll.
It is the AT path, where the block is assembled branch by branch, that could
carry a stale one.

While in there, one thing your report implies but does not say: the same modem
on an ipv6-only APN gets a single /64 and no delegated prefix, so odhcp6c
had nothing to hand the LAN. The proto wwand path already shared its own /64
(RFC 7278), but the dhcpv6 subinterface — which on the ipv6-only RNDIS model is
where the only address lives — did not. It now defaults to extendprefix '1'
there, and only there: an explicit 0 is never overwritten, and a subinterface
you wrote yourself is not touched.

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Aug 24, 2026
…ocks honours its own rule

Both raised in review on openwrt/luci#8917.

bands.js — n74 had been moved ahead of n50/n75 to rescue it from being dead
behind them. That bought its reachability by handing 1475-1517 MHz to a
Japanese FDD band, and 1477/1490/1510 MHz is exactly where the European L-band
SDL carriers (n50/n75) sit. The table's own rule says the more commonly
deployed band wins an overlap — it is applied four lines below for n66 over
n65 — so n74 goes back behind them and keeps only the sliver above 1517.

The same argument settles the sub-1 GHz reorder: n5 (global 850) now precedes
n26 and n18, so 872/880/890 MHz read n5 instead of a regional band. n26 keeps
859-869 and n18 becomes unreachable. Ordering n18 first would relabel ordinary
n5 spectrum as Japan-only, which is the worse of the two errors.

A first-match table cannot be right in every region at once. Where it must
choose, it now consistently chooses what is deployed, and the header says which
bands that costs and why.

format.js — the fallback loop that renders unknown lock keys bypassed the
`enabled` check group() applies, contradicting the documented rule that a
present-but-disarmed lock is omitted. `locks[k] !== false` catches only a
literal false, so a future RAT reported in the shape the daemon already uses
for lte/nr5g ({ enabled: false, values: [...] }) would have been listed as raw
JSON, disarmed and all. Routed through group() instead, which also drops the
JSON.stringify output.
@ddimension

Copy link
Copy Markdown
Author

Both open threads addressed.

bands.js — the trade was not intended, and you framed it correctly. n74
had been moved ahead of n50/n75 to rescue it from being dead behind them, but
that pays for its reachability with 1475-1517 MHz, which is where the European
L-band SDL carriers sit. The table already states the rule that settles it —
the more commonly deployed band is listed first — and applies it four lines
below for n66 over n65. n74 now follows n50/n75 and keeps only the sliver above
1517; 1477/1490/1510 MHz read n50 again.

The same argument decides the sub-1 GHz case you raised alongside it, and it
goes the other way from the current order: n5 (global 850) now precedes n26 and
n18, so 872/880/890 MHz read n5 rather than a regional band. n26 keeps 859-869
and n18 becomes unreachable. Ordering n18 first would relabel ordinary n5
spectrum as a Japan-only band, which is the worse of the two errors — but it is
a regional judgement, so if you read the deployment picture differently for
860-875 MHz, say so and I will take it.

A first-match table cannot be right in every region simultaneously. The header
comment now says which bands the rule costs (n18, n51, n75, n65, n90, and n74
in part) and why, instead of claiming only identical-range ties are unreachable.

format.js — fixed as suggested. The fallback now routes unknown keys
through group(), so the disarmed check applies to them too and the
JSON.stringify output is gone: { n8: { enabled: true, values: [1,2] } }
n8 1, 2, { n8: true }n8 armed, and { enabled: false, … } is omitted
like any other disarmed lock.

Worth noting that the shape you described as hypothetical is what the daemon
emits today: a disarmed cell lock arrives as {"enabled":false,"values":[]},
and on a build predating this it rendered verbatim into the status page under
"Locked to". So this was live, not only latent.

The third thread — the wwand.simlist require — was already dropped in the
previous push, together with the comment correction.

@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 amended head. Both commits are new objects (1a66040 replaces d406f26, 0e41f7c replaces 466fd78), but both messages are byte-identical to the ones I reviewed last round and both are still single "add the package" commits scoped to one package each — so no commit checks this round.

git diff 466fd78..0e41f7c is +32/−13 across four files, and it is exactly the three threads left open from the last two rounds. All three close:

  • bands.js — who owns 1475–1517 MHz. n74 now follows n50/n75, so 1477/1490/1510 MHz resolve to n50 again and n74 keeps only the sliver above 1517. I recomputed reachability across the whole table (interval subtraction in table order, not pairwise containment) rather than taking the new paragraph's word for it, and it is exactly right: the dead set is {n18, n51, n65, n75, n90} — the five bands it names, no more and no fewer. n26's surviving remainder really is 859–869, which is what the n5/n26/n18 swap buys. The sub-1 GHz call is a regional judgement either way, but the comment now states what the rule costs instead of implying only identical-range ties are unreachable, which was the thing worth fixing.
  • format.js — the fallback loop. Unknown keys go through group() now, so the disarmed check applies to them and the JSON.stringify output is gone. One residual nit inline, on what group() does with a payload that is not a values array.
  • Makefile / wwand.simlist. The require is dropped and the comment corrected to five. Checked rather than assumed: each of the handler's five requires (bands, modemopts, rpc, format, modemsid) is actually referenced in protocol/wwand.js; none of the five pulls simlist in transitively, so the count is right at both levels; and wwand.simlist still has a live consumer in view/wwand/modems.js, so removing the require does not orphan the module the app ships. The remaining wwand.simlist mention at wwand.js:550 is prose pointing at the Modems page, not a dangling reference.

No new UCI option or ubus method in this diff, so nothing new to check against the backend-coupling rule. All 13 JS files pass node --check and all three JSON files parse. CI on the head SHA is green (all three FormalityCheck runs).


Generated by Claude Code

Comment on lines +255 to +257
for (var k in locks)
if (k != 'lte' && k != 'nr5g')
group(locks[k], 0, k);

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.

nit: routing the fallback through group() fixes the enabled check, but it also drops the payload for every shape except { values: [...] } — which is the half of the comment above that says "shown rather than silently dropped".

group() reads the value only from var v = l.values || [] at format.js:231,`` so a future key that is a scalar ({ band_lock: 'n78' }) or a plain object with no `values` array (`{ band_lock: { earfcn: 1300, pci: 246 } }`) falls straight past both branches and renders as `band_lock armed`. The old `JSON.stringify` branch listed disarmed locks — which is why it had to go — but it did print what the lock held.

Widening the payload read inside group() closes it without reinstating that branch: treat a non-object, non-true l as a single value, and fall back to the object's own entries when values is absent. { lte: true } keeps saying LTE armed, the known keys are unaffected, and an unknown key keeps its content instead of being reduced to "armed".

Not live today — the daemon emits only lte/nr5g — so purely about the fallback matching what the doc comment promises of it.


Generated by Claude Code

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Aug 25, 2026
Follow-up finding on the previous fix, from review on openwrt/luci#8917.

Routing the fallback loop through group() fixed the disarmed check, but group()
reads the payload only from `l.values` — so a scalar ({ band_lock: 'n78' }) or
an object without a values array ({ band_lock: { earfcn: 1300, pci: 246 } })
rendered as a bare "band_lock armed" and lost what the lock held. The removed
JSON.stringify branch had to go because it listed disarmed locks, but it did
print the payload; dropping it silently traded one half of the doc comment for
the other.

group() now reads the payload from whatever shape arrived: a scalar is its own
value, a lone non-array `values` is a single value, and an object without
`values` is rendered from its own entries. `true` stays payload-free, since
that IS the "armed, no detail" spelling.

The known keys are untouched — lte/nr5g still pair their values by width
(LTE 1:2, 3:4), and a disarmed lock is still omitted.
@ddimension

Copy link
Copy Markdown
Author

Right on both counts — the fix traded one half of the comment for the other.

group() reads the payload only from l.values, so once it became the single
reader, a scalar ({ band_lock: 'n78' }) or an object without that array
({ band_lock: { earfcn: 1300, pci: 246 } }) rendered as a bare
band_lock armed. The disarmed check was honoured and the content was gone.

Widened the payload read inside group(), as you suggested, rather than
reinstating the JSON branch:

input output
{ enabled: true, values: [1,2,3,4] }, width 2 LTE 1:2, 3:4
{ enabled: false, values: [7] } omitted
'n78' band_lock n78
{ earfcn: 1300, pci: 246 } band_lock earfcn=1300, pci=246
true band_lock armed
{ values: 'n41' } band_lock n41

true stays payload-free deliberately — it is the "armed, no detail"
spelling, not a shape that lost something. The known keys are unaffected: lte
and nr5g still pair by width, and a disarmed lock is still omitted.

Checked each shape against the real function rather than by reading it.

@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 amended head. 1a66040 is byte-identical to the d406f26 I reviewed last round; 46fb9e1 replaces 0e41f7c with the same message, and it is still the single "add the package" commit for luci-app-wwand, so no commit checks this round.

git diff 0e41f7c..46fb9e1 is +19/−1 in one file — the group() payload widening from the last open thread. CI on the head SHA is green (all three FormalityCheck runs).

The widening does what your table says for every shape it lists — I ran the function rather than reading it: {enabled:true,values:[1300,246,5230,118]}LTE 1300:246, 5230:118, 'n78'band_lock n78, {values:'n41'}band_lock n41, trueband_lock armed, {enabled:false,values:[7]} omitted, and the known keys still pair by width. Nothing from the previous rounds reopens, and no new UCI option or ubus method here, so nothing new for the backend-coupling rule.

Two inline comments, both on the new branch chain. The one worth a look is a shape the chain does not cover: a bare array ({ lte: [1300, 246] }) reaches the l.values != null test, and since Array.prototype.values is a function that test passes — the row renders as LTE function values() { [native code] }.


Generated by Claude Code

Comment on lines +242 to +245
else if (Array.isArray(l.values))
v = l.values; /* lte/nr5g and anything like them */
else if (l.values != null)
v = [ l.values ]; /* a lone non-array value */

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.

A bare-array payload — { lte: [1300, 246] }, the most obvious shape after { values: [...] } — falls into the l.values != null branch, because Array.prototype.values is a real function on every array. v becomes [ Array.prototype.values ] and the row renders as:

LTE function values() { [native code] }

Ran the actual function over the shapes in your table plus this one: {lte:{enabled:true,values:[1300,246,5230,118]}}LTE 1300:246, 5230:118, {band_lock:'n78'}band_lock n78, {band_lock:{values:'n41'}}band_lock n41 — all as documented; {lte:[1300,246]} and {nr5g:[242,431070,1,78]} are the two that produce the native-code string. An explicit array branch before the l.values tests fixes it and also restores the colon pairing for that shape (LTE 1300:246):

Suggested change
else if (Array.isArray(l.values))
v = l.values; /* lte/nr5g and anything like them */
else if (l.values != null)
v = [ l.values ]; /* a lone non-array value */
else if (Array.isArray(l))
v = l; /* bare array: Array.prototype.values is a function */
else if (Array.isArray(l.values))
v = l.values; /* lte/nr5g and anything like them */
else if (l.values != null)
v = [ l.values ]; /* a lone non-array value */

Generated by Claude Code

Comment on lines +246 to +248
else
v = Object.keys(l).filter(function(kk) { return kk != 'enabled'; })
.map(function(kk) { return '%s=%s'.format(kk, l[kk]); });

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.

nit: the k=v strings from this branch still go through the width pairing below, so a known key that arrives in this shape gets colon-joined key/value pairs rather than the , spelling your table shows. { lte: { earfcn: 1300, pci: 246 } } renders LTE earfcn=1300:pci=246, while the same payload under an unknown key (width 0) correctly renders band_lock earfcn=1300, pci=246. Colon pairing only means something for a positional values array, so dropping the width here keeps the two consistent:

Suggested change
else
v = Object.keys(l).filter(function(kk) { return kk != 'enabled'; })
.map(function(kk) { return '%s=%s'.format(kk, l[kk]); });
else {
v = Object.keys(l).filter(function(kk) { return kk != 'enabled'; })
.map(function(kk) { return '%s=%s'.format(kk, l[kk]); });
width = 0; /* k=v pairs are not positional */
}

Generated by Claude Code

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Aug 25, 2026
…ing k=v

Two follow-up findings on the previous fix, both from review on
openwrt/luci#8917 and both real.

A bare array — { lte: [1300, 246] }, the most obvious shape after
{ values: [...] } — fell into the `l.values != null` branch, because
Array.prototype.values is a real function on every array. v became
[ Array.prototype.values ] and the row rendered as

    LTE function values() { [native code] }

An explicit Array.isArray(l) branch ahead of the l.values tests fixes it and
restores the colon pairing for that shape (LTE 1300:246).

And the k=v branch fed its strings through the width pairing, so the same
payload rendered differently depending on the key it arrived under:
{ lte: { earfcn: 1300, pci: 246 } } gave "LTE earfcn=1300:pci=246" while
{ band_lock: {...} } (width 0) gave "band_lock earfcn=1300, pci=246". Colon
pairing only means something for a positional values array, so that branch
clears the width.

Checked every shape against the real function, not by reading it.
Status and management UI for the wwand cellular connection manager proposed in
openwrt/packages#30185: modem overview, per-modem tools (radio technologies,
band selection, network selection and scan, cell lock), SIM and eSIM profile
management, SMS, and the per-interface migration entry point that converts a
stock cellular interface to `proto wwand` in place.

Everything is driven over the daemon's ubus API; the ACL file lists exactly the
methods the pages call.

Signed-off-by: André Valentin <avalentin@marcant.net>
@ddimension

Copy link
Copy Markdown
Author

Both correct, both fixed — the array one is a good catch.

Array.prototype.values being a real function is exactly why { lte: [1300, 246] } slipped through: Array.isArray(l.values) is false, l.values != null
is true, and the row rendered the native-code string. An explicit
Array.isArray(l) branch ahead of the l.values tests handles it, and the
colon pairing comes back with it.

The width point is right too, and it was inconsistent in a way that would have
been confusing to debug: the same payload rendered differently depending on the
key it arrived under. That branch now clears the width, since colon pairing only
means anything for a positional array.

Ran every shape through the real function again:

input width output
{ enabled: true, values: [1300,246,5230,118] } 2 LTE 1300:246, 5230:118
[1300, 246] 2 LTE 1300:246
[242, 431070, 1, 78] 4 NR5G 242:431070:1:78
{ earfcn: 1300, pci: 246 } 2 LTE earfcn=1300, pci=246
{ earfcn: 1300, pci: 246 } 0 band_lock earfcn=1300, pci=246
'n78' 0 band_lock n78
{ values: 'n41' } 0 band_lock n41
true 0 band_lock armed
{ enabled: false, values: [7] } 2 omitted

The PR body is updated to v1.4.7 as well — you were right that it had fallen
behind the pin again, and for the second time. I have written that down as a
step in the release routine rather than promising to remember it: tag the
commit (not the annotated tag object), bump the feed pin and recompute the
mirror hash, repoint the PR, and edit the body in the same pass.

@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 amended head. 1a66040 is byte-identical to the proto commit I reviewed last round; bc1667f replaces 46fb9e1 with a byte-identical message and is still the single "add the package" commit scoped to applications/luci-app-wwand/ — so no commit checks this round.

git diff 46fb9e1..bc1667f is +5/−1 in one file, exactly the two threads left open from the last round, and both close:

  • Bare-array payload. The explicit Array.isArray(l) branch now sits ahead of the l.values tests, so Array.prototype.values can no longer be mistaken for a payload. I ran the function rather than reading it: {lte:[1300,246]}LTE 1300:246 and {nr5g:[242,431070,1,78]}NR5G 242:431070:1:78, both with the colon pairing restored, in place of the function values() { [native code] } string. Branch order is right — l === true is still tested first, so the "armed, no detail" spelling is unaffected, and typeof l != 'object' still catches scalars, since an array never reaches it.
  • Width on the k=v branch. width = 0 there makes the same payload render identically under a known and an unknown key: {lte:{earfcn:1300,pci:246}}LTE earfcn=1300, pci=246, matching {band_lock:{earfcn:1300,pci:246}}band_lock earfcn=1300, pci=246. Previously the first produced LTE earfcn=1300:pci=246. Reassigning the parameter is local to the call, so the widths passed at the three call sites are untouched.

Every row of the table in your comment reproduces against the real function, including the ones this round did not change ({enabled:false,values:[7]} omitted, 'n78'band_lock n78, {values:'n41'}band_lock n41, trueband_lock armed, and the two known keys pairing by width). Nothing from the previous rounds reopens.

No new UCI option or ubus method in this diff, so nothing new to check against the backend-coupling rule. All 13 JS files pass node --check and all three JSON files parse. CI on the head SHA is green (all three FormalityCheck runs).

One nit inline, and it is about the PR description rather than the code.


Generated by Claude Code

include $(TOPDIR)/rules.mk

LUCI_TITLE:=Modem status and settings pages for the wwand connection manager
LUCI_DEPENDS:=+wwand +luci-base

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.

nit: the PR body edit that went with this push did not land — it still reads "Pairs with openwrt/packages#30185 (wwand 1.2.0)", while the comment accompanying this head says "The PR body is updated to v1.4.7 as well". The 2026-08-22 comment had already moved the pin past 1.2.0 ("which now points at wwand v1.4.1"), so the body has been three releases behind since then, not one.

Anchoring here because this line is where the tree states the dependency, and it carries no version: +wwand accepts whatever the feed pins, so the PR body is the only place a reader learns which wwand release these pages actually need — and it currently names one that predates proto_switch, fcc_auth and the {"enabled":…,"values":[…]} lock shape this round's fmtLocks() is written against.

(No suggestion block — the fix is in the PR description, not in this file.)


Generated by Claude Code

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

Labels

add package Introduces a new package Makefile build script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants