Skip to content

Add generator controls and improve capture Stop/error handling - #136

Open
victorstoica114 wants to merge 13 commits into
sigrokproject:masterfrom
victorstoica114:dla32-acquisition-controls
Open

victorstoica114 wants to merge 13 commits into
sigrokproject:masterfrom
victorstoica114:dla32-acquisition-controls

Conversation

@victorstoica114

@victorstoica114 victorstoica114 commented Oct 14, 2026 •

Copy link
Copy Markdown

Continuous acquisition can currently be overwritten by a stale finite sample
count, generator capabilities lack suitable controls, and Stop joins the
acquisition thread from the GUI. This adds capability-based generator controls,
preserves continuous mode's zero sample limit and requests Stop asynchronously.

Configuration errors are visible. Proposed libsigrok capture outcomes produce
an incomplete-capture warning, survive save/reload, and require explicit
Scan/reselection after device loss. Received partial data remains available.

Ready for review. Capture-status handling depends on the API proposed in
sigrokproject/libsigrok#311, including names/key allocation.
It will need adjustment if that interface changes.

Changes

Commits separate Windows dynamic dependencies, orderly logger shutdown,
MinGW segment-mutex lifetime, generator properties and acquisition handling.
The mutex dispatch remains C++11-compatible. Independent fixes can be split
if maintainers prefer.

Review follow-ups handle a restored input wrapper with no core device, and
preserve an existing capture's outcome/error when a new start is rejected for
having no enabled channels. Old samples can no longer be resaved after losing
their incomplete status merely because a restart failed validation.

A separate focused commit fixes the pre-existing exporter success path:
failed or cancelled writes keep the capture unsaved. Success is emitted only
after all requested samples, END and file close complete without errors.
Stream opening/writing failures are handled as export errors, and temporary
sample buffers use RAII when an output operation throws.

Portable controls and trigger setup follow-up

Session setup files now preserve the session trigger: name, stage order,
physical channel indices and types, condition types and threshold values.
Restoration validates every stage before replacing the live trigger. Explicit
no-trigger settings clear it; legacy setups without trigger settings leave it
alone. Unsupported or malformed settings retain the previous trigger and emit
a diagnostic. A session notification synchronizes existing trace markers and
popup selections, including after restore. This is generic PulseView behavior.

Current Windows/offline validation: build and all 18 Boost cases pass (11 prior
cases plus seven new real-QSettings/libsigrok-demo cases). New cases cover
D0/D7/D15/D31, multi-stage and analog threshold round trips, explicit no trigger,
legacy files, malformed data, unsupported conditions and channel enable state.
A real-object offscreen GUI probe selects rising D0/falling D31 through popup
actions, saves/reopens/restores the full setup, and verifies backend conditions
and popup checks. Removing the setup trigger-save call reproduces the reported
failure as a negative control. No physical device scans or captures occur.
Linux setup restoration on this exact follow-up still needs external retesting.

The external tester has separately confirmed that the earlier automatic rate
refresh fix works on Linux. Companion core checks now total 107 plus C++ cleanup.

Discovery and channel-limit validation (23 October)

Channel enable changes now refresh samplerate and sample-depth choices. Bulk changes are coalesced before querying limits; clamped values are committed to the backend. All-off states hide timing choices, failed rate-list queries hide stale values, and old channel subscriptions are disconnected when selecting another device. Refresh avoids writes during capture or when reconnection is required and respects read-only samplerate capabilities. A separate build commit quotes Windows resource compiler and include paths for directories containing spaces.

Current offline validation (23 October): Windows x64 / GCC 16.2 / Qt 5.15.19 build and existing CTest suite pass, with protocol decoders disabled and shared companion libsigrok/C++ libraries. An offscreen probe links real production GUI objects and a demo device with channel-dependent configuration lists: 8/9/16/17/32 boundaries, backend rate/depth clamps, all-off recovery, bulk coalescing, failed-query recovery and device replacement pass. Disconnecting channel notifications reproduces the stale 250 MS/s list as a negative control. The probe uses public APIs and performs no physical device scans. Companion core: 106 C checks and C++ cleanup pass. Linux hardware validation remains pending; earlier physical qualification below was not repeated for this follow-up.

Earlier Windows validation (15 October)

  • Windows x64 / Qt 5 / dynamic dependencies: build passed with the companion
    libsigrok/C++ bindings.
  • Eleven bounded offline core/GUI cases pass using actual production GUI
    objects and the corrected libraries, offscreen Qt and isolated settings.
    They cover C++ cleanup success/failure/retry, restored unavailable input
    format, rejected restart with preserved samples/outcome, archive resave,
    locked-destination failure, deterministic cancellation, stream open/write
    failures and successful archive/binary controls. Failed/cancelled cases
    emit no success and keep the dirty flag. Successful controls emit success
    once; archives pass CRC, exact-sample and outcome checks.
  • Companion make check: 87 C checks and the added public-API C++ cleanup
    regression program pass. Eight simulated-I/O worker probes pass.
  • Ten fresh real captures with the corrected core pass Buffer/Stream,
    high/sparse masks, unaligned depth, trigger Stop and following captures,
    including raw conversion, lifecycle, archive and stimulus checks.

Earlier physical qualification used PulseView df3b92d with libsigrok
fc588e7: continuous all32 Stream, Buffer filling and quiet trigger wait
were interrupted by actual USB removal. Each showed the incomplete warning,
retired the worker, recorded Windows absence and recovered after explicit
Scan in the same GUI process. Both GUI processes exited normally. These
physical-removal cases were not repeated after the review corrections.

The earlier interrupted Stream archive preserved all 78,839,808 received
samples (315,359,232 logic bytes in 76 chunks), with ZIP CRC/count and
disconnected/error metadata checks. EXP-001 was not reproduced there; its
original archive-write failure/cause remains unresolved. The exporter fix
above addresses false success/dirty-state handling, not that unknown cause.

No waveform correction/filtering is added. Arbitrary-duration lossless
all32 Stream at 50 MS/s remains unqualified. No private captures, journals,
correspondence or vendor binaries are included.

@victorstoica114
victorstoica114 marked this pull request as ready for review October 14, 2026 16:45
Copilot AI balanced review requested due to automatic review settings October 14, 2026 16:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ljalves ljalves left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this PR #115 supposed to do the same as this 1f80ef9??

@victorstoica114

Copy link
Copy Markdown
Author

@ljalves

Is this PR #115 supposed to do the same as this 1f80ef9??

Yes, they address the same underlying stale sample-rate list, and that part of the changes overlaps. Thanks for pointing out the earlier PR.

#115 connects the Channels popup's closed() signal to on_config_changed(). Commit 1f80ef9 instead listens to channel enabled_changed() notifications, so updates do not have to wait for the popup to close and also cover changes made outside that popup. It coalesces bulk changes on a zero-interval timer.

The commit also includes explicit handling for rate/sample-depth limits and clamped selections, all-channels-off states, failed rate-list queries and removal of old subscriptions when the device changes. Refresh is guarded while capturing or when reconnection is required. Those are the additional cases covered by our regression probe; I have not tested #115 against that same probe.

I would keep one refresh mechanism. If #115 is merged first, I can rebase this part of #136 onto it and retain only the additional behavior that is still needed. The remaining generator, acquisition/error handling, export and trigger-setup changes in #136 are independent of this overlap.

Argument parsing runs before Context::create(), so calling the non-static
Context::set_log_level() through the empty shared_ptr is undefined behavior.
Set the global level through sr_log_loglevel_set().

Validated the exact PR source with complete Linux and Windows builds,
18 regression cases per platform and ten startup cases per platform (help,
version, levels 0..5, invalid 6 and repeated levels). The original main fails
null sanitizer checks for levels 0 and 4; the corrected main passes, with the
actual global level checked before sr_init on Linux.

This correction is confined to parsing/global logging before Context creation;
its acceptance checks do not require electrical acquisition.
@victorstoica114

Copy link
Copy Markdown
Author

Published Linux fork follow-up: 62cf4c1 adds sampled OR/HIGH/LOW pulse-width conditions to Stream windows and fixes the hidden Software trigger/Pre toolbar actions. It also integrates the reviewed startup/decoder lifetime corrections and analog idle-compression guards. This is on fork master; this PR's narrower branch remains at 9ffd2de, with decoder ownership reviewed separately in #137.

The final frontend source passes all 77 Qt5 regressions on Linux in VMware. Native GUI captures at 5 and 50 MS/s each save/reopen three frames of 200000 samples; all six frames match independent all32 RAW transposition and SDK replay, with the trigger on the expected D0 closing edge. The shown-toolbar regression covers mode toggles and device/file transitions; the old implementation fails its negative control.

Validation scope and limits: Linux is verified for these cases; Windows of the exact combined tree remains deferred. RAW conversion equality and the bounded D0/D22 witnesses do not certify arbitrary-channel continuity. The separate high-rate Stream discontinuities and USB handover issue remain open. No transport, firmware or segment-memory workaround is included in this frontend publication.

@victorstoica114

Copy link
Copy Markdown
Author

Storage allocation follow-up: 7670c6d is now on fork master. Controlled failures reproduced loss of the final stored-chunk pointer during compaction, deletion of still-owned storage during bulk append, and a leaked successor when deque insertion failed in either append path. The fix assigns the existing deque entry directly and keeps successor arrays locally owned until insertion succeeds. The optional standalone regression covers 31 allocation fault cases, preserving accepted data and allocation ownership; the original paths fail their negative controls.

The final Linux source passes 77 frontend regressions and six real DLA event windows at 5/50 MS/s (66 MB stored), with independent all32 RAW transposition, archive/Session replay, numbered D22 UART checked by two decoders and the bounded D0 timing witness. The installed GUI also saves/reloads a 60 MB collection twice without changing data, source origins or trigger metadata. Scope and standalone test instructions.

This changes host storage ownership, preserving the existing chunk sizes, memory reserve, compaction timing and sample-count commit points. It does not promote early segment compaction or change transport/firmware. Fault injection is isolated to the test process; real system-wide memory exhaustion and Windows of this exact tree remain unqualified. This PR's branch remains separate from fork master; the Stream continuity and USB handover limits remain open.

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.

3 participants