Skip to content

feat: add secure connections and FastFrame support - #192

Open
SaiDeepikaKuchibhotla wants to merge 7 commits into
tektronix:mainfrom
SaiDeepikaKuchibhotla:deepika/ff_support
Open

SaiDeepikaKuchibhotla wants to merge 7 commits into
tektronix:mainfrom
SaiDeepikaKuchibhotla:deepika/ff_support

Conversation

@SaiDeepikaKuchibhotla

Copy link
Copy Markdown
Contributor

Proposed changes

This PR adds two related capabilities to TekHSI:

  1. Secure connections

    • Adds support for secure TLS connections.
    • Supports TLS with HTTP Basic authentication.
    • Adds certificate trust configuration and credential-store support.
  2. Fast-Frame acquisitions

    • Adds support for stopped-scope analog and digital multi-frame waveforms.
    • Preserves per-frame timing and frame metadata.
    • Adds FastFrame transfer-timing diagnostics and summary-frame handling.
    • Adds access to individual frames without changing the current frame.

These changes improve secure instrument communication and provide complete Fast-Frame waveform handling for acquisition and analysis.

Addresses #< fill in issue number here >

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • 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)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have followed the guidelines in the CONTRIBUTING document
  • I have signed the CLA
  • I have checked to ensure there aren't other open Pull Requests for the same update/change
  • I have created (or updated) an Issue to track the status of this update/change and updated the link in this PR description (see above in the Proposed changes section) using the wording Addresses #<issue_number>
  • I have performed a self-review of my code
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • Basic linting passes locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have added necessary documentation (if appropriate)
  • I have updated the Changelog with a brief description of my changes

@read-the-docs-community

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

Copy link
Copy Markdown

Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread examples/auth_helpers.py Fixed
return True, None


def discover(url: str) -> tuple[bool, Path | None, dict, bool]:
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
Comment thread src/tekhsi/security.py Fixed
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Test Results (macos)

path passed subtotal
tests/test_auth_basic.py 8 8
tests/test_client.py 79 79
tests/test_credential_store.py 13 13
tests/test_fastframe_native.py 26 26
tests/test_load_timing.py 15 15
tests/test_logging.py 3 3
tests/test_security.py 63 63
tests/test_security_wiring.py 13 13
tests/test_wfm_digital.py 9 9
TOTAL 229 229

Link to workflow run

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Test Results (ubuntu)

path passed subtotal
tests/test_auth_basic.py 8 8
tests/test_client.py 79 79
tests/test_credential_store.py 13 13
tests/test_fastframe_native.py 26 26
tests/test_load_timing.py 15 15
tests/test_logging.py 3 3
tests/test_security.py 63 63
tests/test_security_wiring.py 13 13
tests/test_wfm_digital.py 9 9
TOTAL 229 229

Link to workflow run

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Test Results (windows)

path passed skipped xpassed subtotal
tests\test_auth_basic.py 8 8
tests\test_client.py 74 5 79
tests\test_credential_store.py 12 1 13
tests\test_fastframe_native.py 26 26
tests\test_load_timing.py 15 15
tests\test_logging.py 3 3
tests\test_security.py 63 63
tests\test_security_wiring.py 13 13
tests\test_wfm_digital.py 9 9
TOTAL 223 1 5 229
tests\test_credential_store.py
('D:\\a\\TekHSI\\TekHSI\\tests\\test_credential_store.py', 118, 'Skipped: POSIX chmod only')

Link to workflow run

@nfelt14 nfelt14 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You also need to make sure to run pre-commit run -a ruff-check and pre-commit run -a ruff-format to fix issues, that is why all the checks are failing.

Comment thread docs/macros.py Outdated
Comment thread examples/fastframe_usage.py Outdated
Comment thread tests/manual/test_scope_fastframe_auth.py Outdated
Comment thread tests/test_docs.py
Comment thread .pre-commit-config.yaml Outdated
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
legacy_tox_ini = """
[tox]
requires = tox>4
isolated_build = True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Comment thread pyproject.toml Outdated
Comment thread tests/manual/conftest.py Outdated
Comment thread docs/security.md
Comment thread docs/security.md Outdated
Comment thread docs/security.md Outdated
Comment thread docs/security.md Outdated
Comment thread docs/security.md
```

The first connection can use `on_trust_prompt` to approve the scope certificate. The certificate fingerprint and credentials are then stored for later connections. A changed certificate raises a certificate-mismatch error rather than silently trusting a different endpoint.
The default store locations are:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is the store on Linux .tektronix, but other platforms just tektronix (no dot)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The leading dot follows the Unix convention for hidden application configuration directories. Windows uses %APPDATA%, while macOS uses its standard Application Support directory, so neither needs a dot-prefixed folder.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not put it in the user folder on Windows too?

Comment thread docs/security.md Outdated
Comment thread docs/security.md Outdated
Comment thread docs/security.md Outdated
| ------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- |
| `UNAUTHENTICATED` | Confirm the scope is in the expected security mode and verify the password. |
| Certificate mismatch | Remove the stale stored certificate only after verifying the new certificate, then trust it deliberately. |
| No active channel | Enable the channel on the scope and check `TEKHSI_SCOPE_CHANNEL`. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Manual integration test information should not live in this usage document. This is for package users only, not package developers.

Comment thread docs/basic_usage.md Outdated
Comment thread docs/basic_usage.md
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Breaking API Changes

src/tekhsi/_tek_highspeed_server_pb2.py:185: WaveformHeader.__slots__:
Attribute value was changed:
  Old: ['bitmask', 'chunksize', 'dataid', 'hasdata', 'horizontalUnits', 'horizontalfractionalzeroindex', 'horizontalspacing', 'horizontalzeroindex', 'iq_centerFrequency', 'iq_fftLength', 'iq_rbw', 'iq_span', 'iq_windowType', 'noofsamples', 'pairtype', 'sourcename', 'sourcewidth', 'transid', 'verticaloffset', 'verticalspacing', 'verticalunits', 'wfmtype']
  New: ('sourcename', 'sourcewidth', 'dataid', 'transid', 'horizontalUnits', 'horizontalspacing', 'horizontalzeroindex', 'horizontalfractionalzeroindex', 'noofsamples', 'chunksize', 'wfmtype', 'bitmask', 'pairtype', 'verticalunits', 'verticalspacing', 'verticaloffset', 'iq_centerFrequency', 'iq_fftLength', 'iq_rbw', 'iq_span', 'iq_windowType', 'hasdata', 'num_frames', 'frame_info', 'current_frame_index', 'probe_details', 'channel_sparam')

src/tekhsi/tek_hsi_connect.py:196: TekHSIConnect.d_datatypes:
Attribute value was changed:
  Old: {1: np.int8}
  New: {1: np.int8, 2: np.int16}

Link to workflow run

Comment thread docs/security.md
- [`TekCertificateMismatch`][tekhsi.security.TekCertificateMismatch]
- [`TekSecurityError`][tekhsi.security.TekSecurityError]
- [`FastFrameLoadTiming`][tekhsi.load_timing.WaveformTransferTiming] (the compatibility alias for [`WaveformTransferTiming`][tekhsi.load_timing.WaveformTransferTiming])
- [`FastFrameDigitalWaveform`](https://tm-data-types.readthedocs.io/en/stable/)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use a dot syntax like tekhsi, it resolves due to the imported libraries in the build process.

Comment thread examples/auth_helpers.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This file is just an example of bad code. We shouldn't have an example that imports private members.

Comment thread pyproject.toml
"missing-class-docstring", # caught by ruff
"missing-module-docstring", # caught by ruff
"no-member", # caught by pyright
"locally-disabled", # local suppressions are reviewed at their usage sites

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why was this added?

Comment thread pyproject.toml
"consider-using-with", # TODO: enable this check
"deprecated-typing-alias", # caught by ruff
"duplicate-code",
"fixme", # caught by ruff

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why were these removed?

Comment thread pyproject.toml
output-format = "text" # colorized could be another option

[tool.pyright]
executionEnvironments = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be necessary if the tests are set up properly. The tests shouldn't be manipulating sys.path

Comment thread pyproject.toml
"src/tekhsi/_tek_highspeed_server_pb2*.py*" = [
"ALL",
]
"src/tekhsi/credential_store.py" = ["ALL"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove total file ignore for these filea

Comment thread pyproject.toml
pydantic = "^2.7.4"
pygments = "^2.17.2"
pymdown-extensions = "^12.1.0"
pymdown-extensions = "^11.0.1"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is not reverted

)

global _logger_initialized # noqa: PLW0603
global _logger_initialized # noqa: PLW0603 # pylint: disable=global-statement

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This pylint error can still be ignored globally, since it is caught by ruff

Comment thread tests/manual/conftest.py


@pytest.fixture
def manual_scope_enabled_fixture() -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should just be an auto use fixture, it doesn't need to be used manually. Also, this env var should be removed, just use the presence of the scope ip address to skip.

This branch was successfully deployed

1 active deployment
package-build — 954c112b Deployed Oct 6, 2026 by SaiDeepikaKuchibhotla via package-build / Build package #469
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.

3 participants