[AliKNS]

The three Triggerscope fixes from v0.3.0-rc4 (#83), as a standalone, non-breaking PR. PR #90 noted that this work was still owed. Left out: dac_output, since a Triggerscope modulator is planned separately. If your local branch already covers this, take what is useful and close this one.

What it fixes

• The serial timeout runs out. LibSerialPort's read/write timeout is a budget that every read spends. Set once at openport, the 10 s budget was spent after a few thousand commands, so a command failed and the port was reset, losing that DAC write.
  • On quickbeam at 400 Hz (2026-10-15), that was about 1 command in 2,500–5,000, the LCAs included; after the fix, 0 in 10,000.
  • writecommand now re-arms both timeouts before every command, so rwtimeout is the limit of one command.
• A per-scope lock. scope.lock (a ReentrantLock) is held by every command function across its write and its reply read, so tasks sharing one scope cannot take each other's replies (for example two LCC1620 attenuators on different DACs). The positional constructor without the lock still works.
• Out-of-range DAC voltages throw. volttooutput, and so setdac, progdac, progwave and the DAC output setter, throws ArgumentError for a non-finite or out-of-range voltage before anything is sent. Before, the code was sent anyway.
  • New exported dac_range_volts(range).
  • A failed command's error now names the port and the cause.

Tests

• The serial calls go through four one-line wrappers (_arm_timeouts!, _flush_port!, _send!, _receive!). test/triggerscope.jl replaces them, plus openport/closeport, with a recorder, the same seam as tcube_fake_sdk.jl, so no test can open a real port.
• The fake models the spent budget: each reply read costs 1 ms of it. Running the file alone: 41 pass.
• Fault check: with the re-arm removed, the timeout testset fails, as do the port-reset, concurrency and lock testsets that come after it.
• Concurrency: two threads, 40 commands each. Every write is followed at once by the same task's reply.
• Full suite (Windows 11, Julia 1.13, Pkg.test()): 3292 pass, 3 broken, 1 fail + 1 error. The failure is skills.jl's symlink testset, which on Windows needs administrator rights to create a symlink (EPERM). The same environmental failure appears on the companion SLM branch, which has no Triggerscope changes; it is not related to this PR.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[kalidke]

Thanks. The diagnosis is right, and LibSerialPort's own code confirms it: unsafe_read subtracts each read's elapsed time from read_timeout_ms, so a timeout set once at openport is a budget that runs out. Re-arming it per command fixes that. The lock now covers every write/reply pair in the package: direct_control.jl, sequence_control.jl, and through them interface_methods.jl, the GUI and LCC1620.

Three changes before merge, plus one small pre-existing bug in a function this PR already touches:

1. The tests reach the host's serial layer, so CI fails on Linux. newscope() calls the keyword constructor, which calls SerialPort(portname) and so sp_get_port_by_name. Windows accepts the made-up name; Linux returns SP_ERR_ARG. That is the six errors in CI, and the same six on kitt (Julia 1.13). Route the constructor's SerialPort(portname) through a one-line wrapper like the other four and replace it in test/triggerscope.jl, so no test touches the OS serial layer on any platform. After that fix, this PR's CI run is the Linux check.

2. The DAC code is computed outside the lock. setdac, progdac and progwave call volttooutput before lock(scope.lock), and setrange updates scope.dacranges outside it too. Suppose one task encodes 5 V on a 0–5 V channel (code 65535) and, while it waits for the lock, another task sets that channel to 0–10 V. The first task then sends 65535, and the board puts out 10 V. A Julia-only probe reproduced this sequence: RANGE1,2 followed by DAC1,65535. Move the range lookup, the encoding, the command string and the range-cache update into the same locked transaction.

3. The range check rejects values that used to work. lo <= voltage <= hi throws for nextfloat(5.0) on 0–5 V, which used to round to the valid code 65535. Any code that computes a voltage landing a hair past the end of the range now throws, which is a behavior change on a working path (decision 0033). Refuse only non-finite values and voltages whose rounded code falls outside 0–65535. Every voltage that produced a valid code before still does, and only codes that were already invalid are refused.

4. While you're there: progttl writes into scope.ttloutputs, which is the TTL channel count (an Int). So progttl(scope, 1, 1, true) throws a MethodError after sending the command and before reading its reply. It should update scope.ttlvalues, after the reply is read.

Left for a follow-up, since this behavior is older than this PR:

• cached dacranges, dacvalues and trigmode are updated before the board replies;
• replies are never checked as acknowledgements;
• openport/closeport run outside the lock, so a concurrent shutdown can close the port between a write and its reply;
• initialize warns and returns when the acknowledgement fails.

For when it's on hardware (not part of the merge decision):

• endpoint, midpoint and negative voltages on all five ranges;
• a sustained 400 Hz run past 10,000 commands, with two devices sharing one scope.

Please put the fixes in one push.

[AliKNS]

Thanks for the review. All four are in one push (ce3cadc):

1. No test touches the OS serial layer. The constructor's SerialPort(portname) now goes through a fifth one-line wrapper, _new_port, beside the other four. test/triggerscope.jl replaces it with a recorder that returns a never-opened SerialPort on a NULL port, so sp_get_port_by_name is never called. libserialport's sp_free_port returns at once on NULL, so the finalizer is harmless. A new testset checks that the constructor records :new_port and nothing else, and that the port is NULL and closed. This PR's CI run is the Linux check.

2. One locked transaction. setdac, progdac and progwave now look up the range, encode, build the command, write and read the reply all under scope.lock. setrange updates dacranges inside the lock too. So do the interface's DAC output and range setters, which also write dacvalues and the GUI caches (the lock is reentrant). A new test replays your sequence: a task waits for the lock to setdac(1, 5.0) on 0–5 V, and the holder sends RANGE1,2. The waiting task now sends DAC1,32768, not 65535. The same check covers progdac, and another checks that setrange's cache update waits for the lock. With the old placement put back, these fail 4 times.

3. Only invalid codes are refused. volttooutput now throws only for a non-finite voltage, or one whose rounded code falls outside 0–65535. Every voltage keeps the code it gave before: the tests compare all five ranges against the old formulas at 101 points each. nextfloat(5.0) and 5.00003 V on 0–5 V give 65535, 5.00004 V (65535.52) throws, and prevfloat(0.0) and −0.00003 V give 0.

4. progttl stores the value in scope.ttlvalues, after the reply is read. Tests: progttl(scope, 1, 3, true) returns the reply and sets ttlvalues[3], ttloutputs stays 16, and a failed command leaves the value unchanged.

The CHANGELOG entries are updated to match.

Tests: test/triggerscope.jl passes 76 of 76, up from 41. The full suite on Windows (Julia 1.13): 3327 pass, 3 broken, and the only failure is the skills.jl symlink testset, which needs Administrator rights on Windows (unrelated, as before).

The follow-ups you listed (caches updated before the reply, replies not checked as acknowledgements, openport/closeport outside the lock, initialize returning on a failed acknowledgement) and the hardware checks are not in this push. I'll run the hardware checks on the rig. One note for them: since today, DAC 3 and DAC 4 on that scope are cabled to the two 642 nm T-Cubes' MOD IN inputs, so the checks will use a spare DAC from 5 to 16.