TCube laser: pass Kinesis boolean arguments as four bytes

.gitignore

@@ -5,5 +5,7 @@
 /docs/Manifest.toml
 /docs/build/
 .DS_Store
+# local-only symlink to the lab instrument archive on the NAS (see CLAUDE.md)
+/manuals
 # Local run output: test records, logs, handoffs (lab decision 0028)
 dev/output/

CHANGELOG.md

@@ -11,6 +11,16 @@ the next version with `-DEV`).
 ## [Unreleased]
 
 ### Fixed
+- **TCube laser: Kinesis boolean arguments are passed as four bytes.** Every
+  Kinesis boolean was bound as a one-byte `Bool`. That is right for return
+  values, but the headers declare arguments as a four-byte type, so the
+  controller could read three bytes of whatever the register held. Arguments
+  (`LD_EnableMaxCurrentAdjust`, `LD_EnableTIAGainAdjust`,
+  `LD_EnableLastMsgTimer`) are now a zero-extended `Cuint` (`KBOOL_ARG`), and
+  returns stay one byte (`KBOOL_RET`); the vendor facts are in
+  `manuals/Thorlabs/TLD001/BINDING.md` (see CLAUDE.md, "Instrument
+  documentation archive"). No driver method calls those three functions yet,
+  so no current behaviour changes; not yet run on hardware.
 - **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

CLAUDE.md

@@ -186,3 +186,50 @@ pull request that raises `Z`, and merge the same fix into `main`.
 hand, after checking the same `lab/tests` coverage.
 
 What the numbers mean (decision 0033): before 1.0, in `0.Y.Z` **raising `Z` is any non-breaking change, new features included, and raising `Y` is an interface break** -- `0.2.4 -> 0.3.0` declares a break and `0.2.4 -> 0.2.5` a compatible release, which is also how Julia's `^0.2` compat bound reads them. A break is anything that lets working downstream code behave differently: a signature, an export, or what a call returns or throws. A bug fix that changes behaviour only on a path that was already broken is not a break. Config types are built by keyword (lab decision 0035): adding a field with a default is not a break, and a positional argument's meaning never changes (add a keyword and deprecate the old form instead). Hardware verification is not tracked in this repo; it is recorded by the downstream rig repo that pins to a given tag. The merge gate is the local suite (see "Testing policy" above) plus `test/contract.jl`'s "Interface Contract" testset, which guards the no-ambiguous-exports and core-method invariants described above; CI confirms it on a reduced matrix.
+
+## Instrument documentation archive (`manuals/`)
+
+`manuals/` is a **local-only symlink** to the lab-wide instrument archive on
+the NAS — vendor manuals, SDK headers, API references and the driver-facing
+notes that explain why a binding is the way it is. It is gitignored and must
+**never** be committed: git stores a symlink as a mode-120000 blob, and on a
+Windows rig without the symlink privilege that checks out as a text file
+containing the path, which is worse than nothing. Each clone makes its own.
+
+```bash
+# Linux (any of the four hosts)
+ln -sfn /mnt/nas/lidkelab/Projects/lab_instruments manuals
+```
+```bat
+REM Windows rig, from the repo root. Needs an elevated prompt, OR Developer
+REM Mode enabled once (Settings > Privacy & security > For developers).
+mklink /D manuals \\192.168.1.21\lidke-lrs\Projects\lab_instruments
+```
+
+`[limitation]` The junction form, `mklink /J`, does **not** work here: junctions
+resolve only to local volumes, so a UNC target fails. `/D` is required, and it
+is the one step in this arrangement that needs a privilege — once per rig, not
+once per clone. This has not been run on either rig yet; if `/D` is refused,
+say so rather than reaching for a mapped drive letter, which differs between
+user sessions and services.
+
+Nothing else is required — no environment variable and no shell profile edit.
+If `manuals/` is absent, make it with the line above.
+
+Layout is manufacturer first, then model: `manuals/Thorlabs/TLD001/`,
+`manuals/Hamamatsu/C11440-22CU/`, with shared vendor SDKs under
+`manuals/<Manufacturer>/SDK/<sdk-id>/`. Start at `manuals/INDEX.md`; the rules
+for adding anything are in `manuals/README.md`.
+
+Within a model directory, `source/` is the vendor original, verbatim and never
+renamed, and `docs/` is the working copy with a predictable name. A
+`BINDING.md`, where one exists, is the distillate a driver author actually
+needs — for example `manuals/Thorlabs/TLD001/BINDING.md` records that a Kinesis
+C++ boolean must be *passed* as a 4-byte `Cuint` but *read back* as a 1-byte
+`Bool`, which this package got wrong twice in opposite directions.
+
+`[policy]` When a driver's behaviour turns on a vendor fact -- a struct layout,
+an ABI width, a scaling constant, a status bit -- record it in that model's
+`BINDING.md` and cite the document in `source/` it came from. Three of the four
+defects in v0.2.3 were found by a rig holding hardware rather than by review,
+because the vendor fact was not written down anywhere a reviewer could check.

src/hardware_implementations/tcube_laser/constants_Tlaser.jl

@@ -8,23 +8,71 @@ const __int64 = Clonglong
 
 const __int32 = Cint
 
+"""
+    BOOL
+
+Four bytes. **Retained for `TLI_DeviceInfo`'s fields only**, which nothing in
+this package calls — see that struct's docstring. Do not use it for a new
+binding: use [`KBOOL_ARG`](@ref) for an argument and [`KBOOL_RET`](@ref) for a
+return.
+
+`[limitation]` The vendor headers do **not** declare `BOOL` for these fields.
+Two rigs have now read their own vendor-installed
+`Thorlabs.MotionControl.TCube.LaserDiode.h` — Kinesis 1.14.10 on the seq-sr rig
+and 1.14.47.22504 on quickbeam — and **both declare lowercase C++ `bool` with
+`#pragma pack(1)` active**. This `Cuint` is therefore wrong for
+`TLI_DeviceInfo`, and is left in place only because no single layout fits both
+versions anyway (they disagree on `serialNo`'s length), so there is nothing to
+change it *to*.
+
+**History, because this was got wrong twice in opposite directions.** v0.2.3
+retyped both boolean roles to a one-byte `Bool`. A copy of the header on the lab
+NAS appeared to contradict that, carrying `typedef unsigned int BOOL` and no
+lowercase `bool` at all, and on its strength the change was reverted — but that
+file is a Clang.jl generation input, hand-edited to parse without the Windows
+SDK, and its typedefs are artefacts of the editing rather than the vendor's ABI.
+Splitting the two roles is the correct answer, and the vendor-installed headers
+since read off both rigs confirm it.
+"""
 const BOOL = Cuint
 
 """
-    CPPBOOL
-
-Julia's `Bool`, i.e. one byte, for the Kinesis entry points the header declares
-as C++ `bool` rather than as a Windows `BOOL`. The distinction is not cosmetic:
-a `bool` return sets only the low byte of the return register, so reading it as
-a 4-byte `BOOL` reads three bytes of whatever happened to be there, and
-`LD_CheckConnection(...) != 0` can then report a disconnected controller as
-connected. Reported by the 642 nm rig from the Kinesis header, 2026-09-23; the
-same file's `tcubeapi.jl` already used `::Bool` for `LD_StartPolling` and
-`LD_StopPolling`, so the package disagreed with itself. Not hardware-verified.
-
-`BOOL` above stays `Cuint` for the genuine Windows `BOOL` uses.
+    KBOOL_ARG
+
+The type to pass a Kinesis C++ `bool` ARGUMENT: a zero-extended `Cuint`
+carrying exactly 0 or 1.
+
+This is robust whichever width the callee really reads. A `bool` callee takes
+the low byte and sees 0 or 1; a `BOOL` callee takes all four and sees 0 or 1.
+Passing a 1-byte `Bool` is NOT robust in this direction: the upper three bytes
+of the register are undefined, so a `false` can arrive as true. That matters
+for `LD_EnableMaxCurrentAdjust(serialNo, enableAdjust, enableDiode)`, whose
+second flag enables the laser diode during a max-current adjustment.
+"""
+const KBOOL_ARG = Cuint
+
+"""
+    KBOOL_RET
+
+The type to read a Kinesis C++ `bool` RETURN: one byte.
+
+The installed vendor header declares these `bool`, which on MSVC x86-64
+returns in `AL` and leaves the rest of `EAX` **undefined**. Reading four bytes
+can therefore turn a `false` into a nonzero value — `LD_CheckConnection`
+reporting a disconnected controller as connected. Reading the low byte is
+correct under the `bool` ABI and still correct under a `BOOL` ABI returning
+0 or 1.
+
+**History, because this was got wrong twice.** v0.2.3 retyped both roles to a
+1-byte `Bool`, which was right for returns and wrong for arguments. A copy of
+the header on the lab NAS appeared to contradict it — but that copy is a
+Clang.jl generation input, hand-edited to parse without Windows headers: its
+`typedef unsigned int BOOL` and its commented-out `#pragma pack` are artefacts
+of that editing, not the vendor's ABI. The installed header has 32 lowercase
+`bool`, zero `BOOL`, and an active `#pragma pack(1)`. Splitting the two roles
+is what is actually correct, and is safe under either reading.
 """
-const CPPBOOL = Bool
+const KBOOL_RET = Bool
 
 struct tagSAFEARRAYBOUND
     cElements::Culong
@@ -67,22 +115,39 @@ end
 """
     TLI_DeviceInfo
 
-**[limitation] This layout is suspect and unverified.** The Kinesis header
-declares the `is*` fields as C++ `bool` (one byte), and they are typed here as
-`BOOL` = `Cuint` (four). If that is right, every field from `isKnownType`
-onward is misaligned and `TLI_GetDeviceInfo` returns nonsense. Nothing in this
-package calls `TLI_GetDeviceInfo`, so the defect is latent rather than active,
-and it is left alone as deferred ABI repair rather than corrected in passing.
-
-Correcting it is more than retyping the five fields, and does NOT need a
-controller -- it needs the vendor header, which this repo does not carry. The
-header declares the structure `#pragma pack(1)` at 100 bytes; this declaration
-is 120, and the divergence starts at `PID`, *before* the first `bool`. So
-retyping the booleans alone would leave it wrong. Repair it against the header,
-with the size asserted, or do not call it. The function signatures in
-`functions_Tlaser.jl` had a related discrepancy and WERE corrected -- see
-`CPPBOOL` -- because there the fix moves no field and the ABI rule is
-unambiguous.
+**[limitation] This layout does not match either Kinesis version we have
+looked at, and it is left alone deliberately, because there is no single
+layout that would match both.**
+
+Two rigs read their installed headers and reported different declarations:
+
+| | Kinesis 1.14.10 (seq-sr rig) | Kinesis 1.14.47.22504 (quickbeam) |
+|---|---|---|
+| `serialNo` | `char serialNo[9]` | `char serialNo[16]` |
+| flags | 1-byte C++ `bool` | 1-byte C++ `bool` |
+| `#pragma pack(1)` | ACTIVE (lines 63-125) | ACTIVE |
+| reported size | — | 100 bytes, `PID` at 85 |
+
+This declaration uses `NTuple{16,Cchar}`, 4-byte `BOOL` flags and default
+alignment, measuring **120 bytes with `PID` at offset 88** (verified by
+execution). It is wrong for both, and the field that differs between the two
+vendor versions is an array LENGTH, so no reinterpretation fixes both at once.
+
+`[policy]` Do not call `TLI_GetDeviceInfo` through this declaration. Nothing
+in this package does — `initialize` uses only `TLI_BuildDeviceList` and
+`TLI_GetDeviceListSize`, both of which return counts and touch no struct
+(verified). If a caller ever needs device info, declare the struct for the
+SDK version in use, assert `sizeof`, and keep it beside the header it was
+read from.
+
+**One copy of this header on the lab NAS contradicts all of the above; do not
+trust it.** `Personal Folders/Sheng/code/generate_lib/lib/` holds a Clang.jl
+generation input, hand-edited to parse without Windows headers: local
+typedefs including `typedef unsigned int BOOL`, `OaIdl.h` and `__declspec`
+stripped, both pack pragmas commented out, every lowercase `bool` rewritten.
+Those are artefacts of the editing. This docstring briefly asserted, on the
+strength of that file, that our layout was correct; it is not, and the round
+trip cost two wrong rulings in opposite directions.
 """
 struct TLI_DeviceInfo
     typeID::DWORD

src/hardware_implementations/tcube_laser/functions_Tlaser.jl

@@ -55,7 +55,7 @@ function LD_Close(serialNo)
 end
 
 function LD_CheckConnection(serialNo)
-    ccall((:LD_CheckConnection, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar},), serialNo)
+    ccall((:LD_CheckConnection, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar},), serialNo)
 end
 
 function LD_Identify(serialNo)
@@ -79,15 +79,15 @@ function LD_GetSoftwareVersion(serialNo)
 end
 
 function LD_LoadSettings(serialNo)
-    ccall((:LD_LoadSettings, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar},), serialNo)
+    ccall((:LD_LoadSettings, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar},), serialNo)
 end
 
 function LD_LoadNamedSettings(serialNo, settingsName)
-    ccall((:LD_LoadNamedSettings, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar}, Ptr{Cchar}), serialNo, settingsName)
+    ccall((:LD_LoadNamedSettings, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar}, Ptr{Cchar}), serialNo, settingsName)
 end
 
 function LD_PersistSettings(serialNo)
-    ccall((:LD_PersistSettings, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar},), serialNo)
+    ccall((:LD_PersistSettings, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar},), serialNo)
 end
 
 function LD_Disable(serialNo)
@@ -111,11 +111,11 @@ function LD_MessageQueueSize(serialNo)
 end
 
 function LD_GetNextMessage(serialNo, messageType, messageID, messageData)
-    ccall((:LD_GetNextMessage, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar}, Ptr{WORD}, Ptr{WORD}, Ptr{DWORD}), serialNo, messageType, messageID, messageData)
+    ccall((:LD_GetNextMessage, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar}, Ptr{WORD}, Ptr{WORD}, Ptr{DWORD}), serialNo, messageType, messageID, messageData)
 end
 
 function LD_WaitForMessage(serialNo, messageType, messageID, messageData)
-    ccall((:LD_WaitForMessage, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar}, Ptr{WORD}, Ptr{WORD}, Ptr{DWORD}), serialNo, messageType, messageID, messageData)
+    ccall((:LD_WaitForMessage, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar}, Ptr{WORD}, Ptr{WORD}, Ptr{DWORD}), serialNo, messageType, messageID, messageData)
 end
 
 function LD_SetOpenLoopMode(serialNo)
@@ -127,7 +127,7 @@ function LD_SetClosedLoopMode(serialNo)
 end
 
 function LD_EnableMaxCurrentAdjust(serialNo, enableAdjust, enableDiode)
-    ccall((:LD_EnableMaxCurrentAdjust, Thorlabs_Tcube_laser), Cshort, (Ptr{Cchar}, CPPBOOL, CPPBOOL), serialNo, enableAdjust, enableDiode)
+    ccall((:LD_EnableMaxCurrentAdjust, Thorlabs_Tcube_laser), Cshort, (Ptr{Cchar}, KBOOL_ARG, KBOOL_ARG), serialNo, enableAdjust, enableDiode)
 end
 
 function LD_RequestMaxCurrentDigPot(serialNo)
@@ -147,7 +147,7 @@ function LD_FindTIAGain(serialNo)
 end
 
 function LD_EnableTIAGainAdjust(serialNo, enable)
-    ccall((:LD_EnableTIAGainAdjust, Thorlabs_Tcube_laser), Cshort, (Ptr{Cchar}, CPPBOOL), serialNo, enable)
+    ccall((:LD_EnableTIAGainAdjust, Thorlabs_Tcube_laser), Cshort, (Ptr{Cchar}, KBOOL_ARG), serialNo, enable)
 end
 
 function LD_DisableOutput(serialNo)
@@ -267,7 +267,7 @@ function LD_GetStatusBits(serialNo)
 end
 
 function LD_StartPolling(serialNo, milliseconds)
-    ccall((:LD_StartPolling, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar}, Cint), serialNo, milliseconds)
+    ccall((:LD_StartPolling, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar}, Cint), serialNo, milliseconds)
 end
 
 function LD_PollingDuration(serialNo)
@@ -279,15 +279,15 @@ function LD_StopPolling(serialNo)
 end
 
 function LD_TimeSinceLastMsgReceived(serialNo, arg2)
-    ccall((:LD_TimeSinceLastMsgReceived, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar}, __int64), serialNo, arg2)
+    ccall((:LD_TimeSinceLastMsgReceived, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar}, __int64), serialNo, arg2)
 end
 
 function LD_EnableLastMsgTimer(serialNo, enable, lastMsgTimeout)
-    ccall((:LD_EnableLastMsgTimer, Thorlabs_Tcube_laser), Cvoid, (Ptr{Cchar}, CPPBOOL, __int32), serialNo, enable, lastMsgTimeout)
+    ccall((:LD_EnableLastMsgTimer, Thorlabs_Tcube_laser), Cvoid, (Ptr{Cchar}, KBOOL_ARG, __int32), serialNo, enable, lastMsgTimeout)
 end
 
 function LD_HasLastMsgTimerOverrun(serialNo)
-    ccall((:LD_HasLastMsgTimerOverrun, Thorlabs_Tcube_laser), CPPBOOL, (Ptr{Cchar},), serialNo)
+    ccall((:LD_HasLastMsgTimerOverrun, Thorlabs_Tcube_laser), KBOOL_RET, (Ptr{Cchar},), serialNo)
 end
 
 function LD_RequestSettings(serialNo)