AuxPreprocessor: stop mutating the macro source; use a tempfile

Macros and uploaded jobs now pass through gplan untouched on disk.
The (MSG,HOOK:...) substitution that lets the host react to ATC
M-codes mid-program now lands in a tempfile that gplan loads instead
of the operator-authored source.

Why we still rewrite at all: gplan (camotics planner) treats
M100/M102/M103 as no-ops by spec and doesn't expose a callback for
user-defined M-codes. Its only in-band channel back to Python during
a running program is the (MSG,...) message stream, so we substitute
hook messages for those M-codes purely as transport. That mechanism
is fine; what was broken was that we wrote the substitution back
over the macro source. So:

- The macro editor opened drop.nc and saw (MSG,HOOK:...) blobs
  instead of the M100/M102/M103 sequence.
- Re-running compounded any rewrite quirk (paren-comment handling,
  consecutive HOOK lines collapsing) on every load.
- Editing a macro accidentally re-rewrote its already-rewritten
  form.

Now:

- AuxPreprocessor.preprocess_to_tempfile() returns a path to a
  rewritten temp file; the source is never modified. The old
  preprocess_file() in-place wrapper is kept (deprecated) for
  the upload path, where mutating the saved upload is fine.
- Planner.load() goes through preprocess_to_tempfile and tracks
  the temp path on the Planner instance, deleting the previous
  tempfile on each new load() so /tmp doesn't fill up.
- Each rewritten (MSG,HOOK:...) line gets a tiny G4 P0.001
  dwell prefix so gplan doesn't collapse consecutive comment-
  only lines into a single block (which was eating all but the
  last hook in a sequence). The dwell appears only in the
  tempfile, never in the source.

Macros on the controller (drop.nc, grab.nc, release.nc, clamp.nc)
restored to the human-readable M100/M102/M103 form.

src/py/bbctrl/AuxPreprocessor.py

@@ -394,7 +394,22 @@ class AuxPreprocessor(object):
                         except ValueError: continue
                         event = _ATC_M_CODES.get(num)
                         if event:
-                            fout.write('(MSG,HOOK:%s:)\n' % event)
+                            # gplan only delivers `(MSG,...)` to the
+                            # message stream when it's attached to an
+                            # executable block (so the host can release
+                            # it on the matching cmd-id ack). A bare
+                            # comment-only line gets collapsed and the
+                            # message is silently dropped, which means
+                            # back-to-back hook lines (like 4 ejects in
+                            # a row) only deliver the last one.
+                            #
+                            # Pair each HOOK line with an essentially-
+                            # zero dwell so it gets a planner block id
+                            # of its own. G4 P0.001 = 1us dwell which
+                            # is below any timer resolution and has no
+                            # observable effect on the machine.
+                            fout.write('G4 P0.001 (MSG,HOOK:%s:)\n'
+                                       % event)
                     code_stripped = _ATC_M_RE.sub('', code).strip()
                     if code_stripped:
                         # Mixed line: keep the residual executable
@@ -410,16 +425,26 @@ class AuxPreprocessor(object):
         return rewrote_any
 
 
-def preprocess_file(src_path, log=None, coupling=None, **_unused):
+def preprocess_to_tempfile(src_path, log=None, coupling=None):
-    """Convenience: rewrite src_path in place if it contains ATC
+    """Run the preprocessor on `src_path` and return the path to a
-    M-codes or needs Z-A coupling injection. Returns True if the
+    rewritten temp file (or None if no rewriting was needed). Caller
-    file was rewritten.
+    owns the temp file and must os.unlink() it when done.
 
-    `coupling` is an optional dict (see AuxPreprocessor.__init__).
+    The original source file is never modified - this is the
-    Extra keyword args are accepted for backwards compat (the old
+    intentional design: the macro / job file the operator authored
-    w_first arg is no longer used)."""
+    is what they see in the macro editor and the file viewer; the
+    rewriting happens only on the in-memory copy that gplan loads.
+
+    Why we rewrite at all: gplan (the camotics planner) treats the
+    user-defined M-codes M100/M102/M103 as no-ops. The only callback
+    channel it exposes during a running program is the (MSG,...)
+    message stream, so the only way for the host to react to those
+    M-codes mid-program is to substitute (MSG,HOOK:<event>:) lines
+    in their place. This rewriting is an implementation detail the
+    operator should never have to know about - hence the tempfile.
+    """
     if not AuxPreprocessor.file_uses_aux(src_path, coupling=coupling):
-        return False
+        return None
     pre = AuxPreprocessor(log=log, coupling=coupling)
     fd, tmp = tempfile.mkstemp(prefix='auxpre_', suffix='.nc',
                                dir=os.path.dirname(src_path) or None)
@@ -427,13 +452,36 @@ def preprocess_file(src_path, log=None, coupling=None, **_unused):
     try:
         rewrote = pre.process(src_path, tmp)
         if rewrote:
-            shutil.move(tmp, src_path)
+            return tmp
-            return True
         os.unlink(tmp)
-        return False
+        return None
     except Exception:
         try:
             os.unlink(tmp)
         except OSError:
             pass
         raise
+
+
+def preprocess_file(src_path, log=None, coupling=None, **_unused):
+    """DEPRECATED in-place version of the preprocessor. Kept for
+    callers that still rewrite their input on disk (chiefly the
+    upload path, where mutating the file is fine because there's no
+    operator-authored source to preserve).
+
+    Returns True if the file was rewritten, False otherwise.
+
+    For new callers prefer preprocess_to_tempfile() which never
+    touches the source."""
+    tmp = preprocess_to_tempfile(src_path, log=log, coupling=coupling)
+    if tmp is None:
+        return False
+    try:
+        shutil.move(tmp, src_path)
+    except Exception:
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        raise
+    return True

src/py/bbctrl/Planner.py

@@ -27,6 +27,7 @@
 
 import json
 import math
+import os
 import re
 import time
 from collections import deque
@@ -76,6 +77,10 @@ class Planner():
         self.planner = None
         self._position_dirty = False
         self.where = ''
+        # Tracks the rewritten temp file (if any) returned by the
+        # AuxPreprocessor for the currently-loaded program. We delete
+        # it on the next load() so it doesn't pile up under /tmp.
+        self._aux_tempfile = None
 
         ctrl.state.add_listener(self._update)
 
@@ -507,28 +512,57 @@ class Planner():
 
     def load(self, path):
         self.where = path
-        path = self.ctrl.get_path('upload', path)
+        src_path = self.ctrl.get_path('upload', path)
-        self.log.info('GCode:' + path)
+        self.log.info('GCode:' + src_path)
-        # Rewrite ATC M-codes (M100..M103) before gplan sees them.
+
-        # preprocess_file is a no-op when no rewriting is needed and
+        # Clean up any leftover temp file from a previous load.
-        # idempotent when run twice on the same file, so this is
+        self._cleanup_aux_tempfile()
-        # safe on every load. W tokens are no longer rewritten - the
+
-        # auxcnc stepper is now exposed as a virtual A axis and gcode
+        # Rewrite ATC M-codes (M100/M102/M103) and inject Z-A
-        # should use A directly.
+        # coupling moves before gplan sees them. The rewriting goes
+        # to a temp file -- the operator's macro / job source is
+        # never modified. This matters because:
+        #
+        #   1. The macro editor reads back the source. If we
+        #      rewrote in place, the operator would open `drop.nc`
+        #      and see (MSG,HOOK:...) blobs instead of the M-code
+        #      sequence they wrote.
+        #   2. Re-running a rewritten file would re-rewrite it; any
+        #      bug in the regex (e.g. with paren comments) would
+        #      compound on every load.
+        #
+        # Why we rewrite at all: gplan treats M100..M103 as no-ops
+        # by spec and exposes no callback for user M-codes. Its only
+        # in-band channel back to Python during a running program is
+        # the (MSG,...) message stream, so we substitute hook
+        # messages for the M-codes purely as transport.
+        load_path = src_path
         try:
-            from bbctrl.AuxPreprocessor import preprocess_file
+            from bbctrl.AuxPreprocessor import preprocess_to_tempfile
             ext = getattr(self.ctrl, 'ext_axis', None)
             coupling = (ext.coupling_for_preprocessor()
                         if ext is not None else None)
-            if preprocess_file(path, log=self.log, coupling=coupling):
+            tmp = preprocess_to_tempfile(
-                self.log.info('Rewrote (ATC / Z-A coupling) in %s' % path)
+                src_path, log=self.log, coupling=coupling)
+            if tmp is not None:
+                self._aux_tempfile = tmp
+                load_path = tmp
+                self.log.info(
+                    'Rewrote (ATC / Z-A coupling) for gplan: %s -> %s'
+                    % (src_path, tmp))
         except Exception:
             self.log.exception('Aux preprocess at load failed; '
                                'attempting to load file unchanged')
         self._sync_position()
-        self.planner.load(path, self.get_config(False, True))
+        self.planner.load(load_path, self.get_config(False, True))
         self.reset_times()
 
+    def _cleanup_aux_tempfile(self):
+        if self._aux_tempfile and os.path.exists(self._aux_tempfile):
+            try: os.unlink(self._aux_tempfile)
+            except OSError: pass
+        self._aux_tempfile = None
+
 
     def stop(self):
         try: