Skip to content

Time a binary reply's silence in milliseconds, not whole seconds - #172

Merged
TGoodhew merged 2 commits into
mainfrom
fix/binary-silence-ms
Nov 7, 2026
Merged

TGoodhew merged 2 commits into
mainfrom
fix/binary-silence-ms

Conversation

@TGoodhew

@TGoodhew TGoodhew commented Nov 7, 2026

Copy link
Copy Markdown
Owner

After a binary reply starts, a stall is now timed in milliseconds rather than in whole 1 s slices. Copilot found this in its review of #169, and it was held back from 1.3.1 until it could be tested on hardware.

The problem

InstrumentManager.QueryBinary reads in 1 s slices after the first byte, and counted a stall as timeout_ms / 1000 silent slices, rounded down (minimum one). So timeout_ms = 1500 gave up after one silent second, and timeout_ms = 500 waited a full second.

The fix

Silence is counted in the milliseconds asked for, and the last slice is only what's left of the timeout: 1500 is a 1000 ms slice then a 500 ms one. A driver may round a short slice up to its next step (300 ms waits about 0.29 s and 500 ms about 1 s over a GPIB-ENET/1000), so the wait can run longer than asked, never shorter. Timeouts in whole seconds, including the 54845A's default, send exactly what they did before.

The first read, which waits for the reply to start, is unchanged and stays one read. Splitting it failed every capture on the 54845A and left the scope unresponsive until it was power-cycled.

Tested

  • Unit tests: the slices sent for 1500, 500 and 3000 ms of silence.
  • On a 54845A (A.04.50, GPIB-ENET/1000, HP 37204A extender), read-only:
    • two instrument_capture_screen calls at about 2 s each;
    • visa_query_binary of :DISPlay:DATA? GIF,SCReen with timeout_ms 1500 and 2500, about 2.1 s each;
    • a rejected dump failing in 4.3 s with -108 and -420 read back;
    • then *IDN? in 20 ms, with headers still off and the error queue empty.
  • Not tested on hardware: an instrument going silent part-way through a reply, which can't be caused on demand. The unit tests cover it.

🤖 Generated with Claude Code

TGoodhew and others added 2 commits November 7, 2026 09:48
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After the first byte, a stall was counted as timeout_ms / 1000 silent 1 s
slices, rounded down: timeout_ms = 1500 gave up after one second, and 500
waited a full one. The silence is now counted in the milliseconds asked
for, and the last slice is only what's left of the timeout. The first read
is unchanged: still one read.

Tested on the 54845A: two captures and two visa_query_binary dumps with
timeout_ms 1500 and 2500, each about 2 s; a rejected dump in 4.3 s; then
*IDN? in 20 ms, headers off and the error queue empty.

Found by Copilot in its review of #169.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 7, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the millisecond-based binary-reply stall handling update.

No open issues 🎉

What changed in this PR

Updates binary-reply stall handling to count silence in milliseconds rather than whole-second slices, addressing the follow-up deferred from #169. The initial read remains unchanged.

Changes:

  • Limits each subsequent read to the remaining silence allowance, preserving whole-second timeout behavior.
  • Adds tests for 500, 1500, and 3000 ms silence allowances.
  • Bumps the shared version from 1.3.1 to 1.3.2.
File Description
tests/​GpibMcp.Tests/​InstrumentManagerTests.cs Verifies silent-read timeout slices and request counts.
src/​GpibMcp.Core/​Instruments/​InstrumentManager.cs Tracks silence in milliseconds and shortens the final read slice.
src/​Directory.Build.props Updates the shared version to 1.3.2.

Review effort: Balanced

@TGoodhew
TGoodhew merged commit 3074181 into main Nov 7, 2026
2 checks passed
@TGoodhew
TGoodhew deleted the fix/binary-silence-ms branch November 7, 2026 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants