Repository navigation
feat(status): make Restart Machine ask the printer instead of re-reading memory - #9
Conversation
…ing memory The retained status is what the agent last *heard*, so re-reading it on a manual refresh returned the same thing the page was already showing. Pressing the button could not tell you anything you did not already know. It now publishes `probe` on the cmd topic and waits for the agent to go and look. That topic already existed for `flush`, the agent already subscribes to it, and the broker ACL already grants inventree write on it -- so no new topic, no ACL change, no new credential. "Restart Machine" is the refresh button whether or not it is called that: InvenTree's machine framework has no generic custom-action mechanism. restart_machine is the one hardcoded action, with its own endpoint and a fixed place in the menu, so it is the only lever a driver gets and it was being wasted. probe_status subscribes before publishing, because the answer is a retained message and a subscription set up afterwards would race the agent. The message already on the topic arrives immediately and is kept as the baseline, so a probe the printer cannot answer -- the normal case for a D30 that has powered itself down -- degrades to exactly what read_status would have returned. The page keeps the old reading and its age instead of going blank. The automatic paths keep reading rather than probing. Neither startup nor the periodic ping should dial a sleeping printer on a schedule nobody asked for.
… governs It governs everything in this driver: InvenTree's task short-circuits on the setting before any driver is called, so switching it off stops ping_machines entirely. There is nothing to add here for that. What it does not govern is the print agent. device.probe_interval_s is the agent's own timer and it is the one that contacts the printer, so turning off Machine Ping stops this page reading without stopping the agent asking. That is the right layering -- the retained topic is a shared bus and InvenTree is one consumer of it, so a setting here reaching across to reconfigure the device agent would be the more surprising behaviour -- but it is not guessable from either side, hence writing it down on both. Also notes the 5-minute ping cadence, which is the sensible floor for probe_interval_s.
|
Good find — fixed rather than shipped, because the gap you identified was between the docstring and the code, and that is the kind of thing that gets trusted later.
Two things fell out of it that I would not have looked for otherwise:
One trap while implementing it. Comparing the raw strings would have been wrong: they sort chronologically only while every producer emits whole seconds, and You are right that 59 tests on 3.12 and on a real 3.9.25, ruff clean. |
…irst probe_status took the first status published after sending the command. The agent publishes on its own events too -- a print starting and finishing, a broker reconnect, another operator pressing the same button -- so any of those landing first was returned as the answer while the real one was read and thrown away. The realistic trigger is a print in flight when someone clicks refresh: the page shows `state=printing` on a status it just asked to have refreshed, until the next ping corrects it. Mild and self-correcting, which is exactly why nobody would report it -- `printing` on a printer that is printing looks right. It now waits for a status whose device_seen_at is later than the one already retained. A print that actually reached the printer also carries a newer observation and is accepted, correctly: that is the same freshness a probe would have produced, obtained the same way. An identical republish, or a state change with an unchanged observation, is not. The fallback improves as a side effect: an unanswered probe now reports the newest status seen while waiting rather than the one that happened to be retained when we subscribed. Comparing parsed instants rather than raw strings, and parse_seen_at moves out of _ago so both callers share it. The strings sort chronologically only while every producer emits whole seconds -- "...:42.5Z" sorts before "...:42Z" because "." precedes "Z" -- so a sub-second stamp would have been discarded as older. Pinned by a test. The docstring promised "returns what it then publishes" while the code guaranteed "first status after the command". Now they agree.
|
All three taken. #3 turned out to be worth more than 'optional hardening' — implementing it surfaced a second-order bug. #3 — baseline no longer depends on labelfab's emission rules. Everything queued before the command goes out is folded into the baseline now: the retained message plus anything the agent published while we were connecting. Your framing was the useful part — the safety was real but lived in the other repo, where it could be relaxed without anyone here noticing. The bug it surfaced: I first drained after the publish, which lets a quick answer be swallowed into the baseline it is supposed to beat. Still returned via the fallback, so no wrong result — but only after blocking for the full #2 — bound corrected. The docstring now says to size request timeouts on #1 — stale docstring fixed in labelfab@ 61 tests on 3.12 and on a real 3.9.25, ruff clean. |
…t's emission rules Review pointed out that when nothing has ever been observed the baseline is None, and the newness filter weakens to "the first message carrying any timestamp at all". That is safe today only because a pre-observation agent publishes null everywhere -- an invariant that lives in the other repo and could be relaxed there without anyone here noticing. Everything queued before the command goes out is now folded into the baseline: the retained message plus whatever the agent happened to publish while we were connecting. "Newer than what we knew" becomes a property of this function rather than a bet on how labelfab emits. Drained before publishing rather than after, which matters more than it looks. Draining afterwards would let a quick answer be swallowed into the baseline it is supposed to beat -- still returned, via the fallback, but only after blocking for the full wait. The test suite noticed: it ran 5s slower with the drain in the wrong place. Also corrects the wait bound in the docstring. wait_s covers the agent's round trip, but reading the retained message and waiting for the command's PUBACK can each cost conn.timeout_s first, so anyone sizing a request timeout wants 2*conn.timeout_s + wait_s. Sub-millisecond against a healthy broker, and a broker slow enough for it to matter has already broken read_status everywhere else.
Restart Machine now asks the printer instead of re-reading the memory the page was already showing (#9), by publishing `probe` on the cmd topic that already existed. Needs labelfab 0.4.0 to be useful -- an older agent ignores the command -- but degrades to today's behaviour against one, since the probe falls back to the retained status.
Pairs with sengine-cloud/labelfab#14, which adds the
probecommand this sends.The problem with the button
The retained status is what the agent last heard. Re-reading it on a manual refresh returned the same thing the page was already showing — pressing the button could not tell you anything you did not already know.
It now publishes
probeon thecmdtopic and waits for the agent to actually go and look at the printer.Why this needed nothing new
The
cmdtopic already existed and already carriesflush, the agent already subscribes to it, and the broker ACL already grantsinventreetopic write se/v1/print/+/cmd. So: no new topic, no ACL change, no new credential.On "custom actions"
Worth recording, because it is the first thing anyone will ask. InvenTree's machine framework has no generic custom-action mechanism — I checked the running pod.
restart_machineis the single hardcoded action, with its ownMachineRestartendpoint and a fixed place in the Machines menu. Adding a new menu item means patching InvenTree.So "Restart Machine" is the refresh button whether or not it is called that, and it was being wasted on a re-read.
Degradation
probe_statussubscribes before publishing, because the answer is a retained message and a subscription set up afterwards would race the agent. The message already on the topic arrives immediately and is kept as the baseline, so a probe the printer cannot answer — the normal case for a D30 that has powered itself down — degrades to exactly whatread_statuswould have returned. The page keeps the old reading and itsseen Xh ago, rather than going blank.The automatic paths (
init_machine,ping_machines) deliberately keep reading rather than probing: neither should dial a sleeping printer on a schedule nobody asked for.Verified
54 tests on 3.12 and on a real 3.9.25 in Docker, ruff clean. The new tests cover the subscribe-before-publish ordering, the fall-back-to-remembered path, a malformed retained payload, and nothing-retained-and-no-answer.