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

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, RAT priority, guarded modem-reset). A SIM cards page (Status → SIM cards) lists every card wwand has seen, by ICCID, with where it sits now and an optional name; with wwand-gps installed, the status page shows a GNSS panel (fix, satellites in view). 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.8.0; the packages here are luci-app-wwand v1.8.0 and luci-proto-wwand v1.7.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 protocols/luci-proto-wwand/Makefile Resolved
@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_repower → modem_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

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

@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

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

@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

@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 formatting — fmt.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

@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 formatting — fmt.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

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Sep 14, 2026
…icate row

From openwrt/luci#8917, verified against the code rather than taken on trust.

**Two rows both labelled Recovery.** The daemon derives `modem.proven` and
`recovery.armed` from the same flag — `armed: !!c.proto_ok` (daemon.uc:2219) and
`proven: !!…counters.proto_ok` (:2321) — so an unproven modem got a standalone
"disarmed" row AND a ladder row saying "not armed", in different words under
different tooltips. The reviewer suggested folding or dropping the older one;
dropping would have lost the only actionable sentence either tooltip carries
(check the control protocol and the bound driver), so it stays for the one case
the ladder cannot cover: a daemon reporting no `recovery` block at all, which a
WH3000 Pro on r68 really does.

The guard is `!modem.recovery`, the exact complement of the ladder's own
`if (rec)`. `== null` would have left a falsy-but-not-null value rendering
NEITHER row. recovery_view() always returns an object today so that gap is
unreachable, but the pair should hold by construction.

**i18n.** `_('steps taken')` and `_('attempts')` were context-free fragments
interpolated into '%s' shells that were not themselves translatable — two words
a translator cannot place or reorder. One msgid per sentence now, the same way
the graph.js legend line was collapsed.

**An unknown hardware action is no longer described.** An `hw.action` this page
does not recognise — a rung added to the daemon after this release — fell through
to the branch that says "no GPIO — software only (…)", asserting something the
daemon never said, under a tooltip promising the answer came from the code that
performs it. It is quoted verbatim now, with the page admitting it is the older
half.

**CodeQL alert 16.** `slowCache` is keyed by modem names out of uci, so a section
called `__proto__` could write through to Object's prototype. Both levels are
Object.create(null). Only an admin can create that name — this is hygiene, not a
vulnerability — but nothing in this file iterates or reflects over those objects,
so the null prototype costs nothing.

Co-Authored-By: Claude Opus 5 <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.

Commit checks

  • fab6bf9 "luci-app-wwand: add modem status and management application for wwand" — the message enumerates every page surface but not the system-log viewer (wwand/logbox.js, on the Status page) or the second access group it needs, luci-app-wwand-log, which grants log read, i.e. the whole system log. A grant that wide belongs in the commit message, not only in the ACL file's own description.

Generated by Claude Code

})
: Promise.resolve(null)
).then(function(pr) {
res[5] = pr && pr.ok !== false ? (pr.profiles || []) : null;

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 denied or failed read resolves to {} here — cachedCall substitutes {} through L.resolveDefault and then caches that for 60 s — so pr.profiles || [] yields [] and the slot renders "No profiles installed", which is exactly the claim esimProfileList() exists to keep apart from "not read". Key off the presence of profiles instead of defaulting it.

Suggested change
res[5] = pr && pr.ok !== false ? (pr.profiles || []) : null;
res[5] = (pr && pr.ok !== false && pr.profiles) ? pr.profiles : null;

Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Confirmed and fixed. cachedCall substitutes {} through L.resolveDefault and
caches it for 60 s, so a denied or failed read rendered as "no profiles
installed" — the one claim esimProfileList() exists to keep apart from "not
read", and for a full minute.

Keyed off the presence of profiles now, as you suggested. Worth noting for the
next reader why that is not the same as a truthiness test on the list: an eUICC
that genuinely has none answers profiles: [], and an empty array is truthy in
JS, so "installed: none" still renders as itself.

While fixing this I found the call underneath it was wrong too, in a way your
thread led me to — see the reply on :327.


return (euicc
? cachedCall(name, 'profiles', 60, function() {
return callEsim(name, 'profiles');

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.

modem_esim is listed only under write in luci-app-wwand.json, while modem_euicc_profiles is under read — and rpcd applies the two as separate permission levels per access group (session.c:1062-1073), so an operator granted luci-app-wwand read-only is denied here on every eUICC box. Every other call this page makes is in the read set. Is pulling the status page into the write scope intended, or should this read go through the read-granted method?


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The finding is right and the diagnosis is exact: modem_esim is in the write set
because its other ops enable, disable and delete profiles, rpcd grants a METHOD
rather than a method with certain arguments, and a read-only operator was
therefore denied the status page on every eUICC box.

I did not take the suggested route, and I would rather show why than assert
it. modem_euicc_profiles is the modem's own enumeration and calls
sim.euicc_profiles(), which requires a QMI UIM client. On an NCM modem there is
none. Measured on a Fibocom FM350-GL running on its eUICC:

ubus call wwand modem_euicc_profiles '{"modem":"wwmodem0"}'
  { "ok": false, "error": "euicc_read", ... }

ubus call wwand modem_esim '{"modem":"wwmodem0","op":"profiles"}'
  { "ok": true, "profiles": [ { "iccid": "...", "provider": "ByteSIM", ... } ] }

So that method would have moved the page from "denied for read-only operators" to
"broken for every NCM modem".

Instead there is now a read-only twin whose op is not a parameter, so it
cannot be turned into a write, and it goes through the same APDU ladder
(MBIM UICC → QMI UIM → AT) the write method uses. Its live ubus signature on an
RG650E:

"modem_esim_profiles":{"modem":"String","slot":"Integer","ubus_rpc_session":"String"}

It is granted in read; modem_esim stays in write. A test drives it over real
ubus and pins that an op argument is refused or ignored, never honoured.

Your thread also turned up a bug of mine two lines up. Writing the call I
passed slot: 0, which looked harmless — the bridge does +(params?.slot ?? 1),
and ?? does not replace an explicit 0, so it went out as the physical UIM
slot, which is 1-based. Every box here would have hidden it: on the same FM350-GL,
slots 0, 1 and 2 all return the same profile because the AT path ignores the
field. QMI does not. The page now passes the active eUICC record's physical,
the same value the daemon's own esim_ready handler uses.

In wwand df32269, here in this push.

if (!confirm(_('Delete %d selected message(s)?').format(sel.length)))
return;

return callSmsDelete(modem, storageSel.value, 0, idx).then(function(res) {

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.

This sends index: 0 together with indices, but the declaration comment in wwand/rpc.js describes the two as separate forms of the call. If the daemon tests index first, a "Delete selected" deletes storage slot 0 and ignores the ticked set — passing null in that position keeps it out of the request entirely. Is index: 0 something the daemon deliberately ignores when indices is present?


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deliberate, and the daemon is where it has to be handled.

simops.uc takes indices when it is a non-empty array and falls back to index
only when that is positive:

let which = (type(indices) == 'array' && length(indices)) ? indices
                                                         : ((+index > 0) ? +index : null);

The comment above it explains why the guard cannot live in the caller: ubus fills
a declared argument with its default when the caller omits it, so index arrives
as 0 whether or not anyone asked for slot 0. Passing null from here would
change nothing on the wire — ubus would substitute 0 again.

That was not reasoned out in advance; it was found on hardware. An empty
selection used to fall through to "delete index 0" and issue a real delete, and
an NR7101 answered +CMS ERROR 321 (invalid memory index) — which it only does
because the command was actually sent. On a card whose slot 0 is occupied, that
message would simply have been gone. SMS storage records are numbered from 1
(TS 51.011 EF_SMS is a linear fixed file), so requiring a positive index refuses
nothing a modem can address.

So "Delete selected" cannot delete slot 0, and the 0 in this call is the ubus
default made visible rather than a request.

if (t) {
out.subsys = t[1];
out.bare = !s ? t[1] : null; /* see classify() */
out.baretail = !s ? rest.slice(t[0].length) : null;

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: baretail is dead — it is copied onto the row at :238 and deleted at :114 without ever being read; classify() uses only bare. Drop it at all three sites.


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Correct — written at :78, copied at :238, deleted at :114, read nowhere;
classify() uses only bare. Gone at all three sites in this push.

Your thread also caught a second one indirectly: routing the eUICC read to the
new method left callEsim in status.js with no callers, so that is gone too.

ddimension pushed a commit to ddimension/wwand that referenced this pull request Sep 14, 2026
…rguments

The status page rendered the eUICC profile list through `modem_esim` with
`op: 'profiles'`. That method is in the rpcd WRITE set and has to stay there —
its other ops enable, disable and delete profiles — and rpcd grants a METHOD,
never a method with certain arguments. So an operator given read-only access was
denied the status page on every eUICC box (openwrt/luci#8917).

`modem_esim_profiles` hard-codes the op, which is the whole point: it is not a
parameter, so no caller can turn the read-granted method into a delete. The live
ubus signature shows it, on a running RG650E:

    "modem_esim_profiles":{"modem":"String","slot":"Integer","ubus_rpc_session":"String"}

NOT the existing read-granted `modem_euicc_profiles`: that one is the modem's own
enumeration and needs a QMI UIM client, so it cannot serve an NCM modem at all.
Measured rather than assumed, on a Fibocom FM350-GL:

    modem_euicc_profiles        -> ok: false, euicc_read
    modem_esim op=profiles      -> ok: true, profiles: [ … ]

This goes through the same APDU ladder as the write method (MBIM UICC -> QMI UIM
-> AT), which is what that modem is read with.

The test drives it over real ubus and pins the property that matters: an `op`
argument is either refused by the argument policy or ignored, never honoured.
The ICCID in it is deliberately invalid, because with a valid one both routes end
in the same backend error and the test would prove nothing — `delete` rejects an
unparsable ICCID before it reaches a backend, `profiles` does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Sep 14, 2026
… a write-scoped read

From openwrt/luci#8917, each checked against the code rather than taken on trust.

**The eUICC profile list needed WRITE permission.** It called `modem_esim` with
`op: 'profiles'`, and that method is in the write ACL because its other ops
delete profiles — rpcd grants a method, not a method with certain arguments. A
read-only operator was denied the status page on every eUICC box. It now calls
the read-granted `modem_esim_profiles` (wwand df32269), whose op is hard-coded so
it cannot be turned into a write.

The reviewer suggested the existing read-granted `modem_euicc_profiles` instead.
That would have broken the case we actually ship: it needs a QMI UIM client, and
on a Fibocom FM350-GL over NCM it answers `euicc_read` while the APDU ladder
returns the profile — measured on hardware, not argued.

**A denied or failed read rendered as "no profiles installed".** `cachedCall`
substitutes `{}` for it and caches that for 60 s, and `{}.profiles || []` is an
empty list — the one claim `esimProfileList()` exists to keep apart from "not
read". Keyed off the presence of `profiles` now; an eUICC that genuinely has none
answers `profiles: []`, and an empty array is truthy, so that still reads as
installed-none.

**The slot was wrong, and every box we have would have hidden it.** Passing `0`
looked harmless because the bridge does `+(params?.slot ?? 1)` — and `??` does
not replace an explicit 0, so it went out as the physical UIM slot, which is
1-based. Verified on a live FM350-GL: slots 0, 1 and 2 all return the same
profile there, because the AT path ignores the field. QMI does not. The page has
the active eUICC record from `modem_sim_slots` and passes its `physical`, the
same value the daemon's own esim_ready handler uses.

**Dead code**: `baretail` in logbox.js, written and deleted and never read, and
the `callEsim` helper this change left unused.

The .pot carries the moved source references; no msgid changed.

Co-Authored-By: Claude Opus 5 <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.


Generated by Claude Code

callGet(name),
L.resolveDefault(callPlmn(name), {}),
L.resolveDefault(callSlots(name), {}),
L.resolveDefault(callEsim(name, 'profiles', 0, '', '', ''), {}),

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.

The slot: 0 fixed on the status page this round is still here: +(params?.slot ?? 1) does not replace an explicit 0, so this asks for physical UIM slot 0 instead of the eUICC's, and QMI does not ignore the field. Same on :334 and at the seven callEsim(data.modem, …, 0, …) calls in wwand/esim.js (:206, :213, :219, :318, :369, :392, :409) — there the eUICC's physical is already derived from data.slots for euiccProfiles at esim.js:65,`` so the same value can be passed on.


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Caught — the status page was fixed and this call site was missed. Fixed in b215b05 (amended into the app commit).

?? only replaces a MISSING value, so the literal 0 travelled all the way to +(params?.slot ?? 1) and asked for physical slot 0, which is not a slot at all — modem_sim_switch_slot refuses anything but > 0.

It stayed invisible because the AT APDU backend ignores the argument entirely: on hardware, slots 0, 1 and 2 return the same profile. A QMI-UIM box would not have been so forgiving.

I did not just swap the 0 for a 1. A fixed number is the wrong card on a dual-SIM box whatever number it is, so the page now asks for the eUICC's own slot, the way the status page does:

var euicc = slots.filter(function(s) { return s.is_euicc })[0];
var slot = euicc ? euicc.physical : 1;

That costs one round trip on load — the slot list has to arrive before the eSIM calls rather than beside them — which is a fair price on a settings page. Where there is no eUICC the call sends 1 rather than null, because the ubus signature types slot as an integer; 1 is the daemon's own default, said out loud instead of relied on.

@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.


Generated by Claude Code

'click': ui.createHandlerFn(self, function() {
if (!confirm(_('Enable profile %s? The connection will re-establish.').format(p.iccid)))
return;
return callEsim(data.modem, 'enable', 0, p.iccid, '', '').then(function(res) { ctx.notifyEsimApply(data.modem, res) });

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.

Only the read path was moved off the literal 0; the seven action calls still pass it — :206, :213, :219, :318, :369, :392, :409 — so enable/disable/delete/download keep addressing physical slot 0 while the profile list they act on comes from the eUICC's slot. render() already receives data.slots, so derive the eUICC slot once at the top of it the way renderNativeProfiles does at esim.js:64-65 and pass it to all seven.


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

All seven, and you are right that these are the half that matters: the read path shows you the wrong list, the write path enables a profile on the wrong card. Fixed in a3c3622.

Done the way you suggested — render() derives the eUICC slot once, the same derivation renderNativeProfiles already had, and all seven calls take it. Having two places compute the same thing was how the read path got fixed and these did not.

For the record on why it survived: the AT APDU backend ignores the argument entirely — on hardware, slots 0, 1 and 2 return the same profile — so no box I have access to could show it. A QMI-UIM box would have.

signature types `slot` as an integer (ubus.uc modem_esim
args), so a null is not something to send through it. 1 is
the daemon's own default, stated explicitly. */
var slot = euicc ? euicc.physical : 1;

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 slot entry can arrive without physical — modemopts.js:437 guards `sl.physical == null` on this same `modem_sim_slots` reply — and then the eUICC branch sends exactly the `null` the comment above says must not go through the integer-typed arg. Same shape at [`esim.js:65`](https://github.com/openwrt/luci/blob/b215b05981167c37352ac715ae0b698e050fa635/applications/luci-app-wwand/htdocs/luci-static/resources/wwand/esim.js#L65``) and status.js:333.``

Suggested change
var slot = euicc ? euicc.physical : 1;
var slot = (euicc && euicc.physical != null) ? euicc.physical : 1;

Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Taken, with one correction to the premise: a slot without physical cannot currently arrive. Both producers set it unconditionally — sim.uc:633 for QMI (physical: i + 1) and modem_ncm.uc:880/885 for the AT backend (physical: 1 / physical: 2). So this is defensive, not a live bug, and I would rather say that than accept a fix under a wrong description.

I took it anyway, for the reason your own pointer makes: modemopts.js:437 already guards the same field on the same reply, and the comment directly below this line says a null must never reach the integer-typed ubus arg. Code that states an invariant should not be the one place that breaks it.

Guarded in all four derivations rather than only this one — esim.js:65, status.js:333 and the new render() derivation too, since they read the same reply and a half-guarded invariant is worse than none.

Comment on lines +335 to +337
box a fixed number is the wrong card anyway, so ask the eUICC's
own slot and leave it unset when there is none, which is the one
case the daemon's default is right for. */

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: "leave it unset when there is none" contradicts both the code and the inner comment eight lines down, which send an explicit 1 precisely so the arg is never unset.

Suggested change
box a fixed number is the wrong card anyway, so ask the eUICC's
own slot and leave it unset when there is none, which is the one
case the daemon's default is right for. */
box a fixed number is the wrong card anyway, so ask the eUICC's
own slot, and send the daemon's own default of 1 when there is
none. */

Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Mine, and plainly wrong: I wrote that comment while the code still sent null, then changed the code to send an explicit 1 and left the sentence describing the old behaviour eight lines above the line that contradicts it.

Reworded close to your suggestion — "send the daemon's own default of 1 when there is none to point at".

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Sep 18, 2026
`callEsim(name, 'profiles', 0, …)` passed a literal 0 as the slot. The daemon
resolves it with `+(params?.slot ?? 1)`, and `??` only replaces a MISSING value
— so the 0 travelled all the way through and asked for physical slot 0, which
is not a slot at all (`modem_sim_switch_slot` refuses anything but > 0).

The status page was fixed this way a few days ago by passing `euicc.physical`;
this call site was missed, and an upstream reviewer caught it (openwrt/luci#8917
on settings.js:333).

It went unnoticed because the AT APDU backend ignores the argument entirely —
verified on hardware, slots 0/1/2 return the same profile — so every box tested
so far hid it. A QMI-UIM box would not.

Fixed by asking for the eUICC's OWN slot, which is also what a dual-SIM box
needs: a fixed number is the wrong card there whatever number it is. That means
the slot list has to be fetched before the eSIM calls rather than beside them,
so the load path gains one round trip and loses the parallel fetch of `slots`.
Worth it on a settings page.

Where there is no eUICC to point at, the call sends 1 rather than null: the ubus
signature types `slot` as an integer, and 1 is the daemon's own default said out
loud.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSrxnGz8wMc3PFwRMDiV1e
ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Sep 18, 2026
The read path was moved off the literal `0` two days ago and the seven action
calls were missed — enable, disable, delete, download, download_status,
notifications, notify. An upstream reviewer found them (openwrt/luci#8917), and
they are the worse half: these change the card rather than read it.

Same mechanism as before. The daemon resolves the argument with
`+(params?.slot ?? 1)`, and `??` only replaces a MISSING value, so a literal 0
travels through and addresses physical slot 0 — which is not a slot at all. It
went unnoticed because the AT APDU backend ignores the argument entirely
(verified on hardware: slots 0/1/2 return the same profile). A QMI-UIM box would
have enabled a profile on a card nobody asked about.

`render()` now derives the eUICC's own slot once, the way renderNativeProfiles
already did for the read path, and all seven calls take it.

And `physical` is now guarded rather than assumed, in all four places that
derive a slot from a `modem_sim_slots` reply. Both producers in the daemon set
it unconditionally today, so this is defensive — but modemopts.js already
guarded it on the same reply, and the comment right below this code says a null
must never reach the integer-typed ubus arg. Code that states an invariant
should not be the place that breaks it.

Also a comment of mine that contradicted its own code: it said the slot is left
unset when there is no eUICC, eight lines above the line that sends an explicit
1 precisely so it never is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSrxnGz8wMc3PFwRMDiV1e
ddimension pushed a commit to ddimension/openwrt-repo that referenced this pull request Sep 18, 2026
wwand — two field reports, both with the reporter doing the hard part:

- mbim: a data class of CUSTOM alone (0x80000000) carries no RAT, and
  `get_data_mode` returned `{ mode: null, … }` for it. That object is truthy, so
  the demotion added in _p10 counted it an answer, the native rung kept the
  choice, and an RM520F-GL carrying NR5G-SA reported no RAT at all. It returns
  null now, and the slow tick reads the serving cell BEFORE walking the
  data-mode ladder — asked first, every rung declined and `backend.choose`
  cached that verdict permanently (ddimension/wwand#30).
- ncm: `slot_status`/`switch_slot` dereferenced a torn-down AT engine. The
  reporter isolated it himself — it only happens with `sim_slot` configured,
  which is what makes step_simslot walk the second pass. Every hop of the switch
  chain is guarded now, not just the entry. And the address read gets one
  delayed retry on the activation path: three of his six deliberate slot
  switches failed once on `AT+CGPADDR=1 -> ERROR` and came up on the next
  attempt (#32).
- init: the bytecode-mismatch hint named only the ddimension feed's symbol,
  which is the wrong one for openwrt/packages users — the two packagings invert
  the option (openwrt/packages#30185 review).

luci-app-wwand — the eSIM WRITE path still addressed physical slot 0: enable,
disable, delete, download and notify. The read path was moved off the literal 0
and these seven were missed (openwrt/luci#8917 review). `render()` derives the
eUICC's own slot once and all seven take it.

Host suite 4669 checks, 0 failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSrxnGz8wMc3PFwRMDiV1e

@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.


Generated by Claude Code

Comment on lines +123 to +125
var esimSlot = 1;
(data.slots || []).forEach(function(sl) {
if (sl.is_euicc && sl.physical != null) esimSlot = sl.physical;

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 settings view already picked this slot for the profiles/backend reads at settings.js:344-350,`` and the two rules differ: that one takes the first eUICC slot, this one the last, so a reply carrying two eUICC slots would list one card and enable/disable/delete on the other. Carry the slot load() chose through `data` and read it here instead of re-deriving; the third copy at :64-67 can then use it too.


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right, and the divergence is exactly as you describe. Fixed in 9b2d363.

load() keeps the one derivation and passes it through data.esimSlot; both esim.js sites read it, so the third copy at :64-67 is gone too. Three spellings became one.

The two rules, for the record

settings.js:344   slots.filter(s => s.is_euicc && s.physical != null)[0]   // FIRST
esim.js:65,125    forEach(sl => { if (…) slot = sl.physical })             // LAST

They agree whenever there is one eUICC, which is why this outlived the review that moved these very calls off a literal slot 0. Two eUICC slots are representable, not hypothetical: is_euicc is probed per slot and independently — sim.uc:638 from the QMI UIM card status, modem_ncm.uc:883,888 from two separate AT probes.

Verified on hardware

I drove the shipped module in a browser against the real modem_sim_slots reply of three modems and captured the slot it puts on the wire:

box modem / transport eUICC at load() rule what esim.js sends
MikroTik Chateau RG650E, QMI physical 2 2 2
Huasifei WH3000 Pro Fibocom, NCM/AT physical 2 2 2
GL-X3000 RM520N-GL, MBIM physical 1 1 1

The first two matter because 2 is not the fallback — the value genuinely travels rather than coinciding with the default. And fed a synthetic reply carrying two eUICC slots, the same module shows the fix doing its work:

load() rule (first):  1
old esim.js (last):   2
what it sends now:    1

What I could not exercise is a real write. On the Chateau, lpac cannot enumerate that card (ok:false, error:"esim"), so the profile panel and its seven buttons never render; on the GL-X3000 they do, but its eUICC sits in slot 1, which is also the fallback, so a click there proves nothing a bug could not also produce. I did not click Disable or Delete on a live eUICC carrying the active profile to confirm a binding that is one variable in one scope — the read path proves the plumbing, and the diff shows all seven reading it.

One thing I took beyond the nit

physical != null admits 0, and 0 is precisely the value that slipped through the daemon's ?? 1 and addressed a slot that is not a slot — the bug this thread started from. Slots are 1-based wherever the daemon builds them (sim.uc:633 and mbim_backend.uc:297 count i + 1; modem_ncm.uc:880,885 say 1 and 2), so a 0 cannot arrive today. Defensive, then, not a live defect — but a guard that admits it states less than what actually holds, so it is > 0 now, here and in status.js:328.

status.js keeps its own, stricter predicate: that panel wants the active eUICC, because it cannot read an inactive card. Different question, deliberately different rule — I left it alone apart from the guard.

@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; no new issues found.


Generated by Claude Code

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Sep 20, 2026
The eUICC's physical slot was derived in three places with TWO rules. The
settings view took the FIRST slot reporting an eUICC and made the `profiles`
and `backend` reads against it; wwand/esim.js took the LAST, in
renderNativeProfiles for the modem-side read and in render() for the seven
action calls (enable, disable, delete, download, download_status,
notifications, notify).

With one eUICC the two agree, which is why this survived the review that
moved these very calls off a literal slot 0. With two, the panel lists one
card's profiles and writes to the other — silently, because both answers are
well-formed. Two eUICC slots are representable: is_euicc is probed per slot
and independently (wwand src-ucode/sim.uc:638 for QMI UIM,
src-ucode/modem_ncm.uc:883,888 for AT).

So load() keeps the one derivation and passes it through `data.esimSlot`;
both esim.js sites read it. Raised in the openwrt/luci#8917 review.

Two guards tightened while here: `physical != null` admits 0, and 0 is
precisely the value that slipped through the daemon's `?? 1` and addressed a
slot that is not a slot. Slots are 1-based wherever the daemon builds them
(sim.uc:633 and mbim_backend.uc:297 count `i + 1`; modem_ncm.uc:880,885 say 1
and 2), so a 0 cannot arrive today — but a guard that admits it states less
than what holds. status.js keeps its own, stricter predicate deliberately:
that panel wants the ACTIVE eUICC, because it cannot read an inactive card.

Verified in a browser against three modems, with the shipped module driven by
the real modem_sim_slots reply:
  - MikroTik Chateau / RG650E / QMI, eUICC on physical 2: sends slot 2, not
    the fallback 1, so the value travels.
  - the same box fed a synthetic two-eUICC reply: old rule 2, load() rule 1,
    the new code sends 1.
  - Huasifei WH3000 Pro / Fibocom / NCM-AT, eUICC on physical 2: sends 2.
  - GL-X3000 / RM520N-GL / MBIM: the profile panel and its action buttons
    render unchanged (eUICC on slot 1 there, so not a discriminating case).

Co-Authored-By: Claude Opus 5 <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.

Commit checks

  • 9f86adc "luci-app-wwand: add modem status and management application for wwand": the body is unchanged but doesn't mention the new Status → SIM cards page, the GNSS panel, or the runtime daemon log-level control (a new set_log_level write grant). Add a bullet for each.

Generated by Claude Code

1..63 table, which names bands it would refuse */
if (settable) {
defs = {};
settable.forEach(function(k) { defs[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.

can() at :1106/:1134 expects settable to possibly contain mode_preference, usage_preference or roaming_preference, but this sets every settable key to []. If a backend lists one of those integer keys, Reset sends an array where a number is expected, and the whole edit is refused. Should only the band lists be blanked?

Suggested change
settable.forEach(function(k) { defs[k] = []; });
settable.forEach(function(k) { if (/_bands$/.test(k)) defs[k] = []; });

Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

re-attaching shows the difference here */
if (ps.tai && ps.tai.mcc != null) {
srvRows.push([ term(_('Attach TAI'), _('Tracking Area Identity recorded at attach: PLMN and Tracking Area Code. Differs from the serving cell TAC when the modem moved without re-attaching')),
'%s/%s · TAC %d'.format(ps.tai.mcc, fmt.fmtMnc(ps.tai.mnc), ps.tai.tac) ]);

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.

This is the only new fmt.fmtMnc() call that leaves out the digit count, so a 3-digit MNC below 100 (310/030) shows as 310/30 here. If the daemon reports mnc_digits on packet_service.tai, pass it:

Suggested change
'%s/%s · TAC %d'.format(ps.tai.mcc, fmt.fmtMnc(ps.tai.mnc), ps.tai.tac) ]);
'%s/%s · TAC %d'.format(ps.tai.mcc, fmt.fmtMnc(ps.tai.mnc, ps.tai.mnc_digits), ps.tai.tac) ]);

Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

"cell-lock editor, showing what the modem ACTUALLY locked"
msgstr ""

#: htdocs/luci-static/resources/statistics/rrdtool/definitions/wwand.js:32

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.

This push didn't regenerate the template. It points at statistics/rrdtool/definitions/wwand.js, which this package doesn't ship, and it is missing the new strings (SIM cards, GNSS, Daemon writes at, Remote SIM supported, not seen, …). Regenerate it from the sources at this head.


Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

Comment on lines +261 to +263
/* IN USE and IN VIEW are different numbers and the difference is the
diagnosis: none in view is an antenna problem, plenty in view and none
in use is a sky or almanac problem. ugps only ever had the first. */

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 is a leftover first version of the comment directly below it, which begins with the same sentence.

Suggested change
/* IN USE and IN VIEW are different numbers and the difference is the
diagnosis: none in view is an antenna problem, plenty in view and none
in use is a sky or almanac problem. ugps only ever had the first. */

Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

if (g.sats_used != null || g.sats_view != null)
rows.push([ term(_('Satellites'), _('In use / in view. Nothing in view is usually the antenna, though a cold receiver or a blocked sky looks the same; many in view with none in use means it can hear them but not yet solve a position, which outdoors typically takes a minute or two.')),
(g.sats_used != null && g.sats_view != null)
? '%d %s %d %s'.format(g.sats_used, _('in use of'), g.sats_view, _('in view'))

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: _('in use of') / _('in view') (and _('parsed') / _('not') in the NMEA stream row) are word fragments placed into a '%d %s %d %s' shell that translators can't see. Use whole format strings such as _('%d in use of %d in view'), so the word order can be translated.


Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

* modem rejected the command.) Found by a full review, 2026-09-19. */
var lock5g = s.taboption(tab, form.Value, 'lock_5g', _('5G NR SA cell lock'),
_('Lock to a 5G SA cell: "pci:arfcn:scs:band" — the subcarrier spacing is in kHz (15, 30, 60 …). The "Lock this 5G cell" button fills in 30 as a default; the modem does not report the actual spacing, so check it against your band before saving.'));
lock5g.placeholder = '242:431070:15:1';

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 placeholder still shows a 15 kHz SCS and band 1. The Tools page's field now shows 242:431070:30:78, and the comment at protocol/wwand.js:322 says the placeholder shows 30.

Suggested change
lock5g.placeholder = '242:431070:15:1';
lock5g.placeholder = '242:431070:30:78';

Generated by Claude Code

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.

fixed, thanks


Generated by Claude Code

@ddimension

Copy link
Copy Markdown
Author

Addressed this review round in a1c529f:

  • Reset button (settings.js) — now blanks only the *_bands$ lists; integer prefs (mode_preference/usage_preference/roaming_preference) a backend may also list in settable are left untouched, so can() no longer gets an array where it expects a number.
  • Attach TAI (status.js) — fmt.fmtMnc(ps.tai.mnc, ps.tai.mnc_digits) so a 3-digit MNC below 100 renders as 030, not 30.
  • wwand.pot — regenerated from the current sources: the stale statistics/rrdtool/definitions/wwand.js reference is gone and the new strings (SIM cards, GNSS, Daemon writes at, Remote SIM supported, …) are in.
  • GNSS satellites (status.js) — removed the leftover first copy of the duplicated block comment.
  • i18n fragments (status.js) — the satellites and NMEA rows now use whole format strings (_('%d in use of %d in view'), _('%d in use'), _('%d in view'), _('%d parsed, %d not')) instead of %d %s-shell fragments.
  • lock_5g placeholder (modemopts.js) — 242:431070:30:78, matching the Tools page and the 30 kHz default the comment describes.
  • Commit message — now mentions the Status → SIM cards page, the GNSS panel, and the set_log_level write grant.

ddimension pushed a commit to ddimension/luci-app-wwand that referenced this pull request Oct 6, 2026
Bring main in sync with the review round on openwrt/luci#8917:

- settings Reset: blank only the *_bands lists; leave integer prefs
  (mode/usage/roaming) that a backend may also list in settable, which
  can() rejects as an array.
- status Attach TAI: pass mnc_digits to fmtMnc so a 3-digit MNC below 100
  renders 030, not 30.
- status GNSS: drop the duplicated 'IN USE and IN VIEW' comment.
- status i18n: satellites and NMEA rows as whole format strings
  (%d in use of %d in view, %d parsed/%d not) instead of %s fragments.
- modemopts: lock_5g placeholder 242:431070:30:78 (30 kHz SCS, band 78).

Regenerate po/templates/wwand.pot.

@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.

Commit checks

  • a1c529f "luci-app-wwand: add modem status and management application for wwand" — the body says the SIM cards page "lists the inserted SIM and the eSIM profiles", but view/wwand/sims.js lists every card wwand has seen, by ICCID, including cards that are gone ("not present, last seen …") and remote-SIM readers. Reword it to match, e.g. "lists every SIM card wwand has seen, by ICCID, with where it is now".

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 1 new commit; no new issues found.


Generated by Claude Code

ddimension pushed a commit to ddimension/wwand that referenced this pull request Oct 24, 2026
Everyone who opened a pull request or an issue, or took part in a
thread, across the wwand repositories and the upstream submissions
openwrt/packages#30185 and openwrt/luci#8917 — by GitHub account,
generated from the history, bots left out. Linked from the README.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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.

The status page graphs the signal live — one canvas per quantity with its
published thresholds, one series per radio technology — so a modem dropping from
5G to LTE, or a hand on the antenna, is visible rather than inferred. The
history is kept in the browser: nothing is buffered on the router and nothing is
sampled while the page is closed. A Status → SIM cards page lists every SIM
card wwand has seen, by ICCID, with where it is now — including cards no
longer present and remote-SIM readers — and, where the modem exposes a
receiver, a GNSS panel shows the fix, satellites in use/view and NMEA health.

Everything is driven over the daemon's ubus API; the ACL file lists exactly the
methods the pages call, including a set_log_level write grant that exposes the
daemon's run-time log level in the UI.

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

This branch has not been deployed

No deployments
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.

4 participants