librespeed-common: add measurement backend

[BKPepe]

📦 Package Details

Maintainer: @BKPepe

Description:

Backend for running librespeed-cli measurements on a router: a locked runner with live progress state, an rpcd ucode plugin exposing start/stop/status/result/history/config over ubus, cron scheduling within a configurable time window, and optional daily min/avg/max aggregation of JSONL history.

Live progress uses --json-stream, proposed for the Go client in librespeed/speedtest-cli#145 (the Rust port will ship the same flag once reviewed and merged in GO) with a client without it the runner falls back to interface counter sampling.

Companion frontend: openwrt/luci#8951.

cc @shenek: This covers similar ground as the Foris Controller librespeed module;. The server list URL is configurable (option server_list), so a Turris deployment can keep using https://librespeed.turris.cz/servers.json.
cc @aleksan4eg: the reForis integration could be reworked on top of this ubus API.

---

🧪 Run Testing Details

• OpenWrt Version: SNAPSHOT
• OpenWrt Target/Subtarget: mpc85xx/p2020
• OpenWrt Device: Turris 1.x

Compile tested for powerpc_8548. Run tested on the device: measurements over ubus and LuCI (start/stop/status/result), streamed progress and the interface-counter fallback, scheduled runs within an hour window, history with daily aggregation, and server list caching including a custom server_list URL.

---

✅ Formalities

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

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[openwrt-ai]

Reviewed the single commit adding utils/librespeed-common; the message matches what the diff does.

Two findings look like they block a working install rather than taste: rpcd is never told about the new ucode plugin (postinst), and the cron entry is written without ever starting crond on a router that has no other cron job. The rest — grep -q in test.sh, the missing jsonfilter dependency, and the silent archive write — are smaller and inline.

Nothing to say about the packaging shape otherwise: the source-less Build/Prepare is fine ($(PKG_BUILD_DIR) is created by $(STAMP_PREPARED) before it runs), conffiles is unindented as it must be, and flock / setsid / start-stop-daemon are all busybox defaults. CI is green on 44917b1.

---

Generated by Claude Code

[openwrt-ai]

Guarding on running means the schedule silently never fires on a router that has no other cron job. busybox's cron init refuses to start when the crontab dir is empty — package/utils/busybox/files/cron:15 does [ -z "$(ls /etc/crontabs/)" ] && return 1 — so on a stock system crond is not running at all. sync_cron then writes the entry, restart_cron finds cron not running and does nothing, and the measurement only starts happening after the next reboot.

restart starts a stopped procd service, and it is what the other crontab-writing packages in this feed do (net/safe-search/Makefile:59, net/wg-installer/wg-server/config/wgserver.init:11):

Suggested change

	/etc/init.d/cron running && /etc/init.d/cron restart
	/etc/init.d/cron restart >/dev/null 2>&1

---

Generated by Claude Code

[openwrt-ai]

grep -q is not wanted in test.sh / test-version.sh: swallowing the matched line means the CI log shows only pass/fail and not the version string that was actually found, which is the one thing you want to read when the check fails.

Suggested change

	/usr/libexec/librespeed-run --version | grep -q librespeed-common \
	|| fail "librespeed-run --version"
	/usr/libexec/librespeed-run --version | grep librespeed-common \
	|| fail "librespeed-run --version"

Same on lines 16-17 for librespeed-aggregate. Line 23 has the same visibility problem for a different reason: ucode ... >/dev/null 2>&1 throws away the syntax/import error, so a plugin that stops loading fails the test with no indication why. Dropping 2>&1 there keeps ucode's message in the log.

---

Generated by Claude Code

[openwrt-ai]

nit: librespeed-run parses the --json-stream events with jsonfilter (lines 197-208), but nothing here depends on it. jsonfilter is only pulled onto an image implicitly, via busybox's +BUSYBOX_CONFIG_NTPD:jsonfilter — on a build with ntpd disabled it is absent, and then the streaming path never extracts reports.json and every run ends in fail. Eight packages in this feed declare it explicitly:

Suggested change

	DEPENDS:=+librespeed-cli +rpcd-mod-ucode
	DEPENDS:=+librespeed-cli +rpcd-mod-ucode +jsonfilter

---

Generated by Claude Code

[openwrt-ai]

nit: archive_path is free-form UCI pointing at persistent storage the user chose, and nothing creates its directory — the runner only does mkdir -p for the raw history path. If the user sets e.g. /srv/librespeed/archive.jsonl before the mount point exists, writefile returns null, the branch is skipped and the cron job goes on doing nothing every night with no trace anywhere. A mkdir of dirname(archive_path) before the write, or at least a warn() on the failed write, would make that diagnosable.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the amended commit (44917b1 → c71a21c). The four earlier findings are all addressed: postinst/postrm reload rpcd (matching what rpcd's own Makefile and luci.mk do), +jsonfilter is declared, restart_cron no longer guards on running, and test.sh keeps grep/ucode output in the log. The commit message still matches the diff.

Nothing in the new hunks blocks a merge. Two remarks inline on the archive write in librespeed-aggregate, both secondary: the new mkdir -p changes behaviour when the target storage is not mounted, and the new warn() lands on a stderr that cron discards. CI is green on c71a21c.

---

Generated by Claude Code

[openwrt-ai]

nit: this script only ever runs from cron (librespeed.init installs 5 0 * * * $AGG), and a cron job's stderr has nowhere to go on a stock system: busybox is built without sendmail reporting (BUSYBOX_DEFAULT_FEATURE_CROND_CALL_SENDMAIL is n), so crond neither mails nor logs the job's output, and the cron init sets no procd_set_param stderr 1, so the instance's stderr — inherited by the job — is not forwarded to logd either. The warning is therefore visible only when someone runs librespeed-aggregate by hand, which is exactly the case that was already diagnosable.

librespeed-run already uses syslog for the same purpose from the same cron context (librespeed-run:94), so the tag is established:

Suggested change

	// Atomic: a reader never sees a half-written archive. A failed write says
	// so on stderr rather than skipping the night's flush in silence.
	if (writefile(tmp, out) != null)
	rename(tmp, archive_path);
	else
	warn(`librespeed-aggregate: cannot write ${tmp}\n`);
	// Atomic: a reader never sees a half-written archive. A failed write goes to
	// syslog -- cron gives the job's stderr nowhere to go.
	if (writefile(tmp, out) != null)
	rename(tmp, archive_path);
	else {
	warn(`librespeed-aggregate: cannot write ${tmp}\n`);
	system(['logger', '-t', 'librespeed', `aggregate: cannot write ${tmp}`]);
	}

---

Generated by Claude Code

[openwrt-ai]

mkdir -p creates the whole missing tree, which changes what happens in the case this option exists for: archive_path points at external storage (/mnt/sda1/librespeed/archive.jsonl, /srv/...) and the mount is not up — at boot before block-mount, or after the disk was pulled. Previously the write failed and nothing was written; now the directory is created on the overlay and every night's aggregate is appended to internal flash instead, invisibly, and gets shadowed once the disk is mounted again.

Creating only the last component (fs.mkdir(dir), which fails when the parent is missing) keeps the "user prepared this location" property while still fixing the case you're after, i.e. an existing mount point with no subdirectory yet. Is writing to the overlay when the target storage is absent intended here, or would the non-recursive form be preferable?

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the amended commit (c71a21c → 4c3efab). The diff is limited to the archive write in librespeed-aggregate, and both remarks from the last round are resolved:

• system(['mkdir', '-p', dir]) → mkdir(dir, 0o755), so a missing mount no longer gets a directory tree built on the overlay; the comment above it states the reasoning. mkdir is an fs export and the 0o literal parses — both are in use in-tree (unet.uc:162, luci.openvpn.uc:80).``
• The failed write now goes to syslog instead of a stderr that cron discards. Dropping warn() in favour of logger alone is a deliberate call the comment spells out, so I'm not re-raising it.

The failure chain reads correctly end to end: mkdir fails silently when the parent is absent, writefile then returns null, and the logger branch makes it visible. The commit message is unchanged and still matches the diff.

One optional nit inline on the log message. CI on 4c3efab is clean — nine build targets green, three cancelled by the concurrency group rather than failing, formality checks green.

---

Generated by Claude Code

[openwrt-ai]

nit (optional): the log line names the file but not why the write failed, and the three causes this hunk exists for want different actions from the admin — mount not up (ENOENT), wrong owner or a read-only filesystem (EACCES/EROFS), full disk (ENOSPC). All three produce the identical cannot write /mnt/.../archive.jsonl.tmp every night.

ucode's fs module keeps the reason in error(); luci.openvpn.uc:5 imports it alongside the rest of the module. Adding error to the import on line 15 and interpolating it here:

system(['logger', '-t', 'librespeed', `aggregate: cannot write ${tmp}: ${error()}`]);

No suggestion block since the import on line 15 has to change too. Nothing here blocks a merge.

---

Generated by Claude Code

[openwrt-ai]

Reviewed 1 new commit; no new issues found.

The amended commit (4c3efab → e8037f7) touches only the failed-write log in librespeed-aggregate, and it is the optional nit from the last round: error joins the fs import and the reason is interpolated into the logger line. Checks that mattered:

• error is a real fs export and the bare-import form is in use in-tree — base64decode.uc:3 imports it the same way, so no fs. prefix is needed.
• The call site is correct: error() consumes and clears the stored errno, and it is read once, immediately after the writefile that failed. The mkdir two lines up cannot leak a stale EEXIST into it, since every writefile failure path sets its own error first.
• The wrapped system([...]) continuation is tab-indented like the rest of the file, and no line in the file exceeds 80 columns.

The commit message is unchanged and still matches the diff. CI on e8037f7 is fully green — ten build targets and all three formality checks.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the amended commit (e8037f7 → 1d65703). A bigger round than the last few: the retention regex fix, value-checked cron fields, sampler/trap lifecycle, a hand-rolled archive write, and an expanded DEPENDS. The commit message still describes what the diff does.

What I checked and found correct:

• The retention fix is the real one here. json_dump goes through libubox's blobmsg_format_json, which emits "epoch": 1755… with a space, so the old match($0, /"epoch":[0-9]+/) never fired and history was never trimmed. Extracting the digits instead of counting from RSTART + 8 fixes it, and pinning the fixture in test.sh to json_dump's own output means the check breaks if either side moves. [[:space:]] is fine for busybox awk.
• New dependencies all name real packages: jshn (package/libs/libubox/Makefile:118), and ucode / ucode-mod-fs / ucode-mod-uci from the UcodeModule calls in package/utils/ucode/Makefile:136-196. ucode is needed on its own account for the two #!/usr/bin/env ucode scripts.
• The archive write. ucode's write() already returns null rather than a short count, flush() returns null on a failed fflush(), and neither a successful flush() nor close() clears the module's stored error — so error() on line 198 still carries the write's reason, and it is read before unlink(tmp) can replace it. length() on a ucode string is a byte count, so it matches what write() returns. Both flush() and unlink are in use in-tree (unet.uc:41, dispatcher.uc:4). Dropping writefile from the import list leaves no other user behind.
• cron_next's skips land on the first minute of the next hour or day and never past it — the minimum skip is 1, and t += (skip - 1) * 60 plus the loop's own t += 60 is exactly skip minutes. Under normal time no candidate minute is missed. One caveat inline.
• valid_field restores IFS on the first loop iteration, ahead of every early return, and correctly rejects empty parts, 1-2-3, 1,,2 and out-of-range values. One rough edge inline.
• The sampler guard is sound: $$ in a background subshell is still the runner's pid in ash, so kill -0 $$ is the right liveness test, and the trap's kill "$sampler_pid" covers the kill -TERM <pid> fallback in stop where no process-group signal reaches the sampler.

Three inline remarks, none of which block a merge: one that makes a reload print a shell error on a malformed days/hours value, and two nits. CI on 1d65703 is fully green — ten build targets and all three formality checks.

---

Generated by Claude Code

[openwrt-ai]

A part with one empty side gets past this check and then reaches the arithmetic test with an empty operand. ${part%-*} leaves a empty for -5, ${part#*-} leaves b empty for 5-, and concatenating the two hides that from the '' arm — "$a$b" is 5 in both cases. Running the function as written under dash:

5-       => vf.sh: 8: [: Illegal number:
BAD
-5       => vf.sh: 8: [: Illegal number:
BAD

The outcome is still correct (return 1, so the field falls back to * / empty), but a option days '5-' typo makes /etc/init.d/librespeed reload print a shell error for what line 96 describes as a silent fallback to the harmless default. Checking the halves separately keeps it quiet:

Suggested change

	case "$a$b" in ''|*[!0-9]*) return 1 ;; esac
	case "$a" in ''|*[!0-9]*) return 1 ;; esac
	case "$b" in ''|*[!0-9]*) return 1 ;; esac

---

Generated by Claude Code

[openwrt-ai]

nit: the awk consumes $line and prints nothing, so when this check trips the CI log carries only FAIL: retention regex does not match jshn output — not what json_dump actually emitted, which is the one thing needed to tell "jshn changed shape" from "the regex is wrong". That is the same reason grep -q is discouraged in these scripts, and the fixture is exactly the value worth keeping in the log. Adding a bare print rule keeps awk as the engine under test:

Suggested change

	echo "$line" | awk 'match($0, /"epoch":[[:space:]]*[0-9]+/) { ok = 1 }
	END { exit !ok }' || fail "retention regex does not match jshn output"
	echo "$line" | awk 'match($0, /"epoch":[[:space:]]*[0-9]+/) { ok = 1 }
	{ print }
	END { exit !ok }' || fail "retention regex does not match jshn output"

---

Generated by Claude Code

[openwrt-ai]

nit (optional): this is 1440 - (hour * 60 + min) local-clock minutes, but it is added as that many real minutes on line 153, so it lands on midnight only when the skipped day is 1440 minutes long.

The two directions are not symmetric, which is why I think the comment above is half right:

• Fall back (25 h day): the jump lands at 23:00 of the same day, the weekday still misses, the next skip is recomputed as 60 and it self-heals — "only shortens one skip", as the comment says.
• Spring forward (23 h day): the skipped day is 1380 real minutes, so t lands at 01:00 of the next day, and minutes 00:00–00:59 of that day are never examined at all.

Concretely, with DST transitions falling on a Sunday: a schedule whose weekday field excludes Sunday and whose time is in hour 0 (0 0 * * 1, which cron_expr's 1d branch can draw) loses one row from next_runs once a year. Only the prediction is affected — crond itself still fires — so this is cosmetic in effect, but the comment reads as if the case were covered.

Ending the day jump at 23:00 rather than midnight and letting the hour branch walk the last hour would cost one extra iteration per skipped day and could not overshoot; it needs a floor so hour 23 does not produce a non-positive skip. Is the day-level jump worth that, given count is only ever 3 here?

---

Generated by Claude Code

[openwrt-ai]

Reviewed 1 new commit; no new issues found.

The amended commit (1d65703 → d531187) is the three remarks from the last round and nothing else. All three land correctly:

• The day jump (librespeed.uc:145-149). I re-implemented the loop against localtime and cross-checked it minute-by-minute with a brute-force walk over 2560 (zone, schedule, start time) combinations in eight zones. The old form missed Mon 2026-03-09 00:00 for 0 0 * * 1 in America/New_York — the row right after the spring-forward Sunday — and the new one returns it; fall-back dates and non-DST zones are unaffected. The only residual mismatch in the sweep is Antarctica/Troll, whose spring-forward is two hours, so the overshoot clears 00:00 and lands at 01:00. That is outside what the comment claims ("lands an hour long") and I would not spend code on it.
• The skip <= 0 floor can only yield 1..60, so hour 23 always advances and the loop cannot stall; i += skip still accounts for the same number of minutes per skipped day, so the five-week horizon is unchanged.
• valid_field (librespeed.init:30-31). Ran the function standalone under dash and bash: 5-, -5, 0-, 1-2-3, 1,,2, 3-1, 7, 1, 2 all return 1 with an empty stderr now, and 5, 00, 1-3, 1,3,5, 0-6 still pass. Splitting the case does not disturb the IFS restore — it still runs on the first iteration, ahead of every early return, and the for list was split before the loop body ran anyway. An empty val returns 0, but sync_cron rewrites an empty days to * and skips the call entirely for an empty hours, so that path is unreachable.
• test.sh:32. The bare { print } rule sits between the match rule and END, so json_dump's output reaches the CI log on both outcomes and awk stays the engine under test.

The commit message is unchanged and still matches the diff. CI on d531187 is fully green — ten build targets and all three formality checks.

---

Generated by Claude Code