From fd42860fbd930ac600a7a4c02cc5f492257ef417 Mon Sep 17 00:00:00 2001 From: little-did-I-know Date: Mon, 14 Sep 2026 08:58:16 -0700 Subject: [PATCH] fix: accept bare NR3 responses on legacy dialect voltage/timebase/rate 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 --- CHANGELOG.md | 14 ++++++++++++++ docs/about/changelog.md | 14 ++++++++++++++ scpi_control/scpi_commands.py | 8 ++++++-- scpi_control/waveform.py | 8 +++++--- tests/test_scpi_command_tables.py | 2 +- tests/test_waveform_parsing.py | 32 ++++++++++++++++++++++++++++++- 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5497808..084a2ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/about/changelog.md b/docs/about/changelog.md index a5497808..084a2ea3 100644 --- a/docs/about/changelog.md +++ b/docs/about/changelog.md @@ -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 diff --git a/scpi_control/scpi_commands.py b/scpi_control/scpi_commands.py index 1434142d..f658d96c 100644 --- a/scpi_control/scpi_commands.py +++ b/scpi_control/scpi_commands.py @@ -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: diff --git a/scpi_control/waveform.py b/scpi_control/waveform.py index f6a35321..3eac7493 100644 --- a/scpi_control/waveform.py +++ b/scpi_control/waveform.py @@ -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) diff --git a/tests/test_scpi_command_tables.py b/tests/test_scpi_command_tables.py index 5821354f..4a01c72b 100644 --- a/tests/test_scpi_command_tables.py +++ b/tests/test_scpi_command_tables.py @@ -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; diff --git a/tests/test_waveform_parsing.py b/tests/test_waveform_parsing.py index c0ae66ac..dad6005f 100644 --- a/tests/test_waveform_parsing.py +++ b/tests/test_waveform_parsing.py @@ -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", [ @@ -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