From ca5c2f7df1cce9b9b28f36c5b6c9ba980a27377d Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 14:33:52 -0600 Subject: [PATCH 1/5] PIStage: route every GCS2 DLL call through a wrapper seam No behaviour change: each inline @ccall moves into pi_stage/gcs2.jl unchanged, so a test can replace the wrappers. Co-Authored-By: Claude Sonnet 5.5 --- src/hardware_implementations/pi_stage/PI.jl | 1 + .../pi_stage/config_methods.jl | 26 +++++++++---------- src/hardware_implementations/pi_stage/gcs2.jl | 19 ++++++++++++++ .../pi_stage/move_methods.jl | 12 ++++----- .../pi_stage/query_methods.jl | 12 ++++----- 5 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 src/hardware_implementations/pi_stage/gcs2.jl diff --git a/src/hardware_implementations/pi_stage/PI.jl b/src/hardware_implementations/pi_stage/PI.jl index f047c1c..f13072b 100644 --- a/src/hardware_implementations/pi_stage/PI.jl +++ b/src/hardware_implementations/pi_stage/PI.jl @@ -5,6 +5,7 @@ module PI global const gcs2path = "C:\\Program Files (x86)\\Physik Instrumente (PI)\\Software Suite\\Development\\C++\\API\\PI_GCS2_DLL_x64.dll" + include("gcs2.jl") include("types.jl") include("move_methods.jl") include("query_methods.jl") diff --git a/src/hardware_implementations/pi_stage/config_methods.jl b/src/hardware_implementations/pi_stage/config_methods.jl index 0d98669..1318329 100644 --- a/src/hardware_implementations/pi_stage/config_methods.jl +++ b/src/hardware_implementations/pi_stage/config_methods.jl @@ -11,7 +11,7 @@ function initialize_original(stage::PIStage) #TODO: Error handling bufferstring = Vector{UInt8}(undef, 1024) #Find number of connected USB devices, specifically the PI C-867 controller - numconnected = @ccall gcs2path.PI_EnumerateUSB(bufferstring::Ptr{UInt8}, 1024::Cint, "PI C-867"::Ptr{UInt8})::Cint + numconnected = PI_EnumerateUSB(bufferstring, 1024, "PI C-867") @info "Number of connected devices: " * string(numconnected) @@ -27,7 +27,7 @@ function initialize_original(stage::PIStage) #TODO: Error handling return end #Connect to usb device - stage.id = @ccall gcs2path.PI_ConnectUSB(bufferstring::Ptr{UInt8})::Cint + stage.id = PI_ConnectUSB(bufferstring) @info "Device ID: " * string(stage.id) @@ -78,12 +78,12 @@ Possibly must use PiMikroMove to calibrate, but this is not ideal, however there """ function referencemove(stage::PIStage) - ismoved = @ccall gcs2path.PI_FRF(stage.id::Cint, "1 2"::Ptr{UInt8})::Cint + ismoved = PI_FRF(stage.id, "1 2") return ismoved end # PI_GetError returns and clears the controller's last GCS error code (0 = none). -_pi_geterror(stage::PIStage) = @ccall gcs2path.PI_GetError(stage.id::Cint)::Cint +_pi_geterror(stage::PIStage) = PI_GetError(stage.id) """ Poll `PI_qFRF` until both axes report referenced; throw if that has not happened within @@ -94,7 +94,7 @@ function _waitforreference(stage::PIStage; timeout::Real = 60.0) referenced = zeros(Cint, 2) deadline = time() + timeout while true - ok = @ccall gcs2path.PI_qFRF(stage.id::Cint, "1 2"::Ptr{UInt8}, referenced::Ptr{Cint})::Cint + ok = PI_qFRF(stage.id, "1 2", referenced) ok == 1 || error("PI_qFRF failed (GCS error $(_pi_geterror(stage)))") all(!=(0), referenced) && return nothing time() > deadline && error("PI stage not referenced after $(timeout) s: " * @@ -108,11 +108,11 @@ end Function to disconnect PI Stage """ function shutdown_original(stage::PIStage) - isconnected = @ccall gcs2path.PI_IsConnected(stage.id::Cint)::Cint + isconnected = PI_IsConnected(stage.id) if isconnected == 1 - @ccall gcs2path.PI_CloseConnection(stage.id::Cint)::Cvoid - isconnected = @ccall gcs2path.PI_IsConnected(stage.id::Cint)::Cint + PI_CloseConnection(stage.id) + isconnected = PI_IsConnected(stage.id) if isconnected == 1 @error "Stage failed to disconnect" @@ -133,7 +133,7 @@ function servo(stage::PIStage, xtoggle::Bool, ytoggle::Bool) # PI_SVO takes `const BOOL*` = 32-bit ints, one per axis. Passing two UInt8 made the DLL # read axis 2's flag from whatever byte followed the array: servo silently OFF on Y, # every PI_MOV refused with GCS error 5 (worked by luck on Julia 1.10, failed on 1.13). - istoggled = @ccall gcs2path.PI_SVO(stage.id::Cint, "1 2"::Ptr{UInt8}, Cint[xtoggle, ytoggle]::Ptr{Cint})::Cint + istoggled = PI_SVO(stage.id, "1 2", Cint[xtoggle, ytoggle]) stage.servostatus = (xtoggle, ytoggle) if istoggled == 1 @@ -147,7 +147,7 @@ end Sets the servo state of the x axis """ function servox(stage::PIStage, xtoggle::Bool) - @ccall gcs2path.PI_SVO(stage.id::Cint, "1"::Ptr{UInt8}, Cint[xtoggle]::Ptr{Cint})::Cint + PI_SVO(stage.id, "1", Cint[xtoggle]) stage.servostatus = (xtoggle, stage.servostatus[2]) end @@ -156,20 +156,20 @@ end Sets the servo state of the y axis """ function servoy(stage::PIStage, ytoggle::Bool) - @ccall gcs2path.PI_SVO(stage.id::Cint, "2"::Ptr{UInt8}, Cint[ytoggle]::Ptr{Cint})::Cint + PI_SVO(stage.id, "2", Cint[ytoggle]) stage.servostatus = (stage.servostatus[1], ytoggle) end function setvel(stage::PIStage,vel::Vector{Float64}) - success = @ccall gcs2path.PI_VEL(stage.id::Cint, "1 2"::Ptr{UInt8}, vel::Ptr{Cdouble})::Cint + success = PI_VEL(stage.id, "1 2", vel) if success == 0 @error "Failed to set velocity" end velocity = Vector{Cdouble}(undef, 2) - success = @ccall gcs2path.PI_qVEL(stage.id::Cint, "1 2"::Ptr{UInt8}, velocity::Ptr{Cdouble})::Cint + success = PI_qVEL(stage.id, "1 2", velocity) if success == 0 @error "Failed to query velocity" diff --git a/src/hardware_implementations/pi_stage/gcs2.jl b/src/hardware_implementations/pi_stage/gcs2.jl new file mode 100644 index 0000000..481ac5c --- /dev/null +++ b/src/hardware_implementations/pi_stage/gcs2.jl @@ -0,0 +1,19 @@ +# One wrapper per PI GCS2 DLL function: the seam test/pi_stage_fake_sdk.jl replaces. +PI_EnumerateUSB(buffer, bufsize, filter) = @ccall gcs2path.PI_EnumerateUSB(buffer::Ptr{UInt8}, bufsize::Cint, filter::Ptr{UInt8})::Cint +PI_ConnectUSB(description) = @ccall gcs2path.PI_ConnectUSB(description::Ptr{UInt8})::Cint +PI_IsConnected(ID) = @ccall gcs2path.PI_IsConnected(ID::Cint)::Cint +PI_CloseConnection(ID) = @ccall gcs2path.PI_CloseConnection(ID::Cint)::Cvoid +PI_GetError(ID) = @ccall gcs2path.PI_GetError(ID::Cint)::Cint +PI_IsControllerReady(ID, piControllerReady) = @ccall gcs2path.PI_IsControllerReady(ID::Cint, piControllerReady::Ptr{Cint})::Cint +PI_FRF(ID, axes) = @ccall gcs2path.PI_FRF(ID::Cint, axes::Ptr{UInt8})::Cint +PI_qFRF(ID, axes, referenced) = @ccall gcs2path.PI_qFRF(ID::Cint, axes::Ptr{UInt8}, referenced::Ptr{Cint})::Cint +PI_SVO(ID, axes, values) = @ccall gcs2path.PI_SVO(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cint})::Cint +PI_VEL(ID, axes, values) = @ccall gcs2path.PI_VEL(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint +PI_qVEL(ID, axes, values) = @ccall gcs2path.PI_qVEL(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint +PI_MOV(ID, axes, values) = @ccall gcs2path.PI_MOV(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint +PI_HLT(ID, axes) = @ccall gcs2path.PI_HLT(ID::Cint, axes::Ptr{UInt8})::Cint +PI_STP(ID) = @ccall gcs2path.PI_STP(ID::Cint)::Cint +PI_qPOS(ID, axes, values) = @ccall gcs2path.PI_qPOS(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint +PI_IsMoving(ID, axes, values) = @ccall gcs2path.PI_IsMoving(ID::Cint, axes::Ptr{UInt8}, values::Ptr{UInt32})::Cint +PI_qTMN(ID, axes, values) = @ccall gcs2path.PI_qTMN(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint +PI_qTMX(ID, axes, values) = @ccall gcs2path.PI_qTMX(ID::Cint, axes::Ptr{UInt8}, values::Ptr{Cdouble})::Cint diff --git a/src/hardware_implementations/pi_stage/move_methods.jl b/src/hardware_implementations/pi_stage/move_methods.jl index 38c87a6..4a3d54d 100644 --- a/src/hardware_implementations/pi_stage/move_methods.jl +++ b/src/hardware_implementations/pi_stage/move_methods.jl @@ -3,7 +3,7 @@ Function to move PI Stage to a specific position """ function move(stage::PIStage, x::Float64, y::Float64) - ok = @ccall gcs2path.PI_MOV(stage.id::Cint, "1 2"::Ptr{UInt8}, [Cdouble(x),Cdouble(y)]::Ptr{Cdouble})::Cint + ok = PI_MOV(stage.id, "1 2", [Cdouble(x),Cdouble(y)]) ok == 1 || @error "PI_MOV refused — stage not connected, not referenced, or servo off" stage.targ_x = x stage.targ_y = y @@ -14,7 +14,7 @@ end Function to move PI Stage, and wait for completion """ function moveandwait(stage::PIStage, x::Float64, y::Float64) - @ccall gcs2path.PI_MOV(stage.id::Cint, "1 2"::Ptr{UInt8}, [Cdouble(x),Cdouble(y)]::Ptr{Cdouble})::Cint + PI_MOV(stage.id, "1 2", [Cdouble(x),Cdouble(y)]) stage.targ_x = x stage.targ_y = y ismoving(stage) @@ -28,7 +28,7 @@ end Function to move PI Stage X axis to a specific position """ function movex(stage::PIStage, x::Float64) - @ccall gcs2path.PI_MOV(stage.id::Cint, "1"::Ptr{UInt8}, [Cdouble(x)]::Ptr{Cdouble})::Cint + PI_MOV(stage.id, "1", [Cdouble(x)]) stage.targ_x = x end @@ -36,7 +36,7 @@ end Function to move PI Stage Y axis to a specific position """ function movey(stage::PIStage, y::Float64) - @ccall gcs2path.PI_MOV(stage.id::Cint, "2"::Ptr{UInt8}, [Cdouble(y)]::Ptr{Cdouble})::Cint + PI_MOV(stage.id, "2", [Cdouble(y)]) stage.targ_y = y end @@ -44,7 +44,7 @@ end Function call to smoothly stop motion of the PI Stage """ function stopmotion(stage::PIStage) - isstopped = @ccall gcs2path.PI_HLT(stage.id::Cint, "1 2"::Ptr{UInt8})::Cint + isstopped = PI_HLT(stage.id, "1 2") if isstopped == 1 @info "Motion successfully stopped" else @@ -56,7 +56,7 @@ end Function call to immediately stop the PI Stage """ function immediatestop(stage::PIStage) - isstopped = @ccall gcs2path.PI_STP(stage.id::Cint)::Cint + isstopped = PI_STP(stage.id) if isstopped == 1 @info "Motion successfully stopped" else diff --git a/src/hardware_implementations/pi_stage/query_methods.jl b/src/hardware_implementations/pi_stage/query_methods.jl index 3356f29..66e1b04 100644 --- a/src/hardware_implementations/pi_stage/query_methods.jl +++ b/src/hardware_implementations/pi_stage/query_methods.jl @@ -5,7 +5,7 @@ Function to update the position of the PI Stage """ function getposition(stage::PIStage) position = Vector{Cdouble}(undef, 2) - @ccall gcs2path.PI_qPOS(stage.id::Cint, "1 2"::Ptr{UInt8}, position::Ptr{Cdouble})::Cint + PI_qPOS(stage.id, "1 2", position) @info "Stage position: " * string(position[1]) * ", " * string(position[2]) stage.real_x = position[1] @@ -17,7 +17,7 @@ Function to update the x position of the PI Stage """ function getxposition(stage::PIStage) xposition = Cdouble(0.0) - @ccall gcs2path.PI_qPOS(stage.id::Cint, "1"::Ptr{UInt8}, [xposition]::Ptr{Cdouble})::Cint + PI_qPOS(stage.id, "1", [xposition]) stage.real_x = xposition end @@ -26,7 +26,7 @@ Function to update the y position of the PI Stage """ function getyposition(stage::PIStage) yposition = Cdouble(0.0) - @ccall gcs2path.PI_qPOS(stage.id::Cint, "2"::Ptr{UInt8}, [yposition]::Ptr{Cdouble})::Cint + PI_qPOS(stage.id, "2", [yposition]) stage.real_y = yposition end @@ -37,7 +37,7 @@ Function to check if the PI Stage is moving, both the x and y axis are checked """ function ismoving(stage::PIStage) ismoving = Vector{UInt32}(undef, 2) - @ccall gcs2path.PI_IsMoving(stage.id::Cint, "1 2"::Ptr{UInt8}, ismoving::Ptr{UInt32})::Cint + PI_IsMoving(stage.id, "1 2", ismoving) stage.ismoving = (Bool(ismoving[1]), Bool(ismoving[2])) end @@ -51,7 +51,7 @@ end """ function findmin(stage::PIStage) minpositions = Vector{Cdouble}(undef, 2) - @ccall gcs2path.PI_qTMN(stage.id::Cint, "1 2"::Ptr{UInt8}, minpositions::Ptr{Cdouble})::Cint + PI_qTMN(stage.id, "1 2", minpositions) stage.range_x = (minpositions[1], stage.range_x[2]) stage.range_y = (minpositions[2], stage.range_y[2]) end @@ -61,7 +61,7 @@ end """ function findmax(stage::PIStage) maxpositions = Vector{Cdouble}(undef, 2) - @ccall gcs2path.PI_qTMX(stage.id::Cint, "1 2"::Ptr{UInt8}, maxpositions::Ptr{Cdouble})::Cint + PI_qTMX(stage.id, "1 2", maxpositions) stage.range_x = (stage.range_x[1], maxpositions[1]) stage.range_y = (stage.range_y[1], maxpositions[2]) end \ No newline at end of file From efe7c05ca8ac781abd381cf2109eb4b019673f93 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 14:38:22 -0600 Subject: [PATCH 2/5] PIStage: initialize is ready only after full success; fake-GCS2 tests Id defaults to -1 and shutdown resets it; connectionstatus is set once, after every step past the connect; initialize waits for PI_IsControllerReady before PI_qFRF; the stage panels' initialize buttons log a failed initialize; stale TODOs and docstrings corrected. Adds test/pi_stage_fake_sdk.jl and test/pi_stage.jl. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 9 + .../pi_stage/config_methods.jl | 95 ++++++---- .../pi_stage/interface_methods.jl | 4 +- .../pi_stage/types.jl | 2 +- .../stage_interface/gui.jl | 24 ++- test/pi_stage.jl | 109 +++++++++++ test/pi_stage_fake_sdk.jl | 175 ++++++++++++++++++ test/runtests.jl | 4 + 8 files changed, 385 insertions(+), 37 deletions(-) create mode 100644 test/pi_stage.jl create mode 100644 test/pi_stage_fake_sdk.jl diff --git a/CHANGELOG.md b/CHANGELOG.md index b6a94c3..1389560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,13 @@ the next version with `-DEV`). ## [Unreleased] +### Added +- Fake-GCS2 tests for `PIStage` (`test/pi_stage_fake_sdk.jl`, `test/pi_stage.jl`): `initialize`'s ordering and cleanup and `shutdown`'s id handling, with no hardware. + ### Fixed +- `PIStage`: `shutdown` could close another object's connection. `id` defaulted to `0`, a valid GCS id, and was never reset; it now defaults to `-1` and `shutdown` resets it. +- `PIStage.initialize` reported the stage connected before it was: `connectionstatus` was set before the connect, and a failed close after a failed reference left it `true`, so a retry answered "already initialized". It is now set only after the whole sequence succeeds, and every step after the connect is inside the cleanup. +- The stage panels' initialize buttons log a failed `initialize` instead of throwing out of the click callback. - **PI stage: `initialize` no longer finishes on an unreferenced stage.** It ignored the return of the reference move (`PI_FRF`), so when the controller rejected it (GCS error 5, e.g. one axis's servo off) the later move to the @@ -22,6 +28,9 @@ the next version with `-DEV`). unchanged. Reported by the MicroscopeAdapt rig; not yet run on hardware (#64). +### Changed +- `PIStage.initialize` waits for `PI_IsControllerReady` after the reference move, before polling `PI_qFRF`, as PI's samples do. Not yet run on hardware; needs a rig check on the C-867. + ### Changed (release process) - **`main` is the development branch**, carrying the next version with `-DEV`; every pull request goes into it. The `0.3rc1` release-candidate diff --git a/src/hardware_implementations/pi_stage/config_methods.jl b/src/hardware_implementations/pi_stage/config_methods.jl index 1318329..83a5711 100644 --- a/src/hardware_implementations/pi_stage/config_methods.jl +++ b/src/hardware_implementations/pi_stage/config_methods.jl @@ -1,7 +1,17 @@ """ -Function to initialize PI Stage, right now this requires calibration using PiMikroMove to work correctly, no documentation on how to calibrate using the PI_GCS2 library +Initialize the PI stage. Connects to the first PI C-867 the GCS2 library enumerates, turns both +servos on, starts the reference move, waits for the controller and for both axes to report +referenced, reads the travel range, waits for motion to stop, and sets `stage.velocity`. + +`connectionstatus` becomes true only when all of that succeeded. A failure after the connect +closes the connection and throws. Enumeration and connect failures log `@error` and return with +`connectionstatus == false`. + +`[limitation]` Not yet run on hardware; the wait for `PI_IsControllerReady` needs a rig check on +the C-867. The stage needs calibration using PiMikroMove to work correctly; there is no +documentation on how to calibrate using the PI_GCS2 library. """ -function initialize_original(stage::PIStage) #TODO: Error handling +function initialize_original(stage::PIStage) if stage.connectionstatus == true @error "Stage already initialized" return @@ -15,15 +25,11 @@ function initialize_original(stage::PIStage) #TODO: Error handling @info "Number of connected devices: " * string(numconnected) - #Set connection status to true - if numconnected > 0 - stage.connectionstatus = true - else + if numconnected <= 0 # The DLL enumerates only controllers nobody has open: a C-867 that Device Manager # still lists is held by another process (a second Julia with an initialized stage — # under any Windows user —, PIMikroMove, or an open COM port). @error "No PI C-867 found by the GCS2 library — controller absent, or held by another process" - stage.connectionstatus = false return end #Connect to usb device @@ -34,48 +40,49 @@ function initialize_original(stage::PIStage) #TODO: Error handling if stage.id < 0 # The connect itself failed (id -1): typically another process already holds the # controller (a second Julia with an initialized stage, PIMikroMove, an open COM port). - stage.connectionstatus = false @error "PI_ConnectUSB failed — the controller is probably held by another process" return end - #Set servo mode to on for both axes, noting axis X is labeled "1" and axis Y is labeled "2" - servo(stage, true, true) - - #Reference stage. A rejected FRF (e.g. GCS error 5, servo off on one axis) used to be - # ignored: every later PI_MOV was refused too, while the driver's cached position said - # the stage was centred. Refuse to come back from initialize unreferenced. - # On failure, close the connection so a retried initialize starts clean. + # Everything after the connect is inside the cleanup: a failure closes the connection so a + # retried initialize starts clean, and connectionstatus is set only once all of it succeeded. try + #Set servo mode to on for both axes, noting axis X is labeled "1" and axis Y is labeled "2" + servo(stage, true, true) + + #Reference stage. A rejected FRF (e.g. GCS error 5, servo off on one axis) used to be + # ignored: every later PI_MOV was refused too, while the driver's cached position said + # the stage was centred. Refuse to come back from initialize unreferenced. if referencemove(stage) != 1 error("PI_FRF refused (GCS error $(_pi_geterror(stage))); stage is not referenced") end - _waitforreference(stage) - catch - shutdown_original(stage) - rethrow() - end + _waitforready(stage; timeout = REFERENCE_TIMEOUT_S[]) + _waitforreference(stage; timeout = REFERENCE_TIMEOUT_S[]) - #Find the max and min position of the axes - getrange(stage) + #Find the max and min position of the axes + getrange(stage) - #Wait for any remaining motion to finish - ismoving(stage) - while stage.ismoving[1] == 1 || stage.ismoving[2] == 1 + #Wait for any remaining motion to finish ismoving(stage) - end + while stage.ismoving[1] == 1 || stage.ismoving[2] == 1 + ismoving(stage) + end - #Set velocity to 1 mm/s - success = setvel(stage, stage.velocity) + #Set the velocity to `stage.velocity` + success = setvel(stage, stage.velocity) + catch + shutdown_original(stage) + rethrow() + end + stage.connectionstatus = true @info "Stage initialized" return end """ -Function to calibrate PI Stage, not implemented yet as there is no documentation for this stage on calibration using the PI_GCS2 library -Possibly must use PiMikroMove to calibrate, but this is not ideal, however there is a CLI - +Start the reference move (`PI_FRF`) on both axes and return the GCS BOOL, 1 if accepted. +`initialize` waits for it to finish. """ function referencemove(stage::PIStage) ismoved = PI_FRF(stage.id, "1 2") @@ -85,6 +92,30 @@ end # PI_GetError returns and clears the controller's last GCS error code (0 = none). _pi_geterror(stage::PIStage) = PI_GetError(stage.id) +""" +How long `initialize` waits, in seconds, for the controller to become ready and then for both +axes to report referenced. A `Ref` so tests can shorten it. +""" +const REFERENCE_TIMEOUT_S = Ref(60.0) + +""" +Poll `PI_IsControllerReady` every 0.1 s until the controller reports ready; throw if the call +fails or it is not ready within `timeout` seconds. + +`[limitation]` Not yet run on hardware; the wait needs a rig check on the C-867. +""" +function _waitforready(stage::PIStage; timeout::Real = REFERENCE_TIMEOUT_S[]) + ready = Ref{Cint}(0) + deadline = time() + timeout + while true + ok = PI_IsControllerReady(stage.id, ready) + ok == 0 && error("PI_IsControllerReady failed (GCS error $(_pi_geterror(stage)))") + ready[] != 0 && return nothing + time() > deadline && error("PI controller not ready after $(timeout) s") + sleep(0.1) + end +end + """ Poll `PI_qFRF` until both axes report referenced; throw if that has not happened within `timeout` seconds or the query itself fails. @@ -119,10 +150,12 @@ function shutdown_original(stage::PIStage) else @info "Stage disconnected" stage.connectionstatus = false + stage.id = Cint(-1) end else @error "Stage already disconnected" stage.connectionstatus = false + stage.id = Cint(-1) end end diff --git a/src/hardware_implementations/pi_stage/interface_methods.jl b/src/hardware_implementations/pi_stage/interface_methods.jl index cdf1aa2..eb5291e 100644 --- a/src/hardware_implementations/pi_stage/interface_methods.jl +++ b/src/hardware_implementations/pi_stage/interface_methods.jl @@ -1,7 +1,7 @@ """ -Function to initialize PI Stage, right now this requires calibration using PiMikroMove to work correctly, no documentation on how to calibrate using the PI_GCS2 library +Initialize the PI stage; see `initialize_original` for the sequence and its failure behaviour. """ -function initialize(stage::PIStage) #TODO: Error handling +function initialize(stage::PIStage) initialize_original(stage) end diff --git a/src/hardware_implementations/pi_stage/types.jl b/src/hardware_implementations/pi_stage/types.jl index 8df1e0f..d92d397 100644 --- a/src/hardware_implementations/pi_stage/types.jl +++ b/src/hardware_implementations/pi_stage/types.jl @@ -28,7 +28,7 @@ function PIStage(; units::String = "Milimeters", dimensions::Int = 2, connectionstatus::Bool = false, - id::Cint = Cint(0), + id::Cint = Cint(-1), x::Float64 = 12.5, y::Float64 = 12.5, targ_x::Float64 = 12.5, diff --git a/src/hardware_interfaces/stage_interface/gui.jl b/src/hardware_interfaces/stage_interface/gui.jl index e186911..a634bfc 100644 --- a/src/hardware_interfaces/stage_interface/gui.jl +++ b/src/hardware_interfaces/stage_interface/gui.jl @@ -164,7 +164,13 @@ function gui1d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - initialize(stage) + try + initialize(stage) + catch err + @error "Failed to initialize the stage" exception = err + return + end + stage.connectionstatus || return getposition(stage) xposition[] = stage.real_x xtarget[] = stage.targ_x @@ -453,7 +459,13 @@ function gui2d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - initialize(stage) + try + initialize(stage) + catch err + @error "Failed to initialize the stage" exception = err + return + end + stage.connectionstatus || return getposition(stage) xposition[], yposition[] = stage.real_x, stage.real_y xtarget[], ytarget[] = stage.targ_x, stage.targ_y @@ -795,7 +807,13 @@ function gui3d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - initialize(stage) + try + initialize(stage) + catch err + @error "Failed to initialize the stage" exception = err + return + end + stage.connectionstatus || return getposition(stage) xposition[], yposition[], zposition[] = stage.real_x, stage.real_y, stage.real_z xtarget[], ytarget[], ztarget[] = stage.targ_x, stage.targ_y, stage.targ_z diff --git a/test/pi_stage.jl b/test/pi_stage.jl new file mode 100644 index 0000000..4f0b809 --- /dev/null +++ b/test/pi_stage.jl @@ -0,0 +1,109 @@ +# PIStage against the fake GCS2 library in `pi_stage_fake_sdk.jl`: initialize's +# ordering and cleanup, and shutdown's id handling. No hardware. +@testset "PI stage (fake GCS2)" begin + PI = MicroscopeControl.HardwareImplementations.PI + F = Main.FakePIStage + ncalls(op) = count(==(op), F.calls) + saved_timeout = PI.REFERENCE_TIMEOUT_S[] + PI.REFERENCE_TIMEOUT_S[] = 0.3 + + try + @testset "never-initialized stage has no id" begin + F.reset!() + stage = PIStage() + @test stage.id == -1 + shutdown(stage) + @test ncalls("PI_CloseConnection") == 0 + end + + @testset "successful initialize" begin + F.reset!() + F.not_ready_polls[] = 2 + F.unreferenced_polls[] = 2 + push!(F.connect_ids, 5) + stage = PIStage() + seen = Ref{Any}(nothing) + F.frf_hook[] = () -> (seen[] = stage.connectionstatus) + initialize(stage) + @test stage.connectionstatus + @test stage.id == 5 + @test seen[] == false + iFRF = findfirst(==("PI_FRF"), F.calls) + iready = findfirst(==("PI_IsControllerReady"), F.calls) + iq = findfirst(==("PI_qFRF"), F.calls) + @test iFRF < iready < iq + @test ncalls("PI_IsControllerReady") == 3 + end + + @testset "refused PI_FRF" begin + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_FRF") + stage = PIStage() + @test_throws ErrorException initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + @test stage.id == -1 + end + + @testset "controller never ready" begin + F.reset!() + push!(F.connect_ids, 5) + F.not_ready_polls[] = typemax(Int) + stage = PIStage() + @test_throws "not ready" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + + @testset "axes never referenced" begin + F.reset!() + push!(F.connect_ids, 5) + F.unreferenced_polls[] = typemax(Int) + stage = PIStage() + @test_throws "not referenced" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + + @testset "throw after the reference" begin + F.reset!() + push!(F.connect_ids, 5) + F.throw!("PI_qTMN") + stage = PIStage() + @test_throws "PI_qTMN threw" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + + @testset "failed close keeps the id; retry cannot report ready" begin + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_FRF") + F.close_leaves_open[] = true + stage = PIStage() + @test_throws ErrorException initialize(stage) + @test !stage.connectionstatus + @test stage.id == 5 + F.enum_count[] = 0 + initialize(stage) + @test !stage.connectionstatus + end + + @testset "shutdown after initialize" begin + F.reset!() + push!(F.connect_ids, 5) + stage = PIStage() + initialize(stage) + shutdown(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + @test stage.id == -1 + shutdown(stage) + @test ncalls("PI_CloseConnection") == 1 + end + finally + PI.REFERENCE_TIMEOUT_S[] = saved_timeout + F.reset!() + end +end diff --git a/test/pi_stage_fake_sdk.jl b/test/pi_stage_fake_sdk.jl new file mode 100644 index 0000000..e0ab6a4 --- /dev/null +++ b/test/pi_stage_fake_sdk.jl @@ -0,0 +1,175 @@ +# A fake PI GCS2 library for the PIStage driver. +# +# Replaces the wrappers in `pi_stage/gcs2.jl` (each a single `@ccall` into a +# Windows DLL that is not on any build machine) with methods that record into +# `Main.FakePIStage`, so the driver's own `initialize`/`shutdown` 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`. +# A wrapper the tests do not need throws, so no test can reach a DLL. + +""" + FakePIStage + +Recorder standing in for the PI GCS2 library: what the driver called, in what +order, and what each call reports back. Failures are per call: `fail!(op)` +makes it return FALSE, `throw!(op)` makes it throw. +""" +module FakePIStage + +"Operation names in the order the driver called them, since the last `reset!`." +const calls = String[] + +"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[] + +"Ids connected and not yet closed." +const open_ids = Set{Int}() + +"Operations that return FALSE." +const failing = Set{String}() + +"Operations that throw." +const throwing = Set{String}() + +"When true, `PI_CloseConnection` leaves the id open: a close that fails." +const close_leaves_open = Ref(false) + +"`PI_IsControllerReady` answers not ready for this many polls." +const not_ready_polls = Ref(0) + +"`PI_qFRF` answers unreferenced for this many polls." +const unreferenced_polls = Ref(0) + +"Called with no arguments inside `PI_FRF`, so a test can observe driver state there." +const frf_hook = Ref{Any}(nothing) + +"Returned by `PI_GetError`." +const error_code = Ref(0) + +const ready_polls = Ref(0) +const referenced_polls = Ref(0) + +function reset!() + empty!(calls) + enum_bytes[] = UInt8[codeunits("d1")..., 0x00] + enum_count[] = 1 + empty!(connect_ids) + empty!(open_ids) + empty!(failing) + empty!(throwing) + close_leaves_open[] = false + not_ready_polls[] = 0 + unreferenced_polls[] = 0 + frf_hook[] = nothing + error_code[] = 0 + ready_polls[] = 0 + referenced_polls[] = 0 + return nothing +end + +fail!(op) = push!(failing, op) +throw!(op) = push!(throwing, op) + +"Record `op`, throw if it is in `throwing`." +function record!(op) + push!(calls, op) + op in throwing && error("FakePIStage: $op threw") + return nothing +end + +"Record `op` and report FALSE if it is in `failing`, else TRUE." +function status!(op) + record!(op) + return op in failing ? Cint(0) : Cint(1) +end + +unmodelled(op) = error("FakePIStage: $op is not modelled") + +end # module FakePIStage + +FakePIStage.reset!() + +@eval MicroscopeControl.HardwareImplementations.PI begin + function PI_EnumerateUSB(buffer, bufsize, filter) + Main.FakePIStage.record!("PI_EnumerateUSB") + bytes = Main.FakePIStage.enum_bytes[] + for i in 1:min(length(bytes), Int(bufsize)) + buffer[i] = bytes[i] + end + return Cint(Main.FakePIStage.enum_count[]) + end + function PI_ConnectUSB(description) + Main.FakePIStage.record!("PI_ConnectUSB") + ids = Main.FakePIStage.connect_ids + id = isempty(ids) ? 0 : popfirst!(ids) + id >= 0 && push!(Main.FakePIStage.open_ids, id) + return Cint(id) + end + function PI_IsConnected(ID) + Main.FakePIStage.record!("PI_IsConnected") + return Int(ID) in Main.FakePIStage.open_ids ? Cint(1) : Cint(0) + end + function PI_CloseConnection(ID) + Main.FakePIStage.record!("PI_CloseConnection") + Main.FakePIStage.close_leaves_open[] || delete!(Main.FakePIStage.open_ids, Int(ID)) + return nothing + end + PI_GetError(ID) = (Main.FakePIStage.record!("PI_GetError"); Cint(Main.FakePIStage.error_code[])) + function PI_IsControllerReady(ID, piControllerReady) + st = Main.FakePIStage.status!("PI_IsControllerReady") + Main.FakePIStage.ready_polls[] += 1 + piControllerReady[] = Main.FakePIStage.ready_polls[] > Main.FakePIStage.not_ready_polls[] ? Cint(1) : Cint(0) + return st + end + function PI_FRF(ID, axes) + hook = Main.FakePIStage.frf_hook[] + hook === nothing || hook() + return Main.FakePIStage.status!("PI_FRF") + end + function PI_qFRF(ID, axes, referenced) + st = Main.FakePIStage.status!("PI_qFRF") + Main.FakePIStage.referenced_polls[] += 1 + done = Main.FakePIStage.referenced_polls[] > Main.FakePIStage.unreferenced_polls[] + referenced[1] = referenced[2] = done ? Cint(1) : Cint(0) + return st + end + PI_SVO(ID, axes, values) = Main.FakePIStage.status!("PI_SVO") + PI_VEL(ID, axes, values) = Main.FakePIStage.status!("PI_VEL") + function PI_qVEL(ID, axes, values) + st = Main.FakePIStage.status!("PI_qVEL") + values[1] = values[2] = 1.0 + return st + end + function PI_IsMoving(ID, axes, values) + st = Main.FakePIStage.status!("PI_IsMoving") + values[1] = values[2] = 0 + return st + end + function PI_qTMN(ID, axes, values) + st = Main.FakePIStage.status!("PI_qTMN") + values[1] = values[2] = 0.0 + return st + end + function PI_qTMX(ID, axes, values) + st = Main.FakePIStage.status!("PI_qTMX") + values[1] = values[2] = 25.0 + return st + end + function PI_qPOS(ID, axes, values) + st = Main.FakePIStage.status!("PI_qPOS") + for i in eachindex(values) + values[i] = 12.5 + end + return st + end + PI_MOV(ID, axes, values) = Main.FakePIStage.unmodelled("PI_MOV") + PI_HLT(ID, axes) = Main.FakePIStage.unmodelled("PI_HLT") + PI_STP(ID) = Main.FakePIStage.unmodelled("PI_STP") +end diff --git a/test/runtests.jl b/test/runtests.jl index 15b7ec5..fea70b4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -8,6 +8,9 @@ const HDF5 = MicroscopeControl.HDF5 # be included at top level, before the testsets. See the file for the seam. include("tcube_fake_sdk.jl") +# Same seam for the PI stage's GCS2 wrappers; see the file. +include("pi_stage_fake_sdk.jl") + # Writes the lab test record summary when LAB_TEST_SUMMARY is set; see the file. include("lab_summary.jl") @@ -708,6 +711,7 @@ lab_summary("Core") do end end + include("pi_stage.jl") include("contract.jl") include("skills.jl") include("gui.jl") From 7acee6cdc99b110f4cf0babe1124f4def82899ed Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 14:53:09 -0600 Subject: [PATCH 3/5] main: 0.2.6-DEV after the 0.2.5 release; an empty [Unreleased] section Decision 0033: after X.Y.Z, main carries X.Y.(Z+1)-DEV. TagOnMerge skips a -DEV version. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ Project.toml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f621ec..0bb5c07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ the README's Installation section: in `0.x.y`, `x` is the breaking component and `y` is the non-breaking one (releases are tagged; between them `main` carries the next version with `-DEV`). +## [Unreleased] + ## [0.2.5] - 2026-09-29 A non-breaking release. It brings the TCube laser's closed-loop (power) mode and diff --git a/Project.toml b/Project.toml index bd83563..f1f971d 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "MicroscopeControl" uuid = "aa70d9ae-4a1e-49fd-870a-8ccfd99f4c3e" -version = "0.2.5" +version = "0.2.6-DEV" authors = ["klidke@unm.edu"] [deps] From 89dcd33b90870d0f01140a5671b7c6c150db5186 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 15:15:25 -0600 Subject: [PATCH 4/5] Review fixes on #73 (P1-P5): check range and velocity reads, bound the motion-stop wait, reclaim own connection on retry, one GUI initialize guard Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 7 +- .../pi_stage/config_methods.jl | 61 +++++--- .../pi_stage/interface_methods.jl | 1 + .../pi_stage/query_methods.jl | 30 ++-- .../objective_positioner_interface/gui.jl | 7 +- .../stage_interface/gui.jl | 24 +--- .../triggerscope_interface/gui.jl | 3 +- src/instrument.jl | 17 +++ test/pi_stage.jl | 134 +++++++++++++++++- test/pi_stage_fake_sdk.jl | 16 ++- 10 files changed, 235 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb50fa..58b9aca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,13 +13,16 @@ the next version with `-DEV`). ### Fixed - `PIStage`: `shutdown` could close another object's connection. `id` defaulted to `0`, a valid GCS id, and was never reset; it now defaults to `-1` and `shutdown` resets it. - `PIStage.initialize` reported the stage connected before it was: `connectionstatus` was set before the connect, and a failed close after a failed reference left it `true`, so a retry answered "already initialized". It is now set only after the whole sequence succeeds, and every step after the connect is inside the cleanup. -- The stage panels' initialize buttons log a failed `initialize` instead of throwing out of the click callback. +- `PIStage.initialize` ignored a FALSE from reading the travel range (`PI_qTMN`/`PI_qTMX`) or setting the velocity, and came up connected with an unset range. Each is now checked; a failure closes the connection and throws. A failed range query no longer writes an uninitialized buffer into `range_x`/`range_y`. +- `PIStage.initialize`'s wait for motion to stop after the reference move had no deadline and ignored `PI_IsMoving`'s return, so a failed query could spin forever. It now polls every 0.1 s, throws on a failed query, and gives up after `REFERENCE_TIMEOUT_S`. +- A `PIStage.initialize` retried after a failed close reported the controller held by another process. The stage now closes its own earlier connection first, and does not reconnect if that close fails too. +- The stage, Triggerscope and objective-positioner panels log a failed `initialize` instead of throwing out of the callback, through one helper, and a stage panel reads no position after an `initialize` that did not connect. ### Changed - `PIStage.initialize` waits for `PI_IsControllerReady` after the reference move, before polling `PI_qFRF`, as PI's samples do. Not yet run on hardware; needs a rig check on the C-867. ### Added -- Fake-GCS2 tests for `PIStage` (`test/pi_stage_fake_sdk.jl`, `test/pi_stage.jl`): `initialize`'s ordering and cleanup and `shutdown`'s id handling, with no hardware. +- Fake-GCS2 tests for `PIStage` (`test/pi_stage_fake_sdk.jl`, `test/pi_stage.jl`): `initialize`'s ordering and cleanup and `shutdown`'s id handling, the range, velocity and motion-stop checks, the reclaim after a failed close, and the GUI guard, with no hardware. ## [0.2.5] - 2026-09-29 diff --git a/src/hardware_implementations/pi_stage/config_methods.jl b/src/hardware_implementations/pi_stage/config_methods.jl index 83a5711..61b77c5 100644 --- a/src/hardware_implementations/pi_stage/config_methods.jl +++ b/src/hardware_implementations/pi_stage/config_methods.jl @@ -17,6 +17,17 @@ function initialize_original(stage::PIStage) return end + if stage.id >= 0 + # An earlier initialize connected and its close failed: this stage still holds the + # controller, and the DLL does not enumerate a controller that is open. Close it first. + @info "Closing this stage's earlier connection (id $(stage.id)) before reconnecting" + shutdown_original(stage) + if stage.id >= 0 + @error "This stage's earlier connection (id $(stage.id)) could not be closed; not reconnecting" + return + end + end + # Create a buffer string bufferstring = Vector{UInt8}(undef, 1024) @@ -60,16 +71,15 @@ function initialize_original(stage::PIStage) _waitforreference(stage; timeout = REFERENCE_TIMEOUT_S[]) #Find the max and min position of the axes - getrange(stage) + getrange(stage) == 1 || + error("PI_qTMN/PI_qTMX failed (GCS error $(_pi_geterror(stage))); travel range unknown") #Wait for any remaining motion to finish - ismoving(stage) - while stage.ismoving[1] == 1 || stage.ismoving[2] == 1 - ismoving(stage) - end + _waitforstop(stage; timeout = REFERENCE_TIMEOUT_S[]) #Set the velocity to `stage.velocity` - success = setvel(stage, stage.velocity) + setvel(stage, stage.velocity) == 1 || + error("PI_VEL/PI_qVEL failed (GCS error $(_pi_geterror(stage))); velocity not set") catch shutdown_original(stage) rethrow() @@ -93,8 +103,9 @@ end _pi_geterror(stage::PIStage) = PI_GetError(stage.id) """ -How long `initialize` waits, in seconds, for the controller to become ready and then for both -axes to report referenced. A `Ref` so tests can shorten it. +How long each of `initialize`'s three waits may take, in seconds: for the controller to report +ready, for both axes to report referenced, and for motion to stop. The budgets are separate, so +`initialize` can wait up to three times this in all. A `Ref` so tests can shorten it. """ const REFERENCE_TIMEOUT_S = Ref(60.0) @@ -116,11 +127,29 @@ function _waitforready(stage::PIStage; timeout::Real = REFERENCE_TIMEOUT_S[]) end end +""" +Poll `PI_IsMoving` every 0.1 s until neither axis is moving; throw if the query fails or motion +has not stopped within `timeout` seconds. Query only: it sends no motion command. +""" +function _waitforstop(stage::PIStage; timeout::Real = REFERENCE_TIMEOUT_S[]) + # PI_IsMoving fills `BOOL*`, bound as UInt32 in gcs2.jl. + moving = zeros(UInt32, 2) + deadline = time() + timeout + while true + ok = PI_IsMoving(stage.id, "1 2", moving) + ok == 1 || error("PI_IsMoving failed (GCS error $(_pi_geterror(stage)))") + stage.ismoving = (moving[1] != 0, moving[2] != 0) + any(!=(0), moving) || return nothing + time() > deadline && error("PI stage still moving after $(timeout) s") + sleep(0.1) + end +end + """ Poll `PI_qFRF` until both axes report referenced; throw if that has not happened within `timeout` seconds or the query itself fails. """ -function _waitforreference(stage::PIStage; timeout::Real = 60.0) +function _waitforreference(stage::PIStage; timeout::Real = REFERENCE_TIMEOUT_S[]) # PI_qFRF fills `BOOL*`: one 32-bit int per axis, like PI_SVO. referenced = zeros(Cint, 2) deadline = time() + timeout @@ -196,18 +225,18 @@ end function setvel(stage::PIStage,vel::Vector{Float64}) - success = PI_VEL(stage.id, "1 2", vel) + setok = PI_VEL(stage.id, "1 2", vel) - if success == 0 + if setok == 0 @error "Failed to set velocity" end - velocity = Vector{Cdouble}(undef, 2) - success = PI_qVEL(stage.id, "1 2", velocity) - - if success == 0 + velocity = zeros(Cdouble, 2) + queryok = PI_qVEL(stage.id, "1 2", velocity) + + if queryok == 0 @error "Failed to query velocity" else stage.velocity = velocity end - return success + return setok == 1 && queryok == 1 ? Cint(1) : Cint(0) end \ No newline at end of file diff --git a/src/hardware_implementations/pi_stage/interface_methods.jl b/src/hardware_implementations/pi_stage/interface_methods.jl index eb5291e..79b5f38 100644 --- a/src/hardware_implementations/pi_stage/interface_methods.jl +++ b/src/hardware_implementations/pi_stage/interface_methods.jl @@ -37,6 +37,7 @@ Function to update the position range of the PI Stage """ function StageInterface.getrange(stage::PIStage) getrange(stage) + return stage.range_y # preserves 0.2.5's return value end diff --git a/src/hardware_implementations/pi_stage/query_methods.jl b/src/hardware_implementations/pi_stage/query_methods.jl index 66e1b04..bb75691 100644 --- a/src/hardware_implementations/pi_stage/query_methods.jl +++ b/src/hardware_implementations/pi_stage/query_methods.jl @@ -36,32 +36,40 @@ BOOL PI_IsMoving (int ID, const char* szAxes, BOOL* pbValueArray) Function to check if the PI Stage is moving, both the x and y axis are checked """ function ismoving(stage::PIStage) - ismoving = Vector{UInt32}(undef, 2) + ismoving = zeros(UInt32, 2) PI_IsMoving(stage.id, "1 2", ismoving) stage.ismoving = (Bool(ismoving[1]), Bool(ismoving[2])) end function getrange(stage::PIStage) - findmin(stage) - findmax(stage) + # Both reads run, so a failed min does not skip the max. + okmin = findmin(stage) + okmax = findmax(stage) + return okmin == 1 && okmax == 1 ? Cint(1) : Cint(0) end """ """ function findmin(stage::PIStage) - minpositions = Vector{Cdouble}(undef, 2) - PI_qTMN(stage.id, "1 2", minpositions) - stage.range_x = (minpositions[1], stage.range_x[2]) - stage.range_y = (minpositions[2], stage.range_y[2]) + minpositions = zeros(Cdouble, 2) + ok = PI_qTMN(stage.id, "1 2", minpositions) + if ok == 1 + stage.range_x = (minpositions[1], stage.range_x[2]) + stage.range_y = (minpositions[2], stage.range_y[2]) + end + return ok end """ """ function findmax(stage::PIStage) - maxpositions = Vector{Cdouble}(undef, 2) - PI_qTMX(stage.id, "1 2", maxpositions) - stage.range_x = (stage.range_x[1], maxpositions[1]) - stage.range_y = (stage.range_y[1], maxpositions[2]) + maxpositions = zeros(Cdouble, 2) + ok = PI_qTMX(stage.id, "1 2", maxpositions) + if ok == 1 + stage.range_x = (stage.range_x[1], maxpositions[1]) + stage.range_y = (stage.range_y[1], maxpositions[2]) + end + return ok end \ No newline at end of file diff --git a/src/hardware_interfaces/objective_positioner_interface/gui.jl b/src/hardware_interfaces/objective_positioner_interface/gui.jl index 9bfe98f..c9b7316 100644 --- a/src/hardware_interfaces/objective_positioner_interface/gui.jl +++ b/src/hardware_interfaces/objective_positioner_interface/gui.jl @@ -1,12 +1,7 @@ using GLMakie function gui(positioner::Zpositioner) - try - initialize(positioner) - catch - @error "Failed to initialize the positioner. Please check the connection." - return - end + MicroscopeControl.gui_initialize(positioner, "positioner") || return fig = Figure(size=(600, 400)) diff --git a/src/hardware_interfaces/stage_interface/gui.jl b/src/hardware_interfaces/stage_interface/gui.jl index a634bfc..9785e19 100644 --- a/src/hardware_interfaces/stage_interface/gui.jl +++ b/src/hardware_interfaces/stage_interface/gui.jl @@ -164,13 +164,7 @@ function gui1d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - try - initialize(stage) - catch err - @error "Failed to initialize the stage" exception = err - return - end - stage.connectionstatus || return + MicroscopeControl.gui_initialize(stage, "stage") || return getposition(stage) xposition[] = stage.real_x xtarget[] = stage.targ_x @@ -459,13 +453,7 @@ function gui2d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - try - initialize(stage) - catch err - @error "Failed to initialize the stage" exception = err - return - end - stage.connectionstatus || return + MicroscopeControl.gui_initialize(stage, "stage") || return getposition(stage) xposition[], yposition[] = stage.real_x, stage.real_y xtarget[], ytarget[] = stage.targ_x, stage.targ_y @@ -807,13 +795,7 @@ function gui3d(stage::Stage) stopmotion(stage) end on(control_buttons[3].clicks) do initialize_click - try - initialize(stage) - catch err - @error "Failed to initialize the stage" exception = err - return - end - stage.connectionstatus || return + MicroscopeControl.gui_initialize(stage, "stage") || return getposition(stage) xposition[], yposition[], zposition[] = stage.real_x, stage.real_y, stage.real_z xtarget[], ytarget[], ztarget[] = stage.targ_x, stage.targ_y, stage.targ_z diff --git a/src/hardware_interfaces/triggerscope_interface/gui.jl b/src/hardware_interfaces/triggerscope_interface/gui.jl index 1673f61..94b6b31 100644 --- a/src/hardware_interfaces/triggerscope_interface/gui.jl +++ b/src/hardware_interfaces/triggerscope_interface/gui.jl @@ -30,7 +30,8 @@ function gui(trig::TRIG) trig_gui_fig[2,1] = stopbutton = Button(trig_gui_fig, label="Stop Device") on(startbutton.clicks) do event - initialize(trig) + MicroscopeControl.gui_initialize(trig, "Triggerscope") + return nothing end on(stopbutton.clicks) do event diff --git a/src/instrument.jl b/src/instrument.jl index 8de077c..503dc22 100644 --- a/src/instrument.jl +++ b/src/instrument.jl @@ -54,6 +54,23 @@ function gui(instrument::AbstractInstrument) error("gui not implemented for $(typeof(instrument))") end +""" + gui_initialize(device, what::AbstractString) -> Bool + +Call `initialize(device)` from a GUI panel or button. A throw is logged with `@error` instead of +escaping into Makie's callback. Returns `false` after a throw, or when the device has a +`connectionstatus` field that is still `false`; otherwise `true`. +""" +function gui_initialize(device, what::AbstractString) + try + initialize(device) + catch err + @error "Failed to initialize the $what" exception = err + return false + end + return !hasproperty(device, :connectionstatus) || device.connectionstatus +end + # ============================================================================= # AbstractSystem - Composite of instruments # ============================================================================= diff --git a/test/pi_stage.jl b/test/pi_stage.jl index 4f0b809..ee76a00 100644 --- a/test/pi_stage.jl +++ b/test/pi_stage.jl @@ -29,9 +29,10 @@ @test stage.id == 5 @test seen[] == false iFRF = findfirst(==("PI_FRF"), F.calls) - iready = findfirst(==("PI_IsControllerReady"), F.calls) - iq = findfirst(==("PI_qFRF"), F.calls) - @test iFRF < iready < iq + @test iFRF < findfirst(==("PI_IsControllerReady"), F.calls) + @test findlast(==("PI_IsControllerReady"), F.calls) < findfirst(==("PI_qFRF"), F.calls) + @test stage.range_x == (0.0, 25.0) + @test stage.range_y == (0.0, 25.0) @test ncalls("PI_IsControllerReady") == 3 end @@ -76,7 +77,7 @@ @test !stage.connectionstatus end - @testset "failed close keeps the id; retry cannot report ready" begin + @testset "failed close keeps the id; a retry reclaims it" begin F.reset!() push!(F.connect_ids, 5) F.fail!("PI_FRF") @@ -85,11 +86,136 @@ @test_throws ErrorException initialize(stage) @test !stage.connectionstatus @test stage.id == 5 + F.close_leaves_open[] = false + delete!(F.failing, "PI_FRF") + push!(F.connect_ids, 6) + empty!(F.calls) + initialize(stage) + @test stage.connectionstatus + @test stage.id == 6 + @test findfirst(==("PI_CloseConnection"), F.calls) < findfirst(==("PI_EnumerateUSB"), F.calls) + end + + @testset "reclaim fails: no reconnect" begin + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_FRF") + F.close_leaves_open[] = true + stage = PIStage() + @test_throws ErrorException initialize(stage) + @test stage.id == 5 + empty!(F.calls) + initialize(stage) + @test !stage.connectionstatus + @test stage.id == 5 + @test ncalls("PI_EnumerateUSB") == 0 + end + + @testset "controller-ready query fails" begin + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_IsControllerReady") + stage = PIStage() + @test_throws "PI_IsControllerReady failed" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + @test stage.id == -1 + end + + @testset "nothing enumerated" begin + F.reset!() F.enum_count[] = 0 + stage = PIStage() + @test initialize(stage) === nothing + @test !stage.connectionstatus + @test stage.id == -1 + @test ncalls("PI_ConnectUSB") == 0 + end + + @testset "connect fails" begin + F.reset!() + push!(F.connect_ids, -1) + stage = PIStage() + @test initialize(stage) === nothing + @test !stage.connectionstatus + @test stage.id == -1 + @test ncalls("PI_SVO") == 0 + end + + @testset "range read fails" begin + for op in ("PI_qTMN", "PI_qTMX") + F.reset!() + push!(F.connect_ids, 5) + F.fail!(op) + stage = PIStage() + @test_throws "travel range unknown" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + end + + @testset "velocity set fails" begin + for op in ("PI_VEL", "PI_qVEL") + F.reset!() + push!(F.connect_ids, 5) + F.fail!(op) + stage = PIStage() + @test_throws "velocity not set" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + end + + @testset "motion stops after polls" begin + F.reset!() + push!(F.connect_ids, 5) + F.moving_polls[] = 2 + stage = PIStage() initialize(stage) + @test stage.connectionstatus + @test ncalls("PI_IsMoving") == 3 + end + + @testset "motion never stops" begin + F.reset!() + push!(F.connect_ids, 5) + F.moving_polls[] = typemax(Int) + stage = PIStage() + @test_throws "still moving" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 @test !stage.connectionstatus end + @testset "IsMoving query fails" begin + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_IsMoving") + stage = PIStage() + @test_throws "PI_IsMoving failed" initialize(stage) + @test ncalls("PI_CloseConnection") == 1 + @test !stage.connectionstatus + end + + @testset "GUI guard" begin + G = MicroscopeControl.gui_initialize + F.reset!() + push!(F.connect_ids, 5) + stage = PIStage() + @test G(stage, "stage") === true + + F.reset!() + F.enum_count[] = 0 + stage = PIStage() + @test G(stage, "stage") === false + + F.reset!() + push!(F.connect_ids, 5) + F.fail!("PI_FRF") + stage = PIStage() + r = @test_logs (:error, r"Failed to initialize the stage") match_mode=:any G(stage, "stage") + @test r === false + end + @testset "shutdown after initialize" begin F.reset!() push!(F.connect_ids, 5) diff --git a/test/pi_stage_fake_sdk.jl b/test/pi_stage_fake_sdk.jl index e0ab6a4..c5b1482 100644 --- a/test/pi_stage_fake_sdk.jl +++ b/test/pi_stage_fake_sdk.jl @@ -53,7 +53,11 @@ const frf_hook = Ref{Any}(nothing) "Returned by `PI_GetError`." const error_code = Ref(0) +"`PI_IsMoving` answers moving on both axes for this many polls." +const moving_polls = Ref(0) + const ready_polls = Ref(0) +const moving_count = Ref(0) const referenced_polls = Ref(0) function reset!() @@ -71,6 +75,8 @@ function reset!() error_code[] = 0 ready_polls[] = 0 referenced_polls[] = 0 + moving_polls[] = 0 + moving_count[] = 0 return nothing end @@ -144,22 +150,24 @@ FakePIStage.reset!() PI_VEL(ID, axes, values) = Main.FakePIStage.status!("PI_VEL") function PI_qVEL(ID, axes, values) st = Main.FakePIStage.status!("PI_qVEL") - values[1] = values[2] = 1.0 + st == 1 && (values[1] = values[2] = 1.0) return st end function PI_IsMoving(ID, axes, values) st = Main.FakePIStage.status!("PI_IsMoving") - values[1] = values[2] = 0 + Main.FakePIStage.moving_count[] += 1 + moving = Main.FakePIStage.moving_count[] <= Main.FakePIStage.moving_polls[] + st == 1 && (values[1] = values[2] = moving ? 1 : 0) return st end function PI_qTMN(ID, axes, values) st = Main.FakePIStage.status!("PI_qTMN") - values[1] = values[2] = 0.0 + st == 1 && (values[1] = values[2] = 0.0) return st end function PI_qTMX(ID, axes, values) st = Main.FakePIStage.status!("PI_qTMX") - values[1] = values[2] = 25.0 + st == 1 && (values[1] = values[2] = 25.0) return st end function PI_qPOS(ID, axes, values) From 4618582138e455f0856ae69b9b026daa16e71281 Mon Sep 17 00:00:00 2001 From: kalidke Date: Tue, 29 Sep 2026 15:29:57 -0600 Subject: [PATCH 5/5] Objective positioner panel: open disconnected after a failed initialize, as in 0.2.5 (#73 Q1) gui_initialize still logs the failure. 0.2.5 opened the panel when initialize returned without connecting (MCL_InitHandle == 0), with its callbacks refusing while connectionstatus is false; the P5 guard's '|| return' stopped that. Co-Authored-By: Claude Opus 5.5 --- src/hardware_interfaces/objective_positioner_interface/gui.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hardware_interfaces/objective_positioner_interface/gui.jl b/src/hardware_interfaces/objective_positioner_interface/gui.jl index c9b7316..abee4b8 100644 --- a/src/hardware_interfaces/objective_positioner_interface/gui.jl +++ b/src/hardware_interfaces/objective_positioner_interface/gui.jl @@ -1,7 +1,9 @@ using GLMakie function gui(positioner::Zpositioner) - MicroscopeControl.gui_initialize(positioner, "positioner") || return + # A failed initialize is logged, and the panel still opens disconnected, as in 0.2.5: + # its callbacks refuse until `connectionstatus` is true. + MicroscopeControl.gui_initialize(positioner, "positioner") fig = Figure(size=(600, 400))