motorRecord: key CDIR on the commanded stroke after a jog-stop backlash take-out

[physwkim]

motorRecord: key CDIR on the commanded stroke after a jog-stop backlash take-out

Summary

After a reverse jog is released, postProcess() publishes CDIR = 1
(forward) for the backlash take-out leg unconditionally, even when that
leg is actually commanded in the reverse direction. The commanded-direction
field CDIR is derived from relpos, but on the MIP_JOG_STOP path relpos
has just been forced to 0 by the drive-to-readback sync at the top of
postProcess(), so the sign test (relpos < 0.0) can never be true. This
patch keys CDIR on the stroke actually commanded (relbpos), which is live
on this path, without changing the take-out motion itself.

The defect

In motorApp/MotorSrc/motorRecord.cc, postProcess():

1. The "make drive values agree with readback" block runs for every MIP
   except {MIP_MOVE, MIP_MOVE_BL, MIP_JOG_BL1, MIP_JOG_BL2} — the
   predicate does not exclude MIP_JOG_STOP. Inside it, pmr->diff = 0.;.

2. The MIP_JOG_STOP || MIP_MOVE arm then computes
   relpos = pmr->diff / pmr->mres; — which is 0 on the JOG_STOP path
   because of step 1.

3. The JOG_STOP branch dispatches the take-out leg toward dval - bdst via
   WRITE_MSG(MOVE_REL, &relbpos) (encoder/readback in use) or
   WRITE_MSG(MOVE_ABS, &bpos).

4. The shared assignment pmr->cdir = (relpos < 0.0) ? 0 : 1; then runs with
   relpos == 0, so (0 < 0.0) is false and cdir = 1 regardless of the
   sign of the stroke actually commanded.

The sibling arms are self-consistent and are not touched:

• The MIP_MOVE case is excluded from the sync in step 1, so its relpos
  is live at the shared cdir assignment (and is re-derived as relpos * frac
  in its own branch).
• The MIP_JOG_BL1 arm (MIP_JOG_BL1 is likewise excluded from the sync)
  re-derives relpos before its cdir assignment.

Only the JOG_STOP path zeroes the value it then keys CDIR on.

Reachability (why this is a real defect, not defensive noise)

Jog an axis in reverse, release it, with backlash enabled and non-trivial:
BDST > 0 and |BDST| >= |MRES| (the fabs(pmr->bdst) >= fabs(pmr->mres)
gate that enters the backlash block). The record commands the backlash
take-out in the negative direction but publishes CDIR = 1 (forward).

Downstream damage from the wrong CDIR:

• ls_active = (rhls && cdir) || (rlls && !cdir) fails to recognise the
  minus limit switch as active during the move.
• The limit-switch re-arm and the skip-retry gate are missed, so the record
  retries into a pressed reverse limit until RCNT > RTRY, and latches
  MISS = 1.
• maybeRetry() derives user_cdir from cdir and takes the wrong branch.

The fix

On the JOG_STOP branch only, set relpos = relbpos; immediately after the
take-out leg is dispatched, so the shared pmr->cdir = (relpos < 0.0) ? 0 : 1;
reflects the true commanded direction. This is valid because the commanded
stroke sign equals sign(relbpos) in both modes:

• use_rel: MOVE_REL &relbpos commands relbpos steps directly.
• absolute: MOVE_ABS &bpos moves from the current raw position
  (drbv / mres) to bpos = (dval - bdst) / mres; the raw stroke is
  bpos - drbv/mres = ((dval - bdst) - drbv) / mres = relbpos.

relbpos is computed live on this path and is unaffected by the diff = 0
sync, so the sign is now correct. The reassignment mirrors the sibling
MIP_MOVE branch, which already reassigns relpos before the same shared
cdir line. The change is CDIR-only: none of the WRITE_MSG dispatches on
the JOG_STOP branch use relpos, so the take-out motion is unchanged.

                if (use_rel == true)
                    WRITE_MSG(MOVE_REL, &relbpos);
                else
                    WRITE_MSG(MOVE_ABS, &bpos);
                /* 'diff' (hence relpos) was zeroed by the drive-to-readback
                 * sync above, so the shared CDIR assignment below would key on
                 * a stale 0.  The take-out leg just commanded here moves by
                 * relbpos (MOVE_REL) or to bpos, whose stroke is also relbpos
                 * (MOVE_ABS); make CDIR reflect that actual direction. */
                relpos = relbpos;
                pmr->mip = MIP_JOG_BL1;

Scope / family check

Every cdir = assignment in motorRecord.cc was audited:

Site	Keyed on	Verdict
JOG_STOP take-out (this patch)	relpos, zeroed by the sync	same defect — fixed
MIP_MOVE shared line	relpos, live (excluded from sync, re-derived)	self-consistent — untouched
MIP_JOG_BL1 arm	relpos, live (excluded from sync, re-derived)	self-consistent — untouched
HOME arms	MIP_HOMF flag + mres sign	distinct — untouched
JOG dispatch	jogf flag + mres/dir sign	distinct — untouched
normal move dispatch	live rdif	distinct — untouched

Testing

Not compiled locally: this checkout has no EPICS support tree configured, so
make cannot build the module here. The change is a one-line addition of a
local-variable assignment plus a comment, in a branch that already reassigns
the same variable in its sibling; it relies on CI to compile. No behavioural
change to the commanded motion — only CDIR (the published commanded
direction) is corrected on the reverse jog-stop backlash take-out.

[kmpeters]

Was this PR created by an LLM? If so, which agent & model were used for it?

[physwkim]

Yes — the agent was Claude Code, the model Claude Opus 4.8. It came out of a session auditing motorRecord.cc for a Rust port of the motor record; I reviewed the analysis and the patch before filing and take responsibility for both. Happy to rework anything that doesn't fit the project's conventions.

[kmpeters]

I don't think you need to rework anything.

We don't have official guidelines yet, but I'm trying to label AI contributions so that people who feel strongly about it can make informed decisions.