fix(canon): act on the Stage 4.5/5/6 results -- jog is dead, presets confirmed
CI for csaxs_bec / test (push) Failing after 28s

Three hardware results on 2026-08-18, three different consequences.

Presets work -- the last protocol guess is now measured
--------------------------------------------------------
`control.cgi?p=<id>` recalls a preset. The inference that control.cgi writes
the same names info.cgi reports held, and with it the transport has no
unverified guesses left. R3 marked resolved; the docstring records that
recall runs at the preset's own stored speeds (300 deg/s on p.1) and moves
every axis at once.

Jog does nothing -- so it now says so
--------------------------------------
Writing c.1.<axis>.speed on its own does not start continuous motion; the
field is a setting the *next move* consumes. The camera accepted the write
and did nothing -- the same silent acceptance that hid the broken stop().

Keeping a no-op that reports success would repeat exactly the mistake that
cost us the stop() bug, so jog() raises with an explanation instead, and
jog_pan/jog_tilt come out of USER_ACCESS: that list is the discoverable API
and must not advertise a capability the hardware lacks. The methods stay so
the failure explains itself rather than surfacing as AttributeError, and the
message points at the two real options -- repeated move_absolute(), or
harvesting the continuous-move command from the camera's own web UI.

Focus was "hard to tell" -- so the test was wrong, not the answer
-----------------------------------------------------------------
check_focus_mode sampled the focus readback for drift and called that the
difference between manual and auto. On hardware that is inconclusive:
autofocus on a static, well-lit scene barely hunts, so "not drifting" proves
nothing either way. A weak test that yields a shrug is worse than no test,
because it looks like coverage.

Replaced with a functional one: set manual, command a focus move, and check
the value both reaches the target and stays there. In auto the camera owns
the focus loop and will not leave it there, so this actually discriminates --
and the image visibly defocuses, which is a second, human-readable check.
The three outcomes (reached-and-held / reached-then-slid / never-reached) map
to different causes and say which.

Tests: 124.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-18 11:57:18 +02:00
co-authored by Claude Opus 5
parent a1ddb64e9f
commit 7ecbf1147d
7 changed files with 165 additions and 47 deletions
+13 -7
View File
@@ -55,7 +55,7 @@ name used is **`ptz_cam`** (from [`device_configs/canon_crn300.yaml`](../../devi
| `dev.ptz_cam.focus.move(f)` | positioner | Manual focus, 0–1 (MF only) | 🟡 [R5](#r5) |
| `dev.ptz_cam.<axis>.stop()` | method | Halt one axis | 🟡 [R9](#r9) |
| `dev.ptz_cam.stop_all()` | USER_ACCESS | Halt all four axes | 🟡 [R9](#r9) |
| `dev.ptz_cam.jog_pan(speed)` / `jog_tilt(speed)` | USER_ACCESS | Continuous move at deg/s; `0` stops | 🟡 [R5](#r5) |
| ~~`dev.ptz_cam.jog_pan(speed)` / `jog_tilt(speed)`~~ | — | **Not supported by this camera**; raises. Removed from USER_ACCESS [[2026-08-18]] | ✅ measured |
| `dev.ptz_cam.recall_preset(id)` | USER_ACCESS | Recall stored PTZ preset | 🟡 [R3](#r3) |
| `dev.ptz_cam.gain.set(v)` | signal (config) | Set gain, verify read-back | 🟡 [R4](#r4) |
| `dev.ptz_cam.iris.set(v)` | signal (config) | Set iris/aperture, verify read-back | 🟡 [R4](#r4) |
@@ -174,7 +174,7 @@ footprint behind one grab API. Default is **`jpeg_poll`**.
## 3. What has already been tested
**120 automated tests, no hardware required** (`pytest tests/tests_devices/test_canon_*.py
**124 automated tests, no hardware required** (`pytest tests/tests_devices/test_canon_*.py
--random-order`, run under `OPHYD_CONTROL_LAYER=dummy`). They prove the **Python logic**
using the in-memory `FakeTransport` and a **mocked HTTP opener** — i.e. they confirm the
device builds the requests we *intend* and parses the response format we *assume*. They
@@ -305,8 +305,11 @@ field; absent ⇒ reported as not moving (move completion then relies purely on
tolerance check). *Confirm:* whether such a flag exists. *Fix:* `is_moving()` / consider
tightening tolerance or adding settle time.
<a name="r3"></a>**R3 — Preset CGI.** `preset.cgi?preset=<id>`. *Symptom:* preset recall
errors or no-op. *Fix:* `_CGI["preset"]` / `_PARAM["preset"]`.
<a name="r3"></a>**R3 — Preset recall. RESOLVED [[2026-08-18]].** There is no
`preset.cgi`; presets are the `p.*` namespace, and recall via **`control.cgi?p=<id>`
was confirmed working against hardware**. This was the last unverified guess in the
transport. ⚠️ Recall runs at the preset's *own* stored speeds — `p.1.pan.speed:=30000`
is 300 °/s, three times the commanded-move ceiling — and moves every axis at once.
<a name="r9"></a>**R9 — Stop command. CONFIRMED BROKEN as shipped; now fixed.**
`stop()` used to send `control.cgi?stop=<axis>`. **Verified against hardware on
@@ -370,7 +373,7 @@ the bench**; Stage 1+ need it on the network.
### Stage 0 — Pre-hardware (no camera needed) — do this first
0.1 **Run the suite.** `OPHYD_CONTROL_LAYER=dummy pytest tests/tests_devices/test_canon_*.py
--random-order`. *Expect 120 passed.* If not, stop — the environment is wrong, not the
--random-order`. *Expect 124 passed.* If not, stop — the environment is wrong, not the
camera.
0.2 **Point the editable install at *this* worktree — before starting BEC.** `csaxs_bec`
@@ -427,7 +430,7 @@ cp csaxs_bec/devices/canon/probe.py /tmp/ && (cd /tmp && python3 probe.py --host
`# SPEC:` tables in `transport/xc.py` you can already confirm from the document. *Every
correction here saves a hardware round-trip.*
**Gate:** 120 green + the canon worktree is the live install + sim device usable in BEC +
**Gate:** 124 green + the canon worktree is the live install + sim device usable in BEC +
probe rehearsal shows a full session + SPEC tables reviewed. → proceed.
### Stage 0.5 — First contact (hardware just arrived) — **start here**
@@ -608,7 +611,10 @@ move `Status` completes. → never completes ⇒ [R5](#r5) (scale so tolerance n
4.4 **Speed.** Move pan with `pan_speed` set low vs. high; confirm the camera honours it.
→ ignored ⇒ [R5](#r5) speed param name.
4.5 **Jog + stop.** Use `feature_check.py --jog` (dry run first) — it picks the jog
4.5 **Jog + stop. RESOLVED [[2026-08-18]]: there is no jog.** Writing
`c.1.<axis>.speed` alone does not start motion — the camera accepted it and did
nothing. `jog()` now raises and the convenience methods are out of `USER_ACCESS`.
Kept below for the record. ~~Use `feature_check.py --jog` (dry run first)~~ — it picks the jog
direction with more headroom and refuses if the axis is too close to a limit.
⚠️ **Do not attempt until [R9](#r9) is verified** — jog starts
*continuous* motion, so a non-working stop means the axis runs to its limit. The jog
+9 -2
View File
@@ -158,11 +158,18 @@ Motion — each axis is a positioner (`move()`/`set()` → ophyd status, `stop()
cam.pan.move(45.0) # blocking; returns a completed status
st = cam.tilt.set(-10.0) # non-blocking; st.wait() to block
cam.zoom.move(5.0)
cam.jog_pan(30.0) # continuous pan at 30 deg/s; jog_pan(0) stops
cam.stop_all() # halt every axis
cam.recall_preset(3) # recall a stored PTZ preset
cam.recall_preset(1) # recall a stored PTZ preset (fast! see below)
```
> **No continuous jog.** MEASURED 2026-08-18: this camera has no continuous-move
> command — `c.1.<axis>.speed` is a setting the next move consumes. `jog_pan`/`jog_tilt`
> now raise rather than silently doing nothing, and are no longer in `USER_ACCESS`.
> Use repeated `move()` calls, or harvest the real command from the camera's web UI.
> **Preset recall is fast.** `p.1.pan.speed:=30000` — recall runs at **300 °/s**, three
> times the ceiling for a commanded move, and moves every axis at once.
Parameters (read via `read()`/`read_configuration()`, set with read-back verification):
`exposure_mode`, `shooting_mode`, `iris`, `gain`, `gain_mode`, `white_balance`,
`shutter_speed`, `image_stabilizer`, `focus_mode` (`manual` | `auto`), `face_detect`
+7 -4
View File
@@ -151,8 +151,6 @@ class CanonPTZCamera(PSIDeviceBase):
USER_ACCESS = [
"stop_all",
"recall_preset",
"jog_pan",
"jog_tilt",
"start_stream",
"stop_stream",
"get_latest",
@@ -394,11 +392,16 @@ class CanonPTZCamera(PSIDeviceBase):
self.transport.recall_preset(int(preset_id))
def jog_pan(self, speed: float) -> None:
"""Continuously pan at *speed* deg/s (sign = direction); 0 stops."""
"""Not supported on the CR-N300 -- kept so the failure explains itself.
MEASURED 2026-08-18: this camera has no continuous-move command;
``c.1.pan.speed`` is a setting the next move consumes. Removed from
``USER_ACCESS`` rather than advertising a capability that does nothing.
"""
self.pan.jog(speed)
def jog_tilt(self, speed: float) -> None:
"""Continuously tilt at *speed* deg/s (sign = direction); 0 stops."""
"""Not supported on the CR-N300 -- see :meth:`jog_pan`."""
self.tilt.jog(speed)
def transport_info(self) -> dict:
+54 -24
View File
@@ -39,9 +39,12 @@ What each check proves
guess (a signed speed with no target) and the measured data suggests
``c.1.pan.speed`` is a *setting consumed by moves* rather than a jog command, so
"no movement" is the expected result, not a fault.
* ``--focus-mode`` -- that ``c.1.focus`` switches between ``manual`` and ``auto``, and
that the focus readback **stops drifting** in manual. This is also the prerequisite
for testing the focus axis at all: the positioner is gated on manual mode.
* ``--focus-mode`` -- that ``c.1.focus`` switches to ``manual`` **and that focus then
obeys**: it commands a focus move and checks the value both reaches the target and
stays there. Drift-sampling was tried first and proved inconclusive on hardware --
autofocus on a static scene barely hunts, so "not drifting" proves nothing. This is
also the prerequisite for testing the focus axis at all: the positioner is gated on
manual mode.
* ``--presets`` -- the last unverified protocol guess. There is no ``preset.cgi``, so
recall is issued as ``control.cgi?p=<id>``. If the axes do not move, that guess is
wrong.
@@ -165,14 +168,26 @@ def check_jog(transport, args) -> dict:
def check_focus_mode(transport, args) -> dict:
_XC, Axis, FocusMode, _limits = _import_transport()
del _XC, _limits
"""Prove manual focus **functionally**, by commanding a focus move that sticks.
An earlier version sampled the focus readback for drift and called that the
difference between the modes. Against hardware that was inconclusive -- autofocus
on a static, well-lit scene barely hunts, so "not drifting" says nothing.
Commanding a focus change is decisive instead: in manual the value goes where it
is told and stays; in auto the camera owns the focus loop and will not leave it
there. The image visibly defocuses too, which is a second, human-readable check.
"""
_XC, Axis, FocusMode, HARDWARE_LIMITS = _import_transport()
del _XC
limits = HARDWARE_LIMITS[Axis.FOCUS]
report: dict = {"check": "focus_mode", "sent": False}
before = transport.get_focus_mode()
print(f"\n[5] focus mode -- currently {before.value}")
if not args.go:
print(" DRY RUN -- would set manual, sample drift, then restore.")
print(f" DRY RUN -- would set manual, command a {args.focus_delta:+g} focus")
print(" move, and check it holds.")
report.update({"before": before.value, "dry_run": True})
return report
@@ -182,29 +197,42 @@ def check_focus_mode(transport, args) -> dict:
print(f" set manual -> reads back {now.value}")
report.update({"before": before.value, "after_manual": now.value})
# In manual the readback should hold still; in auto it hunts.
first = transport.get_position(Axis.FOCUS)
time.sleep(args.focus_settle)
second = transport.get_position(Axis.FOCUS)
drift = second - first
report.update({"focus_first": first, "focus_second": second, "focus_drift": drift})
print(
f" focus value {first:.4f} -> {second:.4f} over {args.focus_settle}s "
f"(drift {drift:+.4f})"
)
if now != FocusMode.MANUAL:
print(" RESULT: the mode did NOT change -- check the c.1.focus wire tokens (R6).")
print(" RESULT: the mode did NOT change -- check the c.1.focus token (R6).")
report["focus_mode_works"] = False
elif abs(drift) <= args.tolerance:
print(" RESULT: manual mode holds -- the focus axis is now testable:")
print(" motion_check.py --axis focus --delta 0.05 --go")
return report
# Move focus somewhere it is not, staying inside the range.
here = transport.get_position(Axis.FOCUS)
delta = args.focus_delta if here + args.focus_delta <= limits.high else -args.focus_delta
target = min(limits.high, max(limits.low, here + delta))
print(f" focus {here:.4f} -> commanding {target:.4f}")
transport.move_absolute(Axis.FOCUS, target)
time.sleep(args.focus_settle)
landed = transport.get_position(Axis.FOCUS)
time.sleep(args.focus_settle)
held = transport.get_position(Axis.FOCUS)
report.update(
{"focus_before": here, "focus_target": target, "focus_landed": landed, "focus_held": held}
)
print(f" landed {landed:.4f}, still {held:.4f} after {args.focus_settle}s")
reached = abs(landed - target) <= args.focus_tolerance
stayed = abs(held - landed) <= args.focus_tolerance
if reached and stayed:
print(" RESULT: focus went where it was told and stayed -- manual mode works,")
print(" and the focus axis is functional.")
report["focus_mode_works"] = True
elif reached and not stayed:
print(" RESULT: focus reached the target then drifted away -- something is")
print(" still driving it. Is the camera really out of auto?")
report["focus_mode_works"] = False
else:
print(" RESULT: mode reads manual but the value still drifts. Suspect the")
print(" camera is ignoring the write, or something else holds control.")
print(" RESULT: focus did not reach the target. Either the write is ignored")
print(" or the 0..2048 scaling (R5) is wrong for this axis.")
report["focus_mode_works"] = False
print(" Look at the image: it should be visibly defocused now.")
if args.restore_focus_mode and before != FocusMode.MANUAL:
transport.set_focus_mode(before)
print(f" restored {before.value}")
@@ -348,7 +376,9 @@ def main(argv: list[str] | None = None) -> int:
parser.add_argument("--jog-axis", default="pan", choices=("pan", "tilt"))
parser.add_argument("--jog-speed", type=float, default=3.0, help="deg/s")
parser.add_argument("--jog-seconds", type=float, default=2.0)
parser.add_argument("--focus-settle", type=float, default=3.0, help="drift sample window")
parser.add_argument("--focus-settle", type=float, default=2.0, help="settle/hold window")
parser.add_argument("--focus-delta", type=float, default=0.15, help="focus move to test")
parser.add_argument("--focus-tolerance", type=float, default=0.03)
parser.add_argument(
"--restore-focus-mode",
action="store_true",
+25 -9
View File
@@ -481,12 +481,24 @@ class XCTransport(CameraTransport):
self._request("control", params)
def jog(self, axis: Axis, speed: float) -> None:
if not self._has_control:
raise CanonControlPrivilegeError("control privilege required to jog")
if axis not in (Axis.PAN, Axis.TILT):
raise CanonCommandError(f"continuous jog not supported for axis {axis.value}")
# SPEC: continuous-move encoding. Modelled as a signed speed on the axis.
self._request("control", {_PARAM[f"{axis.value}_speed"]: int(round(speed * _SPEED_SCALE))})
"""Not supported on this camera -- raises.
MEASURED 2026-08-18: writing ``c.1.<axis>.speed`` on its own does **not**
start continuous motion. That field is a *setting consumed by the next move*,
not a jog command, and no continuous-move CGI was found in the info.cgi
surface. The camera accepted the write and did nothing -- the same silent
acceptance that hid the broken ``stop()``.
Rather than keep a no-op that reports success, this raises. A caller that
wants continuous motion can issue successive absolute moves, or the real
continuous-move command can be harvested from the camera's own web UI
(DevTools -> Network while dragging its PTZ control) and implemented here.
"""
raise CanonCommandError(
f"continuous jog is not supported by this camera (axis {axis.value}); "
"c.1.<axis>.speed is a move setting, not a jog command. Use repeated "
"move_absolute() calls, or harvest the real command from the web UI."
)
def is_moving(self, axis: Axis) -> bool:
# MEASURED: c.1.<axis>.status is 0 while the axis is idle. Focus has no such
@@ -564,9 +576,13 @@ class XCTransport(CameraTransport):
``p.count:=100`` slots, of which only ``p.1`` ("home") is enabled by default,
and the currently selected one is reported as ``p:=<id>``.
SPEC/UNVERIFIED: recall is issued as ``control.cgi?p=<id>``, following the
pattern that control.cgi writes the same names info.cgi reports. Confirm at
commissioning Stage 6; if it is wrong, this one line is what changes.
**Recall via ``control.cgi?p=<id>`` was CONFIRMED against hardware on
2026-08-18** -- the inference that control.cgi writes the same names info.cgi
reports held, and this was the last unverified guess in the transport.
Note that recall runs at the preset's **own stored speeds** (``p.1.pan.speed``
is 30000, i.e. 300 deg/s -- three times the ceiling for a commanded move) and
moves every axis at once. It is the fastest motion this camera makes.
"""
if not self._has_control:
raise CanonControlPrivilegeError("control privilege required to recall presets")
@@ -29,6 +29,8 @@ def _args(**kw):
jog_speed=3.0,
jog_seconds=0.05,
focus_settle=0.02,
focus_delta=0.15,
focus_tolerance=0.03,
restore_focus_mode=False,
preset=1,
preset_settle=0.02,
@@ -88,12 +90,39 @@ def test_focus_mode_dry_run_sends_nothing(fake):
assert not any(c.startswith("set_focus_mode") for c in fake.call_log)
def test_focus_mode_switches_to_manual_and_confirms_it_holds(fake):
def test_focus_mode_proves_manual_by_commanding_a_move_that_sticks(fake):
"""Drift-sampling was inconclusive on hardware; commanding a move is decisive."""
fake.set_focus_mode(FocusMode.AUTO)
report = check_focus_mode(fake, _args(go=True))
assert report["before"] == "auto"
assert report["after_manual"] == "manual"
assert report["focus_mode_works"] is True
# It actually moved focus, rather than just watching it.
assert report["focus_landed"] == pytest.approx(report["focus_target"])
assert report["focus_target"] != report["focus_before"]
def test_focus_mode_fails_when_focus_will_not_hold(fake):
"""Reaching the target then sliding away means something still drives focus."""
class SlidingFocus(FakeTransport):
def get_position(self, axis):
pos = super().get_position(axis)
if axis is Axis.FOCUS:
self._positions[axis] = min(1.0, pos + 0.2) # creeps every read
return pos
t = SlidingFocus()
t.connect()
t.claim_control()
report = check_focus_mode(t, _args(go=True))
assert report["focus_mode_works"] is False
def test_focus_mode_stays_inside_the_range_near_the_top(fake):
fake.move_absolute(Axis.FOCUS, 0.95)
report = check_focus_mode(fake, _args(go=True))
assert 0.0 <= report["focus_target"] <= 1.0
def test_focus_mode_detects_a_write_the_camera_ignores(fake):
@@ -515,3 +515,30 @@ def test_stop_reports_an_axis_it_cannot_read_rather_than_failing_silently():
t.claim_control()
with pytest.raises(CanonCommandError, match="pan"):
t.stop(Axis.PAN)
def test_xc_jog_raises_rather_than_silently_doing_nothing():
"""MEASURED 2026-08-18: this camera has no continuous-move command.
Writing c.1.<axis>.speed alone does not start motion -- it is a setting the next
move consumes -- and the camera accepted the write and did nothing. A no-op that
reports success is exactly the failure mode that hid the broken stop(), so jog
now raises instead.
"""
opener = RecordingOpener(bodies={"open.cgi": REAL_OPEN_BODY})
t = make_xc(opener)
t.connect()
t.claim_control()
opener.requests.clear()
with pytest.raises(CanonCommandError, match="not supported"):
t.jog(Axis.PAN, 5.0)
assert not opener.requests, "it must not send anything it knows will be ignored"
def test_device_no_longer_advertises_jog_in_user_access():
"""USER_ACCESS is the discoverable API; it must not list a dead capability."""
from csaxs_bec.devices.canon.canon_ptz import CanonPTZCamera
assert "jog_pan" not in CanonPTZCamera.USER_ACCESS
assert "jog_tilt" not in CanonPTZCamera.USER_ACCESS
assert "recall_preset" in CanonPTZCamera.USER_ACCESS # this one does work