diff --git a/.github/scripts/versions.py b/.github/scripts/versions.py new file mode 100644 index 0000000..c7ce464 --- /dev/null +++ b/.github/scripts/versions.py @@ -0,0 +1,83 @@ +"""Project.toml version rules for CI.yml's version check and TagOnMerge.yml (lab decision 0033). + +Before 1.0 a version is 0.Y.Z. main carries the next version with -DEV (after X.Y.Z it is +X.Y.(Z+1)-DEV); a release is the commit that drops -DEV and is tagged vX.Y.Z. + + python3 versions.py read < Project.toml print the version; exit 1 unless X.Y.Z or X.Y.Z-DEV + python3 versions.py check-pr PR BASE exit 1, with the reason, if PR may not follow BASE + python3 versions.py selftest run the cases below +""" +import re +import sys + +VERSION = re.compile(r"([0-9]+)\.([0-9]+)\.([0-9]+)(-DEV)?") + + +def parse(v): + """(core tuple, is_dev) for X.Y.Z or X.Y.Z-DEV; ValueError for anything else.""" + m = VERSION.fullmatch(v) + if m is None: + raise ValueError(f"invalid version: {v} (expected X.Y.Z or X.Y.Z-DEV)") + return tuple(int(x) for x in m.group(1, 2, 3)), m.group(4) is not None + + +def check_pr(pr, base): + """None if a pull request may move BASE's version to PR, else the reason it may not. + + The release PR's X.Y.Z (or the X.Y.Z a -DEV version leads to) must be new. Against a + released base the core must go up; against a -DEV base it may stay (ordinary work, or the + release that drops -DEV) or go up (a break raising Y), never down. + """ + (pc, _), (bc, bdev) = parse(pr), parse(base) + if bdev and pc < bc: + return f"Project.toml version {pr} is below the base branch's {base}" + if not bdev and pc <= bc: + return f"Project.toml version {pr} must be greater than the base branch's {base}" + return None + + +def selftest(): + reads = {"0.2.4": True, "0.2.5-DEV": True, "1.0.0-DEV": True, + "0.2": False, "0.2.4-dev": False, "0.2.4-rc1": False, "v0.2.4": False} + for v, ok in reads.items(): + try: + parse(v) + got = True + except ValueError: + got = False + assert got == ok, f"parse({v!r}) accepted={got}, expected {ok}" + prs = [ + ("0.2.4", "0.2.3", True), # release on a released base (this repo today) + ("0.2.3", "0.2.3", False), # no bump + ("0.2.2", "0.2.3", False), # down + ("0.2.5-DEV", "0.2.4", True), # main after a release + ("0.2.4-DEV", "0.2.4", False), # -DEV of a released version + ("0.2.5-DEV", "0.2.5-DEV", True), # ordinary work on main + ("0.2.5", "0.2.5-DEV", True), # the release drops -DEV + ("0.3.0-DEV", "0.2.5-DEV", True), # a break raises Y + ("0.2.4", "0.2.5-DEV", False), # below the base + ] + for pr, base, ok in prs: + got = check_pr(pr, base) is None + assert got == ok, f"check_pr({pr!r}, {base!r}) ok={got}, expected {ok}" + print(f"versions.py selftest: {len(reads) + len(prs)} cases pass") + + +if __name__ == "__main__": + cmd = sys.argv[1] if len(sys.argv) > 1 else "" + if cmd == "read": + import tomllib # Python 3.11+, as on ubuntu-latest + + v = tomllib.load(sys.stdin.buffer)["version"] + try: + parse(v) + except ValueError as e: + sys.exit(str(e)) + print(v) + elif cmd == "check-pr" and len(sys.argv) == 4: + reason = check_pr(sys.argv[2], sys.argv[3]) + sys.exit(reason) # None exits 0 + elif cmd == "selftest": + selftest() + else: + sys.exit(__doc__) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 73dba80..195255e 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -93,31 +93,21 @@ jobs: # to update refs/remotes/origin/ for a plain `git fetch # origin `, and `git show origin/:...` below needs it. run: git fetch origin "${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}" --depth=1 - - name: Check Project.toml version is a real, untagged bump + - name: Check Project.toml version against the base branch + # Rules (lab decision 0033) live in .github/scripts/versions.py, + # shared with TagOnMerge.yml; its selftest runs first. run: | - # ubuntu-latest ships Python >=3.11, so tomllib is stdlib. Each - # snippet below is kept on one physical line on purpose: a `run: |` - # block scalar requires every line to carry the step's own - # indentation, and Python does not tolerate an indented first line. - read_version() { - python3 -c 'import re, sys, tomllib; v = tomllib.load(sys.stdin.buffer)["version"]; print(v) if re.fullmatch(r"[0-9]+\.[0-9]+\.[0-9]+", v) else sys.exit(f"invalid version for tagging: {v}")' - } - pr_version=$(read_version < Project.toml) - # The base may still carry a prerelease version (e.g. 1.0.0-DEV before - # the 0.x reset); parse it leniently and only compare against a plain X.Y.Z. - base_version=$(git show "origin/${{ github.base_ref }}:Project.toml" | python3 -c 'import sys, tomllib; print(tomllib.load(sys.stdin.buffer)["version"])') + python3 .github/scripts/versions.py selftest + pr_version=$(python3 .github/scripts/versions.py read < Project.toml) + base_version=$(git show "origin/${{ github.base_ref }}:Project.toml" | python3 .github/scripts/versions.py read) - tag="v${pr_version}" + # A release X.Y.Z, or the X.Y.Z a -DEV version leads to, must not be tagged yet. + tag="v${pr_version%-DEV}" if git rev-parse -q --verify "refs/tags/$tag" >/dev/null; then - echo "bump Project.toml version; $tag is already tagged" + echo "Project.toml is $pr_version but $tag is already tagged; move to the next version" exit 1 fi - - if [[ "$base_version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - python3 -c 'import sys; parse = lambda v: tuple(int(x) for x in v.split(".")); pr, base = sys.argv[1], sys.argv[2]; sys.exit(f"Project.toml version {pr} must be greater than base branch version {base}") if not (parse(pr) > parse(base)) else None' "$pr_version" "$base_version" - else - echo "base branch version $base_version is a prerelease; skipping ordering check (version reset)" - fi + python3 .github/scripts/versions.py check-pr "$pr_version" "$base_version" docs: name: Documentation # The longest job in this workflow by a wide margin, and it deploys diff --git a/.github/workflows/TagOnMerge.yml b/.github/workflows/TagOnMerge.yml index 42c923b..1e4384e 100644 --- a/.github/workflows/TagOnMerge.yml +++ b/.github/workflows/TagOnMerge.yml @@ -5,6 +5,12 @@ on: branches: - main +# This repo's own tag workflow: LidkeLab/.github has no shared tag-on-merge +# (its julia-ci.yml does not tag; checked 2026-09-29). It implements lab +# decisions 0033 (a -DEV version is development, never tagged) and 0009's +# amendment (tag only a tree that a passing lab/tests record covers); keep +# the two in step if a shared one appears. + # No `concurrency` group here on purpose: GitHub keeps at most one pending run # per group and cancels the older one even with cancel-in-progress: false, so a # queued merge could silently lose its tag. Without a group, two merges landing @@ -17,28 +23,36 @@ jobs: permissions: contents: write actions: write # to trigger CI.yml's workflow_dispatch for the new tag + statuses: read # lab/tests records + pull-requests: read # the merged pull request's head commit steps: - uses: actions/checkout@v4 with: fetch-depth: 0 - - name: Tag the merged version if it doesn't exist yet + - name: Tag a release whose tree has a passing lab/tests record id: tag + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} run: | # Local tag refs from checkout can be stale; get the real state of # the remote before deciding whether the tag already exists. git fetch --tags --force - # ubuntu-latest ships Python >=3.11, so tomllib is stdlib. Kept on - # one physical line: a `run: |` block scalar requires every line to - # carry the step's own indentation, and Python does not tolerate an - # indented first line. - version=$(python3 -c 'import re, sys, tomllib; v = tomllib.load(sys.stdin.buffer)["version"]; print(v) if re.fullmatch(r"[0-9]+\.[0-9]+\.[0-9]+", v) else sys.exit(f"invalid version for tagging: {v}")' < Project.toml) + # Version rules live in .github/scripts/versions.py (shared with + # CI.yml's version check). A -DEV version is development: no tag. + version=$(python3 .github/scripts/versions.py read < Project.toml) + if [[ "$version" == *-DEV ]]; then + echo "Project.toml is $version, a development version; nothing to tag." + exit 0 + fi tag="v${version}" + head=$(git rev-parse HEAD) if git rev-parse -q --verify "refs/tags/$tag" >/dev/null; then - if [ "$(git rev-parse "refs/tags/$tag^{commit}")" != "$(git rev-parse HEAD)" ]; then - echo "main has moved past $tag without a version bump; bump Project.toml" + if [ "$(git rev-parse "refs/tags/$tag^{commit}")" != "$head" ]; then + echo "main has moved past $tag without a version change; set Project.toml to the next -DEV version" exit 1 fi # Still emit the tag so a rerun (e.g. after a transient dispatch @@ -48,6 +62,27 @@ jobs: exit 0 fi + # A merge or squash makes a commit the record never saw. It is + # covered when its tree is byte-identical to the tree of a commit + # with a passing lab/tests: this commit itself, or the head of the + # pull request it merged. + tree=$(git rev-parse "HEAD^{tree}") + candidates="$head $(gh api "repos/$REPO/commits/$head/pulls" --jq '.[].head.sha' || true)" + covered="" + for c in $candidates; do + state=$(gh api "repos/$REPO/commits/$c/status" --jq '[.statuses[] | select(.context == "lab/tests")][0].state // ""' || true) + [ "$state" = "success" ] || continue + if [ "$(gh api "repos/$REPO/git/commits/$c" --jq '.tree.sha' || true)" = "$tree" ]; then + covered=$c + break + fi + done + if [ -z "$covered" ]; then + echo "::error::Not tagging $tag: no commit with a passing lab/tests record has this tree ($tree). Checked: $candidates. Run record_tests.jl on $head (on main), then re-run this job." + exit 1 + fi + echo "lab/tests covers $head: its tree equals that of $covered." + git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git tag -a "$tag" -m "Release $tag" diff --git a/.gitignore b/.gitignore index ddcefb2..c55a1b8 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,6 @@ /Manifest.toml /docs/Manifest.toml /docs/build/ -.DS_Store \ No newline at end of file +.DS_Store +# Local run output: test records, logs, handoffs (lab decision 0028) +dev/output/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d4a789..3371a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,85 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project follows Julia's pre-1.0 versioning convention, described in the README's Installation section: in `0.x.y`, `x` is the breaking component -and `y` is the non-breaking one (every merge to `main` is tagged). +and `y` is the non-breaking one (releases are tagged; between them `main` carries +the next version with `-DEV`). ## [Unreleased] +## [0.2.4] - 2026-09-29 + +Safety patch for the TCube laser driver. **v0.2.3 and every earlier tag are +affected.** + +**UPGRADE WARNING: this release applies `setpower` values that were never +applied before.** On v0.2.3 and earlier, `setpower(l, x); light_on(l)` never +delivered `x`: the diode ran on the controller's stored setpoint. From 0.2.4 +it delivers `x`, so a rig whose `setpower` values were never really used (the +usual order, for example MicroscopeSeqSR's 405 nm laser) will run currents it +has never run, checked only against its ceiling, and `max_current` defaults to +160 mA. `light_on` while the output is already on also re-sends +`drive_current`, undoing a lower value set from Kinesis or the front panel. +Before repinning a rig to 0.2.4: +- check every `setpower` value that precedes a `light_on`; +- set `max_current` to the diode's rating; +- set the controller's current-limit potentiometer at or below that rating; +- run a hardware check of the new sequence. + +**Hardware verification: NOT DONE in this repository.** The controller +behaviour below was found on the 642 nm rig (recorded in PR #66); the fix is +exercised against a fake controller that models it +(`test/tcube_output_order.jl`), and those tests fail on v0.2.3. + +### Fixed +- **`light_on(::TCubeLaser)` ran the diode at the controller's stored + setpoint, not the requested current.** The Thorlabs TLD001 ignores + `LD_SetLaserSetPoint` while its output is disabled and, on the next + `LD_EnableOutput`, runs on whatever setpoint it had stored. `setpower` sent + the setpoint whenever it was called and `light_on` only enabled the output, + so the ordinary `setpower(l, x); light_on(l)` ran at the stale value. + Observed on the 642 nm rig's TLD001 (serial 64849775) on 2026-09-28; on + 2026-09-29 a script in that order drove the diode for about 13 s at the + controller's ~160 mA limit, above the diode's absolute maximum. Now + `light_on` re-checks the requested current against the ceiling, enables, + and sends the setpoint immediately; if that setpoint fails it disables the + output again and throws. `light_off` and `shutdown` zero the setpoint + before disabling, so the controller's stored value is 0 and the next enable + starts dark. + - **Rigs pinned to v0.2.3 or earlier:** call `light_on` before `setpower`, + and `setpower(l, 0.0)` before `light_off`, or move to v0.2.4. + - `[limitation]` Between the enable and the setpoint that follows it (one + USB round trip) the controller runs on its stored setpoint: 0 after this + driver's `light_off` or `shutdown`, but anything up to the controller's + current limit if other software (the Kinesis GUI, a session that died) + left it there. In that window the current-limit potentiometer is the only + hardware bound: 160 mA on the 642 nm rig, above that diode's absolute + maximum. + +### Changed +- **`light_on(::TCubeLaser)` sends `drive_current` every time**, including + when the output is already on (see the upgrade warning). +- **`light_on(::TCubeLaser)` before any `setpower` enables the output at + setpoint 0 and warns.** It used to run at whatever the controller had + stored, which is the hazard above. +- **A failed zeroing in `light_off(::TCubeLaser)` logs `@error` instead of + throwing**, because the output is disabled regardless; a failed disable + still throws, as before. +- `setpower(::TCubeLaser, ...)` says when the output is off that the current + will be applied by `light_on`. + +### Changed (release process) +- **Lab decision 0033: `main` carries the next version with `-DEV`.** + `TagOnMerge` skips a `-DEV` version quietly, and CI's version check accepts + `X.Y.Z-DEV` (rules and their selftest in `.github/scripts/versions.py`). +- **`TagOnMerge` tags only a commit whose tree is identical to that of a + commit with a passing `lab/tests` record** (decision 0009's amendment), the + merged commit or its pull request's head; otherwise it fails and says why. +- `test/test_groups.toml` (the whole suite as group Core), `test/lab_summary.jl` + and an ignored `dev/output/`, so admiral's `record_tests.jl` can record this + package. `DAQmx`'s `[sources]` entry is committed in the form `Pkg.test()` + rewrites it to (`rev = "main"`), so a test run leaves the tree clean; + `[sources]` is read only in the root project, so no dependent sees it. + ### Fixed (documentation) - **Depending on this package needs more than pinning the tag, and the docs did not say so.** MicroscopeControl depends on the unregistered `DAQmx.jl` and diff --git a/CLAUDE.md b/CLAUDE.md index ed538b9..fe3ef39 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -164,4 +164,4 @@ Some hardware modules are commented out in `MicroscopeControl.jl` while under de ### Versioning -This package is 0.x and not yet registered; install a pinned tag per the README's Installation Notes. Policy, following Julia's pre-1.0 convention: while the version is `0.x.y`, **`x` is the breaking component and `y` is the non-breaking one** -- `0.2.0 -> 0.3.0` declares a breaking release and `0.2.0 -> 0.2.1` a compatible one, which is also how Julia's `^0.2` compat bound reads them. So bump `x` only when working downstream code can behave differently (a signature, an export, or what a call returns or throws), and bump `y` for everything else, including bug fixes that change behaviour on a path that was already broken. Every merge to `main` is tagged automatically by `.github/workflows/TagOnMerge.yml`. 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. +This package is 0.x and not yet registered; install a pinned tag per the README's Installation Notes. Policy, following Julia's pre-1.0 convention: while the version is `0.x.y`, **`x` is the breaking component and `y` is the non-breaking one** -- `0.2.0 -> 0.3.0` declares a breaking release and `0.2.0 -> 0.2.1` a compatible one, which is also how Julia's `^0.2` compat bound reads them. So bump `x` only when working downstream code can behave differently (a signature, an export, or what a call returns or throws), and bump `y` for everything else, including bug fixes that change behaviour on a path that was already broken. `.github/workflows/TagOnMerge.yml` tags a merge to `main` only when its Project.toml version is a release `X.Y.Z` (a `-DEV` version is development and is skipped, lab decision 0033) and a passing `lab/tests` record covers that commit's tree (decision 0009); an untested tree is not tagged, and the job says why. 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. diff --git a/Project.toml b/Project.toml index 87019fa..1f56ba2 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "MicroscopeControl" uuid = "aa70d9ae-4a1e-49fd-870a-8ccfd99f4c3e" -version = "0.2.3" +version = "0.2.4" authors = ["klidke@unm.edu"] [deps] @@ -23,7 +23,7 @@ Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2" TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76" [sources] -DAQmx = {url = "https://github.com/LidkeLab/DAQmx.jl.git"} +DAQmx = {rev = "main", url = "https://github.com/LidkeLab/DAQmx.jl.git"} [compat] CEnum = "0.5.0" diff --git a/README.md b/README.md index a8376d5..a917137 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ Since this package is under active development and not yet registered, install i ```julia using Pkg -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` **You must also declare this package's unregistered dependency in your own @@ -88,7 +88,7 @@ writes both entries for you: ```julia using Pkg Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` If you write the TOML by hand, `[sources]` alone is **not** enough — Julia @@ -110,7 +110,7 @@ remove the requirement entirely. Advance the pinned tag deliberately when you want a newer release. `Pkg.develop` (tracking `main` directly, no tag) is for contributors working on the package itself, not for rig code that depends on it. -This package follows Julia's pre-1.0 versioning convention: while the version is `0.x.y`, `x` is the breaking component and `y` is the non-breaking one, so `0.2.0 -> 0.3.0` declares a breaking release and `0.2.0 -> 0.2.1` a compatible one. Every merge to `main` is tagged (`.github/workflows/TagOnMerge.yml`). Hardware verification is not tracked here; it is recorded by the downstream rig repo that pins to a given tag. +This package follows Julia's pre-1.0 versioning convention: while the version is `0.x.y`, `x` is the breaking component and `y` is the non-breaking one, so `0.2.0 -> 0.3.0` declares a breaking release and `0.2.0 -> 0.2.1` a compatible one. A release is tagged `vX.Y.Z` when it merges to `main` (`.github/workflows/TagOnMerge.yml`), and only once a recorded test run covers that exact tree; between releases `main` carries the next version with `-DEV` and is not tagged. Hardware verification is not tracked here; it is recorded by the downstream rig repo that pins to a given tag. ## Claude Code skills diff --git a/skills/mc-extend/SKILL.md b/skills/mc-extend/SKILL.md index 98a22d1..98490da 100644 --- a/skills/mc-extend/SKILL.md +++ b/skills/mc-extend/SKILL.md @@ -106,8 +106,9 @@ tag, so the report must say which tag, from the environment: gh issue create --repo LidkeLab/MicroscopeControl.jl --title "PIStage: (v0.2.0, Windows)" --body-file report.md ``` -Upstream tags every merge to `main`, so a merged fix is pinnable the same day; -offer a PR if you have the fix. Two honest workarounds while you wait: +Upstream tags releases, not every merge: between releases `main` carries a +`-DEV` version, so a merged fix is pinnable once the release that carries it is +tested and tagged. Offer a PR if you have the fix. Two honest workarounds while you wait: - **Re-bind the C call under your own name** in your repo, with the corrected signature, and call that from your system code. MC is untouched: @@ -371,7 +372,8 @@ checked) in the rig repo; upstream's CLAUDE.md says it is not tracked in MC. verification line. 5. **Version** (traced from upstream CLAUDE.md): minor bump for an interface change (signature, export or dispatch contract), patch for everything else; - every merge to `main` is tagged. A new driver adds exports and a new + releases are the tagged commits, and `main` carries `X.Y.Z-DEV` between + them. A new driver adds exports and a new interface is an interface change, so expect a minor bump. Pin a tag, never `main`. 6. **Refresh the installed skills** after moving the pin: `install_skills()` diff --git a/skills/mc-extend/references/rig-causes.md b/skills/mc-extend/references/rig-causes.md index aeb0552..23359e2 100644 --- a/skills/mc-extend/references/rig-causes.md +++ b/skills/mc-extend/references/rig-causes.md @@ -16,6 +16,7 @@ and each looks like a broken `ccall`. | `TCubeLaser` does not respond | Kinesis serial number wrong or the device is open in the Kinesis GUI. It is addressed by `serialNo` (Thorlabs Kinesis), plus an `NIdaq` AO channel for modulation. Before **v0.2.3** every Kinesis status code was assigned to an unread `err`, so a failed `LD_Open` looked exactly like a successful one; from v0.2.3 each one throws naming the call and the code. | Match `serialNo` to the Kinesis GUI's device list; close that GUI. | | `setpower(::TCubeLaser, current)` drives more current than asked for, or `TCubeLaser` rejects a small current | **[fixed in v0.2.3]** the range check only logged `@error` and then sent the setpoint anyway, so an out-of-range request reached the diode; and `min_current` defaulted to **60.0 mA**, a lower bound that rejected safe small currents while protecting nothing. From v0.2.3 the check throws an `ArgumentError` before any setpoint is computed, and `min_current` defaults to `0.0`. | On v0.2.3+, `setpower(laser, 500.0)` throws. On an older pinned tag, do the range check in your own system code before calling. | | A `TCubeLaser` rig ceiling is ignored after `initialize` | **[fixed in v0.2.3]** `initialize` overwrote `light.max_current` with the controller's own limit (160-220 mA), so a caller's `max_current=80.0` was gone by the time `setpower` validated against it. From v0.2.3 the controller's value goes to the new `controller_max_current` field and `setpower` enforces the smallest of `max_current`, `controller_max_current` and `max_setcurrent`. | Print `laser.max_current` after `initialize`: on an older tag it equals the controller's limit, not yours. | +| `light_on(::TCubeLaser)` drives more current than `setpower` asked for, up to the controller's limit | **[fixed in v0.2.4]** The TLD001 ignores a setpoint sent while its output is off and, at the next enable, runs on the setpoint it had stored; before v0.2.4 `setpower` followed by `light_on` ran at that stale value (seen on a rig at the ~160 mA limit). From v0.2.4 `light_on` sends the setpoint right after enabling and `light_off`/`shutdown` zero it before disabling. A separate cause with the same symptom: a control source that includes the front-panel potentiometer (`LD_GetControlSource` reading 5) makes the diode follow the knob whatever the setpoint. | On an older tag, call `light_on` before `setpower`, and `setpower(l, 0.0)` before `light_off`. If the current still ignores the setpoint while the output is on, turn the front-panel knob fully down: that is the potentiometer, not the driver. | | `laser.properties.power` on a `TCubeLaser` does not match a power meter | Expected, on every version. `setpower` takes a drive **current in mA**; `properties.power` is `current * max_power / ` under a `"mW"` label, an uncalibrated linear model the driver cannot measure and that the bench table in `TCubeLaserControl.jl` contradicts. The controller reports no optical power in the open-loop mode this driver uses. **[limitation]** the pair is **deprecated from v0.2.3** and scheduled for removal in a future 0.3.0. | From v0.2.3 read `laser.drive_current` (mA, `NaN` before the first accepted `setpower`), which is what the driver acted on, and also written to `export_state`'s attributes. Convert to optical units in your own system code; the bench table and the conditional scaling formula beside it are the only optical data the package has, and both are one rig's assumptions. | | `initialize(::TCubeLaser)` fails once, then every retry fails at `LD_Open` | **[fixed in v0.2.3]** a failure after a successful `LD_Open` — a mode change, a readings request — threw without closing, and a controller left open refuses the next `LD_Open`, so the first failure poisoned the retry. From v0.2.3 the handle is closed on the way out and the original error is the one raised. | On an older pinned tag, call `TCubeLaserControl.LD_Close(laser.serialNo)` before retrying, or power-cycle the cube. | | `setupIO(::TCubeLaser)` throws `BoundsError`, or modulates the wrong analogue output | It hardcoded `devs[2]` and `channelsAO[2]`. **[fixed in v0.2.3]** the defaults are unchanged (a working rig keeps working) but the indices are validated with a message naming what discovery found, and `TCubeLaser(serialNo; daq_device=, ao_channel=)` names them explicitly. | `NIDAQcard.showdevices(NIdaq())` from Julia; pass `daq_device=`/`ao_channel=`. | diff --git a/skills/mc-system-design/SKILL.md b/skills/mc-system-design/SKILL.md index 8eb558d..2e0112b 100644 --- a/skills/mc-system-design/SKILL.md +++ b/skills/mc-system-design/SKILL.md @@ -34,7 +34,7 @@ MicroscopeControl and Pkg writes both entries for you: ```julia Pkg.add(url="https://github.com/LidkeLab/DAQmx.jl.git") -Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.3") +Pkg.add(url="https://github.com/LidkeLab/MicroscopeControl.jl.git", rev="v0.2.4") ``` Writing the TOML by hand needs **both** a `[deps]` and a `[sources]` entry — diff --git a/src/hardware_implementations/tcube_laser/interface_methods.jl b/src/hardware_implementations/tcube_laser/interface_methods.jl index 465f9ca..d5f67ca 100644 --- a/src/hardware_implementations/tcube_laser/interface_methods.jl +++ b/src/hardware_implementations/tcube_laser/interface_methods.jl @@ -261,13 +261,49 @@ end """ light_on(light::TCubeLaser) -Enable the controller's output, then record it. `properties.is_on` is a -*requested* state across this package, but it is at least written after the -call that was supposed to make it true rather than before it, so a failed -enable no longer leaves the field claiming the laser is on. +Enable the controller's output, then send the requested setpoint. + +The controller ignores a setpoint sent while its output is off (see +[`TCubeLaser`](@ref)). So `drive_current` is validated first against the +current ceiling (nothing is sent if that throws), the output is enabled, and +only then is its setpoint code sent. Called while the output is already on, +this re-sends `drive_current`, replacing any lower value set from Kinesis or +the front panel. If no [`setpower`](@ref) has been called, setpoint 0 is sent, with a +warning. If the setpoint fails after the enable, the output is disabled again +and the error rethrown; `properties.is_on` is written only once both steps +have succeeded (or to what the failed disable left the output as). + +`[limitation]` Between the enable and the setpoint the controller runs on its +stored setpoint, bounded in hardware only by its current-limit potentiometer; +see [`TCubeLaser`](@ref). """ function LightSourceInterface.light_on(light::TCubeLaser) - check_err(LD_EnableOutput(light.serialNo), "LD_EnableOutput", light.serialNo) + serialNo = light.serialNo + if isnan(light.drive_current) + code = UInt16(0) + @warn "TCubeLaser $(serialNo): light_on before any setpower; sending setpoint 0, since the controller's stored setpoint cannot be trusted" + else + check_current(light, light.drive_current) + code = setpoint_code(light, light.drive_current) + end + check_err(LD_EnableOutput(serialNo), "LD_EnableOutput", serialNo) + try + check_err(LD_SetLaserSetPoint(serialNo, code), "LD_SetLaserSetPoint", serialNo) + catch + disabled = false + try + disabled = LD_DisableOutput(serialNo) == 0 + catch + end + if disabled + light.properties.is_on = false + @error "TCubeLaser $(serialNo): setpoint after enable failed; output disabled" + else + light.properties.is_on = true + @error "TCubeLaser $(serialNo): setpoint after enable failed and the disable failed; the output may still be ON at the controller's stored setpoint" + end + rethrow() + end light.properties.is_on = true println("$(light.laser_color)" * "_laser is on") return nothing @@ -301,6 +337,13 @@ sub-code request sees its own number, and `export_state` writes that number to the HDF5 attributes. There is no field here that reports what actually went to the wire. +# The controller ignores setpoints while the output is off + +The setpoint is sent on every call, but the controller ignores it while the +output is off (see [`TCubeLaser`](@ref)); it then takes effect only because +[`light_on`](@ref) sends it again after enabling. Sending it anyway covers an +output enabled outside this object. + # What is recorded On success, `light.drive_current` holds the current that was accepted, in mA. @@ -320,17 +363,43 @@ function LightSourceInterface.setpower(light::TCubeLaser, current::Float64) check_err(LD_SetLaserSetPoint(light.serialNo, current_setpoint), "LD_SetLaserSetPoint", light.serialNo) light.drive_current = current light.properties.power = legacy_power(light, current) # deprecated; see legacy_power - println("Laser current set to $current mA") + println("Laser current set to $current mA" * (light.properties.is_on ? "" : " (output off: applied at light_on)")) return nothing end +""" + zero_setpoint(light::TCubeLaser) + +Send setpoint 0 while the output is still on, so the controller does not keep +the last current for its next enable. Never throws: returns `nothing` on +success, otherwise the failure (a status code or an exception) for the caller +to report. +""" +function zero_setpoint(light::TCubeLaser) + try + status = LD_SetLaserSetPoint(light.serialNo, UInt16(0)) + return status == 0 ? nothing : "status $status" + catch err + return err + end +end + """ light_off(light::TCubeLaser) -Disable the controller's output, then record it. See [`light_on`](@ref) on the -ordering. +Zero the controller's setpoint, then disable its output, then record it. + +The controller keeps its setpoint across a disable (see [`TCubeLaser`](@ref)), +so the setpoint is zeroed while the output is still on: the next enable starts +dark. A failed zero is logged, before the disable, and does not throw, since +the output is still turned off; a failed disable throws and leaves +`properties.is_on` unchanged. +`drive_current` is not changed: it is the request the next [`light_on`](@ref) +re-sends. """ function LightSourceInterface.light_off(light::TCubeLaser) + zeroed = zero_setpoint(light) + isnothing(zeroed) || @error "TCubeLaser $(light.serialNo): zeroing the setpoint before disable failed ($zeroed); the controller's stored setpoint was not cleared" check_err(LD_DisableOutput(light.serialNo), "LD_DisableOutput", light.serialNo) light.properties.is_on = false println("$(light.laser_color)" * "_laser is off") @@ -340,12 +409,15 @@ end """ shutdown(light::TCubeLaser) -Disable the output and close the connection. A failed disable throws, but the -connection is closed either way: leaving the Kinesis handle open would also +Zero the setpoint, disable the output and close the connection. The zero +comes first, as in [`light_off`](@ref) (see [`TCubeLaser`](@ref)). A failed zero is logged; a failed disable throws, but +the connection is closed either way: leaving the Kinesis handle open would also block the reconnection a caller needs in order to retry the disable. """ function shutdown(light::TCubeLaser) serialNo = light.serialNo + zeroed = zero_setpoint(light) + isnothing(zeroed) || @error "TCubeLaser $serialNo: zeroing the setpoint before disable failed ($zeroed); the controller's stored setpoint was not cleared" try check_err(LD_DisableOutput(serialNo), "LD_DisableOutput", serialNo) light.properties.is_on = false diff --git a/src/hardware_implementations/tcube_laser/types.jl b/src/hardware_implementations/tcube_laser/types.jl index fc1e9ab..e03519c 100644 --- a/src/hardware_implementations/tcube_laser/types.jl +++ b/src/hardware_implementations/tcube_laser/types.jl @@ -55,6 +55,21 @@ number the driver actually acted on. `setpower` validates against the *smallest* of `min_current`'s counterparts: `max_current`, `controller_max_current` (ignored while `NaN`) and `max_setcurrent`. See [`check_current`](@ref). + +# The setpoint only takes while the output is on + +The Thorlabs TLD001 ignores `LD_SetLaserSetPoint` while its output is disabled +and, on the next `LD_EnableOutput`, runs on whatever setpoint it had stored +(observed on the 642 nm rig's TLD001 64849775, 2026-09-28). So a `setpower` +value reaches the diode only because `light_on` sends it again right after +enabling, and `light_off` and `shutdown` zero the setpoint before disabling, +so the next enable starts dark. + +`[limitation]` Between the enable and the setpoint that follows it (one USB +round trip) the controller runs on its stored setpoint: 0 after this driver's +`light_off` or `shutdown`, but anything up to the controller's current limit if +other software left it there. In that window the current-limit potentiometer +is the only hardware bound; set it at or below the diode's rating. """ mutable struct TCubeLaser <: LightSource unique_id::String diff --git a/test/lab_summary.jl b/test/lab_summary.jl new file mode 100644 index 0000000..6805b40 --- /dev/null +++ b/test/lab_summary.jl @@ -0,0 +1,40 @@ +# The lab test record (admiral decision 0009). `record_tests.jl` runs `Pkg.test()` with +# LAB_TEST_SUMMARY= and reads the result from that file, one entry per test group of +# test/test_groups.toml. Until this package adopts the lab's group layout (decision 0008), +# the whole suite is the one group Core, and this wrapper is the only record-specific code. + +using Test, TOML + +""" + lab_summary(f, group) + +Run `f()`, the package's top-level `@testset`, and when LAB_TEST_SUMMARY is set write its +counts there as `group`. A failing suite still writes its summary and then rethrows, so +`Pkg.test()` fails exactly as it did without the wrapper. +""" +function lab_summary(f, group::AbstractString) + t0 = time() + counts, failure = try + ts = f() + c = Test.get_test_counts(ts) + (pass = c.passes + c.cumulative_passes, fail = 0, error = 0, + broken = c.broken + c.cumulative_broken), nothing + catch e + e isa Test.TestSetException || rethrow() + (pass = e.pass, fail = e.fail, error = e.error, broken = e.broken), e + end + path = get(ENV, "LAB_TEST_SUMMARY", "") + if !isempty(path) + result = Dict{String, Any}( + "ran" => true, "passed" => counts.fail + counts.error == 0, "pass" => counts.pass, + "fail" => counts.fail, "error" => counts.error, "broken" => counts.broken, + "seconds" => round(time() - t0; digits = 1)) + open(path, "w") do io + TOML.print(io, Dict("julia" => string(VERSION), + "host" => first(split(gethostname(), '.')), + "groups" => Dict(group => result)); sorted = true) + end + end + failure === nothing || throw(failure) + return nothing +end diff --git a/test/runtests.jl b/test/runtests.jl index 5faf0e5..15b7ec5 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -8,6 +8,10 @@ const HDF5 = MicroscopeControl.HDF5 # be included at top level, before the testsets. See the file for the seam. include("tcube_fake_sdk.jl") +# Writes the lab test record summary when LAB_TEST_SUMMARY is set; see the file. +include("lab_summary.jl") + +lab_summary("Core") do @testset "MicroscopeControl.jl" begin @testset "Simulated Camera" begin cam = SimCamera(exposure_time=0.01) @@ -377,7 +381,7 @@ include("tcube_fake_sdk.jl") laser = TCubeLaser("00000000") laser.properties.is_on = true shutdown(laser) - @test FakeKinesis.calls == ["LD_DisableOutput", "LD_Close"] + @test FakeKinesis.calls == ["LD_SetLaserSetPoint", "LD_DisableOutput", "LD_Close"] @test laser.properties.is_on == false # A failed disable throws -- and the handle is closed anyway, @@ -395,7 +399,7 @@ include("tcube_fake_sdk.jl") end @test err isa ErrorException @test occursin("LD_DisableOutput", err.msg) - @test FakeKinesis.calls == ["LD_DisableOutput", "LD_Close"] # closed regardless + @test FakeKinesis.calls == ["LD_SetLaserSetPoint", "LD_DisableOutput", "LD_Close"] # closed regardless # The disable failed, so the output is not recorded as off: the # field follows the call, not the request. @test stuck.properties.is_on == true @@ -626,6 +630,8 @@ include("tcube_fake_sdk.jl") @test_logs export_state(laser, nothing) @test TCube.EXPORT_STATE_2ARG_WARNED[] end + + include("tcube_output_order.jl") end @testset "NIdaq digital output" begin @@ -706,3 +712,4 @@ include("tcube_fake_sdk.jl") include("skills.jl") include("gui.jl") end +end # lab_summary diff --git a/test/tcube_fake_sdk.jl b/test/tcube_fake_sdk.jl index a784876..f7767f6 100644 --- a/test/tcube_fake_sdk.jl +++ b/test/tcube_fake_sdk.jl @@ -81,12 +81,27 @@ driver's default scale. """ const diode_limit_raw = Ref{Int}(23830) +"Whether the fake controller's output is currently enabled." +const output_on = Ref{Bool}(false) + +""" +The controller's stored setpoint. Like the real TLD001 it changes only when +`LD_SetLaserSetPoint` succeeds while the output is on. +""" +const stored = Ref{Int}(0) + +"The stored setpoint at each successful `LD_EnableOutput`, i.e. what it ran on." +const enable_log = Int[] + "Forget the recorded history and restore the default responses." -function reset!(; limit_raw::Integer=23830) +function reset!(; limit_raw::Integer=23830, stored::Integer=0) empty!(calls) empty!(status) empty!(setpoints) empty!(throws) + empty!(enable_log) + output_on[] = false + FakeKinesis.stored[] = stored diode_limit_raw[] = limit_raw return nothing end @@ -119,11 +134,24 @@ end LD_RequestReadings(serialNo) = Main.FakeKinesis.record!("LD_RequestReadings") LD_RequestLaserDiodeMaxCurrentLimit(serialNo) = Main.FakeKinesis.record!("LD_RequestLaserDiodeMaxCurrentLimit") - LD_EnableOutput(serialNo) = Main.FakeKinesis.record!("LD_EnableOutput") - LD_DisableOutput(serialNo) = Main.FakeKinesis.record!("LD_DisableOutput") + function LD_EnableOutput(serialNo) + st = Main.FakeKinesis.record!("LD_EnableOutput") + if st == 0 + Main.FakeKinesis.output_on[] = true + push!(Main.FakeKinesis.enable_log, Main.FakeKinesis.stored[]) + end + return st + end + function LD_DisableOutput(serialNo) + st = Main.FakeKinesis.record!("LD_DisableOutput") + st == 0 && (Main.FakeKinesis.output_on[] = false) + return st + end function LD_SetLaserSetPoint(serialNo, laserDiodeCurrent) push!(Main.FakeKinesis.setpoints, laserDiodeCurrent) - return Main.FakeKinesis.record!("LD_SetLaserSetPoint") + st = Main.FakeKinesis.record!("LD_SetLaserSetPoint") + st == 0 && Main.FakeKinesis.output_on[] && (Main.FakeKinesis.stored[] = Int(laserDiodeCurrent)) + return st end function LD_GetLaserDiodeMaxCurrentLimit(serialNo) Main.FakeKinesis.record!("LD_GetLaserDiodeMaxCurrentLimit") diff --git a/test/tcube_output_order.jl b/test/tcube_output_order.jl new file mode 100644 index 0000000..6718ce2 --- /dev/null +++ b/test/tcube_output_order.jl @@ -0,0 +1,167 @@ +# The TLD001 ignores LD_SetLaserSetPoint while its output is disabled and runs +# on its stored setpoint at the next enable (642 nm rig, 2026-09-28). These +# cases pin the order the driver sends things in, against the fake controller +# in `tcube_fake_sdk.jl`, which models that behaviour. +using Test +using MicroscopeControl +using MicroscopeControl.HardwareImplementations.TCubeLaserControl + +@testset "TCube setpoint follows the output (0.2.4)" begin + TCube = MicroscopeControl.HardwareImplementations.TCubeLaserControl + FK = Main.FakeKinesis + + # A fresh, initialized laser on a controller holding a stale + # above-full-scale setpoint word, as found on the rig. + function fresh() + FK.reset!(stored = 32767) + l = TCubeLaser("00000000") + redirect_stdout(devnull) do + initialize(l) + end + empty!(FK.calls) + empty!(FK.setpoints) + return l + end + quiet(f) = redirect_stdout(f, devnull) + last_index(op) = findlast(==(op), FK.calls) + first_index(op) = findfirst(==(op), FK.calls) + + @testset "a: setpower with output off, then light_on" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + end + @test FK.output_on[] + @test FK.stored[] == TCube.setpoint_code(l, 10.0) + @test last_index("LD_SetLaserSetPoint") > first_index("LD_EnableOutput") + @test FK.enable_log == [32767] # it ran on the stale word only until the setpoint + end + + @testset "b: light_off zeroes before disable; next light_on starts dark" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + empty!(FK.calls); empty!(FK.setpoints) + light_off(l) + end + @test FK.setpoints == [0x0000] + @test first_index("LD_SetLaserSetPoint") < first_index("LD_DisableOutput") + @test FK.stored[] == 0 + @test !FK.output_on[] + @test l.drive_current == 10.0 + quiet() do + light_on(l) + end + @test last(FK.enable_log) == 0 + @test FK.stored[] == TCube.setpoint_code(l, 10.0) + end + + @testset "c: shutdown zeroes before disable" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + empty!(FK.calls) + shutdown(l) + end + @test first_index("LD_SetLaserSetPoint") < first_index("LD_DisableOutput") + @test FK.stored[] == 0 + @test "LD_Close" in FK.calls + end + + @testset "d: light_on without setpower sends 0 and warns" begin + l = fresh() + @test_logs (:warn, r"setpoint 0") match_mode = :any quiet() do + light_on(l) + end + @test FK.stored[] == 0 + end + + @testset "e: light_on above the ceiling never enables" begin + l = fresh() + l.drive_current = TCube.effective_max_current(l) + 1 + @test_throws ArgumentError quiet() do + light_on(l) + end + @test !("LD_EnableOutput" in FK.calls) + end + + @testset "f: setpoint failing after enable disables the output" begin + l = fresh() + quiet() do + setpower(l, 10.0) + end + FK.fail!("LD_SetLaserSetPoint") + @test_logs (:error, r"output disabled") match_mode = :any begin + @test_throws ErrorException quiet() do + light_on(l) + end + end + @test last_index("LD_DisableOutput") > first_index("LD_EnableOutput") + @test !FK.output_on[] + @test !l.properties.is_on + end + + @testset "g: failed zero in light_off is logged, output still off" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + end + FK.fail!("LD_SetLaserSetPoint") + @test_logs (:error, r"zeroing the setpoint") match_mode = :any quiet() do + light_off(l) + end + @test !FK.output_on[] + @test !l.properties.is_on + end + + @testset "h: failed disable throws and is_on stays true" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + end + FK.fail!("LD_DisableOutput") + @test_throws ErrorException quiet() do + light_off(l) + end + @test l.properties.is_on + end + + @testset "i: setpoint and then disable both failing leaves is_on true ($how)" for how in (:status, :throw) + l = fresh() + quiet() do + setpower(l, 10.0) + end + FK.fail!("LD_SetLaserSetPoint") + how === :status ? FK.fail!("LD_DisableOutput") : FK.throw!("LD_DisableOutput") + @test_logs (:error, r"may still be ON") match_mode = :any begin + @test_throws ErrorException quiet() do + light_on(l) + end + end + @test FK.output_on[] + @test l.properties.is_on + end + + @testset "j: a failed zero is logged even when the disable then throws" begin + l = fresh() + quiet() do + setpower(l, 10.0) + light_on(l) + end + FK.fail!("LD_SetLaserSetPoint") + FK.fail!("LD_DisableOutput") + @test_logs (:error, r"stored setpoint was not cleared") match_mode = :any begin + @test_throws ErrorException quiet() do + light_off(l) + end + end + @test l.properties.is_on + end + + FK.reset!() +end diff --git a/test/test_groups.toml b/test/test_groups.toml new file mode 100644 index 0000000..ab76819 --- /dev/null +++ b/test/test_groups.toml @@ -0,0 +1,6 @@ +# Test groups (admiral decision 0008). Minimal form, so record_tests.jl (decision 0009) can +# record this package: the whole current suite is Core. Adopting the full 0008 layout +# (group folders, the lab runner) waits for the package-standard pass. +# Core test/*.jl, included from runtests.jl plain Pkg.test() + +[Core]