diff --git a/csaxs_bec/devices/canon/COMMISSIONING.md b/csaxs_bec/devices/canon/COMMISSIONING.md index 995e96e9..1d78ea4e 100644 --- a/csaxs_bec/devices/canon/COMMISSIONING.md +++ b/csaxs_bec/devices/canon/COMMISSIONING.md @@ -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..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. -**R3 — Preset CGI.** `preset.cgi?preset=`. *Symptom:* preset recall -errors or no-op. *Fix:* `_CGI["preset"]` / `_PARAM["preset"]`. +**R3 — Preset recall. RESOLVED [[2026-08-18]].** There is no +`preset.cgi`; presets are the `p.*` namespace, and recall via **`control.cgi?p=` +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. **R9 — Stop command. CONFIRMED BROKEN as shipped; now fixed.** `stop()` used to send `control.cgi?stop=`. **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..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 diff --git a/csaxs_bec/devices/canon/README.md b/csaxs_bec/devices/canon/README.md index 83da31eb..a7bc5e37 100644 --- a/csaxs_bec/devices/canon/README.md +++ b/csaxs_bec/devices/canon/README.md @@ -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..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` diff --git a/csaxs_bec/devices/canon/canon_ptz.py b/csaxs_bec/devices/canon/canon_ptz.py index 94b0d3e9..c6869cb4 100644 --- a/csaxs_bec/devices/canon/canon_ptz.py +++ b/csaxs_bec/devices/canon/canon_ptz.py @@ -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: diff --git a/csaxs_bec/devices/canon/feature_check.py b/csaxs_bec/devices/canon/feature_check.py index 8c3fd72b..63d8597d 100644 --- a/csaxs_bec/devices/canon/feature_check.py +++ b/csaxs_bec/devices/canon/feature_check.py @@ -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=``. 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", diff --git a/csaxs_bec/devices/canon/transport/xc.py b/csaxs_bec/devices/canon/transport/xc.py index c1fe677a..61823b07 100644 --- a/csaxs_bec/devices/canon/transport/xc.py +++ b/csaxs_bec/devices/canon/transport/xc.py @@ -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..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..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..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:=``. - SPEC/UNVERIFIED: recall is issued as ``control.cgi?p=``, 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=`` 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") diff --git a/tests/tests_devices/test_canon_feature_check.py b/tests/tests_devices/test_canon_feature_check.py index e0a8ac29..51291673 100644 --- a/tests/tests_devices/test_canon_feature_check.py +++ b/tests/tests_devices/test_canon_feature_check.py @@ -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): diff --git a/tests/tests_devices/test_canon_transport.py b/tests/tests_devices/test_canon_transport.py index 9aa02bb5..e293e454 100644 --- a/tests/tests_devices/test_canon_transport.py +++ b/tests/tests_devices/test_canon_transport.py @@ -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..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