Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
little-did-I-know merged 1 commit into from
Sep 14, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`synthesize_combined()` and `stream()` needed no changes -- both already
dispatch through `synthesize()` per component/chunk.

### Fixed

- **`get_waveform()` raised `CommandError: Invalid voltage scale response`
on a real SDS1104X-E.** The legacy Siglent dialect's documented format for
`C1:VDIV?`/`C1:OFST?`/`TDIV?`/`SARA?` is unit-suffixed (RC01020-E01C
p.139/142), so `legacy` was deliberately excluded from
`BARE_NR3_DIALECTS`. A real SDS1104X-E has now been observed returning a
bare NR3 value with no unit suffix at all for `C1:VDIV?` (e.g. `"1.00E+00"`
instead of `"1.00E+00V"`) (GitHub issue #177). `legacy` is now included in
`BARE_NR3_DIALECTS` alongside `modern`/`tektronix`/`lecroy`, so
`_parse_value_with_units` falls back to parsing a bare float for legacy too
when the primary unit-suffix check doesn't match, while the documented
unit-suffixed format still parses via that primary check first.

## [7.3.0] - 2026-08-30

### Added
Expand Down
14 changes: 14 additions & 0 deletions docs/about/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`synthesize_combined()` and `stream()` needed no changes -- both already
dispatch through `synthesize()` per component/chunk.

### Fixed

- **`get_waveform()` raised `CommandError: Invalid voltage scale response`
on a real SDS1104X-E.** The legacy Siglent dialect's documented format for
`C1:VDIV?`/`C1:OFST?`/`TDIV?`/`SARA?` is unit-suffixed (RC01020-E01C
p.139/142), so `legacy` was deliberately excluded from
`BARE_NR3_DIALECTS`. A real SDS1104X-E has now been observed returning a
bare NR3 value with no unit suffix at all for `C1:VDIV?` (e.g. `"1.00E+00"`
instead of `"1.00E+00V"`) (GitHub issue #177). `legacy` is now included in
`BARE_NR3_DIALECTS` alongside `modern`/`tektronix`/`lecroy`, so
`_parse_value_with_units` falls back to parsing a bare float for legacy too
when the primary unit-suffix check doesn't match, while the documented
unit-suffixed format still parses via that primary check first.

## [7.3.0] - 2026-08-30

### Added
Expand Down
8 changes: 6 additions & 2 deletions scpi_control/scpi_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,12 @@ def __repr__(self) -> str:
# Dialects whose numeric queries return a bare NR3 value with no unit suffix.
# LeCroy joins because CHDR OFF also suppresses the trailing unit token on
# LeCroy responses -- e.g. C1:VDIV? returns "200E-3", not "200E-3 V" (MAUI
# p.7-46) -- unlike Siglent legacy, which keeps the unit.
BARE_NR3_DIALECTS = frozenset({"modern", "tektronix", "lecroy"})
# p.7-46). Legacy Siglent's documented format is unit-suffixed (RC01020-E01C
# p.139/142) and the primary unit-suffix check in _parse_value_with_units
# still tries that first, but real SDS1104X-E hardware has been observed
# returning a bare NR3 with no unit for C1:VDIV? (GitHub issue #177), so
# legacy now also gets the bare-NR3 fallback like the other three dialects.
BARE_NR3_DIALECTS = frozenset({"modern", "tektronix", "lecroy", "legacy"})


def is_flat_trigger(dialect: str) -> bool:
Expand Down
8 changes: 5 additions & 3 deletions scpi_control/waveform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,11 @@ def _strip_prefix(value: str) -> str:
# all: modern Siglent (:CHANnel:SCALe?, :CHANnel:OFFSet?,
# :TIMebase:SCALe?, :ACQuire:SRATe? -- guide pp.46,56,58,476),
# Tektronix (HEADer OFF strips the echo, leaving a bare NR3), and
# LeCroy (CHDR OFF). Legacy Siglent always echoes a unit and keeps
# the strict check above (audit-pinned, see
# test_value_parsing_requires_units).
# LeCroy (CHDR OFF). Legacy Siglent's manual-documented format
# normally carries the unit and gets caught by the primary check
# above first, but real SDS1104X-E hardware has been observed
# returning a bare NR3 with no unit for C1:VDIV? (GitHub issue
# #177), so legacy is now included in this fallback too.
if self._dialect in BARE_NR3_DIALECTS:
try:
value = float(cleaned)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_scpi_command_tables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def test_flat_trigger_dialects(self):

def test_bare_nr3_dialects(self):
assert "modern" in BARE_NR3_DIALECTS
assert "legacy" not in BARE_NR3_DIALECTS
assert "legacy" in BARE_NR3_DIALECTS

def test_valid_public_token_unsupported_on_dialect_raises(self):
# STOP is the :TRIGger:STOP command on modern, not a trigger-mode token;
Expand Down
32 changes: 31 additions & 1 deletion tests/test_waveform_parsing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,34 @@ def test_voltage_scale_parsing_accepts_units(response):
assert waveform._get_voltage_scale("C1") == pytest.approx(2.0)


@pytest.mark.parametrize(
"query,expected",
[
("C1:VDIV?", 1.0),
("C1:OFST?", 1.0),
("TDIV?", 1.0),
("SARA?", 1.0),
],
)
def test_legacy_dialect_accepts_bare_nr3_without_units(query, expected):
# Real SDS1104X-E hardware (legacy dialect) has been observed returning a
# bare NR3 numeric value with no unit suffix at all for these queries
# (GitHub issue #177), contradicting the documented unit-suffixed format.
# The mock's default dialect is legacy, modeling an SDS1104X-E.
connection = MockConnection(custom_responses={query: "1.00E+00"})
with Oscilloscope("mock", connection=connection) as scope:
waveform = scope.waveform

parser = {
"C1:VDIV?": lambda: waveform._get_voltage_scale("C1"),
"C1:OFST?": lambda: waveform._get_voltage_offset("C1"),
"TDIV?": waveform._get_timebase,
"SARA?": waveform._get_sample_rate,
}[query]

assert parser() == pytest.approx(expected)


@pytest.mark.parametrize(
"query,expected_exception",
[
Expand All @@ -68,7 +96,9 @@ def test_voltage_scale_parsing_accepts_units(response):
],
)
def test_value_parsing_requires_units(query, expected_exception):
connection = MockConnection(custom_responses={query: "1.00E+00"})
# A genuinely unparseable response (neither unit-suffixed nor a valid
# bare float) must still raise, even for bare-NR3-tolerant dialects.
connection = MockConnection(custom_responses={query: "ERROR"})
with Oscilloscope("mock", connection=connection) as scope:
waveform = scope.waveform

Expand Down