fix(webconfig): publish the board probe safely and test the gap
Codex review of the previous commit found two real problems. The routes go live in startLanMode()/startSetupMode(), one tick before probeBoardCommands() first runs, so a status request can land in between. That made _board_cmds a genuine cross-task race despite the comment claiming otherwise, and — worse — let /api/status answer "no board commands" before the board had been asked. The page reads status once, so a browser that loaded in that window hid working FEM controls for the whole session. The probe result is now published and read under _mux, and /api/status OMITS board_cmds until the probe has run, which is not the same as reporting an empty list. The page treats the absent field as "not known yet" and re-polls with backoff; the audit waits for it the same way, rather than auditing a board it only half knows. Probing from the start path instead would close the window, but Callbacks::execCommand is documented as loop-task-only and `start webconfig` is itself mid-handleCommand, so that trades a race for re-entrancy. The audit never executed fan.lo or fan.hi, which have no getter and so no round-trip: they were only ever matched as strings. They now run for real, including the four rejection paths, and the mock grew --probe-delay so the retry path is exercised rather than assumed.
This commit is contained in:
1 parent
3d2d0b5da6
commit
2c057b0fc6
5 files changed
+106
-18
No files matched your search
@@ -12,6 +12,13 @@ that come back an error, so the two stay honest about each other.
|
||||
python3 scripts/webconfig_mock_server.py --port 8137 &
|
||||
python3 scripts/webconfig_cli_audit.py
|
||||
|
||||
Board-specific commands (Board::handleCommand) are offered only when the node
|
||||
says it answers them, so one run exercises one shape of board. The mock claims
|
||||
none by default, like a Heltec V3; drive the other shapes with --board-cmds:
|
||||
|
||||
python3 scripts/webconfig_mock_server.py --port 8137 \
|
||||
--board-cmds radio.fem.rxgain,radio.fem.txgain,fan &
|
||||
|
||||
Exits non-zero if anything fails that is not in EXPECTED_FAILURES. Stdlib only.
|
||||
"""
|
||||
|
||||
@@ -169,6 +176,21 @@ BOARD_ROUND_TRIPS = {
|
||||
"fan": [("set fan on", "get fan", "on 41.0C fan=on cd=0s")],
|
||||
}
|
||||
|
||||
# Board commands with no getter to read back, so a round-trip cannot reach them.
|
||||
# Run in order, and drive the rejection path too: a set-only key that silently
|
||||
# accepted anything would look identical to one that works.
|
||||
# [(command, should_succeed, expected substring of the reply)]
|
||||
BOARD_SET_PROBES = {
|
||||
"fan": [
|
||||
("set fan.lo 40", True, "OK"),
|
||||
("set fan.hi 70", True, "OK"),
|
||||
("set fan.lo 200", False, "0..100"), # out of range
|
||||
("set fan.lo 90", False, "< fan.hi"), # crosses the upper bound
|
||||
("set fan.hi 10", False, "> fan.lo"), # crosses the lower bound
|
||||
("set fan.hi 130", False, "<= 120"), # out of range
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
class Client:
|
||||
def __init__(self, base):
|
||||
@@ -179,8 +201,17 @@ class Client:
|
||||
# rather than hardcoding a number that drifts when the slot is resized.
|
||||
status = json.load(self._open("/api/status"))
|
||||
self.max_cmds = status.get("max_cmds", 24)
|
||||
# Board::handleCommand() commands this node probed for at startup.
|
||||
self.board_cmds = [c for c in status.get("board_cmds", "").split(",") if c]
|
||||
# Board::handleCommand() commands this node probed for at startup. The
|
||||
# field is absent until the probe has run, which is NOT the same as an
|
||||
# empty list, so wait for it rather than audit a board half-known.
|
||||
for _ in range(20):
|
||||
if "board_cmds" in status:
|
||||
break
|
||||
time.sleep(0.25)
|
||||
status = json.load(self._open("/api/status"))
|
||||
else:
|
||||
sys.exit("node never reported board_cmds; it has not probed its board")
|
||||
self.board_cmds = [c for c in status["board_cmds"].split(",") if c]
|
||||
|
||||
def _open(self, path, data=None):
|
||||
headers = {"Content-Type": "application/json"}
|
||||
@@ -295,6 +326,16 @@ def main():
|
||||
% (getc, got, want, setr["reply"]))
|
||||
failures.append((getc, got))
|
||||
|
||||
# Set-only board commands, which no round-trip can reach.
|
||||
probes = [p for gate in cli.board_cmds for p in BOARD_SET_PROBES.get(gate, [])]
|
||||
if probes:
|
||||
print("\nset-only board commands : %d" % len(probes))
|
||||
for (cmd, want_ok, want), (_, res) in zip(probes, cli.run([p[0] for p in probes])):
|
||||
if res["ok"] == want_ok and want in res["reply"]:
|
||||
continue
|
||||
print(" FAIL %-30s %s" % (cmd, res["reply"]))
|
||||
failures.append((cmd, res["reply"]))
|
||||
|
||||
# Commands the portal refuses must be refused clearly, not run and fudged.
|
||||
print("\nrefused with a reason : ", end="")
|
||||
refused = []
|
||||
|
||||
@@ -155,6 +155,7 @@ class State:
|
||||
# Board::handleCommand() commands this "board" answers. Empty by default:
|
||||
# the mock is a Heltec V3, which implements no such hook at all.
|
||||
self.board_cmds = [c for c in args.board_cmds.split(",") if c]
|
||||
self.probe_delay = args.probe_delay
|
||||
self.cfg = default_config(args.setup)
|
||||
# latched at AP start, like WebConfigServer::_initial_setup
|
||||
self.initial_setup = args.setup and self.cfg["wifi"]["ssid"] == ""
|
||||
@@ -186,7 +187,7 @@ class State:
|
||||
return c
|
||||
|
||||
def status_json(self, authed):
|
||||
return {
|
||||
out = {
|
||||
"mode": "setup" if self.setup_mode else "lan",
|
||||
"auth": authed,
|
||||
"needs_setup": self.cfg["wifi"]["ssid"] == "",
|
||||
@@ -198,9 +199,14 @@ class State:
|
||||
"role": "Repeater", "board": "Heltec V3 (mock)",
|
||||
"uptime_s": int(time.time() - self.start),
|
||||
"runtime_slots": 6, "max_slots": 6, "active_slots": self.active_slots,
|
||||
"board_cmds": ",".join(self.board_cmds),
|
||||
"max_cmds": CLI_MAX_CMDS,
|
||||
}
|
||||
# ABSENT, not empty, until the board has been probed: the node's routes
|
||||
# go live a tick before probeBoardCommands() runs, and the page has to
|
||||
# retry rather than latch "this board has no board commands".
|
||||
if time.time() - self.start >= self.probe_delay:
|
||||
out["board_cmds"] = ",".join(self.board_cmds)
|
||||
return out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1119,6 +1125,9 @@ def main():
|
||||
ap.add_argument("--port", type=int, default=8080)
|
||||
ap.add_argument("--setup", action="store_true", help="first-boot setup wizard mode")
|
||||
ap.add_argument("--active-slots", type=int, default=5, help="server slots to expose (2 or 5)")
|
||||
ap.add_argument("--probe-delay", type=float, default=0.0,
|
||||
help="seconds to withhold board_cmds from /api/status, simulating "
|
||||
"the gap before the node has probed its board")
|
||||
ap.add_argument("--board-cmds", default="",
|
||||
help="comma list of Board::handleCommand() commands to answer, e.g. "
|
||||
"radio.fem.rxgain,radio.fem.txgain,fan (default: none, like a Heltec V3)")
|
||||
|
||||
@@ -477,6 +477,10 @@ void WebConfigServer::finalizeTeardown() {
|
||||
// not the hardware returns — starts with something else.
|
||||
//
|
||||
// Loop task only: execCommand() reaches the CLI, which the async task must not.
|
||||
// The routes are already live by the time this first runs — createServer()
|
||||
// happens in startLanMode()/startSetupMode(), one tick earlier — so the result
|
||||
// is published under _mux and read the same way, and /api/status leaves the
|
||||
// field out entirely until then rather than reporting a premature "nothing".
|
||||
void WebConfigServer::probeBoardCommands() {
|
||||
char cmd[48], reply[160];
|
||||
uint8_t mask = 0;
|
||||
@@ -486,6 +490,7 @@ void WebConfigServer::probeBoardCommands() {
|
||||
_cb->execCommand(cmd, reply);
|
||||
if (reply[0] == '>') mask |= (uint8_t)(1 << i);
|
||||
}
|
||||
WCLock lock(_mux);
|
||||
_board_cmds = mask;
|
||||
_board_cmds_probed = true;
|
||||
}
|
||||
@@ -796,17 +801,29 @@ void WebConfigServer::handleStatus(AsyncWebServerRequest* req) {
|
||||
doc["board"] = _board_name;
|
||||
// Board-specific CLI commands this board answers; the page hides the controls
|
||||
// for everything absent here rather than offering one that cannot work.
|
||||
char board_cmds[80];
|
||||
size_t n = 0;
|
||||
board_cmds[0] = 0; // the nothing-supported case
|
||||
for (size_t i = 0; i < WC_BOARD_CMD_COUNT && n < sizeof(board_cmds) - 1; i++) {
|
||||
if (!(_board_cmds & (1 << i))) continue;
|
||||
int w = snprintf(&board_cmds[n], sizeof(board_cmds) - n, "%s%s",
|
||||
n ? "," : "", WC_BOARD_CMDS[i]);
|
||||
if (w < 0) break;
|
||||
n += (size_t)w; // snprintf NUL-terminates; a truncating w stops the loop
|
||||
// ABSENT until the probe has run, which is not the same as an empty list: the
|
||||
// page must not latch "this board has none" from a status served in the gap
|
||||
// between the routes going live and the first tick.
|
||||
bool probed;
|
||||
uint8_t board_mask;
|
||||
{
|
||||
WCLock lock(_mux);
|
||||
probed = _board_cmds_probed;
|
||||
board_mask = _board_cmds;
|
||||
}
|
||||
if (probed) {
|
||||
char board_cmds[80];
|
||||
size_t n = 0;
|
||||
board_cmds[0] = 0; // the nothing-supported case
|
||||
for (size_t i = 0; i < WC_BOARD_CMD_COUNT && n < sizeof(board_cmds) - 1; i++) {
|
||||
if (!(board_mask & (1 << i))) continue;
|
||||
int w = snprintf(&board_cmds[n], sizeof(board_cmds) - n, "%s%s",
|
||||
n ? "," : "", WC_BOARD_CMDS[i]);
|
||||
if (w < 0) break;
|
||||
n += (size_t)w; // snprintf NUL-terminates; a truncating w stops the loop
|
||||
}
|
||||
doc["board_cmds"] = board_cmds;
|
||||
}
|
||||
doc["board_cmds"] = board_cmds;
|
||||
doc["uptime_s"] = millis() / 1000;
|
||||
doc["runtime_slots"] = RUNTIME_MQTT_SLOTS;
|
||||
doc["max_slots"] = MAX_MQTT_SLOTS;
|
||||
|
||||
@@ -201,8 +201,9 @@ private:
|
||||
char _stats_json[1024] = {0};
|
||||
|
||||
// Which Board::handleCommand() commands this board actually answers, as a
|
||||
// bitmask over WC_BOARD_CMDS. Probed once per start on the loop task and then
|
||||
// only read, so the async status handler needs no lock for it.
|
||||
// bitmask over WC_BOARD_CMDS. Written once by the loop task in
|
||||
// probeBoardCommands() and read by the async status handler, both under _mux:
|
||||
// the routes are serving before the first tick, so the two really do race.
|
||||
uint8_t _board_cmds = 0;
|
||||
bool _board_cmds_probed = false;
|
||||
|
||||
|
||||
+22
-2
@@ -648,8 +648,9 @@ function boot(){
|
||||
st.fw=s.fw||"";st.build=s.build_date||"";st.role=s.role||"";st.board=s.board||"";
|
||||
// Board-specific CLI commands this node answered for at startup. Controls
|
||||
// for anything absent stay hidden: on this board they are not commands.
|
||||
st.boardCmds=(s.board_cmds||"").split(",").filter(Boolean);
|
||||
applyBoardCmds();
|
||||
// The field is ABSENT, not empty, until the node has finished probing, and
|
||||
// this page reads status once — so latch it only when it is really there.
|
||||
setBoardCmds(s.board_cmds);
|
||||
$("#h-name").textContent=s.name||"MeshCore";
|
||||
$("#h-sub").textContent=s.role+" · "+shortVer()+" · "+s.board;
|
||||
var b=$("#h-badge");b.classList.remove("hide");
|
||||
@@ -1609,6 +1610,25 @@ var CLI_BOARD_KEYS=[
|
||||
["fan","fan.hi","Fan-on temperature in C {above fan.lo, max 120}",2]
|
||||
];
|
||||
function boardHas(gate){return st.boardCmds.indexOf(gate)>=0}
|
||||
// An absent board_cmds means the node has not finished probing its board, which
|
||||
// it does on the first tick after the routes go live — so in practice the very
|
||||
// first retry finds it. Retry rather than read absent as "no board commands",
|
||||
// which would hide working controls for the whole session. Backs off to ~15s
|
||||
// total: generous against a one-tick gap, and bounded, since a node that never
|
||||
// answers is one where hiding the controls is the right outcome anyway.
|
||||
function setBoardCmds(v,tries){
|
||||
if(v===undefined||v===null){
|
||||
tries=tries||0;
|
||||
if(tries<10)setTimeout(function(){
|
||||
api("/api/status").then(function(s){setBoardCmds(s.board_cmds,tries+1)})
|
||||
.catch(function(){});
|
||||
},Math.min(300*(tries+1),2000));
|
||||
return;
|
||||
}
|
||||
st.boardCmds=v.split(",").filter(Boolean);
|
||||
cli.built=0; // the command table is keyed on this; force a rebuild
|
||||
applyBoardCmds();
|
||||
}
|
||||
// Board-specific rows in the settings form. Hidden unless the node answered for
|
||||
// the command at startup, so the form never shows a control that cannot apply.
|
||||
function applyBoardCmds(){
|
||||
|
||||
Reference in new issue
Block a user