Visitar URL original
fix(tsp): batch load_script() writes to respect max write length by Abdullah-Builds · Pull Request #623 · tektronix/tm_devices · GitHub
Skip to content

fix(tsp): batch load_script() writes to respect max write length - #623

Open
Abdullah-Builds wants to merge 9 commits into
tektronix:mainfrom
Abdullah-Builds:fix/tsp-load-script-write-chunking
Open

Abdullah-Builds wants to merge 9 commits into
tektronix:mainfrom
Abdullah-Builds:fix/tsp-load-script-write-chunking

Conversation

@Abdullah-Builds

Copy link
Copy Markdown

Proposed changes

TSP devices require a write termination character to be sent at least
once every ~1000 characters, or the instrument's receive buffer
overruns. load_script() currently sends the entire script (the
loadscript header, body, and endscript footer) as a single write,
so any script whose content exceeds that limit before a termination
occurs triggers a communication overrun on the instrument — including
a single long line with no embedded newline (e.g. one long print()
statement), which is the exact case in the reproduction steps below.

This splits the full script text into fixed-size chunks bounded by a
new _MAX_TSP_SCRIPT_WRITE_LENGTH constant (1000, matching the
documented device limit) and sends each chunk as its own write(),
rather than validating and rejecting long lines.

I want to flag up front that #611 already attempted this issue with a
validation approach (raising ValueError on an oversized line), and
@nfelt14 requested changes on it with:

The proper solution is to batch the data in small enough sizes to
send to the device.

This PR is that batching approach instead. Chunking is a pure
character-count split with no attempt to align to line boundaries, so
concatenating the sent chunks always reconstructs the original script
text exactly. Scripts that already fit under the max write length
continue to be sent as a single write, identical to current behavior
— this is a pure addition for the oversized case, not a behavior
change for existing callers.

Addresses #500

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Functionality update (non-breaking change which updates or changes existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • CI/CD update (an update to the CI/CD workflows, scripts, and/or configurations)
  • Documentation update (an update to enhance the user experience when reading through the docs)

Testing

Added two tests to tests/test_smu.py:

  • test_smu_load_script_max_write_length — reproduces the exact bug
    scenario (one line with no newline, over _MAX_TSP_SCRIPT_WRITE_LENGTH
    characters on its own) and asserts no individual write exceeds the
    limit, more than one write occurs, and concatenating the writes
    reproduces the original script text exactly.
  • test_smu_load_script_short_script_single_write — regression check
    that scripts already under the limit are still sent in exactly one
    write, unchanged from current behavior.

Verified locally:

pytest tests/test_smu.py -v      # 6/6 passed
pytest tests/                    # 852 passed; 4 pre-existing failures
                                  # in my sandbox are unrelated (missing
                                  # `ping` binary, confirmed present on
                                  # the unmodified tree too)
ruff check / ruff format --check # clean
pyright                          # no new errors vs. the unmodified file

Checklist

@CLAassistant

CLAassistant commented Aug 16, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@nfelt14

nfelt14 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Please add a proper PR summary/title

@nfelt14

nfelt14 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Please sign your commits in accordance with the contributing guidelines

@Abdullah-Builds Abdullah-Builds changed the title feat:Resolved gh pr edit --title "fix(tsp): batch load_script() writes to respect max write length" Aug 17, 2026
@Abdullah-Builds Abdullah-Builds changed the title gh pr edit --title "fix(tsp): batch load_script() writes to respect max write length" fix(tsp): batch load_script() writes to respect max write length Aug 17, 2026
@Abdullah-Builds
Abdullah-Builds force-pushed the fix/tsp-load-script-write-chunking branch from acdced4 to 1a9d1f7 Compare August 17, 2026 18:50
@Abdullah-Builds

Copy link
Copy Markdown
Author

@nfelt14 waiting for the approvals

@nfelt14
nfelt14 self-requested a review August 24, 2026 22:40
@codecov

codecov Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d55e839) to head (ecfe334).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #623   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          166       166           
  Lines         4866      4873    +7     
  Branches       437       439    +2     
=========================================
+ Hits          4866      4873    +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nfelt14

nfelt14 commented Aug 25, 2026 •

Copy link
Copy Markdown
Collaborator

One of your commits isn't verified as signed

@Abdullah-Builds
Abdullah-Builds force-pushed the fix/tsp-load-script-write-chunking branch from 674f01e to 1319e39 Compare August 25, 2026 17:30
@Abdullah-Builds

Copy link
Copy Markdown
Author

One of your commits isn't verified as signed

done

@Abdullah-Builds

Copy link
Copy Markdown
Author

@nfelt14 waiting for approval

@Abdullah-Builds

Copy link
Copy Markdown
Author

@adityakb waiting for approvals

Comment thread CHANGELOG.md Resolved
@Abdullah-Builds
Abdullah-Builds force-pushed the fix/tsp-load-script-write-chunking branch 2 times, most recently from 1c87b73 to 83863a1 Compare September 1, 2026 17:44
Signed-off-by: Muhammad Abdullah khan <khan.abdullah135790@gmail.com>
@Abdullah-Builds
Abdullah-Builds force-pushed the fix/tsp-load-script-write-chunking branch from 83863a1 to ecfe334 Compare September 1, 2026 17:49
@nfelt14

nfelt14 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

@nfelt14 waiting for approval

I would like one of the TSP experts, @ldantek to look at it.

@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Test Results (ubuntu)

path passed subtotal
tests/test_abstract_device_expect_esr.py 1 1
tests/test_afgs.py 2 2
tests/test_alias_dict.py 1 1
tests/test_all_device_drivers.py 112 112
tests/test_alternative_connection_addresses.py 19 19
tests/test_awgs.py 4 4
tests/test_config_parser.py 50 50
tests/test_device_manager.py 18 18
tests/test_devices_legacy_tsp_ieee_cmds.py 3 3
tests/test_extension_mixin.py 1 1
tests/test_generate_waveform.py 6 6
tests/test_helpers.py 94 94
tests/test_ieee_scpi_cmds.py 1 1
tests/test_logging.py 5 5
tests/test_mainframe.py 13 13
tests/test_margin_testers.py 5 5
tests/test_pi_device.py 1 1
tests/test_psu.py 1 1
tests/test_rest_api_device.py 10 10
tests/test_schema.py 7 7
tests/test_scopes.py 10 10
tests/test_singleton.py 1 1
tests/test_smu.py 6 6
tests/test_ss.py 1 1
tests/test_tm_devices.py 471 471
tests/test_unsupported_device_type.py 1 1
tests/test_verification_functions.py 13 13
TOTAL 857 857

Link to workflow run

@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Test Results (macos)

path passed subtotal
tests/test_abstract_device_expect_esr.py 1 1
tests/test_afgs.py 2 2
tests/test_alias_dict.py 1 1
tests/test_all_device_drivers.py 112 112
tests/test_alternative_connection_addresses.py 19 19
tests/test_awgs.py 4 4
tests/test_config_parser.py 50 50
tests/test_device_manager.py 18 18
tests/test_devices_legacy_tsp_ieee_cmds.py 3 3
tests/test_extension_mixin.py 1 1
tests/test_generate_waveform.py 6 6
tests/test_helpers.py 94 94
tests/test_ieee_scpi_cmds.py 1 1
tests/test_logging.py 5 5
tests/test_mainframe.py 13 13
tests/test_margin_testers.py 5 5
tests/test_pi_device.py 1 1
tests/test_psu.py 1 1
tests/test_rest_api_device.py 10 10
tests/test_schema.py 7 7
tests/test_scopes.py 10 10
tests/test_singleton.py 1 1
tests/test_smu.py 6 6
tests/test_ss.py 1 1
tests/test_tm_devices.py 471 471
tests/test_unsupported_device_type.py 1 1
tests/test_verification_functions.py 13 13
TOTAL 857 857

Link to workflow run

@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Test Results (windows)

path passed subtotal
tests\test_abstract_device_expect_esr.py 1 1
tests\test_afgs.py 2 2
tests\test_alias_dict.py 1 1
tests\test_all_device_drivers.py 112 112
tests\test_alternative_connection_addresses.py 19 19
tests\test_awgs.py 4 4
tests\test_config_parser.py 50 50
tests\test_device_manager.py 18 18
tests\test_devices_legacy_tsp_ieee_cmds.py 3 3
tests\test_extension_mixin.py 1 1
tests\test_generate_waveform.py 6 6
tests\test_helpers.py 94 94
tests\test_ieee_scpi_cmds.py 1 1
tests\test_logging.py 5 5
tests\test_mainframe.py 13 13
tests\test_margin_testers.py 5 5
tests\test_pi_device.py 1 1
tests\test_psu.py 1 1
tests\test_rest_api_device.py 10 10
tests\test_schema.py 7 7
tests\test_scopes.py 10 10
tests\test_singleton.py 1 1
tests\test_smu.py 6 6
tests\test_ss.py 1 1
tests\test_tm_devices.py 471 471
tests\test_unsupported_device_type.py 1 1
tests\test_verification_functions.py 13 13
TOTAL 857 857

Link to workflow run

ldantek
ldantek previously approved these changes Sep 10, 2026
@nfelt14

nfelt14 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

@Abdullah-Builds, are you going to make the changes necessary to get this ready for merge?

@Abdullah-Builds

Copy link
Copy Markdown
Author

I apologize for the delay, as I have been busy over the past few days. I will make the necessary changes as requested by Idantek.

@read-the-docs-community

read-the-docs-community Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

@Abdullah-Builds

Copy link
Copy Markdown
Author

@nfelt14 Moved the changelog entry into the Unreleased "Fixed" section and added an "Added" entry for write_long()

This branch was previously deployed

1 inactive (outdated) deployment
package-build — ecfe334a Deployed Sep 1, 2026 by Abdullah-Builds via package-build / Build package #1917
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.

4 participants