fix(moonraker): Stop Fluidd freezing on MMU status updates - #75
fix(moonraker): Stop Fluidd freezing on MMU status updates#75jansobczak wants to merge 2 commits into
Conversation
0b37083 to
fba1383
Compare
martinbogo
left a comment
There was a problem hiding this comment.
Thanks for this. The core finding is real and we want the fix: register_notification("mmu_ace:status_update") derives notify_status_update, which collides with Moonraker's own subscription push and delivers malformed params (a single element, no eventtime). Renaming it is the right call, and the diff-and-suppress approach for MMU status is sound.
Four things before this can merge.
1. Please retarget to develop. This targets master, which is release-only here. It merges into develop cleanly, so this is mechanical.
2. Strip mmu / mmu_machine before forwarding to GoKlipper. This is the blocking one. You add both to the hardcoded objects/list in patch_objects_list, gated only on is_goklipper_running(). Clients build their subscriptions from objects/list, so on every model they will now send objects/subscribe containing objects gklib does not implement. Our own code documents why that is dangerous - mmu_ace.py:854 notes that querying a non-existent object "causes a nil pointer panic" in gklib on KS1M 2.6.9.3. A wedged gklib needs a power cycle, and mid-print that kills the print.
The pattern already exists in the file: motion_report (kobra.py:1613-1621) and bed_mesh (kobra.py:1270-1280) are both deleted from args['objects'] on objects/query and objects/subscribe before forwarding, then synthesised back onto the response by patch_status. Please mirror that - gklib never needs to see mmu/mmu_machine. (Worth noting the bed_mesh handling is not evidence that gklib tolerates unknown objects; it is stripped for exactly this reason.) You tested on KS1M 2.7.1.4, and this approach removes the need to test all six models across two firmwares each.
3. There is a lost-update hole in the diff cache. The non-subscription branch deliberately does not update _last_pushed_mmu_status. So: baseline X, MMU changes to Y, a client issues printer.objects.query and is served the full Y, MMU reverts to X, and the next push compares X against baseline X, finds no diff, and emits nothing. The client is stuck showing Y indefinitely. That is reachable with an ACE load/select that starts and aborts while Fluidd is polling. Also _last_pushed_mmu_status is never cleared in reinit() or _disable_ace(), unlike _last_gate_fingerprint which is explicitly reset.
4. Tests, plus one comment. tests/ currently has no references to patch_status, register_status_patcher or is_subscription_update. The DummyServer harness in tests/test_kobra.py and tests/test_mmu_ace.py makes these cheap: a registered patcher receives both arguments; the non-subscription path returns full MMU status; the subscription path suppresses an unchanged object and emits a changed one.
Separately, patch_status now runs twice per gklib push - once in the _process_status_update wrapper and once in KlippyAPI.send_status during fan-out, since KlippyAPI is the host Subscribable. Correctness depends on the outer wrapper calling patch_status before delegating, and nothing states that. A future reorder would silently stop MMU updates reaching Fluidd, so please add a comment making the invariant explicit.
Minor: kobra.py:113 still declares status_patchers: List[Callable[[dict], dict]] with the old single-argument hint.
One last thing - your branch is based on a pre-20260716 master tip and your on-printer validation was against build 20260711_01_test, so it does not carry over to the develop-merged result. Please rebase onto develop and re-test. While you are there, a useful field check: after the rename, MMU state reaches the browser only by piggybacking on gklib status deltas, so on an idle printer with no gklib traffic, does a gate-map edit or MMU_SELECT still show up promptly?
I checked the arity change itself and it is safe in-tree (one definition at kobra.py:826, one registrant at mmu_ace.py:1610), and the full test suite passes on the merged tree - so that part is fine. It is items 2 and 3 that need work.
Fixes bugs that broke live status updates in Fluidd. - fluidd subscription never included in the client, server was fine - notification mmu_ace:status_update collision with Klipper notify_status_update which stoped live update (thermals etc) - fix edit filaments at MMU use partial update when need use full update when required Signed-off-by: Jan Sobczak <jasobeczek@gmail.com>
… MMU status diff-cache lost-update hole - patch_objects_list: strip mmu/mmu_machine from objects/query and objects/subscribe args before forwarding to GoKlipper, which doesn't implement these objects and panics if queried for them. - MmuAcePatcher.patch_status: update the subscription diff cache on the non-subscription (query) path too, closing a lost-update hole. - Reset the diff cache on reinit() and _disable_ace() via a new MmuAceController._on_ace_disabled callback hook. - Document the patch_status ordering invariant in patch_status_updates(). - Fix stale status_patchers type hint. - Add test coverage for all of the above. Signed-off-by: Jan Sobczak <jasobeczek@gmail.com>
fba1383 to
d9bd5b9
Compare
|
Retarget this to the "develop" branch please @jansobczak |
|
@martinbogo apologies it was late |
Summary
Fixes two bugs that broke Fluidd's live status updates for the MMU integration on Kobra S1 Max: a notification-name collision that silently froze the entire Fluidd UI after any MMU gate change and a missing
objects/listentry that kept the gate/spool display from updating.Fixes also how the update status of the mmu is handled (diff or full) since fixing those two bug introduced a new problem - user unable to edit filaments in Fluidd - that was not visible before.
Why
Clicking an MMU gate tile (or spools icon) in Fluidd would eventually freeze all live updates (thermals, print status) with no error anywhere. Separately,
MMU_SELECTchanges weren't reflected in the gate/spool display even though the command succeeded server-side.Changes
mmu_ace.py: givemmu_ace:status_updatean explicitnotify_name— the derived name collided with Klipper's ownnotify_status_update, due to how Moonraker is shortening the names. Whenregister_notification()is called without an explicitnotify_name, Moonraker derives one by taking everything after the last colon in the event name (event_name.split(':')[-1]), then prefixes it withnotify_to build the actual method sent so it become"notify_status_update". That's the exact same wire method Moonraker's own core status-update push already uses for every subscribed printer object (temperatures, print state, etc.)kobra.py: addmmu/mmu_machineto the hardcodedobjects/listresponse - Fluidd builds its subscription from this list, so without the entry it never subscribed to MMU status at all.kobra.py,mmu_ace.py: editpatch_statusaddis_subscription_updatewhich push full or diff status update depending on the subscription - this fix the issue with edit filaments in Fluidd where user was unable to edit filaments data due to constant full refreshTesting
Kobra S1 Max, firmware
2.7.1.4, Rinkhals build20260711_01_test.tests/still passRelated issues
Fixes #
Related to #
Notes for reviewers