cli: ask majestic to apply a setting, instead of only writing it down - #2366
Merged
Conversation
`cli -s .video0.fps 20` edited /etc/majestic.yaml, printed nothing and exited
0, while the camera carried on streaming at the old rate. Whether a reload was
needed depended on the page you happened to read: of the wiki pages that change
a setting, ten say nothing, four say restart and one says reboot.
majestic already re-reads the file on SIGHUP, diffs it against the tree in
force and prices the difference on its own ladder -- a live key is pushed
straight at the SDK, a service key restarts only that service, and only a
pipeline key rebuilds -- so most settings now apply without the stream
noticing. Repeat signals inside 3s collapse into one reload, so a burst of
writes still reloads once. This is the pattern wifibroadcast already applies
by hand after its own writes; it is only being made the default.
The signal is conditional on majestic already running, which is what keeps
provisioning untouched: customizer.sh seeds a device at S30 and majestic does
not start until S95, so those writes take exactly the path they always did,
silently. Reads and an explicit -i/-o are left alone -- sensor_cli and the wfb
applet both address other files through them.
Also refuse a malformed setting path rather than storing it. Writing the file
cannot fail, so `cli -s .video0.bitrate: 4000` recorded the trailing colon and
said nothing; that line is in t40_lite_movols-mo-805p's customizer.sh and is
why six of that device's settings have never applied.
Routing the writes through the HTTP API was the other candidate and was
measured on a lab camera first: of the fourteen keys wifibroadcast seeds,
six answer 404 -- including .outgoing.wfb, which gates the wfb relay -- because
they are read from the config but never declared in the schema. It would also
have had to guess whether majestic was up, and S98wifibroadcast and S98datalink
both run after S95majestic, where the answer is "starting".
CMD now comes from ${0##*/}: `cut -d / -f 4` needs a path of exactly four
fields, so ./cli fell through to the silent default. The api) applet goes --
nothing creates /usr/sbin/api, and --data-urlencode "$@" only ever bound $1.
widgetii
requested review from
cronyx,
flyrouter and
viktorxda
as code owners
September 5, 2026 09:46
PR Summary by QodoApply CLI configuration writes to running Majestic
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
pidof answers that the process exists, which is not the same as being ready to be signalled. S95majestic starts it with start-stop-daemon -b, so the call returns at the fork and boot carries straight on into the S98 scripts -- wifibroadcast writes fourteen keys there -- while majestic is still working its way down main() towards init_libevent(), which is where the SIGHUP handler is installed. Until it gets there the default action applies, and for SIGHUP that is to terminate. So a reload asked for in that window does not reload the camera, it takes the streamer away for the rest of the boot. /proc/<pid>/status answers the question exactly rather than by timing guess: SigCgt is the caught-signal mask, SIGHUP is signal 1 and therefore its lowest bit, so the mask ends in an odd hex digit precisely when the handler is in place. Signal only those pids, and a write that lands early simply does not reload -- which is what it did before this branch existed. Measured on a hi3516ev200. The process is visible to pidof with SigCgt still 0000000000000000, and reaches 0000000000004007 once the handler is installed. Firing 25 writes straight into that window: with this check majestic is still alive afterwards and streaming; with an unconditional killall -1 in the same window it is dead. Worth noting the hazard is older than this branch -- wifibroadcast:82 and :212 already send killall -1 at S98, and the wiki has long told people to run killall -HUP majestic by hand. Majestic ignoring SIGHUP until it can handle it would fix the class for every sender; this fixes the one the tree controls.
This was referenced Sep 5, 2026
openipc-ai
enabled auto-merge (squash)
September 5, 2026 11:10
widgetii
added a commit
to OpenIPC/wiki
that referenced
this pull request
Sep 5, 2026
OpenIPC/firmware#2366 made `cli -s` write /etc/majestic.yaml and then ask Majestic to reload, so the pages telling people to prefer the HTTP API because `cli` does not apply anything are describing firmware that no longer exists. `cli` and the API now do the same job, and each is better at something: `cli` is the one that works before Majestic is running, which is the whole of first boot, and it can write a key this build does not declare; the API validates, and can be called from another machine. Three claims were wrong rather than merely stale: The SIGHUP row said a reload tears the pipeline down and builds it again. It has not meant that for a while -- Majestic prices the difference and takes the cheapest option that carries it, which is why setting a bitrate no longer costs the stream. Rewritten to describe the ladder. The plugin table attributed reloading to who sent the signal, "what the WebUI and cli use to apply a change". Both produce either outcome depending on the key, so the row now splits on what actually decides it: a rebuild reloads the plugin, an in-place change does not. And majestic-config.md now says why a saved file stays short. widgetii/majestic PR 608 stopped Majestic writing its own defaults into majestic.yaml, so a value you never set is absent rather than frozen at whatever the default was when you last pressed Save -- which is what lets a corrected default reach a camera that has already been configured.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cli -s .video0.fps 20edits/etc/majestic.yaml, prints nothing and exits 0 — and thecamera carries on streaming at the old rate. Nothing in the tool says a reload is needed,
and the wiki is not consistent about it either: of the pages that change a setting, ten say
nothing about restarting, four say restart, and one says reboot.
Writing the file also cannot fail, so a malformed setting path is stored rather than
reported. The tree ships one:
builder/devices/t40_lite_movols-mo-805p/general/overlay/usr/share/openipc/customizer.sh:15-19,25says
cli -s .video0.bitrate: 4000, and the trailing colon is why six of that device'ssettings have never applied. Nobody noticed, because there was nothing to notice.
Shows on every board —
general/overlay/usr/sbin/extutilsis in the shared overlay.What this does
cli -s/cli -dstill write the file, and then ask majestic to reload if it is running.signal_hup_cbalready parses before tearing anything down, diffs the file against the treein force, prices the difference on the
CFG_RELOAD_*ladder — a live key goes straight atthe SDK, a service key restarts only that service, and only a pipeline key rebuilds — and
debounces repeat signals inside 3s into one reload. So most settings now apply without the
stream noticing. This is the pattern
wifibroadcast:82and:212already use by hand;it is only being made the default.
The signal is conditional on majestic already running, which is what keeps provisioning
untouched:
customizer.shseeds a device at S30 and majestic does not start until S95, sothose writes take exactly the path they always did, silently. Reads and an explicit
-i/-oare left alone, so
sensor_cliand thewifibroadcast cliapplet are unaffected.Plus a key-shape check that refuses a malformed path before the write,
CMDfrom${0##*/}(
cut -d / -f 4needs a path of exactly four fields, so./clifell through to the silentdefault), and removal of the dead
api)applet — nothing creates/usr/sbin/api, and its--data-urlencode "$@"only ever bound$1.Why not route the writes through the HTTP API
That was the first design, and it was measured on a lab camera before being abandoned.
Of the fourteen keys
wifibroadcast'svideo_settingsseeds, six answer 404 from/api/v1/set—outgoing.wfb,records.notime,fpv.noiseLevel,fpv.enabled,isp.exposure,video0.noiseLevel. They are read by majestic but never declared in itsschema.
.outgoing.wfbgates the wfb relay, so this would have broken video on everywifibroadcast camera. The file path is forward- and backward-compatible across a key rename;
the API path is not, and
wifibroadcast:183-188says so in a comment.It would also have had to guess whether majestic was up, and
S98wifibroadcastandS98datalinkboth run afterS95majestic, where the honest answer is "starting".Hardware tested on
hi3516ev200 (lab camera, OpenIPC 2.6.04.16, majestic master+a805052). The new
extutilswascopied over the shipped one, exercised, and the camera restored afterwards —
extutilsandmajestic.yamlmd5s both back to their originals.Evidence
Before — the write lands in the file, the running camera ignores it, and a malformed path is
stored without complaint:
After:
Off-camera gates:
Follow-ups this turned up
zoom.shsendskillall -10 majestic, which is SIGUSR1, caught by the bundledthread pool's
thread_hold()and never released. Those nine calls should be deleted ratherthan corrected, since
cli -snow asks for the reload itself.builder'st40_lite_movols-mo-805pstill has the six trailing colons; that is a fix forthat repo, and this change is what makes them visible.
Scope
general/package/all-patches/linux/general/overlay/or in a sharedload_<vendor>script hardcodes a value specific to my boardLD_PRELOAD, and no binaries that cannot be rebuilt from source