feat(dewesoftx): add dewesoftx daq driver - #443
Conversation
|
Claude finished @maxleblang's task in 14m 3s —— View job Code review
OverviewSound fit for Posted 3 inline findings (session-restart anchoring bug that drops the first batch on every Two things not called out inline:
|
Greptile SummaryThe PR adds an experimental Windows-only DewesoftX DAQ driver that attaches through COM, binds synchronous analog channels, tracks storage-session timestamps, and emits Instro measurements.
Confidence Score: 4/5The PR should not merge until ring-buffer wraparound is handled without silently dropping an entire available buffer. The new read and fetch paths treat equal wrapped positions as proof that no samples arrived, although the producer may have advanced by one or more complete buffer lengths. Files Needing Attention: packages/instro-unstable/instro/unstable/daq/drivers/dewesoftx.py Important Files Changed
|
|
|
||
| # Start a storing session | ||
| name = f"run_{datetime.now().strftime('%Y-%m-%d_%H-%M-%S')}" | ||
| self._app.StartStoring(name) |
There was a problem hiding this comment.
start() calls StartStoring() but never re-anchors self._t0_ns (it's only set in open()). Since StartStoreTimeUTC now points at the new session, the next read_analog() hits _resync_session(), sees the anchor changed, logs a restart, and returns DewesoftXData(channels={}) — silently dropping the first batch on every hardware-timed start. Set self._t0_ns from self._app.Data.StartStoreTimeUTC right after StartStoring() returns.
| self._ai_hw_timing_config = HWTimingConfig( | ||
| sample_rate=rate, | ||
| sample_period=round(1e9 / rate), | ||
| samples_per_channel=max(1, rate // 10), |
There was a problem hiding this comment.
rate is self._app.Data.SampleRate, an untyped COM Any, so rate // 10 can be a float (mypy won't catch it since it's Any). HWTimingConfig.samples_per_channel is typed int. InstroDAQ.configure_ai_hw_sample_rate (instro/daq/daq.py:890) wraps the equivalent expression in int(...) — do the same here: max(1, int(rate // 10)).
| target = self._ai_hw_timing_config.samples_per_channel | ||
| rate = self._ai_hw_timing_config.sample_rate | ||
| last_resync = time.monotonic() | ||
| while True: |
There was a problem hiding this comment.
This inner loop never checks the background-daemon stop event. If DewesoftX stops storing externally, _resync_session() (called every 0.5s at line 194) raises RuntimeError on every daemon cycle indefinitely — _background_daemon logs and continues but there's no path back to a healthy state short of daq.stop(). Consider surfacing a clearer terminal condition or having the driver recover once storing resumes.
Summary
This PR implements a DAQ driver to "configure" and stream synchronous data out of a running DewesoftX instance into
instro, essentially treating the DewesoftX instance as a physical DAQ. There are more first class ways to integrate Dewesoft support withinstrobut this is good enough for the time being. Additionally, this driver is pretty universal as it doesn't matter what Dewesoft DAQ is connected as long as its data is going through DewesoftX.Type of change
fix)feat)feat!/fix!)refactor)docs)chore)Verification
This video demonstrates a simulated channels in DewesoftX being streamed to Nominal Core via the
DewesoftXDriverininstroRecording.2026-08-26.154311.mp4
Tests
Early unstable driver development. This needs more iterations before tests and docs.
Checklist
feat(driver): add support for Keysight E36300)