fix(tsp_control): split load_script() by line when total length exceeds threshold - #581
Closed
bestcal wants to merge 3 commits into
Closed
fix(tsp_control): split load_script() by line when total length exceeds threshold#581bestcal wants to merge 3 commits into
bestcal wants to merge 3 commits into
Conversation
|
|
bestcal
force-pushed
the
fix-tsp-load-script-len
branch
from
May 17, 2026 01:45
03f6138 to
1e50789
Compare
Author
|
pre-commit.ci autofix |
…ds threshold Fixes tektronix#500 The TSP load_script() method previously sent the entire script command (including loadscript/endscript wrapper) in a single write() call. Devices enforce a maximum write length; scripts longer than ~1000 chars would silently fail or error out. This change introduces a hybrid strategy: - If total script command length >= 1000, send it line-by-line: loadscript <name>, each line of the body, then endscript. - Otherwise, keep the original single-write behavior for backward compatibility with simulation/backplane tests. A regression test is added that asserts: - A 45-line long script is split into multiple write() calls - A short script uses a single write() call
bestcal
force-pushed
the
fix-tsp-load-script-len
branch
from
May 17, 2026 01:56
cc98674 to
f52fac7
Compare
Author
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
nfelt14
requested changes
May 18, 2026
| # script_body argument is overwritten by file contents | ||
| script_body = Path(file_path).read_text(encoding="utf-8").strip() | ||
|
|
||
| tsp_max_write_length = 1000 # TSP devices require a write_termination within 1000 chars |
Collaborator
There was a problem hiding this comment.
I would recommend making this a private, class constant. Then subclasses can overwrite it if needed.
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
| "pytest-order>=1.4.0", |
Collaborator
There was a problem hiding this comment.
This shouldn't be necessary, please remove this
- Move tsp_max_write_length to a private class constant _TSP_MAX_WRITE_LENGTH so subclasses can override it if needed. - Remove the unnecessary [dependency-groups] section from pyproject.toml.
ldantek
reviewed
May 20, 2026
| assert written_calls[1] == "loadscript longscript" | ||
| # Every call should be ≤ 1000 characters | ||
| for idx, call in enumerate(written_calls): | ||
| assert len(call) <= 1000, ( |
Contributor
There was a problem hiding this comment.
This should access the private variable from the device driver
ldantek
requested changes
May 20, 2026
Collaborator
|
Closing this PR based on a lack of interaction and updates. |
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
4 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.
Fixes #500
Problem
The
load_script()method inTSPControlpreviously assembled the entireTSP script command—including the
loadscript <name>, body, andendscriptwrapper—into a single string, then sent it via one
write()call.Real TSP devices enforce a maximum write length (commonly 1024 chars). When
the total command exceeds this, the script silently fails or raises a device
error. Issue #500 reports exactly this failure with long user scripts.
Solution
Introduce a hybrid split strategy based on a
TSP_MAX_WRITE_LENGTH = 1000threshold:
len(script_command) >= 1000loadscript name, then each line of the body viaself.write(line), finallyendscriptlen(script_command) < 1000Key implementation details:
device-specific limits and protocol overhead.
(they are idempotent for TSP).
Testing
A new regression test
test_load_script_split_long_linesis added intests/test_smu.pythat asserts:load_script()splits into multiplewrite()calls, each ≤ 1000 chars.print("hello"), ~40 chars assembled):load_script()still uses a singlewrite()call, matching existingsimulator expectations.
Full test run (
pytest tests/test_smu.py -k "test_smu or test_load_script_split_long_lines") passes 5/5:Checklist