Repository navigation
Add visa_read to collect a pending VISA response without writing - #9
Merged
Merged
Conversation
Every VISA read was tied to the write that triggered it, so a long acquisition had to run inside a single visa_query whose timeout spanned the whole sweep. visa_read reads whatever is waiting in the output buffer and sends nothing: write the sweep, then read it later with a timeout sized to the acquisition. The timeout is reset from scratch on every call (ARCHITECTURE §8.2), since an acquisition-sized override is the normal case for this tool and must not leak into later queries. An empty buffer follows the locked read-style contract (§5.2): success=True, raw=None, timed_out=True, so the agent can tell "not ready yet" from a broken connection. Other VISA I/O errors still fail. No CLI subcommand: each CLI invocation opens its own session, so it could never see a response left by an earlier `lablink visa write`. serial_read and ssh_read_stream already follow this; the rule is now written down in agent_development.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
What changed
This adds a
visa_readMCP tool. It reads whatever is already waiting in an instrument's output buffer and sends nothing. Before this, every VISA read was tied to the write that triggered it. A 30-second sweep therefore had to run inside onevisa_query, with a 30-second timeout held open for the whole acquisition. Now youvisa_writethe sweep, do other work, andvisa_readwith a timeout sized to the acquisition.lablink/interfaces/visa/driver.pyvisa_read_impl(alias, timeout_ms=None)sits next tovisa_query_implandvisa_write_impland follows their structure: the same session lookup, the same_no_session_resultpath, the sameReadResultshape, and an event-log entry on every return path.session.raw.timeoutfrom scratch on every call (ARCHITECTURE §8.2). A long override is the normal case for this tool, so a 40 s read can't leak its timeout into the next fast query.visa_write(alias, "READ?")→visa_read(alias, timeout_ms=40000)sequence. It also says what an empty buffer does. One warning goes beyond the task: don't send the instrument another command between the write and the read, because on most instruments a new query discards the pending response. An agent doing other work in the meantime would hit this.tests/interfaces/test_visa.pygets aTestReadclass covering:timeout_msafter an overridetests/test_demo_bench.pyadds an end-to-end test against the real SCPI simulator over TCP using the real driver. It writesREAD?, reads back all 8 channels, then checks that a second read with nothing pending returnstimed_out=True, raw=None.README.md:visa_readadded to the protocol table and the per-driver tool reference.CHANGELOG.md: entry under[Unreleased]→ Added.docs/agent_docs/agent_development.md: one new CLI rule (see below).docs/ARCHITECTURE.mdis unchanged. It doesn't list the VISA driver's operations; it only mentionsvisa_queryas an example.The full suite passes (
.venv/bin/python -m pytest -q: 393 passed). It also passed before the change.Where I departed from the task
timeout_msand then errors." But ARCHITECTURE §5.2 marks the read-style result contract as locked: a timeout with no data issuccess=True, raw=None, timed_out=True, andsuccess=Falseis reserved for a broken connection. That split is what lets an agent tell "not ready yet, call again" from "something is wrong," and "not ready yet" is exactly what happens when a sweep runs longer than expected.serial_readandssh_read_streambehave the same way. Sovisa_readreturnstimed_out=True, raw=Noneon a VISA timeout, and still returnssuccess=Falsefor any other VISA I/O error. The task's main point still holds: nothing pending gives a timeout, never an empty string, and the docstring says so. If you really wantsuccess=Falsehere, it's a one-line change, but it would break §5.2 and should come with an edit to that section.lablink visa readCLI subcommand. Each CLI invocation opens and closes its own session, andconnectsends*IDN?first. A CLI read can therefore never see a response left by an earlierlablink visa write: over a TCP socket the new connection has its own buffer, and on USB/GPIB the*IDN?discards the pending response. The command would always time out, which would mislead users. There's precedent:serial_readandssh_read_streamhave no CLI commands either. I wrote this down as a general rule inagent_development.mdso the next read-style tool doesn't hit the same question.Notes
visa_readnow appears in that row. It has only been checked against the simulator and mocks, not real hardware.Co-Authored-By: Claude Opus 5, while the session's default trailer is Opus 5.5. I used your wording, since your instructions take precedence.🤖 Generated with Claude Code
🤖 Generated with Claude Code