From 1c240d5972e9442f7b4d763d6c33149b86344378 Mon Sep 17 00:00:00 2001 From: Ali Kazemi Date: Tue, 29 Sep 2026 02:16:47 -0600 Subject: [PATCH 1/5] PI N-472: NUL-terminated connect string, checked connect, retryable object, working stop initialize handed PI_ConnectUSB a description with every 0x00 stripped and no terminator, so the DLL read past the vector until it met a zero by chance; it also set connectionstatus before the connect was checked, so a -1 return was logged as "Stage initialized" and a retry was refused. shutdown never cleared the flag. stopmotion passed a Vector{String} where the DLL wants one string. Verified on the rig (C-885 SN 124014300): init, stop (returns 1), shutdown, re-init on the same object, and the held-controller refusal. No motion. Adds a "PI N472 (no hardware)" testset that skips its DLL path whenever a controller enumerates, so the suite never commands attached hardware. Bumps to 0.2.4; documents the ccall-boundary rules in CLAUDE.md and the skills. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 29 ++++++++++ CLAUDE.md | 15 +++++ Project.toml | 4 +- README.md | 4 +- skills/mc-extend/references/rig-causes.md | 1 + skills/mc-system-design/SKILL.md | 2 +- .../references/driver-caveats.md | 9 +++ .../pi_n472/interface_methods.jl | 58 ++++++++++++++----- test/runtests.jl | 48 +++++++++++++++ 9 files changed, 150 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d4a789..a7edcf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,35 @@ and `y` is the non-breaking one (every merge to `main` is tagged). ## [Unreleased] +### Fixed + +PI N-472 actuator driver (`PI_N472`), initialize/shutdown/stop. Same family +as the v0.1.1 C-867 fix: memory-layout accidents at the GCS2 boundary that +work most of the time. **Hardware verification: NOT DONE** on the actuator +itself; the no-controller path is exercised by the new "PI N472 (no +hardware)" testset against the installed DLL, and the rest is traced from the +source. + +- **`initialize` handed `PI_ConnectUSB` a description with no NUL + terminator.** The enumeration buffer was stripped of every `0x00` byte and + passed as a bare `Vector{UInt8}`, so the DLL read past its end until it met + a zero by chance. That explains an initialize that "usually works". It now + passes the first enumerated line as a Julia `String`, which is always + NUL-terminated at the `Ptr{Cchar}` boundary. The buffer grew from 128 to + 1024 bytes to match the C-867 driver. +- **A failed connect was silent and poisoned the object.** `connectionstatus` + was set before `PI_ConnectUSB` was called and its `-1` return never checked, + so every later GCS call failed quietly, "Stage initialized" was still logged, + and a retry was refused as "already initialized". The flag is now set only + after a successful connect; a failure logs the description and + `PI_GetInitError()` and leaves the object retryable. +- **`shutdown` never cleared `connectionstatus`**, so re-initializing the same + object in one session was always refused. It now clears the flag. +- **`stopmotion` could not reach the controller.** It passed `stage.axes`, a + `Vector{String}`, where the DLL wants one space-separated `Ptr{Cchar}` + string; the pointer handed over pointed at string references, not + characters. It now joins the axes like every other call in the driver. + ### Fixed (documentation) - **Depending on this package needs more than pinning the tag, and the docs did not say so.** MicroscopeControl depends on the unregistered `DAQmx.jl` and diff --git a/CLAUDE.md b/CLAUDE.md index ed538b9..1b36d1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,6 +119,21 @@ Hardware implementations use `ccall` for vendor SDKs: - `mcl_stage/*.jl` - Mad City Labs NanoDrive - Serial devices (CrystaLaser, Vortran, Triggerscope) use `LibSerialPort` +Rules at the `ccall` boundary, each learned from a bug that "usually worked" +(C-867 servo, v0.1.1; N-472 initialize, v0.2.4): +- A `Ptr{Cchar}` argument (GCS2 axes lists, USB descriptions) gets a Julia + `String`, which is always NUL-terminated. Never a `Vector{UInt8}` with the + zeros filtered out, and never a `Vector{String}`; join axes with a space first. +- A `BOOL*` argument is 32-bit (`Cuint`/`Cint`), one element per axis, never `UInt8`. +- Set a driver's `connectionstatus` only after the connect call's return value + is checked, and clear it in `shutdown`, so a failed or closed object can be + initialized again. + +The test suite must never command attached hardware. Driver tests that touch a +vendor DLL run only its failure paths, and skip when the device enumerates +(see the "PI N472 (no hardware)" testset: on the rig, the C-885 is plugged in +and `initialize` would turn its servos on). + ### Camera Image Data Convention **Convention:** Image data is stored and displayed as column-major `(H, W, N)` arrays where `data[row, col]` = `data[y, x]`. diff --git a/Project.toml b/Project.toml index 87019fa..1f56ba2 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "MicroscopeControl" uuid = "aa70d9ae-4a1e-49fd-870a-8ccfd99f4c3e" -version = "0.2.3" +version = "0.2.4" authors = ["klidke@unm.edu"] [deps] @@ -23,7 +23,7 @@ Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2" TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76" [sources] -DAQmx = {url = "https://github.com/LidkeLab/DAQmx.jl.git"} +DAQmx = {rev = "main", url = "https://github.com/LidkeLab/DAQmx.jl.git"} [compat] CEnum = "0.5.0" diff --git a/README.md b/README.md index a8376d5..f1c63fe 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ Since this package is under active development and not yet registered, install i ```julia using Pkg -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` **You must also declare this package's unregistered dependency in your own @@ -88,7 +88,7 @@ writes both entries for you: ```julia using Pkg Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` If you write the TOML by hand, `[sources]` alone is **not** enough — Julia diff --git a/skills/mc-extend/references/rig-causes.md b/skills/mc-extend/references/rig-causes.md index aeb0552..5e2c176 100644 --- a/skills/mc-extend/references/rig-causes.md +++ b/skills/mc-extend/references/rig-causes.md @@ -10,6 +10,7 @@ and each looks like a broken `ccall`. | Observed symptom | Possible causes | How to tell | |---|---|---| | `initialize(stage::PIStage)` logs `@error` ("No PI C-867 found ..." or "PI_ConnectUSB failed ...") and returns; `stage.connectionstatus` stays `false`. It does **not** throw. | Controller absent or unpowered; USB enumeration; **or** held by another process (a second Julia session with an initialized stage, PIMikroMove, an open COM port). None of these is established by the error alone. | Check `stage.connectionstatus` after every `initialize`. Close PIMikroMove and every other Julia; check Task Manager for stray `julia.exe`. If it then connects, it was contention, not the driver. | +| `initialize(stage::N472)` logs `@error` ("No PI C-885 found ..." or "PI_ConnectUSB failed ...") and returns; `stage.connectionstatus` stays `false`. It does **not** throw. **[fixed in v0.2.4]** Before that release the flag was set *before* the connect was checked, so a failed connect logged "Stage initialized", left every later GCS call failing silently, and a retry was refused as "already initialized"; and the description handed to `PI_ConnectUSB` had no NUL terminator, so a connect could fail by chance from one run to the next. | Same causes as the C-867 row: controller absent or unpowered, or held by another process. On a pre-0.2.4 copy, add: the driver itself. | As for the C-867 row. On a pre-0.2.4 copy, an `initialize` that "usually works" is the driver, not the rig; rebuild the `N472()` object before retrying, since `shutdown` did not clear the flag either. | | `MLSLM` SDK calls do nothing useful | The board is claimed by another process (vendor GUI or a dead Julia), or the SDK never found it. | `MLSLM()` is pure: its `n_boards_found` field is a **default of 0** and is never updated, so it diagnoses nothing. The only board count is what `initializesdk()` prints to stdout from `Create_SDK`. Read that output; if it reports 0 boards with the board powered, close the vendor GUI and every other Julia, then reboot if a dead process still holds it. | | Serial device times out, returns garbage, or "port busy" | Wrong COM assignment (Windows renumbers COM ports when USB topology changes), or the port is open elsewhere. Applies only to the **Triggerscope** (`Triggerscope4`, default `portname="COM3"`) and, through it, `LCC1620`. | Device Manager: match the port to the device. From Julia, `MicroscopeControl.HardwareImplementations.Triggerscope.LibSerialPort.list_ports()`. One `Triggerscope4` per physical port, shared by its dependents (`mc-system-design`). | | A DAQ-backed light (`CrystaLaser`, `VortranLaser`, `DaqTrLight`) `@warn`s at construction ("No NI-DAQ devices found ..." or "Failed to initialize NI-DAQ ..."), then `initialize` returns normally (Vortran's may `@warn` "insufficient DO channels for initialization") and `setpower`/`light_on` `@warn` about missing channels and do nothing, or only some calls work | These devices are **not** serial. They drive NI-DAQ AO/DO channels through an `NIdaq` built in their constructor, which picks `devs[1]` (Crysta, Vortran) or `devs[device_index]` (DaqTrLight, default 2). Discovery runs AO then DO inside one `try`, so it can **partially** succeed: AO channels kept, DO empty. Causes: wrong device index, missing NI-DAQmx runtime, card not enumerated, or a DO-less card. | Inspect `light.channelsAO` and `light.channelsDO` right after construction; `NIDAQcard.showdevices(NIdaq())` from Julia; compare with NI MAX. Fix the index or the runtime, not the driver. | diff --git a/skills/mc-system-design/SKILL.md b/skills/mc-system-design/SKILL.md index 8eb558d..2e0112b 100644 --- a/skills/mc-system-design/SKILL.md +++ b/skills/mc-system-design/SKILL.md @@ -34,7 +34,7 @@ MicroscopeControl and Pkg writes both entries for you: ```julia Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` Writing the TOML by hand needs **both** a `[deps]` and a `[sources]` entry — diff --git a/skills/mc-system-design/references/driver-caveats.md b/skills/mc-system-design/references/driver-caveats.md index 1560c3a..89a4d4e 100644 --- a/skills/mc-system-design/references/driver-caveats.md +++ b/skills/mc-system-design/references/driver-caveats.md @@ -93,6 +93,15 @@ Resolving to a device-specific method is not the same as the operation working: - `getposition(::N472)` returns the SDK success code and stores positions in `stage.pos`; `move(::N472, pos::Vector{Float64})` takes a vector, not `x, y, z` (traced). +- `stopmotion(::N472)` **[fixed in v0.2.4]**: before that release it passed + `stage.axes` (a `Vector{String}`) where the DLL wants one space-separated + string, so the halt never reached the controller (traced). +- `initialize(::N472)` / `shutdown(::N472)` **[fixed in v0.2.4]**: `initialize` + now sets `connectionstatus` only after `PI_ConnectUSB` succeeds and leaves + the object retryable on failure; `shutdown` clears the flag so the same + object can be initialized again. Before 0.2.4 a failed connect was reported + as "Stage initialized" and both a retry and a re-initialize after `shutdown` + were refused (verified on the rig: init, stop, shutdown, init on one object). - `capture` returns the `SINGLE_FRAME` enum on `SimCamera` and the frame on the hardware cameras, and `getdata` after `capture` is valid **only** on `SimCamera` (DCAM4 and DCX release their buffers before `capture` returns). diff --git a/src/hardware_implementations/pi_n472/interface_methods.jl b/src/hardware_implementations/pi_n472/interface_methods.jl index 7ea9d25..8c33c48 100644 --- a/src/hardware_implementations/pi_n472/interface_methods.jl +++ b/src/hardware_implementations/pi_n472/interface_methods.jl @@ -1,3 +1,12 @@ +""" + _cstring(buf) -> String + +The bytes of `buf` up to its first NUL, as a `String`. +""" +function _cstring(buf::Vector{UInt8}) + i = findfirst(==(0x00), buf) + return String(buf[1:(i === nothing ? end : i - 1)]) +end function initialize(stage::N472) if stage.connectionstatus == true @@ -5,28 +14,40 @@ function initialize(stage::N472) return end - # Create a buffer string - buffersize = 128 - devstring = zeros(UInt8, buffersize) + # Enumerate the C-885 controllers the GCS2 DLL can see. The DLL lists only + # controllers nobody has open: one that Device Manager still shows but is + # missing here is held by another process (a second Julia, PIMikroMove, an + # open COM port). + buffersize = 1024 + buffer = zeros(UInt8, buffersize) controllername = "C-885" - numdevice = PI_EnumerateUSB(devstring, buffersize, controllername) - devstring = filter(x -> x != 0x00, devstring) - - - - #Set connection status to true - if numdevice > 0 - stage.connectionstatus = true - else - @error "No devices connected" + numdevice = PI_EnumerateUSB(buffer, buffersize, controllername) + if numdevice <= 0 + @error "No PI C-885 found by the GCS2 library — controller absent, or held by another process" stage.connectionstatus = false return end + # The buffer holds one NUL-terminated description per line. Pass the first + # line as a `String`: Julia strings are always NUL-terminated for + # `Ptr{Cchar}`, whereas the old code stripped every 0x00 byte and handed + # the DLL a bare `Vector{UInt8}`, terminated only by whatever happened to + # follow it in memory (usually a zero, so it usually worked). + devstring = String(first(split(_cstring(buffer), '\n'))) + @info "PI device: " * devstring + #Connect to usb device stage.id = PI_ConnectUSB(devstring) - @info "PI device: " * String(devstring) @info "Device ID: " * string(stage.id) + if stage.id < 0 + # The connect itself failed (id -1): typically another process already + # holds the controller. Leave the flag cleared so `initialize` can be + # retried on the same object. + stage.connectionstatus = false + @error "PI_ConnectUSB failed for \"$devstring\" (init error $(PI_GetInitError())) — the controller is probably held by another process" + return + end + stage.connectionstatus = true #Query the unit of the physical position axes = join(stage.axes, " ") @@ -67,6 +88,9 @@ function shutdown(stage::N472) else @info "Stage not connected" end + # Clear the flag so the same object can be initialized again; before this + # a second `initialize` after `shutdown` was refused as "already initialized". + stage.connectionstatus = false return end @@ -95,7 +119,11 @@ function StageInterface.home(stage::N472) end function StageInterface.stopmotion(stage::N472) - success = PI_HLT(stage.id, stage.axes) + # Every GCS2 axes argument is one space-separated string. `stage.axes` is a + # Vector{String}; passing it as Ptr{Cchar} handed the DLL a pointer to + # string references, not characters, so the halt never reached the axes. + axes = join(stage.axes, " ") + success = PI_HLT(stage.id, axes) return success end diff --git a/test/runtests.jl b/test/runtests.jl index 5faf0e5..633461a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -702,6 +702,54 @@ include("tcube_fake_sdk.jl") end end + @testset "PI N472 (no hardware)" begin + N = MicroscopeControl.HardwareImplementations.PI_N472 + + @testset "_cstring stops at the first NUL" begin + buf = zeros(UInt8, 32) + buf[1:14] .= codeunits("PI C-885 SN 42") + buf[20] = UInt8('x') # stale bytes past the terminator are ignored + @test N._cstring(buf) == "PI C-885 SN 42" + @test N._cstring(codeunits("abc") |> collect) == "abc" + @test N._cstring(UInt8[0x00]) == "" + end + + @testset "shutdown clears connectionstatus" begin + stage = N472() + stage.connectionstatus = true + stage.id = Cint(-1) # never connected; PI_IsConnected(-1) is FALSE + if isfile(N.PI_GCS2) + @test_logs (:info, "Stage not connected") shutdown(stage) + @test stage.connectionstatus == false + else + @test_skip "PI GCS2 DLL not installed" + end + end + + @testset "initialize without a controller leaves the object retryable" begin + # Never command a real controller from the suite: on a rig with a + # C-885 attached, `initialize` connects, zeroes the origin and turns + # the servos on. Run this branch only when enumeration finds nothing. + present = isfile(N.PI_GCS2) && + N.PI_EnumerateUSB(zeros(UInt8, 1024), 1024, "C-885") > 0 + if present + @test_skip "PI C-885 attached; not commanding real hardware from the suite" + elseif isfile(N.PI_GCS2) + stage = N472() + # No C-885 is plugged into a build box: enumeration finds nothing, + # initialize must say so and leave the flag cleared (before this + # the flag was set before the connect was checked). + @test_logs (:error, r"No PI C-885 found") initialize(stage) + @test stage.connectionstatus == false + @test stage.id == 0 + # and a second call is not refused as "already initialized" + @test_logs (:error, r"No PI C-885 found") initialize(stage) + else + @test_skip "PI GCS2 DLL not installed" + end + end + end + include("contract.jl") include("skills.jl") include("gui.jl") From 7bc9436096efde0d604a48ebe3e043ed0ed32880 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 09:58:49 -0600 Subject: [PATCH 2/5] Move PR #67 to 0.3rc1 and correct its release notes Per the ruling that this work collects on 0.3rc1: Project.toml, the README and mc-system-design's install pins return to the 0.3rc1 values (no 0.2.4 bump), and DAQmx's [sources] loses the unrelated rev = "main". Markers read v0.3.0. CHANGELOG: one hardware statement (not verified in this repository; the author's no-motion exercise on a C-885 described as such); the connect string is described as relying on filter's implementation rather than missing a terminator, and not as the cause of the intermittent initialize; the id/shutdown fix and the behaviour changes a pinned rig will see (throw on setup failure, re-zeroing re-initialize, first enumerated controller, default id, setvel's return) are listed individually. CLAUDE.md: provenance without a version, BOOL* rule scoped to GCS2, shutdown clears the id too, driver tests use recorded fakes. rig-causes merges the C-867 and C-885 rows; driver-caveats drops the rig claim. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 64 ++++++++++++++----- CLAUDE.md | 17 ++--- Project.toml | 4 +- README.md | 4 +- skills/mc-extend/references/rig-causes.md | 3 +- skills/mc-system-design/SKILL.md | 2 +- .../references/driver-caveats.md | 15 +++-- 7 files changed, 72 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7edcf2..f525133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,32 +11,66 @@ and `y` is the non-breaking one (every merge to `main` is tagged). ### Fixed -PI N-472 actuator driver (`PI_N472`), initialize/shutdown/stop. Same family -as the v0.1.1 C-867 fix: memory-layout accidents at the GCS2 boundary that -work most of the time. **Hardware verification: NOT DONE** on the actuator -itself; the no-controller path is exercised by the new "PI N472 (no -hardware)" testset against the installed DLL, and the rest is traced from the -source. - -- **`initialize` handed `PI_ConnectUSB` a description with no NUL - terminator.** The enumeration buffer was stripped of every `0x00` byte and - passed as a bare `Vector{UInt8}`, so the DLL read past its end until it met - a zero by chance. That explains an initialize that "usually works". It now - passes the first enumerated line as a Julia `String`, which is always - NUL-terminated at the `Ptr{Cchar}` boundary. The buffer grew from 128 to - 1024 bytes to match the C-867 driver. +PI N-472 actuator driver (`PI_N472`): initialize, shutdown and stop. The +lifecycle is covered by fake-GCS2 tests that run on every machine +(`test/pi_n472_fake_sdk.jl`). Hardware: not verified in this repository. The +author reports exercising initialize, `stopmotion` with no motion in +progress, shutdown, re-initialize and a refused second object on a C-885 +(SN 124014300), with no motion commanded; stop during motion and the +connect-failure branch were not exercised on hardware. + - **A failed connect was silent and poisoned the object.** `connectionstatus` was set before `PI_ConnectUSB` was called and its `-1` return never checked, so every later GCS call failed quietly, "Stage initialized" was still logged, and a retry was refused as "already initialized". The flag is now set only after a successful connect; a failure logs the description and - `PI_GetInitError()` and leaves the object retryable. + `PI_GetInitError()` and leaves the object retryable. The intermittent + initialize seen on the rig is more likely another process holding the + controller, made sticky by this bug, than anything in the connect string; + that is not established, so do not treat it as fixed until the rig says so. - **`shutdown` never cleared `connectionstatus`**, so re-initializing the same object in one session was always refused. It now clears the flag. +- **`shutdown` could close another object's connection.** `id` defaulted to + `0`, a valid GCS ID, and `shutdown` never reset it, so a second `shutdown` on + a closed object, or a `shutdown` on one never initialized, closed whichever + controller then held ID 0. `id` now defaults to `-1` and `shutdown` resets it. - **`stopmotion` could not reach the controller.** It passed `stage.axes`, a `Vector{String}`, where the DLL wants one space-separated `Ptr{Cchar}` string; the pointer handed over pointed at string references, not characters. It now joins the axes like every other call in the driver. +- **The connect string relied on an implementation detail.** `initialize` + filtered every `0x00` out of the enumeration buffer and passed the bare + `Vector{UInt8}`. On current Julia that happens to leave a zero just past the + shrunk vector, so the DLL did see a terminated string, but by accident of + `filter`'s implementation, and carrying the enumeration's trailing newline + (and every further description when several controllers are attached). It + now passes the first description, whitespace-stripped, as a `String`, which + Julia always NUL-terminates at a `Ptr{Cchar}` boundary. The buffer grew from + 128 to 1024 bytes to match the C-867 driver. + +### Changed + +Behaviour a rig pinned to an earlier tag will see from the PI N-472 driver, +each on its own: + +- **`initialize(::N472)` now throws when a setup step fails.** After the + connect, every step (reference mode, `PI_POS`, servos, travel range, + velocity) is checked; a failure closes the connection, clears + `connectionstatus` and `id`, and throws with the step and its GCS error + code. Before, failures were ignored and "Stage initialized" was logged on a + half-initialized stage. Enumeration and connect failures still log `@error` + and return, as `initialize(::PIStage)` does. **Breaking** under this + package's versioning rule: what the call throws changed. +- **Re-initializing after `shutdown` now re-zeroes the frame.** A second + `initialize` on the same object was refused and did nothing; it now runs the + full sequence: reference mode off, `PI_POS` redefining the current position + as `homepos`, servos on. A script that used shutdown-then-initialize as a + reconnect now resets its coordinates to wherever the actuator sits. +- **With several C-885s attached, `initialize` connects to the first + enumerated one.** There is no selection by serial number. +- **`N472()` defaults `id` to `-1`**, not `0`. +- **`setvel(::N472)` returns `FALSE` when `PI_VEL` fails.** It used to return + only the status of the `PI_qVEL` read-back. ### Fixed (documentation) - **Depending on this package needs more than pinning the tag, and the docs did diff --git a/CLAUDE.md b/CLAUDE.md index 40e4e28..45d9dc6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,19 +120,20 @@ Hardware implementations use `ccall` for vendor SDKs: - Serial devices (CrystaLaser, Vortran, Triggerscope) use `LibSerialPort` Rules at the `ccall` boundary, each learned from a bug that "usually worked" -(C-867 servo, v0.1.1; N-472 initialize, v0.2.4): +(C-867 servo, v0.1.1; N-472 stopmotion): - A `Ptr{Cchar}` argument (GCS2 axes lists, USB descriptions) gets a Julia `String`, which is always NUL-terminated. Never a `Vector{UInt8}` with the zeros filtered out, and never a `Vector{String}`; join axes with a space first. -- A `BOOL*` argument is 32-bit (`Cuint`/`Cint`), one element per axis, never `UInt8`. +- A GCS2 `BOOL*` argument is 32-bit (`Cuint`/`Cint`), one element per axis, + never `UInt8`. - Set a driver's `connectionstatus` only after the connect call's return value - is checked, and clear it in `shutdown`, so a failed or closed object can be - initialized again. + is checked, and clear it and the device id in `shutdown`, so a failed or + closed object can be initialized again and a stale id cannot close another + object's connection. -The test suite must never command attached hardware. Driver tests that touch a -vendor DLL run only its failure paths, and skip when the device enumerates -(see the "PI N472 (no hardware)" testset: on the rig, the C-885 is plugged in -and `initialize` would turn its servos on). +The test suite must never command attached hardware. Driver tests replace the +vendor wrappers with a recorder (`test/tcube_fake_sdk.jl`, +`test/pi_n472_fake_sdk.jl`), so they run on every machine and never reach a DLL. ### Camera Image Data Convention diff --git a/Project.toml b/Project.toml index 1f56ba2..87019fa 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "MicroscopeControl" uuid = "aa70d9ae-4a1e-49fd-870a-8ccfd99f4c3e" -version = "0.2.4" +version = "0.2.3" authors = ["klidke@unm.edu"] [deps] @@ -23,7 +23,7 @@ Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2" TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76" [sources] -DAQmx = {rev = "main", url = "https://github.com/LidkeLab/DAQmx.jl.git"} +DAQmx = {url = "https://github.com/LidkeLab/DAQmx.jl.git"} [compat] CEnum = "0.5.0" diff --git a/README.md b/README.md index f1c63fe..a8376d5 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ Since this package is under active development and not yet registered, install i ```julia using Pkg -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") ``` **You must also declare this package's unregistered dependency in your own @@ -88,7 +88,7 @@ writes both entries for you: ```julia using Pkg Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") ``` If you write the TOML by hand, `[sources]` alone is **not** enough — Julia diff --git a/skills/mc-extend/references/rig-causes.md b/skills/mc-extend/references/rig-causes.md index 5e2c176..cc7e6f1 100644 --- a/skills/mc-extend/references/rig-causes.md +++ b/skills/mc-extend/references/rig-causes.md @@ -9,8 +9,7 @@ and each looks like a broken `ccall`. | Observed symptom | Possible causes | How to tell | |---|---|---| -| `initialize(stage::PIStage)` logs `@error` ("No PI C-867 found ..." or "PI_ConnectUSB failed ...") and returns; `stage.connectionstatus` stays `false`. It does **not** throw. | Controller absent or unpowered; USB enumeration; **or** held by another process (a second Julia session with an initialized stage, PIMikroMove, an open COM port). None of these is established by the error alone. | Check `stage.connectionstatus` after every `initialize`. Close PIMikroMove and every other Julia; check Task Manager for stray `julia.exe`. If it then connects, it was contention, not the driver. | -| `initialize(stage::N472)` logs `@error` ("No PI C-885 found ..." or "PI_ConnectUSB failed ...") and returns; `stage.connectionstatus` stays `false`. It does **not** throw. **[fixed in v0.2.4]** Before that release the flag was set *before* the connect was checked, so a failed connect logged "Stage initialized", left every later GCS call failing silently, and a retry was refused as "already initialized"; and the description handed to `PI_ConnectUSB` had no NUL terminator, so a connect could fail by chance from one run to the next. | Same causes as the C-867 row: controller absent or unpowered, or held by another process. On a pre-0.2.4 copy, add: the driver itself. | As for the C-867 row. On a pre-0.2.4 copy, an `initialize` that "usually works" is the driver, not the rig; rebuild the `N472()` object before retrying, since `shutdown` did not clear the flag either. | +| `initialize` on a PI C-867 (`PIStage`) or C-885 (`N472`) logs `@error` ("No PI C-867 found ...", "No PI C-885 found ..." or "PI_ConnectUSB failed ...") and returns; `stage.connectionstatus` stays `false`. It does **not** throw; a failure *after* the connect does (a refused reference move on the C-867, any setup step on the C-885). **[N472 fixed in v0.3.0]** Before that release the N472 set the flag before checking the connect, so a failed connect logged "Stage initialized" and a retry was refused as "already initialized". | Controller absent or unpowered; USB enumeration; **or** held by another process (a second Julia session with an initialized stage, PIMikroMove, an open COM port). None of these is established by the error alone. | Check `stage.connectionstatus` after every `initialize`. Close PIMikroMove and every other Julia; check Task Manager for stray `julia.exe`. If it then connects, it was contention, not the driver. On an N472 before 0.3.0, build a fresh `N472()` before retrying: neither a failed connect nor `shutdown` cleared the flag. | | `MLSLM` SDK calls do nothing useful | The board is claimed by another process (vendor GUI or a dead Julia), or the SDK never found it. | `MLSLM()` is pure: its `n_boards_found` field is a **default of 0** and is never updated, so it diagnoses nothing. The only board count is what `initializesdk()` prints to stdout from `Create_SDK`. Read that output; if it reports 0 boards with the board powered, close the vendor GUI and every other Julia, then reboot if a dead process still holds it. | | Serial device times out, returns garbage, or "port busy" | Wrong COM assignment (Windows renumbers COM ports when USB topology changes), or the port is open elsewhere. Applies only to the **Triggerscope** (`Triggerscope4`, default `portname="COM3"`) and, through it, `LCC1620`. | Device Manager: match the port to the device. From Julia, `MicroscopeControl.HardwareImplementations.Triggerscope.LibSerialPort.list_ports()`. One `Triggerscope4` per physical port, shared by its dependents (`mc-system-design`). | | A DAQ-backed light (`CrystaLaser`, `VortranLaser`, `DaqTrLight`) `@warn`s at construction ("No NI-DAQ devices found ..." or "Failed to initialize NI-DAQ ..."), then `initialize` returns normally (Vortran's may `@warn` "insufficient DO channels for initialization") and `setpower`/`light_on` `@warn` about missing channels and do nothing, or only some calls work | These devices are **not** serial. They drive NI-DAQ AO/DO channels through an `NIdaq` built in their constructor, which picks `devs[1]` (Crysta, Vortran) or `devs[device_index]` (DaqTrLight, default 2). Discovery runs AO then DO inside one `try`, so it can **partially** succeed: AO channels kept, DO empty. Causes: wrong device index, missing NI-DAQmx runtime, card not enumerated, or a DO-less card. | Inspect `light.channelsAO` and `light.channelsDO` right after construction; `NIDAQcard.showdevices(NIdaq())` from Julia; compare with NI MAX. Fix the index or the runtime, not the driver. | diff --git a/skills/mc-system-design/SKILL.md b/skills/mc-system-design/SKILL.md index 2e0112b..8eb558d 100644 --- a/skills/mc-system-design/SKILL.md +++ b/skills/mc-system-design/SKILL.md @@ -34,7 +34,7 @@ MicroscopeControl and Pkg writes both entries for you: ```julia Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") ``` Writing the TOML by hand needs **both** a `[deps]` and a `[sources]` entry — diff --git a/skills/mc-system-design/references/driver-caveats.md b/skills/mc-system-design/references/driver-caveats.md index 89a4d4e..d741069 100644 --- a/skills/mc-system-design/references/driver-caveats.md +++ b/skills/mc-system-design/references/driver-caveats.md @@ -93,15 +93,16 @@ Resolving to a device-specific method is not the same as the operation working: - `getposition(::N472)` returns the SDK success code and stores positions in `stage.pos`; `move(::N472, pos::Vector{Float64})` takes a vector, not `x, y, z` (traced). -- `stopmotion(::N472)` **[fixed in v0.2.4]**: before that release it passed +- `stopmotion(::N472)` **[fixed in v0.3.0]**: before that release it passed `stage.axes` (a `Vector{String}`) where the DLL wants one space-separated string, so the halt never reached the controller (traced). -- `initialize(::N472)` / `shutdown(::N472)` **[fixed in v0.2.4]**: `initialize` - now sets `connectionstatus` only after `PI_ConnectUSB` succeeds and leaves - the object retryable on failure; `shutdown` clears the flag so the same - object can be initialized again. Before 0.2.4 a failed connect was reported - as "Stage initialized" and both a retry and a re-initialize after `shutdown` - were refused (verified on the rig: init, stop, shutdown, init on one object). +- `initialize(::N472)` / `shutdown(::N472)` **[fixed in v0.3.0]**: `initialize` + sets `connectionstatus` only after `PI_ConnectUSB` succeeds and leaves the + object retryable on failure, and throws (after closing the connection) when + a setup step fails; `shutdown` clears the flag and resets `id` to `-1`. + Before 0.3.0 a failed connect was reported as "Stage initialized", both a + retry and a re-initialize after `shutdown` were refused, and a stale `id` of + `0` could close another object's controller (fake-SDK tests). - `capture` returns the `SINGLE_FRAME` enum on `SimCamera` and the frame on the hardware cameras, and `getdata` after `capture` is valid **only** on `SimCamera` (DCAM4 and DCX release their buffers before `capture` returns). From 7237b2fe902cb7c50537339f0a1de1fa5c50af1e Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 10:05:33 -0600 Subject: [PATCH 3/5] Check every N472 initialize step and close on failure; fake-GCS2 tests Co-Authored-By: Claude Sonnet 5.5 --- .../pi_n472/helper.jl | 8 +- .../pi_n472/interface_methods.jl | 77 ++++++------ src/hardware_implementations/pi_n472/types.jl | 2 +- test/pi_n472.jl | 112 ++++++++++++++++++ test/pi_n472_fake_sdk.jl | 109 +++++++++++++++++ test/runtests.jl | 52 +------- 6 files changed, 266 insertions(+), 94 deletions(-) create mode 100644 test/pi_n472.jl create mode 100644 test/pi_n472_fake_sdk.jl diff --git a/src/hardware_implementations/pi_n472/helper.jl b/src/hardware_implementations/pi_n472/helper.jl index 2ee4e7e..1baadea 100644 --- a/src/hardware_implementations/pi_n472/helper.jl +++ b/src/hardware_implementations/pi_n472/helper.jl @@ -71,9 +71,9 @@ function setvel(stage::N472,vel::Vector{Float64}) @error "Failed to set velocity" end - success = PI_qVEL(stage.id, axes, stage.velocity) - if success == FALSE + qsuccess = PI_qVEL(stage.id, axes, stage.velocity) + if qsuccess == FALSE @error "Failed to query velocity" end - return success -end \ No newline at end of file + return (success == FALSE || qsuccess == FALSE) ? FALSE : qsuccess +end diff --git a/src/hardware_implementations/pi_n472/interface_methods.jl b/src/hardware_implementations/pi_n472/interface_methods.jl index 8c33c48..ca33411 100644 --- a/src/hardware_implementations/pi_n472/interface_methods.jl +++ b/src/hardware_implementations/pi_n472/interface_methods.jl @@ -14,67 +14,62 @@ function initialize(stage::N472) return end - # Enumerate the C-885 controllers the GCS2 DLL can see. The DLL lists only - # controllers nobody has open: one that Device Manager still shows but is - # missing here is held by another process (a second Julia, PIMikroMove, an - # open COM port). + # An absent, unpowered or held controller fails here or at the connect below. buffersize = 1024 buffer = zeros(UInt8, buffersize) controllername = "C-885" numdevice = PI_EnumerateUSB(buffer, buffersize, controllername) if numdevice <= 0 - @error "No PI C-885 found by the GCS2 library — controller absent, or held by another process" + @error "No PI C-885 found by the GCS2 library (absent, unpowered, or held by another process)" stage.connectionstatus = false return end - # The buffer holds one NUL-terminated description per line. Pass the first - # line as a `String`: Julia strings are always NUL-terminated for - # `Ptr{Cchar}`, whereas the old code stripped every 0x00 byte and handed - # the DLL a bare `Vector{UInt8}`, terminated only by whatever happened to - # follow it in memory (usually a zero, so it usually worked). - devstring = String(first(split(_cstring(buffer), '\n'))) + # Descriptions are '\n'-separated with one NUL at the end; pass the first as a String (NUL-terminated for Ptr{Cchar}). + devstring = String(strip(first(split(_cstring(buffer), '\n')))) @info "PI device: " * devstring #Connect to usb device stage.id = PI_ConnectUSB(devstring) @info "Device ID: " * string(stage.id) if stage.id < 0 - # The connect itself failed (id -1): typically another process already - # holds the controller. Leave the flag cleared so `initialize` can be - # retried on the same object. + # Connect failed (id -1): leave the flag cleared so initialize can be retried. stage.connectionstatus = false - @error "PI_ConnectUSB failed for \"$devstring\" (init error $(PI_GetInitError())) — the controller is probably held by another process" + @error "PI_ConnectUSB failed for \"$devstring\" (init error $(PI_GetInitError())); the controller may be held by another process" return end stage.connectionstatus = true - #Query the unit of the physical position - axes = join(stage.axes, " ") - #unitstring = zeros(UInt8, buffersize) - #success = PI_qPUN(stage.id, axes, unitstring, buffersize) - #stage.units = String(unitstring) + # Every step from here is checked; on failure close the connection so a retry starts clean. + try + axes = join(stage.axes, " ") + failed(step) = error("N472 initialize: $step failed (GCS error $(PI_GetError(stage.id)))") - #query reference mode - refmode = zeros(BOOL, 3) - - success = PI_RON(stage.id, axes, refmode) - success = PI_qRON(stage.id, axes, refmode) - @info "Reference mode: " * string(refmode) + #query reference mode + refmode = zeros(BOOL, 3) - # set the current position as the reference position - success = set_refpos(stage) + PI_RON(stage.id, axes, refmode) == FALSE && failed("PI_RON") + PI_qRON(stage.id, axes, refmode) == FALSE && failed("PI_qRON") + @info "Reference mode: " * string(refmode) - # turn on servo - for i in eachindex(stage.axes) - servo(stage, i, TRUE) - end - #Query the travel range - success = PI_qTMN(stage.id, axes, stage.minpos) - success = PI_qTMX(stage.id, axes, stage.maxpos) + # set the current position as the reference position + set_refpos(stage) == FALSE && failed("set_refpos (PI_POS)") - #set velocity - success = setvel(stage, stage.velocity) + # turn on servo + for i in eachindex(stage.axes) + servo(stage, i, TRUE) == FALSE && failed("servo axis $i") + end + + #Query the travel range + PI_qTMN(stage.id, axes, stage.minpos) == FALSE && failed("PI_qTMN") + PI_qTMX(stage.id, axes, stage.maxpos) == FALSE && failed("PI_qTMX") + + #set velocity + setvel(stage, stage.velocity) == FALSE && failed("setvel") + catch + shutdown(stage) + rethrow() + end @info "Stage initialized" return @@ -88,9 +83,9 @@ function shutdown(stage::N472) else @info "Stage not connected" end - # Clear the flag so the same object can be initialized again; before this - # a second `initialize` after `shutdown` was refused as "already initialized". + # Clearing both lets the object be re-initialized and stops a stale id closing another object's connection. stage.connectionstatus = false + stage.id = Cint(-1) return end @@ -119,9 +114,7 @@ function StageInterface.home(stage::N472) end function StageInterface.stopmotion(stage::N472) - # Every GCS2 axes argument is one space-separated string. `stage.axes` is a - # Vector{String}; passing it as Ptr{Cchar} handed the DLL a pointer to - # string references, not characters, so the halt never reached the axes. + # GCS2 axes arguments are one space-separated string. axes = join(stage.axes, " ") success = PI_HLT(stage.id, axes) return success diff --git a/src/hardware_implementations/pi_n472/types.jl b/src/hardware_implementations/pi_n472/types.jl index 50976a8..fbee13f 100644 --- a/src/hardware_implementations/pi_n472/types.jl +++ b/src/hardware_implementations/pi_n472/types.jl @@ -27,7 +27,7 @@ function N472(; dimensions::Int=1, axes::Vector{String}=["1", "3", "5"], connectionstatus::Bool=false, - id::Cint=Cint(0), + id::Cint=Cint(-1), pos::Vector{Float64}=[0.0, 0.0, 0.0], minpos::Vector{Float64}=[0.0, 0.0, 0.0], maxpos::Vector{Float64}=[7.0, 7.0, 7.0], diff --git a/test/pi_n472.jl b/test/pi_n472.jl new file mode 100644 index 0000000..45f14a1 --- /dev/null +++ b/test/pi_n472.jl @@ -0,0 +1,112 @@ +@testset "PI N472 (no hardware)" begin + N = MicroscopeControl.HardwareImplementations.PI_N472 + F = Main.FakeGCS2 + quiet(f) = Base.CoreLogging.with_logger(f, Base.CoreLogging.NullLogger()) + setup_ops = ["PI_RON", "PI_qRON", "PI_POS", "PI_SVO", "PI_qSVO", "PI_qTMN", "PI_qTMX", "PI_VEL", "PI_qVEL"] + + @testset "_cstring stops at the first NUL" begin + buf = zeros(UInt8, 32) + buf[1:14] .= codeunits("PI C-885 SN 42") + buf[20] = UInt8('x') # stale bytes past the terminator are ignored + @test N._cstring(buf) == "PI C-885 SN 42" + @test N._cstring(codeunits("abc") |> collect) == "abc" + @test N._cstring(UInt8[0x00]) == "" + end + + @testset "first description is passed as a String" begin + F.reset!() + F.enum_bytes[] = UInt8[codeunits("d1\nd2")..., 0x00, codeunits("junk")...] + F.enum_count[] = 2 + stage = N472() + quiet(() -> initialize(stage)) + @test F.lastarg["PI_ConnectUSB"] isa String + @test F.lastarg["PI_ConnectUSB"] == "d1" + @test stage.connectionstatus == true + @test stage.id == 0 + end + + @testset "description is stripped" begin + F.reset!() + F.enum_bytes[] = UInt8[codeunits(" d1\r\n")..., 0x00] + quiet(() -> initialize(N472())) + @test F.lastarg["PI_ConnectUSB"] == "d1" + end + + @testset "failed connect leaves the object retryable" begin + F.reset!() + append!(F.connect_ids, [-1, 0]) + stage = N472() + @test_logs (:error, r"PI_ConnectUSB failed") match_mode=:any initialize(stage) + @test stage.connectionstatus == false + @test stage.id == -1 + @test !any(op -> op in setup_ops, F.calls) + quiet(() -> initialize(stage)) + @test stage.connectionstatus == true + @test stage.id == 0 + end + + @testset "no controller found" begin + F.reset!() + F.enum_count[] = 0 + stage = N472() + @test_logs (:error, r"No PI C-885 found") match_mode=:any initialize(stage) + @test stage.connectionstatus == false + @test !("PI_ConnectUSB" in F.calls) + @test_logs (:error, r"No PI C-885 found") match_mode=:any initialize(stage) + end + + @testset "stopmotion sends one axes string" begin + F.reset!() + stage = N472() + quiet(() -> initialize(stage)) + quiet(() -> stopmotion(stage)) + @test F.lastarg["PI_HLT"] == "1 3 5" + end + + @testset "shutdown closes only its own connection" begin + F.reset!() + A = N472(); B = N472() + quiet(() -> initialize(A)) + quiet(() -> shutdown(A)) + append!(F.connect_ids, [0, 0]) + quiet(() -> initialize(B)) + @test 0 in F.open_ids + quiet(() -> shutdown(A)) + @test 0 in F.open_ids + @test count(==("PI_CloseConnection"), F.calls) == 1 + @test A.id == -1 + end + + @testset "a fresh object holds no connection" begin + F.reset!() + @test N472().id == -1 + B = N472() + quiet(() -> initialize(B)) + @test 0 in F.open_ids + quiet(() -> shutdown(N472())) + @test 0 in F.open_ids + @test !("PI_CloseConnection" in F.calls) + end + + @testset "a failing setup step closes the connection: $op" for op in setup_ops + F.reset!() + F.error_code[] = 7 + push!(F.failing, op) + stage = N472() + err = try + quiet(() -> initialize(stage)) + nothing + catch e + e + end + @test err isa ErrorException + @test occursin("GCS error 7", err.msg) + @test !(0 in F.open_ids) + @test stage.connectionstatus == false + @test stage.id == -1 + empty!(F.failing) + F.error_code[] = 0 + quiet(() -> initialize(stage)) + @test stage.connectionstatus == true + end +end diff --git a/test/pi_n472_fake_sdk.jl b/test/pi_n472_fake_sdk.jl new file mode 100644 index 0000000..1a53adc --- /dev/null +++ b/test/pi_n472_fake_sdk.jl @@ -0,0 +1,109 @@ +# A fake PI GCS2 library for the N-472 driver. +# +# Replaces the GCS2 wrappers in `functions_GCS2.jl` (each a single `ccall` into +# a Windows DLL that is not on any build machine) with methods that record +# into `Main.FakeGCS2`, so the driver's own `initialize`/`shutdown`/`stopmotion` +# run unmodified and no test can command a rig's controller. Same seam, and +# same caveats, as `tcube_fake_sdk.jl`: the replacements are global and +# permanent for the process, and this file must be included at top level, +# early, into `Main`. See that file for the full list. + +""" + FakeGCS2 + +Recorder standing in for the PI GCS2 library: what the driver called, in what +order, with which description/axes string, and what each call reports back. +""" +module FakeGCS2 + +"Operation names in the order the driver called them, since the last `reset!`." +const calls = String[] + +"The `szDescription`/`szAxes` argument of the last call per operation, as passed." +const lastarg = Dict{String,Any}() + +"Bytes `PI_EnumerateUSB` writes into the caller's buffer." +const enum_bytes = Ref(UInt8[]) + +"Count `PI_EnumerateUSB` returns." +const enum_count = Ref(1) + +"Ids `PI_ConnectUSB` returns, in order; when empty it returns 0." +const connect_ids = Int[] + +"Operations that return FALSE." +const failing = Set{String}() + +"Ids connected and not yet closed; one id space shared by every object." +const open_ids = Set{Int}() + +"Returned by `PI_GetError` and `PI_GetInitError`." +const error_code = Ref(0) + +function reset!() + empty!(calls) + empty!(lastarg) + enum_bytes[] = UInt8[codeunits("d1")..., 0x00] + enum_count[] = 1 + empty!(connect_ids) + empty!(failing) + empty!(open_ids) + error_code[] = 0 + return nothing +end + +function record!(op, arg=nothing) + push!(calls, op) + arg === nothing || (lastarg[op] = arg) + return nothing +end + +"Record `op` and report FALSE if it is in `failing`, else TRUE." +function status!(op, arg) + record!(op, arg) + return op in failing ? Cuint(0) : Cuint(1) +end + +end # module FakeGCS2 + +FakeGCS2.reset!() + +@eval MicroscopeControl.HardwareImplementations.PI_N472 begin + function PI_EnumerateUSB(szBuffer, iBufferSize, szFilter) + Main.FakeGCS2.record!("PI_EnumerateUSB") + bytes = Main.FakeGCS2.enum_bytes[] + n = min(length(bytes), Int(iBufferSize)) + for i in 1:n + szBuffer[i] = bytes[i] + end + return Cint(Main.FakeGCS2.enum_count[]) + end + function PI_ConnectUSB(szDescription) + Main.FakeGCS2.record!("PI_ConnectUSB", szDescription) + ids = Main.FakeGCS2.connect_ids + id = isempty(ids) ? 0 : popfirst!(ids) + id >= 0 && push!(Main.FakeGCS2.open_ids, id) + return Cint(id) + end + function PI_IsConnected(ID) + Main.FakeGCS2.record!("PI_IsConnected") + return Int(ID) in Main.FakeGCS2.open_ids ? TRUE : FALSE + end + function PI_CloseConnection(ID) + Main.FakeGCS2.record!("PI_CloseConnection") + delete!(Main.FakeGCS2.open_ids, Int(ID)) + return nothing + end + PI_GetError(ID) = (Main.FakeGCS2.record!("PI_GetError"); Cint(Main.FakeGCS2.error_code[])) + PI_GetInitError() = (Main.FakeGCS2.record!("PI_GetInitError"); Cint(Main.FakeGCS2.error_code[])) + PI_HLT(ID, szAxes) = Main.FakeGCS2.status!("PI_HLT", szAxes) + PI_RON(ID, szAxes, pbValueArray) = Main.FakeGCS2.status!("PI_RON", szAxes) + PI_qRON(ID, szAxes, pbValueArray) = Main.FakeGCS2.status!("PI_qRON", szAxes) + PI_POS(ID, szAxes, pdValueArray) = Main.FakeGCS2.status!("PI_POS", szAxes) + PI_SVO(ID, szAxes, pbValueArray) = Main.FakeGCS2.status!("PI_SVO", szAxes) + PI_qSVO(ID, szAxes, pbValueArray) = Main.FakeGCS2.status!("PI_qSVO", szAxes) + PI_qTMN(ID, szAxes, pdValueArray) = Main.FakeGCS2.status!("PI_qTMN", szAxes) + PI_qTMX(ID, szAxes, pdValueArray) = Main.FakeGCS2.status!("PI_qTMX", szAxes) + PI_VEL(ID, szAxes, pdValueArray) = Main.FakeGCS2.status!("PI_VEL", szAxes) + PI_qVEL(ID, szAxes, pdValueArray) = Main.FakeGCS2.status!("PI_qVEL", szAxes) +end diff --git a/test/runtests.jl b/test/runtests.jl index 633461a..5a196c4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -8,6 +8,10 @@ const HDF5 = MicroscopeControl.HDF5 # be included at top level, before the testsets. See the file for the seam. include("tcube_fake_sdk.jl") +# Likewise for the PI N-472's GCS2 wrappers, so `initialize`/`shutdown` run +# against a recorder and never a rig's controller. Top level, before the testsets. +include("pi_n472_fake_sdk.jl") + @testset "MicroscopeControl.jl" begin @testset "Simulated Camera" begin cam = SimCamera(exposure_time=0.01) @@ -702,53 +706,7 @@ include("tcube_fake_sdk.jl") end end - @testset "PI N472 (no hardware)" begin - N = MicroscopeControl.HardwareImplementations.PI_N472 - - @testset "_cstring stops at the first NUL" begin - buf = zeros(UInt8, 32) - buf[1:14] .= codeunits("PI C-885 SN 42") - buf[20] = UInt8('x') # stale bytes past the terminator are ignored - @test N._cstring(buf) == "PI C-885 SN 42" - @test N._cstring(codeunits("abc") |> collect) == "abc" - @test N._cstring(UInt8[0x00]) == "" - end - - @testset "shutdown clears connectionstatus" begin - stage = N472() - stage.connectionstatus = true - stage.id = Cint(-1) # never connected; PI_IsConnected(-1) is FALSE - if isfile(N.PI_GCS2) - @test_logs (:info, "Stage not connected") shutdown(stage) - @test stage.connectionstatus == false - else - @test_skip "PI GCS2 DLL not installed" - end - end - - @testset "initialize without a controller leaves the object retryable" begin - # Never command a real controller from the suite: on a rig with a - # C-885 attached, `initialize` connects, zeroes the origin and turns - # the servos on. Run this branch only when enumeration finds nothing. - present = isfile(N.PI_GCS2) && - N.PI_EnumerateUSB(zeros(UInt8, 1024), 1024, "C-885") > 0 - if present - @test_skip "PI C-885 attached; not commanding real hardware from the suite" - elseif isfile(N.PI_GCS2) - stage = N472() - # No C-885 is plugged into a build box: enumeration finds nothing, - # initialize must say so and leave the flag cleared (before this - # the flag was set before the connect was checked). - @test_logs (:error, r"No PI C-885 found") initialize(stage) - @test stage.connectionstatus == false - @test stage.id == 0 - # and a second call is not refused as "already initialized" - @test_logs (:error, r"No PI C-885 found") initialize(stage) - else - @test_skip "PI GCS2 DLL not installed" - end - end - end + include("pi_n472.jl") include("contract.jl") include("skills.jl") From 28c381825cd1ddaa65cca79b9ee5bcb946c3c9a6 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 10:27:51 -0600 Subject: [PATCH 4/5] Review nits: CLAUDE.md provenance, simpler setvel return stopmotion never worked; the connect string is what worked by accident. setvel's return is unchanged in value. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 5 +++-- src/hardware_implementations/pi_n472/helper.jl | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 45d9dc6..04df747 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,8 +119,9 @@ Hardware implementations use `ccall` for vendor SDKs: - `mcl_stage/*.jl` - Mad City Labs NanoDrive - Serial devices (CrystaLaser, Vortran, Triggerscope) use `LibSerialPort` -Rules at the `ccall` boundary, each learned from a bug that "usually worked" -(C-867 servo, v0.1.1; N-472 stopmotion): +Rules at the `ccall` boundary, learned from the C-867 servo bug (v0.1.1), the +N-472 connect string that worked only by accident of `filter`, and the N-472 +`stopmotion` that never worked: - A `Ptr{Cchar}` argument (GCS2 axes lists, USB descriptions) gets a Julia `String`, which is always NUL-terminated. Never a `Vector{UInt8}` with the zeros filtered out, and never a `Vector{String}`; join axes with a space first. diff --git a/src/hardware_implementations/pi_n472/helper.jl b/src/hardware_implementations/pi_n472/helper.jl index 1baadea..bbcc4c3 100644 --- a/src/hardware_implementations/pi_n472/helper.jl +++ b/src/hardware_implementations/pi_n472/helper.jl @@ -75,5 +75,5 @@ function setvel(stage::N472,vel::Vector{Float64}) if qsuccess == FALSE @error "Failed to query velocity" end - return (success == FALSE || qsuccess == FALSE) ? FALSE : qsuccess + return success == FALSE ? FALSE : qsuccess end From 2ca73ebea9bb21362cb499e1d890cd5b9dff5443 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 14:31:08 -0600 Subject: [PATCH 5/5] CHANGELOG: initialize(::N472) throwing on a failed setup step is not a break It changes behaviour only on a path that was already broken ("Stage initialized" was logged on a half-set-up stage), which main's versioning rule (CLAUDE.md, decision 0033) does not count as a break; #64's identical PIStage change is under Fixed. The entry stays under Changed. Ruled with #67 going into main at 0.2.5-DEV. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc44324..4b8ccfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,8 +70,9 @@ each on its own: `connectionstatus` and `id`, and throws with the step and its GCS error code. Before, failures were ignored and "Stage initialized" was logged on a half-initialized stage. Enumeration and connect failures still log `@error` - and return, as `initialize(::PIStage)` does. **Breaking** under this - package's versioning rule: what the call throws changed. + and return, as `initialize(::PIStage)` does. Not a break under this + package's versioning rule: it changes behaviour only on a path that was + already broken. - **Re-initializing after `shutdown` now re-zeroes the frame.** A second `initialize` on the same object was refused and did nothing; it now runs the full sequence: reference mode off, `PI_POS` redefining the current position