Merge #73 (4618582): PI stage fixes, checked status returns and a bounded stop wait

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: kalidke

CHANGELOG.md

@@ -10,6 +10,20 @@ the next version with `-DEV`).
 
 ## [Unreleased]
 
+### 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.
+- `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, 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
 
 A non-breaking release. It brings the TCube laser's closed-loop (power) mode and

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")

src/hardware_implementations/pi_stage/config_methods.jl

@@ -1,100 +1,160 @@
 """
-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
     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)
 
     #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)
 
-    #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
-    stage.id = @ccall gcs2path.PI_ConnectUSB(bufferstring::Ptr{UInt8})::Cint
+    stage.id = PI_ConnectUSB(bufferstring)
 
     @info "Device ID: " * string(stage.id)
 
     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)
+        _waitforready(stage; timeout = REFERENCE_TIMEOUT_S[])
+        _waitforreference(stage; timeout = REFERENCE_TIMEOUT_S[])
+
+        #Find the max and min position of the axes
+        getrange(stage) == 1 ||
+            error("PI_qTMN/PI_qTMX failed (GCS error $(_pi_geterror(stage))); travel range unknown")
+
+        #Wait for any remaining motion to finish
+        _waitforstop(stage; timeout = REFERENCE_TIMEOUT_S[])
+
+        #Set the velocity to `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()
     end
 
-    #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
-        ismoving(stage)
-    end
-
-    #Set velocity to 1 mm/s
-    success = setvel(stage, stage.velocity)
-
+    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 = @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)
+
+"""
+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)
+
+"""
+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_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
     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,21 +168,23 @@ 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"
         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
 
@@ -133,7 +195,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 +209,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,25 +218,25 @@ 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
+    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 = @ccall gcs2path.PI_qVEL(stage.id::Cint, "1 2"::Ptr{UInt8}, velocity::Ptr{Cdouble})::Cint
-    
-    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

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

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
 
@@ -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
 
 

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,23 +28,23 @@ 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
 
 """
 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
 
 """
 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