luci-app-hh4xmodem: application to monitor and control the MDM9207 modem found in HH40V and HH41V routers.#8713
Conversation
This comment has been minimized.
This comment has been minimized.
4e383e1 to
01259c6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
01259c6 to
5ed0a2d
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Fresh review of this new package. Main concern: the UCI settings written by the uci-defaults script (modem_ip, modem_port, refresh_interval, network.mode_N) are not read by any backend — the modem address comes from environment variables in hh4xmodem-pack and the mode labels are hardcoded in the view, so configuring these does nothing. See inline comments for that plus the unmanaged polling loop and a few nits (license header, leftover debug element). The Signed-off-by formality failure is already reported by the CI bot.
Generated by Claude Code
| uci set hh4xmodem.settings=settings | ||
| uci set hh4xmodem.settings.modem_ip='192.168.225.1' | ||
| uci set hh4xmodem.settings.modem_port='2016' | ||
| uci set hh4xmodem.settings.refresh_interval='5' |
There was a problem hiding this comment.
These UCI options (modem_ip, modem_port, refresh_interval) have no backend consumer, so configuring them does nothing. hh4xmodem-pack reads the modem address from the environment variables MODEM_IP/MODEM_PORT (hh4xmodem-pack:9-10), and nothing in the package ever exports those env vars from UCI. The ucode backend imports only fs and never reads UCI either, and refresh_interval is unused (the dashboard polls on a hardcoded 3000 ms interval). Either wire these options into the backend (e.g. have the ucode plugin read uci get hh4xmodem.settings.modem_ip and pass it to the helper via the environment) or drop the settings section. Which path reads these values at runtime?
Generated by Claude Code
| } | ||
|
|
||
| function startPolling() { | ||
| setInterval(function() { |
There was a problem hiding this comment.
This raw setInterval is never stored or cleared, so the 3000 ms polling loop keeps firing RPC calls even after the user navigates away from the dashboard (it only stops on a full page reload). LuCI views should use the managed poll API ('require poll' + poll.add(fn, interval)), which is automatically torn down when the view is left. It also lets the interval honor the refresh_interval UCI option instead of the hardcoded 3000.
Generated by Claude Code
| uci set hh4xmodem.network=network | ||
| uci set hh4xmodem.network.mode_0='2G Only' | ||
| uci set hh4xmodem.network.mode_1='3G Only' | ||
| uci set hh4xmodem.network.mode_2='2G/3G Auto' | ||
| uci set hh4xmodem.network.mode_3='4G/LTE Only' | ||
| uci set hh4xmodem.network.mode_4='2G/3G Auto (fallback)' | ||
| uci set hh4xmodem.network.mode_5='3G/4G Auto' | ||
| uci set hh4xmodem.network.mode_6='2G/3G/4G Auto' |
There was a problem hiding this comment.
nit: these network.mode_N labels are written to UCI but never read — dashboard.js hardcodes its own networkModes array (dashboard.js:242-250)`` for the mode selector. The two also disagree (e.g. mode 0 here is 2G Only vs `2G Only (GSM)` there). Either consume these UCI labels in the view or drop them to avoid drift.
Generated by Claude Code
| /* | ||
| * sms.js - SMS inbox viewer for HH4xModem | ||
| * Copyright (C) 2026 HH4xModem Contributors | ||
| * Licensed under the GNU General Public License v2. |
There was a problem hiding this comment.
nit: this header declares GPLv2, but the package is Apache-2.0 (Makefile sets PKG_LICENSE:=Apache-2.0, and the other source files carry the Apache-2.0 notice). calllog.js:4 has the same GPLv2 line. Please make the per-file license consistent with the package license.
Generated by Claude Code
| createInfoRow('CGI', signal.CGI || '--') | ||
| ]) | ||
| ]), | ||
| E('div', { 'id': 'poll-debug', 'style': 'text-align:center;font-size:0.8em;color:var(--text-color-medium);padding:0.5em' }, ['.']) |
There was a problem hiding this comment.
nit: the poll-debug element only shows a polling timestamp / ERR string and looks like leftover debugging UI. Consider removing it (and the dbg updates in startPolling) before merge.
Generated by Claude Code
4e41c93 to
e2d9523
Compare
|
All review points have been addressed in the last commit. Please take another look. |
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the changes since 5ed0a2d. The prior nits are all resolved (license headers now Apache-2.0, poll-debug element removed, managed poll.add replacing the raw setInterval, network.mode_N UCI labels dropped, and modem_ip/modem_port are now read by the ucode backend via get_modem_env). The refactor introduced a few new issues, flagged inline — most notably a command-injection regression in get_sms_data where the previously-quoted key arg is now interpolated unquoted into a shell command.
Generated by Claude Code
| function get_sms_data(key, page) { | ||
| key = key || 'inbox'; | ||
| page = page || 1; | ||
| return run_script(sprintf('/usr/bin/hh4xmodem-get-sms %s %d', key, page)); |
There was a problem hiding this comment.
Command injection: key is interpolated into the shell command via %s without quoting. It comes straight from the RPC request args (get_sms_data → request.args.key, exposed in the read ACL), so any session with read access to this app can pass e.g. inbox; reboot or $(...) and have it executed by the shell popen spawns. The previous revision quoted this (shellquote(key)); this refactor dropped the quoting. get_calllog_data is fine because both of its args go through %d/int().
| return run_script(sprintf('/usr/bin/hh4xmodem-get-sms %s %d', key, page)); | |
| return run_script(sprintf('/usr/bin/hh4xmodem-get-sms %s %d', shellquote(key), page)); |
Generated by Claude Code
| function delete_sms(id) { | ||
| id = int(id || 0); | ||
| if (id < 1) return { error: 'Invalid SMS ID', code: -1 }; | ||
| let r = pack_call('DeleteSMS', sprintf('{"DelFlag":3,"SMSArray":[%d]}', id)); |
There was a problem hiding this comment.
This now reports success: true even on failure. pack_call never returns null — every path returns an object, including its error objects ({ error: ..., code: -1 }). So r != null is essentially always true, and a failed delete (empty/unparseable response, no JSON, etc.) is masked as success. The previous revision tested r?.result != null, which correctly distinguished success from an error object. Check for the absence of an error key instead:
| let r = pack_call('DeleteSMS', sprintf('{"DelFlag":3,"SMSArray":[%d]}', id)); | |
| let r = pack_call('DeleteSMS', sprintf('{"DelFlag":3,"SMSArray":[%d]}', id)); | |
| if (r != null && r.error == null) return { success: true }; | |
| return r || { error: "Delete failed", code: -1 }; |
Generated by Claude Code
| let params = sprintf('{"NetworkMode":%d,"NetselectionMode":%d,"NetworkBand":%d,"DomesticRoam":%d,"DomesticRoamGuard":%d}', | ||
| mode, cur.NetselectionMode ?? 0, cur.NetworkBand ?? 255, cur.DomesticRoam ?? 0, cur.DomesticRoamGuard ?? 0); | ||
| let r = pack_call('SetNetworkSettings', params); | ||
| if (r != null && type(r) == 'object') return { success: true, mode: mode, response: r }; |
There was a problem hiding this comment.
Same success-masking issue as delete_sms: pack_call always returns a non-null object, and its error objects ({ error: ..., code: -1 }) satisfy r != null && type(r) == 'object', so a failed SetNetworkSettings is reported as success: true with the error tucked into response. Gate on the absence of an error key, e.g. if (r != null && type(r) == 'object' && r.error == null).
Generated by Claude Code
|
|
||
| render: function (data) { | ||
| var modemData = data[0]; | ||
| var refreshInterval = parseInt(data[1]) || 3000; |
There was a problem hiding this comment.
nit: refreshInterval is passed to startPolling → poll.add(fn, interval), whose interval is in seconds (the UCI default refresh_interval='5' is 5 s, which is correct). But the fallback here is 3000, i.e. 3000 seconds (~50 min) when UCI is unset/unreadable — this looks like a leftover of the old hardcoded 3000 ms setInterval. A sane default would be a handful of seconds, e.g. parseInt(data[1]) || 5.
Generated by Claude Code
f955618 to
e44cfc6
Compare
|
All review points from the last round have been addressed:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
e44cfc6 to
b123f93
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the changes since e2d9523 (the branch was squashed/force-pushed; reviewed the net diff against the prior head). The earlier success-masking fixes (delete_sms, set_network_mode now gate on r.error == null) and the refresh_interval seconds-unit fix are correctly applied, and delete_sms_bulk plus its ACL entry look consistent with the JS caller. One new issue flagged inline: the get_sms_data rewrite swaps shell injection for malformed JSON by reusing shellquote to build a JSON params string.
Generated by Claude Code
| function get_sms_data(key, page) { | ||
| key = key || 'inbox'; | ||
| page = page || 1; | ||
| let sms = pack_call('GetSMSListByContactNum', sprintf('{"Page":%d,"key":%s}', page, shellquote(key))) || { SMSList: [] }; |
There was a problem hiding this comment.
This produces malformed JSON. shellquote wraps the value in single quotes ('inbox'), but this string is a JSON params payload, not a shell argument: hh4xmodem-pack splices it verbatim into the JSON-RPC envelope as "params":%s (hh4xmodem-pack:20), yielding {...,"key":'inbox'} — invalid JSON the modem's parser rejects. This fires even for the default key='inbox', so SMS listing is broken, not just the injection case the previous round flagged. Encode the string as JSON with %J instead of shell-quoting it (ucode's %J printf format emits a properly escaped double-quoted JSON string):
| let sms = pack_call('GetSMSListByContactNum', sprintf('{"Page":%d,"key":%s}', page, shellquote(key))) || { SMSList: [] }; | |
| let sms = pack_call('GetSMSListByContactNum', sprintf('{"Page":%d,"key":%J}', page, key)) || { SMSList: [] }; |
The same shellquote-in-JSON construction is on line 95 in get_sms_list — apply the same %J fix there.
Generated by Claude Code
Fix get_sms_data and get_calllog_data by replacing run_script with direct pack_call to avoid rpcd popen data retrieval failures. Drop now-unused hh4xmodem-get-sms and hh4xmodem-get-calllog shell scripts. Add adaptive signal metrics display: show RSRP/RSSI/RSRQ/SINR for LTE, RSCP/RSSI/RSRQ/EcIo for 3G based on NetworkType. Add bulk SMS delete with checkbox selection, select-all, and delete-selected toolbar in the SMS view. Fix malformed JSON by replacing %s+shellquote() with %J format in sprintf JSON payloads — shellquote wraps in single quotes (invalid JSON), %J emits properly quoted/escaped JSON strings. Also fix delete_sms and set_network_mode success masking checks and set refresh_interval default to 3 seconds (poll.add unit). Signed-off-by: Devon Openheim <devopenm@proton.me>
9e39b53 to
b3d85eb
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed the net change since b123f93 (the previously-malformed-JSON fix). The three sites in hh4xmodem.uc (get_sms_data, get_sms_list, send_sms) now build their JSON params with the %J ucode format directive instead of %s + shellquote(). %J JSON-encodes and escapes the value, so key/phone/content produce valid, properly-quoted JSON strings — this correctly resolves the prior finding, and no sites were missed. No new issues introduced; the commit message matches the change.
Generated by Claude Code
Description
New LuCI application to monitor and control the MDM9207 modem found in HH40V and HH41V routers.
Features:
Communication with the modem is done via serial AT commands through
rpcd+ucode-mod-socket. All backend scripts are in/usr/bin/hh4xmodem-*.This is an initial submission — no prior package exists.
Why this approach is better than AT Commands
AT commands are sequential — one command, one response, one at a time. Querying 20 modem parameters (network info + signal + SMS + call log) requires 20 round-trips over a serial line at 115200 baud, taking 8–12 seconds.
The pack protocol bundles multiple queries into a single TCP request and returns all results in one JSON response. Our hh4xmodem-pack helper retrieves 15+ data points in under 2 seconds — a 5–6× speedup.
AT responses are unstructured text: +CSQ: 15,99, +CREG: 0,1, etc. Every app reinvents fragile regex parsing. The pack protocol returns structured JSON — no parsing, no edge cases, no locale-dependent issues.
The MDM9207 exposes its management interface over TCP (192.168.225.1:2016). Serial AT over /dev/ttyUSB2 is:
TCP is reliable, fast, and multi-client safe.
We bypass the shell entirely. The ucode backend (hh4xmodem.uc) runs inside rpcd, opens a TCP socket to the modem via ucode-mod-socket, and returns data directly to LuCI. No scripts, no pipes, no temporary files, no popen().
LuCI View (JS) → rpcd (ucode) → TCP socket → modem core_app
Each layer is thin, testable, and replaceable. Adding a new API endpoint takes one line in the backend.
###Screenshot

Maintainer
@devopnem
Tested on
OpenWrt version: OpenWrt 25.12.4
LuCI version: LuCI openwrt-25.12 branch
Web browser(s): librewolf 151.0.1-2
Checklist
Signed-off-by: Devon Openheim <devopenm@proton.me>row.<package name>: titlefirst line subject for packages.PKG_VERSIONin the Makefile. (N/A — initial package, no prior version)