Hooks v2: block unpause until hook completes

- Blocking hooks (block_unpause: true, default for tool-change) run
  in a background thread and gate Mach.unpause() via can_unpause()
- Machine stays in HOLDING state while hook runs — AVR steppers idle,
  spindle state preserved, position locked
- auto_resume option to unpause automatically after hook completes
- E-stop cancels any running hook immediately
- Hook status pushed to frontend via state (hook_busy, hook_event)
- GET /api/hooks/status endpoint for polling
- Non-blocking hooks (program-start, program-end, etc.) fire-and-forget

src/py/bbctrl/Hooks.py

@@ -2,41 +2,65 @@
 #
 # Hooks - External event triggers during G-code execution
 #
-# Watches planner state changes and fires webhooks / runs scripts when
+# Integrates with the controller's pause/unpause cycle to run external
-# specific events occur (tool change, program start/end, pause, etc.)
+# actions (webhooks, scripts) at specific points during G-code execution.
 #
-# Configuration is loaded from hooks.json in the controller directory:
+# ## How tool-change hooks work (the important one):
+#
+#   G-code: T5 M6
+#
+#   1. Planner replaces M6 with tool-change override G-code (configurable).
+#      Default: "M0 M6 (MSG, Change tool)"
+#
+#   2. Planner emits: set(tool,5), pause(program), message("Change tool")
+#      These are sent to the AVR as serial commands.
+#
+#   3. AVR finishes current move, enters HOLDING state.
+#      Reports back: xx=HOLDING, pr="Program pause"
+#
+#   4. Pi: Mach._update() sees HOLDING, flushes CommandQueue.
+#      CommandQueue executes callbacks: state.set('tool', 5) fires.
+#
+#   5. Hooks._on_state_change() sees tool changed.
+#      Sets self._hook_busy = True, runs the hook in a thread.
+#      While _hook_busy, Mach.unpause() is blocked via can_unpause().
+#
+#   6. Machine sits in HOLDING. UI shows "Change tool" message.
+#      User cannot resume yet (unpause is gated).
+#
+#   7. Hook thread finishes (toolchanger done). Sets _hook_busy = False.
+#      If auto_resume is set, calls unpause automatically.
+#      Otherwise user clicks Continue in UI.
+#
+#   8. Mach.unpause() → planner.restart() → AVR UNPAUSE → motion resumes.
+#
+# ## Configuration (hooks.json):
 #
 #   {
 #     "tool-change": {
 #       "type": "webhook",
 #       "url": "http://toolchanger.local/api/change",
 #       "method": "POST",
-#       "timeout": 30,
+#       "timeout": 120,
-#       "wait": true
+#       "block_unpause": true,
+#       "auto_resume": true
 #     },
 #     "program-start": {
 #       "type": "script",
-#       "command": "/usr/local/bin/dust-collector on"
+#       "command": "/usr/local/bin/dust-collector on",
-#     },
+#       "block_unpause": false
-#     "program-end": {
-#       "type": "webhook",
-#       "url": "http://homeassistant.local:8123/api/services/switch/turn_off",
-#       "method": "POST",
-#       "headers": {"Authorization": "Bearer TOKEN"},
-#       "body": {"entity_id": "switch.dust_collector"}
-#     },
-#     "pause": {
-#       "type": "script",
-#       "command": "/usr/local/bin/notify 'CNC paused'"
 #     }
 #   }
 #
+#   block_unpause: if true, unpause is blocked until hook completes
+#   auto_resume:   if true AND block_unpause, auto-unpause after hook done
+#
 ################################################################################
 
 import os
 import json
 import subprocess
+import threading
 import traceback
 from urllib.request import Request, urlopen
 from urllib.error import URLError
@@ -48,7 +72,6 @@ HOOK_EVENTS = [
     'program-start',    # Program begins running
     'program-end',      # M2/M30 - program ends
     'pause',            # M0/M1 - program pause
-    'probe-start',      # Probe cycle begins
     'estop',            # Emergency stop triggered
     'homing-start',     # Homing cycle begins
     'homing-end',       # Homing cycle completes
@@ -62,6 +85,12 @@ class Hooks:
         self.log = ctrl.log.get('Hooks')
         self.hooks = {}
 
+        # Hook execution state
+        self._hook_busy = False        # True while a blocking hook runs
+        self._hook_busy_event = None   # Which event is blocking
+        self._hook_error = None        # Error from last hook, if any
+        self._hook_thread = None
+
         # Track state for edge detection — must be set before add_listener
         # because add_listener fires immediately with current state
         self._last_cycle = ctrl.state.get('cycle', 'idle')
@@ -76,6 +105,8 @@ class Hooks:
         ctrl.state.add_listener(self._on_state_change)
         self._initialized = True
 
+    # -- Config management --
+
     def _get_config_path(self):
         return self.ctrl.get_path(filename='hooks.json')
 
@@ -104,13 +135,34 @@ class Hooks:
     def get_config(self):
         return self.hooks
 
+    # -- Unpause gating (called from Mach) --
+
+    def can_unpause(self):
+        """Returns True if no blocking hook is running.
+        Called by Mach.unpause() to gate resume."""
+        if self._hook_busy:
+            self.log.info('Unpause blocked: hook "%s" still running' %
+                          self._hook_busy_event)
+            return False
+        return True
+
+    def get_status(self):
+        """Return current hook execution status for the UI."""
+        return {
+            'busy': self._hook_busy,
+            'event': self._hook_busy_event,
+            'error': self._hook_error,
+        }
+
+    # -- State change listener --
+
     def _on_state_change(self, update):
         """Called on every state update from the controller."""
         if not self._initialized:
             return
         state = self.ctrl.state
 
-        # Detect tool change (tool number changed)
+        # Detect tool change (tool number changed while HOLDING)
         if 'tool' in update:
             new_tool = update['tool']
             if new_tool != self._last_tool:
@@ -134,11 +186,17 @@ class Hooks:
                     self._fire('homing-end', {})
                 self._last_cycle = new_cycle
 
-        # Detect state changes
+        # Detect AVR state changes
         if 'xc' in update or 'xx' in update:
             new_state = state.get('xx', '')
             if new_state != self._last_state:
                 if new_state == 'ESTOPPED':
+                    # Cancel any running hook on estop
+                    if self._hook_busy:
+                        self.log.warning('E-stop: cancelling hook "%s"' %
+                                         self._hook_busy_event)
+                        self._hook_busy = False
+                        self._hook_busy_event = None
                     self._fire('estop', {})
                 self._last_state = new_state
 
@@ -161,9 +219,10 @@ class Hooks:
                     'data': data,
                 }, custom_name=event)
 
+    # -- Hook execution --
+
     def _fire(self, event, context, custom_name=None):
         """Fire a hook event."""
-        # Look up by event name, or by custom name for custom events
         hook = self.hooks.get(event)
         if custom_name and not hook:
             hook = self.hooks.get(custom_name)
@@ -176,34 +235,89 @@ class Hooks:
         state = self.ctrl.state
         context.update({
             'event': event,
-            'position': state.get_position() if hasattr(state, 'get_position') else {},
+            'position': (state.get_position()
+                         if hasattr(state, 'get_position') else {}),
             'state': state.get('xx', ''),
             'cycle': state.get('cycle', 'idle'),
         })
 
+        block_unpause = hook.get('block_unpause', event == 'tool-change')
+        auto_resume = hook.get('auto_resume', False)
+
+        if block_unpause:
+            # Run in thread, block unpause until done
+            self._hook_busy = True
+            self._hook_busy_event = event
+            self._hook_error = None
+
+            # Update UI state so frontend knows we're busy
+            self.ctrl.state.set('hook_busy', True)
+            self.ctrl.state.set('hook_event', event)
+
+            self._hook_thread = threading.Thread(
+                target=self._run_hook_blocking,
+                args=(hook, event, context, auto_resume),
+                daemon=True
+            )
+            self._hook_thread.start()
+        else:
+            # Fire and forget (non-blocking)
+            self._execute_hook(hook, context)
+
+    def _run_hook_blocking(self, hook, event, context, auto_resume):
+        """Runs in a background thread. Blocks unpause until complete."""
+        try:
+            self._execute_hook(hook, context)
+            self.log.info('Hook "%s" completed successfully' % event)
+        except Exception as e:
+            self._hook_error = str(e)
+            self.log.error('Hook "%s" failed: %s' % (event, e))
+        finally:
+            self._hook_busy = False
+            self._hook_busy_event = None
+
+            # Schedule UI update on the ioloop thread
+            self.ctrl.ioloop.call_later(0, self._hook_finished, auto_resume)
+
+    def _hook_finished(self, auto_resume):
+        """Called on the ioloop after a blocking hook completes."""
+        self.ctrl.state.set('hook_busy', False)
+        self.ctrl.state.set('hook_event', '')
+
+        if self._hook_error:
+            self.ctrl.state.set('hook_error', self._hook_error)
+            self.log.error('Hook error: %s' % self._hook_error)
+        else:
+            self.ctrl.state.set('hook_error', '')
+
+        if auto_resume and not self._hook_error:
+            self.log.info('Hook done, auto-resuming')
+            try:
+                self.ctrl.mach.unpause()
+            except Exception as e:
+                self.log.error('Auto-resume failed: %s' % e)
+
+    def _execute_hook(self, hook, context):
+        """Execute a single hook (webhook or script). May block."""
         hook_type = hook.get('type', 'webhook')
 
-        try:
         if hook_type == 'webhook':
             self._fire_webhook(hook, context)
         elif hook_type == 'script':
             self._fire_script(hook, context)
         else:
-                self.log.error('Unknown hook type: %s' % hook_type)
+            raise Exception('Unknown hook type: %s' % hook_type)
-        except Exception:
-            self.log.error('Hook %s failed: %s' % (event, traceback.format_exc()))
 
     def _fire_webhook(self, hook, context):
         """Fire a webhook HTTP request."""
         url = hook.get('url')
         if not url:
-            self.log.error('Webhook missing url')
+            raise Exception('Webhook missing url')
-            return
 
         method = hook.get('method', 'POST').upper()
-        timeout = hook.get('timeout', 10)
+        timeout = hook.get('timeout', 30)
-        headers = hook.get('headers', {})
+        headers = dict(hook.get('headers', {}))
-        body = hook.get('body', {})
+        body = dict(hook.get('body', {}))
 
         # Merge context into body
         body['_context'] = context
@@ -212,26 +326,21 @@ class Hooks:
         headers['Content-Type'] = 'application/json'
 
         req = Request(url, data=data, headers=headers, method=method)
-
         self.log.info('Webhook %s %s' % (method, url))
 
-        try:
         resp = urlopen(req, timeout=timeout)
         self.log.info('Webhook response: %d' % resp.status)
-        except URLError as e:
+
-            self.log.error('Webhook failed: %s' % e)
+        if resp.status >= 400:
-        except Exception as e:
+            raise Exception('Webhook returned %d' % resp.status)
-            self.log.error('Webhook error: %s' % e)
 
     def _fire_script(self, hook, context):
-        """Fire a local script/command."""
+        """Fire a local script/command. Blocks until complete."""
         command = hook.get('command')
         if not command:
-            self.log.error('Script hook missing command')
+            raise Exception('Script hook missing command')
-            return
 
-        timeout = hook.get('timeout', 30)
+        timeout = hook.get('timeout', 120)
-        wait = hook.get('wait', True)
 
         # Pass context as environment variables
         env = os.environ.copy()
@@ -245,24 +354,18 @@ class Hooks:
         if 'new_tool' in context:
             env['HOOK_NEW_TOOL'] = str(context['new_tool'])
 
-        self.log.info('Script: %s (wait=%s)' % (command, wait))
+        self.log.info('Script: %s' % command)
 
-        try:
-            if wait:
         result = subprocess.run(
             command, shell=True, env=env,
             timeout=timeout,
             capture_output=True, text=True
         )
-                if result.returncode != 0:
+
-                    self.log.error('Script failed (%d): %s' %
-                                   (result.returncode, result.stderr))
-                else:
         if result.stdout.strip():
-                        self.log.info('Script output: %s' % result.stdout.strip())
+            self.log.info('Script stdout: %s' % result.stdout.strip())
-            else:
+
-                subprocess.Popen(command, shell=True, env=env)
+        if result.returncode != 0:
-        except subprocess.TimeoutExpired:
+            raise Exception('Script failed (%d): %s' %
-            self.log.error('Script timed out after %ds' % timeout)
+                            (result.returncode,
-        except Exception as e:
+                             result.stderr.strip() or 'non-zero exit'))
-            self.log.error('Script error: %s' % e)

src/py/bbctrl/Mach.py

@@ -349,6 +349,10 @@ class Mach(Comm):
 
     def unpause(self):
         if self._is_paused():
+            # Gate unpause on hook completion
+            if hasattr(self.ctrl, 'hooks') and \
+               not self.ctrl.hooks.can_unpause():
+                return
             self.ctrl.state.set('optional_pause', False)
             self._unpause()
 

src/py/bbctrl/Web.py

@@ -776,6 +776,11 @@ class HooksSaveHandler(bbctrl.APIHandler):
         self.get_ctrl().hooks.save_config(self.json)
 
 
+class HooksStatusHandler(bbctrl.APIHandler):
+    def get(self):
+        self.write_json(self.get_ctrl().hooks.get_status())
+
+
 class HooksFireHandler(bbctrl.APIHandler):
     def put_ok(self, event):
         data = self.json if hasattr(self, 'json') and self.json else {}
@@ -959,6 +964,7 @@ class Web(tornado.web.Application):
             (r'/api/remote-diagnostics', RemoteDiagnosticsHandler),
             (r'/api/hooks', HooksGetHandler),
             (r'/api/hooks/save', HooksSaveHandler),
+            (r'/api/hooks/status', HooksStatusHandler),
             (r'/api/hooks/fire/([\w-]+)', HooksFireHandler),
             (r'/(.*)', StaticFileHandler,
              {'path': bbctrl.get_resource('http/'),