Skip to content

Commit c6cccec

Browse files
Ambient Code Botclaude
andcommitted
Address review comments on sigrok driver PR
- Skip empty lines in CSV parser to prevent strict-zip failures - Use pytest.approx() for float comparison in tests - Fix stderr pipe deadlock in capture_stream by inheriting stderr - Fix channel selection to handle device names and empty channel maps - Handle multi-line VCD value changes and $dumpvars blocks - Fix source_archive URL to use correct repo name - Add missing config: wrapper in README exporter YAML example Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent aa1ab2d commit c6cccec

6 files changed

Lines changed: 68 additions & 26 deletions

File tree

python/packages/jumpstarter-driver-sigrok/README.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ pip3 install --extra-index-url https://pkg.jumpstarter.dev/simple/ jumpstarter-d
1818
export:
1919
sigrok:
2020
type: jumpstarter_driver_sigrok.driver.Sigrok
21-
driver: fx2lafw # sigrok driver (demo, fx2lafw, rigol-ds, etc.)
22-
conn: auto # optional: USB VID.PID, serial path, or "auto" for auto-detect
23-
channels: # optional: map device channels to friendly names
24-
D0: clk
25-
D1: mosi
26-
D2: miso
27-
D3: cs
21+
config:
22+
driver: fx2lafw # sigrok driver (demo, fx2lafw, rigol-ds, etc.)
23+
conn: auto # optional: USB VID.PID, serial path, or "auto" for auto-detect
24+
channels: # optional: map device channels to friendly names
25+
D0: clk
26+
D1: mosi
27+
D2: miso
28+
D3: cs
2829
```
2930
3031
### Configuration Parameters

python/packages/jumpstarter-driver-sigrok/jumpstarter_driver_sigrok/csv.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ def _extract_csv_data_lines(lines: list[str]) -> list[str]:
7070

7171
for _i, line in enumerate(lines):
7272
line = line.strip()
73+
if not line:
74+
continue
7375
# Skip comment lines
7476
if line.startswith(";"):
7577
continue

python/packages/jumpstarter-driver-sigrok/jumpstarter_driver_sigrok/driver.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ async def capture_stream(self, config: CaptureConfig | dict):
108108
process = await asyncio.create_subprocess_exec(
109109
*cmd,
110110
stdout=asyncio.subprocess.PIPE,
111-
stderr=asyncio.subprocess.PIPE,
111+
stderr=None,
112112
)
113113

114114
try:
@@ -167,18 +167,19 @@ def _channel_args(self, selected_names: list[str] | None) -> list[str]:
167167
Returns:
168168
List of args like ["-C", "D0=vcc,D1=cs,D2=miso"]
169169
"""
170+
if selected_names:
171+
resolved = [self._resolve_channel(name) for name in selected_names]
172+
if self.channels:
173+
channel_map = ",".join(f"{dev}={self.channels.get(dev, dev)}" for dev in resolved)
174+
else:
175+
channel_map = ",".join(resolved)
176+
return ["-C", channel_map] if channel_map else []
177+
170178
if not self.channels:
171179
return []
172180

173-
# Filter channels if specific names requested
174-
if selected_names:
175-
selected_lower = {name.lower() for name in selected_names}
176-
filtered = {dev: user for dev, user in self.channels.items() if user.lower() in selected_lower}
177-
else:
178-
filtered = self.channels
179-
180181
# Build channel map: device_name=user_name
181-
channel_map = ",".join(f"{dev}={user}" for dev, user in filtered.items())
182+
channel_map = ",".join(f"{dev}={user}" for dev, user in self.channels.items())
182183
return ["-C", channel_map] if channel_map else []
183184

184185
def _config_args(self, cfg: CaptureConfig, *, continuous: bool = False) -> list[str]:
@@ -274,7 +275,10 @@ def _resolve_channel(self, name_or_dn: str) -> str:
274275
"""
275276
candidate = name_or_dn.strip()
276277

277-
# If already a device channel name, return as-is
278+
if not self.channels:
279+
return candidate
280+
281+
# If already a device channel name (key in channel map), return as-is
278282
if candidate in self.channels:
279283
return candidate
280284

@@ -283,4 +287,8 @@ def _resolve_channel(self, name_or_dn: str) -> str:
283287
if user_name.lower() == candidate.lower():
284288
return dev_name
285289

290+
# Accept device-style names (e.g., "D0", "A1") even if not in channel map
291+
if candidate[:1].isalpha() and candidate[1:].isdigit():
292+
return candidate
293+
286294
raise ValueError(f"Channel '{name_or_dn}' not found in channel map {self.channels}")

python/packages/jumpstarter-driver-sigrok/jumpstarter_driver_sigrok/driver_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ def test_decode_csv_format(demo_client):
302302
assert isinstance(sample.values, dict)
303303

304304
# Verify timing progresses (1/100kHz = 0.00001s per sample)
305-
assert sample.time == sample.sample * 0.00001
305+
assert sample.time == pytest.approx(sample.sample * 0.00001, rel=1e-6, abs=1e-12)
306306

307307
# Verify values are present
308308
assert len(sample.values) > 0

python/packages/jumpstarter-driver-sigrok/jumpstarter_driver_sigrok/vcd.py

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,52 @@ def parse_vcd(data: bytes, sample_rate: str) -> Iterator[dict]:
4444
if line == "$enddefinitions $end":
4545
break
4646

47-
# Parse and yield value changes one by one
47+
# Parse value changes across timestamp blocks (supports multi-line changes)
4848
sample_idx = 0
49+
current_time_s: float | None = None
50+
current_values: dict[str, int | float] = {}
4951

5052
for line in lines:
5153
line = line.strip()
52-
if not line or line.startswith("$"):
54+
if not line:
55+
continue
56+
if line.startswith("$"):
57+
# Handle $dumpvars block: parse value changes until $end
58+
if line.startswith("$dumpvars"):
59+
if current_time_s is None:
60+
current_time_s = 0.0
61+
# If $dumpvars and $end are on the same line, skip
62+
if "$end" not in line or line == "$dumpvars":
63+
continue
5364
continue
5465

55-
# Timestamp line (e.g., "#100 1! 0" 1#")
5666
if line.startswith("#"):
57-
sample_data = _parse_vcd_timestamp_line(line, timescale_multiplier, channel_map)
58-
if sample_data is not None:
59-
sample_data["sample"] = sample_idx
60-
yield sample_data
67+
# Flush previous timestamp block
68+
if current_time_s is not None and current_values:
69+
yield {"sample": sample_idx, "time": current_time_s, "values": current_values}
6170
sample_idx += 1
71+
current_values = {}
72+
73+
parts = line.split(maxsplit=1)
74+
time_str = parts[0][1:]
75+
if time_str:
76+
current_time_s = int(time_str) * timescale_multiplier
77+
else:
78+
current_time_s = 0.0
79+
80+
# Inline values on the same line (if present)
81+
if len(parts) > 1:
82+
_parse_vcd_value_changes(parts[1], channel_map, current_values)
83+
continue
84+
85+
# Value change line (may appear after # or inside $dumpvars)
86+
if current_time_s is None:
87+
current_time_s = 0.0
88+
_parse_vcd_value_changes(line, channel_map, current_values)
89+
90+
# Flush final block
91+
if current_time_s is not None and current_values:
92+
yield {"sample": sample_idx, "time": current_time_s, "values": current_values}
6293

6394

6495
def _parse_timescale(line: str) -> float:

python/packages/jumpstarter-driver-sigrok/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ raw-options = { 'root' = '../../../'}
1818

1919
[tool.hatch.metadata.hooks.vcs.urls]
2020
Homepage = "https://jumpstarter.dev"
21-
source_archive = "https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip"
21+
source_archive = "https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip"
2222

2323
[tool.pytest.ini_options]
2424
addopts = "--cov --cov-report=html --cov-report=xml"

0 commit comments

Comments
 (0)