Release 0.2.5: TCube power mode, PI N-472 and PI stage fixes, Kinesis booleans - #71
Merged
Merged
Conversation
…-checked on the 642 nm rig (0.3.0)
Implements dev/output/plan-laser-modes.md (rev 3): DiodeLaser under
LightSource; ConstantCurrent / ConstantPhotocurrent mode types;
TCubeLaser{M} with a PhotodiodeLoop; setcurrent!, setoutputpower!,
setlevel!, measured_current, measured_photocurrent,
indicated_output_power and loop_status; mode-dispatched current and
power panels (range shown, out-of-range textbox turns red, no command or
read on open); SimDiodeLaser twin; contract test and API map walk to the
device leaves. setpower throws for DiodeLaser; mode is a required
keyword (plan section 8.1 contingency). Brings in the Kinesis boolean
split from fix/revert-cppbool.
Hardware-checked on TLD001 64849775 with a power meter before the fibre:
open loop at 70/90/110 mA is correct, and closed loop regulates from 1 to
5 mW (measured = requested - 0.46 mW, so the 224.2 W/A calibration holds).
Above ~5 mW closed loop held ~21 mW, because the controller's photodiode
reading clips at ~3213 counts; that needs the PD range / TIA gain set-up
redone (CALIBRATION.md). Fixed from the rig: setpoints are ignored with the
output off (now sent right after enabling, zeroed before disabling);
polling starts first; signed photocurrent with 0x8000 = over range; pot
needs adjust mode and the clamp is the controller's reported limit.
Adds CALIBRATION.md with the calibration procedure, today's measurements
and the manual's PD range / gain procedure.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…2 nm rig A setpoint jumped from 0 locks the TLD001's loop at ~21 mW / 90 mA whatever was requested (3 of 3 at 10 mW), while the same target reached in steps regulates exactly, as the Kinesis application does (9.30 mW). light_on and setoutputpower! now ramp upward closed-loop steps, RAMP_STEP_mW = 3 mW every RAMP_STEP_S = 10 ms (40 mW in ~0.2 s; the USB write is the floor). Measured with a power meter before the fibre: 10 / 20 / 40 mW requested gave 9.30 / 19.04 / 38.74 mW. The mechanism is not known; the ramp is empirical. The photodiode UNDER-range flag warns instead of refusing: it was set at 1 mW while the loop regulated correctly. CALIBRATION.md records the 09-29 session, the PD range / gain check (1 mA in range, 386 uA at the limit) and corrects the previous day's diagnosis: the 98 uA "clip" was this lock, not the photodiode channel. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…llback; verified to 70 mW Jumps from 0 to 10, 40 and 70 mW regulated correctly later the same night (9.30/9.31/9.30/9.31 mW at 10 mW, 68.5 mW at 70 mW), so the default is one write, the fastest. The ramp that worked around the earlier lock (3 of 3 failures at 10 mW, cause unknown) stays in the driver as an opt-in fallback, RAMP_STEP_mW[] = 3.0 / RAMP_STEP_S[] = 0.01, verified 10 of 10 at 10-70 mW. Closed loop measured over 1-70 mW: 0.56, 1.55, 2.54, 4.52, 9.30, 19.04, 38.74, 68.59 mW for 1, 2, 3, 5, 10, 20, 40, 70 mW requested (224.2 W/A). Tests cover both the default and the ramp. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…150 mA Measured at the laser head (before the filter): 22.23 mW at 90 mA vs 20.67 at the usual position (T = 0.93), 73.5 mW at 70 mW requested, 75.9 at 72. The diode is an Ushio HL6366DG (80 mW rated, 90 mW absolute maximum), so 70 mW at the usual position is ~94 % of rating; max_current 150 mA leaves the loop headroom over the ~142 mA it needs. Also records today's 13 s at the limit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e fake SDK Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…bject, 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Resolved so that 0.2.4's released TCube safety behaviour holds on this branch's API: - light_on keeps this branch's structure (require_clamp, intended_code, send_setpoint_ramped) with 0.2.4's failure handling: if the setpoint fails after the enable, the disable's status is checked, is_on becomes false only if it succeeded and true otherwise, and both failures are logged (review blocker 1). Open loop checks drive_current against the ceiling before the enable, and light_on warns when it sends 0 because nothing was requested. - zero_then_disable (light_off, shutdown) is 0.2.4's: one write of 0 with no status read and no read-back wait, logged if it fails, then the disable (review should-fix 4). Three assertions of this branch's call sequences change accordingly, and "a zero that cannot be confirmed" becomes "a zero that fails". - The fake SDK keeps this branch's controller model; 0.2.4's names (stored, output_on, enable_log, reset!(stored=)) are views of it, so test/tcube_output_order.jl runs unchanged. - Project.toml takes main's 0.2.5-DEV; README and skill pins stay at v0.2.4; rig-causes takes 0.2.4's row for the ignored-setpoint hazard. tcube_output_order.jl errors at this commit: it builds TCubeLaser without `mode` and calls setpower, which the next commits restore (decision 0035). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s failed-disable path Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ds in open loop, old export_state keys and properties.power kept (decision 0035) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…sweep 0.3.0 references Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…lamp above max_current Both modes zero and disable the output before the mode command. Open loop lowers the max-current pot only when the controller's limit exceeds max_current, never raises it. The closed-loop enter_mode! drops its own disable and clears pd.max_current_clamp first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…re emitting require_clamp reads the status word and the controller's limit afresh and refuses if the loop bit is gone or the limit is above the programmed clamp. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…driver knows, and detect a loop lock Deletes the RAMP_STEP_mW and RAMP_STEP_S globals (new in #66, never released) in favour of ramp_step_mW, ramp_step_s, lock_check_s and lock_ratio keywords. light_on ramps from 0 and setoutputpower! from the previous request's code, not from the stale LD_GetLaserSetPoint. check_lock refuses a measured photocurrent above lock_ratio x the request and the output is disabled. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…the click Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d, drop SETPOINT_NEEDS_OUTPUT Docstring examples use the placeholder serial and the documented diode rating. CALIBRATION.md keeps the current calibration and gains a short Controller facts section; the session diary and the ceiling section are cut (to dev/output/t16-pr66-calibration-rig-history.md). The stale bench-table comment in TCubeLaserControl.jl is deleted. The SETPOINT_NEEDS_OUTPUT constant's text now lives on send_setpoint. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d only after full success require_clamp refused only a limit more than 1 mA above the recorded clamp, which is more than one pot step (0.83 mA): at a 160 mA ceiling a pot one step up (160.74 mA) passed. It now refuses a limit above max_current, or more than half a step above the clamp. enter_mode! records the clamp only after the whole closed-loop sequence succeeded, so a failure after programming the pot (at LD_SetClosedLoopMode, say) leaves it NaN. setoutputpower!'s docstring said an under-range flag with the output on refuses; the code warns, as the rig showed the loop still regulates there. Lane 2 report items 1, 2 and 4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mode defaults to ConstantCurrent(), and closed loop requires max_current and accepts the ramp and lock keywords (stored, not simulated), so one construction line serves a rig and its simulated twin (captain's ruling). Tests: the power-mode re-check at a 160 mA ceiling (one pot step) and a re-initialize failing at LD_SetClosedLoopMode; SimDiodeLaser() is now a ConstantCurrent twin. Suite: 1682 pass, 3 broken, 0 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
v0.2.3 retyped nine returns and three arguments from `BOOL` (`Cuint`) to a
one-byte `Bool`, on a second-hand report that the Kinesis header declares
C++ `bool`. The header says otherwise, and it was on the lab NAS the whole
time:
Thorlabs.MotionControl.TCube.LaserDiode.h:37
typedef unsigned int BOOL;
27 uppercase `BOOL`, zero lowercase `bool`. The original binding was correct
and the change was a regression.
It is wrong in the dangerous direction for ARGUMENTS: passing one byte where
the callee reads four leaves the upper three undefined, so a `false` can
arrive as true. The call that matters is
`LD_EnableMaxCurrentAdjust(serial, enableAdjust, enableDiode)`, whose second
flag enables the laser diode during a max-current adjustment — the call the
closed-loop clamp sequence depends on being able to pass as false. Four bytes
is safe under either ABI for an argument; one byte is not.
The same report also drove a `[limitation]` warning on `TLI_DeviceInfo`
saying it was packed to 100 bytes and misaligned from `PID` onward. The
header's `#pragma pack(1)` is COMMENTED OUT, and the fields are the same
4-byte `BOOL`. Verified: our declaration is 120 bytes with `PID` at offset
88, which is exactly the header's default-alignment arithmetic. That warning
was false and is deleted — it would have sent the next reader hunting a
defect that does not exist.
Both docstrings now record the history, so nobody re-applies either "fix".
Nothing in this package calls the affected functions, so the regression is
latent here; a downstream calls `LD_CheckConnection`.
Suite: 959 passed, 8 broken, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rig repo diffed the installed vendor header against the copy on the lab NAS and settled this properly. The NAS copy is a Clang.jl generation input, hand-edited to parse without Windows headers: the vendor preamble was replaced with local typedefs including `typedef unsigned int BOOL`, `OaIdl.h` and `__declspec` were removed, both pack pragmas were commented out, and every lowercase `bool` was rewritten to `BOOL`. It carries no ABI information of its own, and I treated it as authoritative. The INSTALLED header has 32 lowercase `bool`, zero `BOOL`, and an active `#pragma pack(1)`. So v0.2.3 was half right and my revert was half wrong, and neither pure type is correct. The two roles differ: - `KBOOL_ARG = Cuint`, zero-extended 0 or 1, for arguments. Robust under either ABI: a `bool` callee reads the low byte, a `BOOL` callee reads four, both see 0 or 1. A 1-byte argument is NOT robust — the upper three bytes are undefined, so a `false` can arrive as true, and `LD_EnableMaxCurrentAdjust`'s second flag enables the diode. - `KBOOL_RET = Bool`, one byte, for returns. A `bool` return sets only `AL` and leaves the rest of `EAX` undefined, so reading four bytes can turn `false` into nonzero — `LD_CheckConnection` reporting a disconnected controller as connected. Reading the low byte is right under `bool` and still right under `BOOL` returning 0 or 1. Nine returns and four argument positions, split accordingly. `TLI_DeviceInfo`'s warning is restored and corrected rather than deleted: the real layout is 100 bytes with `PID` at 85 under `pack(1)` with 1-byte flags, against this declaration's verified 120 and 88. Still not fixed — correcting it needs field types AND packing, and a controller to read a device list back from. The docstring now names the NAS copy as untrustworthy so nobody relitigates this from it a third time. Suite: 959 passed, 8 broken, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 405 rig read its installed Kinesis 1.14.10 header and confirmed the 642
rig's reading independently: 32 lowercase C++ `bool`, zero `BOOL`, no `BOOL`
typedef, and `#pragma pack(1)` ACTIVE around `TLI_DeviceInfo`. It also found
something neither of us had — the two versions differ in an ARRAY LENGTH:
1.14.10 char serialNo[9]
the 642 rig char serialNo[16]
So the struct layout varies by SDK version in more than one dimension, and no
single declaration can be right for both. Ours matches neither: 120 bytes with
`PID` at 88, against a packed 100-with-85 on one and something different again
on the other.
The docstring now records both observed variants as a table, states the policy
(do not call `TLI_GetDeviceInfo` through this declaration; declare it per SDK
version beside the header it came from, and assert `sizeof`), and confirms by
inspection that `initialize` touches only the two counting `TLI_` functions,
so nothing reaches the struct.
It also names the NAS copy as untrustworthy, with the reason, so this is not
relitigated from that file a fourth time.
The boolean split stands and both rigs endorse it: 4-byte zero-extended
arguments, low-byte returns.
Suite: 959 passed, 8 broken, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two rigs have now read their own vendor-installed Kinesis header -- 1.14.10 on seq-sr, 1.14.47.22504 on quickbeam -- and both declare lowercase C++ `bool` with `#pragma pack(1)` active. The `BOOL` docstring still asserted the opposite, on the strength of a copy on the NAS that turns out to be a Clang.jl generation input hand-edited to parse without the Windows SDK. That claim also contradicted the `KBOOL_RET` docstring three lines above it. The docstring was additionally unattached: it sat between `const KBOOL_RET` and `struct tagSAFEARRAYBOUND`, so Julia bound it to the SAFEARRAY bound struct rather than to `BOOL`. Moved above `const BOOL = Cuint`, where it belongs. No binding changes. `BOOL` is still four bytes and still used only by `TLI_DeviceInfo`, which nothing calls; it stays because the two vendor versions disagree on `serialNo`'s length, so there is no single layout to change it to. Also record the instrument archive: `manuals/` is a local-only, gitignored symlink to the lab-wide archive on the NAS, documented in CLAUDE.md with the reason it must never be committed. Local suite: 959 pass, 8 broken, 0 fail. Rebased onto main for its pull request: the two dev/output plan files are dropped (decision 0028; they stay in fix/revert-cppbool's history), CLAUDE.md and .gitignore keep main's text beside this commit's, and CHANGELOG gains the [Unreleased] entry for the Kinesis boolean split. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it; initialize records a failed disable light_on and setoutputpower! zero the setpoint before the disable on any failure after the enable, setoutputpower! covers a send failure as well as a lock, and a failed initial disable in initialize sets is_on true. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…hangelog entry Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… construction, never raising Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… the limit-drift wording Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…document light_on's re-ramp from 0 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…s version numbers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fresh when recorded off Final-check nits N1 and N2 on c17d729, under the captain's ruling that no configuration that initialized in 0.2.4 may newly fail. N1: lower_open_loop_clamp! catches a failed search (adjust mode refused, a position that does not read back, or even the lowest position above max_current), re-reads the controller's limit, warns that max_current is enforced in software only, and lets initialize go on. The search has only ever lowered the potentiometer (raise = false), so nothing is less safe than before it ran; a failed re-read keeps the earlier reading, an upper bound for the same reason. This also absorbs the residual of a max_current at or above 17.25 mA but under a controller's real position-20 floor. Closed loop still refuses, since there the clamp is the only protection. N2: setcurrent! decides "on" from is_on, else from a fresh status read. The polled word can still report the output on for about one poll after a light_off, and a send then waited out its 1 s confirm and threw where 0.2.4 did not. Tests: the fake gains stale_bits, a polled status word that lags until an LD_RequestStatusBits. "open loop never newly fails" now proves both N1 paths (a pot walked to its floor, and adjust mode refused): initialize succeeds, warns, and the software ceiling holds. The stale-bit test asserts outcomes both ways: a stale "off" with the output on sends and confirms, and a stale "on" after light_off sends nothing and light_on then applies the request. One assertion changed: setcurrent! with the output recorded off now makes a fresh read (LD_RequestStatusBits, then LD_GetStatusBits). Suite: 1722 pass, 3 broken, 0 fail. tcube_output_order.jl (0.2.4's cases, unedited): 39/39. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # CHANGELOG.md # test/runtests.jl
…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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iled enable; is_on from the enable Codex's post-merge review of 0.2.4, C1-C4, as Keith and the captain ruled for 0.2.5 (simplified: no separate C4 flag or acknowledge call). C1 and C4: between an enable and the setpoint that follows it the diode runs on the controller's stored setpoint, which software cannot clear while the output is off. The only bound is the current limit stored in the controller (front-panel encoder or software), so open-loop light_on reads it fresh (LD_RequestLaserDiodeMaxCurrentLimit, LD_GetLaserDiodeMaxCurrentLimit, in mA) before every enable and refuses above max_current. That bounds the stale pulse and the failed-zero case alike. Closed loop already refused above its clamp. The N1 warnings now say light_on will refuse. C2: LD_EnableOutput moves inside light_on's rollback, so an enable that reports failure (and may have taken) is zeroed and disabled. C3: properties.is_on is true from the moment the enable is sent. CHANGELOG: the C entries in 0.2.5's header as a deliberate safety change a rig may notice; the two "### Changed" headings get their own titles (PI N-472, TCube laser). One assertion changed: open-loop light_on's call list now starts with the limit read. Suite: 1813 pass, 3 broken, 0 fail. Not yet run on hardware. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cstrings; drop a redundant is_on The 0.2.5 header now says every 0.2.4 line keeps its meaning except, deliberately, an open-loop rig whose stored current limit is above max_current, which is refused at light_on (including a max_current below about 17.25 mA, or a limit that could not be lowered). lower_open_loop_clamp! and initialize no longer say max_current is enforced in software only as in 0.2.4; they point to light_on's refusal. light_on's second is_on = true after the try is gone (it is set before the enable). No logic change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release 0.2.5 into
main, as one PR (Keith, 2026-09-29).This branch is
mainplus merge commits of three reviewed PRs, then the 0.2.5 version commit:stopmotion.DiodeLaserinterface.The one conflict was CHANGELOG
[Unreleased]→ Fixed; both sides are kept.Project.tomlis0.2.5, with no-DEV, so TagOnMerge tagsv0.2.5once lab/tests covers the merged tree. The CHANGELOG has one[0.2.5]section with the same upgrade warning as 0.2.4.Then 7977efc fixes C1-C4 from Codex's post-merge review of v0.2.4:
light_onreads the current limit stored in the controller fresh before every enable, and refuses if it is abovemax_current. This is a deliberate safety change a rig may notice. Software cannot clear the controller's stored setpoint while the output is off, so between the enable and the setpoint the diode runs on it. The stored limit is the only bound on that interval, and this bounds both the stale pulse and the failed-zero case.properties.is_onis true from the moment the enable is sent.Each has a test on the fake SDK. None of it has yet run on hardware.
Tests: the local suite at 7977efc (kitt, Julia 1.13) is 1813 pass, 3 broken, 0 fail. lab/tests on 7977efc follows as a commit status.
🤖 Generated with Claude Code