Skip to content

fix: accept bare NR3 responses on legacy dialect voltage/timebase/rate queries - #178

Merged
little-did-I-know merged 1 commit into
mainfrom
fix/legacy-dialect-bare-nr3-voltage-scale
Sep 14, 2026
Merged

little-did-I-know merged 1 commit into
mainfrom
fix/legacy-dialect-bare-nr3-voltage-scale

Conversation

@little-did-I-know

Copy link
Copy Markdown
Owner

Summary

  • Real SDS1104X-E hardware returns a bare NR3 value with no unit suffix for C1:VDIV? (e.g. "1.00E+00" instead of the RC01020-E01C-documented "1.00E+00V"), which made get_waveform() raise CommandError: Invalid voltage scale response.
  • The legacy Siglent dialect was deliberately excluded from BARE_NR3_DIALECTS on the assumption it always echoes a unit. This bug report disproves that assumption on real hardware.
  • legacy now joins the same bare-NR3 fallback already used for modern/tektronix/lecroy (scpi_control/scpi_commands.py), so _parse_value_with_units accepts a bare float for C1:VDIV?, C1:OFST?, TDIV?, and SARA? when no unit suffix is present, while the documented unit-suffixed format still parses via the primary check first.
  • Updated the two tests that pinned the old (incorrect) assumption: test_bare_nr3_dialects and test_value_parsing_requires_units (repurposed to verify genuinely unparseable responses still raise).
  • Added a new regression test reproducing the exact bug (bare NR3 on the legacy-dialect mock, matching SDS1104X-E).

Fixes #177

Test plan

  • New test test_legacy_dialect_accepts_bare_nr3_without_units fails against pre-fix code, passes after the fix
  • pytest tests/test_waveform_parsing.py tests/test_scpi_command_tables.py tests/test_mock_dialects.py -v — 116 passed
  • pytest -m "not slow" -n 8 — 3015 passed, 61 skipped, no regressions
  • CHANGELOG.md / docs/about/changelog.md mirror verified in sync

…e queries

A real SDS1104X-E returns "1.00E+00" (no unit suffix) for C1:VDIV?
instead of the RC01020-E01C-documented "1.00E+00V", which made
get_waveform() raise CommandError. Legacy was deliberately excluded
from BARE_NR3_DIALECTS on the assumption it always echoes a unit;
that assumption doesn't hold on real hardware. Legacy now joins the
same bare-NR3 fallback already used for modern/Tektronix/LeCroy,
while the unit-suffixed check still runs first for compliant
responses.

Fixes #177
@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@little-did-I-know
little-did-I-know merged commit c974c10 into main Sep 14, 2026
30 checks passed
@little-did-I-know
little-did-I-know deleted the fix/legacy-dialect-bare-nr3-voltage-scale branch September 14, 2026 16:14
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.

[Bug]: Expected wrong value for SDS1104X scope

1 participant