Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 18, 2025

Refactor OAMS to use async callback-driven action model

Plan

  • Update fps.py for safer ADC callback handling

    • Clamp ADC readings to [0.0, 1.0]
    • Wrap callback invocations in try/except
    • Validate callbacks are callable when added
  • Refactor oams.py to async callback model

    • Add pending action queue infrastructure (_enqueue_action, _deliver_action_result)
    • Implement async _start_load_spool and _start_unload_spool
    • Keep synchronous load_spool/unload_spool wrappers for backward compatibility
    • Convert G-code handlers to non-blocking (start action and return immediately)
    • Add per-action timeouts and safe delivery mechanisms
    • Update abort_current_action to notify pending callbacks
  • Convert oams_manager.py to async API

    • Implement _load_filament_for_group_async using async OAMS API
    • Implement _unload_filament_for_fps_async using async OAMS API
    • Update runout reload callback to use async unload/load chain
    • Add defensive timer unregistering and exception handling
  • Test and validate changes

    • Run Python syntax checks
    • Verify no breaking changes to existing functionality
Original prompt

Refactor OAMS to use an asynchronous, callback-driven action model and convert OAMS manager to the async API

Summary

  • Convert the OAMS action handling (load/unload/calibrate) to an async-first design where commands dispatched to the MCU return immediately and completion is handled via callbacks registered per action.
  • Add a pending-action queue with per-action timeouts and safe delivery of action results from MCU responses.
  • Make G-code handlers non-blocking: OAMS_LOAD_SPOOL, OAMS_UNLOAD_SPOOL, and calibration commands start an action and return immediately (they post completion notifications via M118 when the MCU reports results).
  • Keep synchronous compatibility wrappers for load_spool/unload_spool that internally use the async API and wait cooperatively with short reactor pauses and a timeout. This preserves backward compatibility while encouraging non-blocking usage.
  • Update OAMSManager to use the new async start APIs for loads/unloads (the manager's infinite-runout flow now triggers unload then load asynchronously and continues in callbacks). This removes blocking reactor pauses and makes complex multi-step flows robust.
  • Make FPS ADC callback more robust by clamping values and protecting callbacks from raising exceptions.

Files changed

  • klipper_openams/src/oams.py

    • Replaced blocking action-status polling with a pending-action queue, _enqueue_action, _deliver_action_result and per-action timeouts.
    • Implemented _start_load_spool / _start_unload_spool async entrypoints which dispatch MCU commands and enqueue the expected action completion handlers. They accept a callback(success, code, value, meta) and timeout.
    • Kept load_spool/unload_spool synchronous wrappers that use the async API and coop-wait the reactor for completion (with timeout) for backward compatibility.
    • Converted gcode handlers for load/unload/calibrate to start actions and return immediately.
    • Added safer logging, exception handling and abort_current_action behaviour that notifies pending callbacks.
  • klipper_openams/src/oams_manager.py

    • Replaced internal synchronous calls to oam.load_spool / oam.unload_spool with _load_filament_for_group_async / _unload_filament_for_fps_async which dispatch async OAMS actions and supply callbacks that continue the flow when complete.
    • The runout reload callback now starts unload asynchronously and chains the load as a callback; it pauses the monitor if unload/load fails and posts pause messages to the printer.
    • Added defensive timer unregistering and exception handling around monitor lifecycle.
  • klipper_openams/src/fps.py

    • Clamp ADC readings to [0.0,1.0] and wrap callback invocations in try/except to avoid exceptions bubbling out of the ADC callback.
    • Validate that added callbacks are callable.

Why this change

  • The previous code used busy-wait loops that polled for action_status and paused the reactor repeatedly; while reactor.pause yields back to Klipper, these patterns make control flow hard to follow and can lead to invisible hangs and complex race conditions. The async queue and per-action timeout ensure we never wait indefinitely and make it straightforward to coordinate multi-step actions (unload then load) without blocking the reactor.

Testing notes / migration

  • Test on a single OAMS unit first.
  • Watch the Klipper logs: the async code uses M118 via gcode.run_script for completion notifications and logs action timeouts or failures.
  • If the MCU firmware can be updated in the future to echo a per-action unique id, the matching logic could be tightened; for now actions are matched by the reported action code.

No images attached.

This pull request was created as a result of the following prompt from Copilot chat.

Refactor OAMS to use an asynchronous, callback-driven action model and convert OAMS manager to the async API

Summary

  • Convert the OAMS action handling (load/unload/calibrate) to an async-first design where commands dispatched to the MCU return immediately and completion is handled via callbacks registered per action.
  • Add a pending-action queue with per-action timeouts and safe delivery of action results from MCU responses.
  • Make G-code handlers non-blocking: OAMS_LOAD_SPOOL, OAMS_UNLOAD_SPOOL, and calibration commands start an action and return immediately (they post completion notifications via M118 when the MCU reports results).
  • Keep synchronous compatibility wrappers for load_spool/unload_spool that internally use the async API and wait cooperatively with short reactor pauses and a timeout. This preserves backward compatibility while encouraging non-blocking usage.
  • Update OAMSManager to use the new async start APIs for loads/unloads (the manager's infinite-runout flow now triggers unload then load asynchronously and continues in callbacks). This removes blocking reactor pauses and makes complex multi-step flows robust.
  • Make FPS ADC callback more robust by clamping values and protecting callbacks from raising exceptions.

Files changed

  • klipper_openams/src/oams.py

    • Replaced blocking action-status polling with a pending-action queue, _enqueue_action, _deliver_action_result and per-action timeouts.
    • Implemented _start_load_spool / _start_unload_spool async entrypoints which dispatch MCU commands and enqueue the expected action completion handlers. They accept a callback(success, code, value, meta) and timeout.
    • Kept load_spool/unload_spool synchronous wrappers that use the async API and coop-wait the reactor for completion (with timeout) for backward compatibility.
    • Converted gcode handlers for load/unload/calibrate to start actions and return immediately.
    • Added safer logging, exception handling and abort_current_action behaviour that notifies pending callbacks.
  • klipper_openams/src/oams_manager.py

    • Replaced internal synchronous calls to oam.load_spool / oam.unload_spool with _load_filament_for_group_async / _unload_filament_for_fps_async which dispatch async OAMS actions and supply callbacks that continue the flow when complete.
    • The runout reload callback now starts unload asynchronously and chains the load as a callback; it pauses the monitor if unload/load fails and posts pause messages to the printer.
    • Added defensive timer unregistering and exception handling around monitor lifecycle.
  • klipper_openams/src/fps.py

    • Clamp ADC readings to [0.0,1.0] and wrap callback invocations in try/except to avoid exceptions bubbling out of the ADC callback.
    • Validate that added callbacks are callable.

Why this change

  • The previous code used busy-wait loops that polled for action_status and paused the reactor repeatedly; while reactor.pause yields back to Klipper, these patterns make control flow hard to follow and can lead to invisible hangs and complex race conditions. The async queue and per-action timeout ensure we never wait indefinitely and make it straightforward to coordinate multi-step actions (unload then load) without blocking the reactor.

Testing notes / migration

  • Test on a single OAMS unit first.
  • Watch the Klipper logs: the async code uses M118 via gcode.run_script for completion notifications and logs action timeouts or failures.
  • If the MCU firmware can be updated in the future to echo a per-action unique id, the matching logic could be tightened; for now actions are matched by the reported action code.

No images attached.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@lindnjoe lindnjoe marked this pull request as ready for review October 18, 2025 18:39
@lindnjoe lindnjoe merged commit 632aaae into main Oct 18, 2025
1 check failed
Copilot AI requested a review from lindnjoe October 18, 2025 18:40
Copilot finished work on behalf of lindnjoe October 18, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants