From 91d646926e792f17ab58c84b05124960596d9dd4 Mon Sep 17 00:00:00 2001 From: "Charles J. Cliffe" <247927+cjcliffe@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:35:55 -0400 Subject: [PATCH] more review+ and fix sessions --- AGENT-LOG.md | 103 ++++++++++++++++++++++++++++ docs/design/audio-subsystem.md | 6 +- docs/design/bookmark-system.md | 10 +-- docs/design/configuration-system.md | 6 +- docs/design/modem-system.md | 2 +- docs/design/sdr-device-layer.md | 12 ++-- docs/design/signal-flow.md | 8 ++- docs/design/threading.md | 10 +-- docs/design/visual-data-pipeline.md | 8 +-- docs/design/visual-rendering.md | 17 ++--- 10 files changed, 142 insertions(+), 40 deletions(-) diff --git a/AGENT-LOG.md b/AGENT-LOG.md index fb900ab..a475cd1 100644 --- a/AGENT-LOG.md +++ b/AGENT-LOG.md @@ -821,3 +821,106 @@ content). No changes were made to any design document. | File | Reason | |------|--------| | `docs/design/visual-architecture.md` | Split into visual-rendering.md and visual-data-pipeline.md | + +## Session 62: Design Doc Verification Review + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +- Cross-verified all 10 `docs/design/` files against source (queues, IOThread, threading lifecycle, modems, visual pipeline/rendering, config, bookmarks, SDR device layer) +- Confirmed the docs are largely accurate; queue capacities, thread bugs, DataTree serialization, and modem registration all checked out +- Corrected three accuracy issues: setter-call claims, TuningContext font thresholds, and SpectrumCanvas right-click peak behavior + +### Files Modified + +| File | Action | +|------|--------| +| `docs/design/sdr-device-layer.md` | Clarified `name`/`driver`/`hardware` are set during enumeration (only `serial`/`tuner`/`manufacturer`/`product` setters are unused) | +| `docs/design/visual-rendering.md` | Fixed `DrawTuner` font-size thresholds (height ≥28px leaves width-based size unchanged); corrected right-click to "reset peak-hold accumulator" instead of "toggle" | +| `docs/design/modem-system.md` | Noted `ModemFMStereo` factory key is `"FMS"` while UI name is "FM Stereo" | + +## Session 63: Design Doc Commentary Cleanup + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +- Re-reviewed all `docs/design/` files for leftover reverse-engineering commentary (statements speculating on earlier designs, historical code, or irrelevant alternatives rather than describing current behavior) +- Removed six such statements: "earlier designs intended..." (sdr-device-layer), "was `glFlush()`" (visual-rendering), "not a documentation error" meta-note (bookmark), "rather than a fourth `ScopeMode` enum value" (visual-data-pipeline), editorial class-name note (bookmark), and "likely unintentional" speculation (bookmark) +- Verified remaining "not" statements are legitimate behavior descriptions (real bugs, unused parameters) + +### Files Modified + +| File | Action | +|------|--------| +| `docs/design/sdr-device-layer.md` | Dropped "earlier designs intended..." speculation about device ID contents | +| `docs/design/visual-rendering.md` | Simplified `EndDraw()` note (removed history of `glFlush()`) | +| `docs/design/visual-data-pipeline.md` | Removed "rather than a fourth ScopeMode enum value" aside | +| `docs/design/bookmark-system.md` | Removed meta-note "not a documentation error", editorial class-name note, and "likely unintentional" speculation | + +## Session 64: Design Doc Full Accuracy Verification + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +Cross-verified all 10 `docs/design/` files against source in full detail (queue wiring/capacities, ReBuffer pool, modem rates/inheritance, Datathreads/kits, DataTree serialization, DeviceConfig keys, bookmark recovery, SDREnumerator state, VisualProcessor `isOutputEmpty`/double-EMA ordering, GLPanel/GLFont/themes, canvas OnIdle-vs-OnPaint). Confirmed the docs are accurate overall; the two subtlest claims (`isOutputEmpty` all-outputs semantics and the reversed double-EMA update order) were verified correct. Applied six fixes — three real inaccuracies plus three minor precision notes. + +### Files Modified + +| File | Action | +|------|--------| +| `docs/design/threading.md` | Moved macOS `pthread_create`/2MB-stack attribution to `DemodulatorInstance::run()` (was falsely in the demod `.cpp` files); qualified the 50ms-heartbeat claim (SDRThread/SDR enumerator/RigThread don't use a timed pop); added the commented-out main wx `SCHED_RR` block | +| `docs/design/visual-data-pipeline.md` | Fixed scope spectrum "1024 points" → FFT size 1024 producing 512 output points (`fftSize/2`); clarified `FFT_DISTRIBUTOR_BUFFER_IN_SECONDS` sizes the buffer (rate limiting is `linesPerSecond`) | +| `docs/design/audio-subsystem.md` | Noted the sample-rate mismatch check runs only on the initial pop, not on the mid-mixing exhaustion pop | + +## Session 65: Design Doc Full Accuracy Verification (Round 2) + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +- Re-verified all 10 `docs/design/` files against source via parallel exploration + spot-checks; confirmed all queue capacities, ReBuffer mechanics, thread lifecycle/priorities/exit codes, macOS `pthread_join` bug, modem hierarchies/rates, audio mixing/WAV details, GLPanel/GLFont/theme claims, FFT/scope constants, config keys/defaults, bookmark bands/recovery, and SDR device layer +- Found one factual error (stream args) and three minor precision issues; all corrected +- Double-checked each finding directly against source before editing (confirmed `SDRDevicesDialog::OnUseSelected` populates stream args; confirmed `ImagePanel` is a plain `wxPanel`, not an `InteractiveCanvas` subclass) + +### Files Modified + +| File | Action | +|------|--------| +| `docs/design/sdr-device-layer.md` | Corrected "CubicSDR does not populate specific stream argument keys" — stream args ARE populated from `getStreamArgsInfo(SOAPY_SDR_RX, 0)` in `SDRDevicesDialog::OnUseSelected`, persisted via `setStreamOpts`, and flowed through `CubicSDR::setStreamArgs()` into `SDRThread::setStreamArgs()` | +| `docs/design/threading.md` | Clarified `stopRig()` also joins and deletes the rig thread + `RigThread` objects (not just `isTerminated`) | +| `docs/design/signal-flow.md` | Noted `pipeAudioVisualData`/`pipeDemodIQVisualData` are created/wired only under `CUBICSDR_ENABLE_VIEW_SCOPE`/`CUBICSDR_ENABLE_VIEW_DEMOD` | +| `docs/design/visual-rendering.md` | Corrected keyboard-reset claim: `freqMove` stops by velocity decay (not hard-reset on key-up), and UP/DOWN key-up re-arms `mouseZoom` | + +## Session 66: Design Doc Accuracy Verification (Round 3) + Fixes + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +- Re-verified all 10 `docs/design/` files against source via parallel exploration agents plus direct spot-checks; confirmed the docs are accurate overall +- Found and applied 5 substantive corrections plus several minor precision fixes; every impactful claim was re-checked directly in source before editing + +### Files Modified + +| File | Action | +|------|--------| +| `docs/design/visual-rendering.md` | Removed false `glEnable(GL_LINE_SMOOTH)`/`glLineWidth` "demod edge" attribution (not in PrimaryGLContext; `glLineWidth` only in `DrawRangeSelector`); corrected arrow-key summary (no "full bandwidth" case; Shift = 10x); added note that `DrawFreqBwInfo` is drawn by SpectrumCanvas, not WaterfallCanvas | +| `docs/design/visual-data-pipeline.md` | Noted `VisualDataDistributor`/`VisualDataReDistributor` are declared-but-unused scaffolding; live distribution is via `FFTDataDistributor`/processors' base `distribute()` | +| `docs/design/configuration-system.md` | Corrected `save()` failure claim: `DataTree::SaveToFileXML` always returns true, so the error branch is unreachable; clarified session sample rate is nearest-supported selection, not clamping | +| `docs/design/sdr-device-layer.md` | Corrected remote add/remove: `removeRemote` is dead code (no UI call site); clarified `SDRThread::streamArgs` stores/applies (device declares the arg set) | +| `docs/design/signal-flow.md` | Filled concrete max sizes for `audioVisOutputQueue` (1) and `audioSinkOutputQueue` (1000) in the per-demod queue table | +| `docs/design/audio-subsystem.md` | Corrected sample-rate-check timing (runs on the second callback, not the initial pop); latency figure is illustrative (device rate configurable); gain applied during summation | +| `docs/design/threading.md` | Corrected "blocking push/pop" to "blocking push / timed pop" | +| `docs/design/bookmark-system.md` | Noted ranges are renamed by editing the label field, not a button | + +## Session 67: Redundant "not-a-thing" note cleanup + +**Date:** 2026-08-06 +**Model:** opencode/deepseek-v4-flash-free + +- Removed a redundant note added in Session 66 (the `DrawFreqBwInfo`-in-waterfall clarification duplicate at `visual-rendering.md:446`) per user feedback +- Scanned `docs/design/` for similar absence/negative-framing notes; judged most justified (setters-never-called, unjoined threads, expandState staleness) but trimmed three redundant/editorial ones + +| File | Action | +|------|--------| +| `docs/design/visual-rendering.md` | Shortened the Mouse-wheel row (was doubly redundant: "not wired" + "absent from event table") | +| `docs/design/configuration-system.md` | Dropped redundant "it is absent when no view state is saved" restatement | +| `docs/design/bookmark-system.md` | Removed editorial "latent defect rather than an active bug" verdict, kept the facts | diff --git a/docs/design/audio-subsystem.md b/docs/design/audio-subsystem.md index 2d2ae8a..486f10d 100644 --- a/docs/design/audio-subsystem.md +++ b/docs/design/audio-subsystem.md @@ -89,11 +89,11 @@ The `audioCallback` function runs in the RtAudio real-time thread: 5. Return 0 on success; return 1 if the controller is terminated, which instructs RtAudio to stop the stream Key properties: -- **Buffer size:** The RtAudio buffer is 1024 frames by default (`nBufferFrames`), which at 48 kHz yields ~21 ms latency per buffer -- **Sample rate matching:** Whenever a new `currentInput` is popped (either on first access or when the current packet is exhausted mid-mixing), the callback checks if its sample rate matches the controller's. If not, it pops and discards packets until it finds a matching one or the queue is exhausted. If no matching packet is found, `currentInput` is left as `nullptr` and the thread is skipped +- **Buffer size:** The RtAudio buffer is 1024 frames by default (`nBufferFrames`), which at a 48 kHz device rate yields ~21 ms of audio per buffer (latency varies with the configured device sample rate) +- **Sample rate matching:** When the callback finds `currentInput` is `nullptr` at the start of a poll, it pops a packet and `continue`s to the next thread without mixing it. On the following callback, `currentInput` is non-null and the sample rate is checked against the controller's. If it does not match, the callback pops and discards packets until it finds a matching one or the queue is exhausted. If no matching packet is found, `currentInput` is left as `nullptr` and the thread is skipped. When a packet is exhausted mid-mixing and the next is popped inline, no rate check is performed. - **First-packet latency:** When a new packet is popped from a queue, the callback immediately continues to the next thread without mixing it. This introduces a one-callback-cycle delay before a newly queued packet produces output, avoiding partial consumption of a fresh packet - **Underflow handling:** If a bound thread runs out of data, the callback continues with the next thread. RtAudio buffer underflows (reported via `status` flag) are counted in the controller's `underflowCount` field -- **Gain staging:** Per-thread `gain` (0.0–2.0, default 1.0) is applied before mixing; global normalization prevents clipping +- **Gain staging:** Per-thread `gain` (0.0–2.0, default 1.0) is applied as samples are summed into the mix (`data[i]*gain`); global normalization prevents clipping ### Real-Time Design Constraints diff --git a/docs/design/bookmark-system.md b/docs/design/bookmark-system.md index 72b9aeb..ec3a76a 100644 --- a/docs/design/bookmark-system.md +++ b/docs/design/bookmark-system.md @@ -31,7 +31,7 @@ Represents a saved demodulator configuration: | `bandwidth` | `int` | Demodulator bandwidth in Hz | | `node` | `DataNode*` | Full demodulator state (serialized via `DemodulatorMgr::saveInstance()`) | -The `node` field stores the complete demodulator configuration including modem settings, gain, squelch, output device, and other parameters. This allows exact restoration when a bookmark is loaded. The class has no constructor, so `frequency`, `bandwidth`, and `node` are all uninitialized when a `BookmarkEntry` is default-constructed. The destructor calls `delete node` on this raw pointer, which is undefined behavior if `node` was never assigned. In practice, all construction paths (`demodToBookmarkEntry()` and `nodeToBookmark()`) assign `node`, so this is a latent defect rather than an active bug. +The `node` field stores the complete demodulator configuration including modem settings, gain, squelch, output device, and other parameters. This allows exact restoration when a bookmark is loaded. The class has no constructor, so `frequency`, `bandwidth`, and `node` are all uninitialized when a `BookmarkEntry` is default-constructed. The destructor calls `delete node` on this raw pointer, which is undefined behavior if `node` was never assigned. In practice, all construction paths (`demodToBookmarkEntry()` and `nodeToBookmark()`) assign `node`. ### BookmarkRangeEntry (`src/BookmarkMgr.h`) @@ -141,7 +141,7 @@ On first run (no bookmark file exists), `loadDefaultRanges()` populates standard | 13 cm lower | 2300–2310 MHz | | 13 cm upper | 2390–2450 MHz | -> **Note:** The 17 meters band range (17.044–19.092 MHz) in the source is incorrect per international allocations. The ITU 17m band is 18.068–18.168 MHz. The source value spans ~2 MHz and bleeds into adjacent bands. This is a bug in `BookmarkMgr.cpp`, not a documentation error. +> **Note:** The 17 meters band range (17.044–19.092 MHz) in the source is incorrect per international allocations. The ITU 17m band is 18.068–18.168 MHz. The source value spans ~2 MHz and bleeds into adjacent bands. ## Persistence @@ -235,7 +235,7 @@ Recovery dialogs call `loadFromFile` with `backup=false`, so no `.lastloaded` or None of the three bookmark dialog subclasses override `doClickCancel()`. The base `ActionDialog` class defines `doClickCancel()` as a no-op, so clicking Cancel simply closes the dialog. Clicking Cancel in `ActionDialogBookmarkLoadFailed` or `ActionDialogBookmarkBackupLoadFailed` causes the app to continue with empty bookmarks. Clicking Cancel in `ActionDialogBookmarkCatastophe` causes the app to continue without exiting. `ActionDialogBookmarkCatastophe`'s OK action calls `disableSave(true)`, which prevents **all** saves on close — not just bookmarks, but also `AppConfig`. -> **Note:** The class names are misleading — `ActionDialogBookmarkBackupLoadFailed` actually loads the `.lastloaded` file, not the `.backup` file. `ActionDialogBookmarkCatastophe` offers to exit without saving to preserve files for manual recovery. +> **Note:** `ActionDialogBookmarkBackupLoadFailed` loads the `.lastloaded` file. `ActionDialogBookmarkCatastophe` offers to exit without saving to preserve files for manual recovery. ## UI Integration @@ -262,7 +262,7 @@ Both systems expose public accessors: `BookmarkView::getExpandState()`/`setExpan During search, expand states are overridden: ranges are forced collapsed, while recents and bookmark groups are forced expanded. Expand/collapse events are suppressed during search to prevent user actions from conflicting with the forced states. -`loadFromFile()` does not clear `BookmarkMgr::expandState` before repopulating it. Old group expand states persist across reloads for groups that no longer exist in the loaded file. Note the asymmetry: `bmData`, `recents`, `ranges`, and `bmDataSorted` are all cleared at the start of `loadFromFile()`, but `expandState` is not. This is likely unintentional but harmless due to the default-`true` behavior of `getExpandState()`. +`loadFromFile()` does not clear `BookmarkMgr::expandState` before repopulating it. Old group expand states persist across reloads for groups that no longer exist in the loaded file. `bmData`, `recents`, `ranges`, and `bmDataSorted` are all cleared at the start of `loadFromFile()`, but `expandState` is not. This has no visible effect because `getExpandState()` returns `true` for unknown keys. ### Demodulator Interaction @@ -274,7 +274,7 @@ During search, expand states are overridden: ranges are forced collapsed, while ### Additional UI Features - **Search/Filter:** Keyword search filters bookmark tree in real-time -- **Range management:** Add, remove, rename, and update frequency band ranges +- **Range management:** Add, remove, and update frequency band ranges; ranges are renamed by editing the label field in the properties panel - **Recording controls:** Start/stop audio recording from active demodulators - **Status bar hint:** A static hint ("Drag & Drop to create / move bookmarks, Group and arrange bookmarks, quick Search by keywords.") is shown in the status bar when the mouse enters the bookmark panel - **Frequency/bandwidth editing:** Double-click the frequency or bandwidth fields in the properties panel to open a FrequencyDialog (active demodulators only) diff --git a/docs/design/configuration-system.md b/docs/design/configuration-system.md index 7f3c57c..6e7e70f 100644 --- a/docs/design/configuration-system.md +++ b/docs/design/configuration-system.md @@ -198,7 +198,7 @@ Note: `DeviceConfig::save()` writes the `antenna`, `streamOpts`, `settings`, `ri 1. Create a `DataTree` with root node named `cubicsdr_config` 2. Populate window, recording, device, manual device, and rig nodes. The `window` node and all its children are only written when both `winW` and `winH` are non-zero (a zero-sized window produces no window config). 3. Get config file path from `getConfigFileName()` -4. Call `DataTree::SaveToFileXML()`; `save()` returns `false` and logs an error if the file cannot be written. +4. Call `DataTree::SaveToFileXML()`. Note that `SaveToFileXML` unconditionally writes the file and always returns `true`, so `save()` cannot detect or report a disk-write failure; its error branch is effectively unreachable. **Load** (`AppConfig::load()`): 1. Determine config file path @@ -245,7 +245,7 @@ Sessions capture the complete demodulator state for save/restore. ``` -Note: The `` element is stored internally as a percent-encoded `wstring` (not plain text), so the earlier example shows its encoded form. The `` section is only written when the waterfall canvas view state is active; it is absent when no view state is saved. +Note: The `` element is stored internally as a percent-encoded `wstring` (not plain text), so the earlier example shows its encoded form. The `` section is only written when the waterfall canvas view state is active. ### Save Flow (`SessionMgr::saveSession()`) @@ -260,7 +260,7 @@ Note: The `` element is stored internally as a percent-encoded `wstring 1. Load `DataTree` from file 2. Validate root node name is `cubicsdr_session` 3. Terminate all existing demodulators -4. Parse header: version, sample rate (clamped to device limits), solo mode +4. Parse header: version, sample rate (selected to the nearest supported value when outside the device's range, or the manual fallback when no rate list is available), solo mode 5. Parse demodulators: create each via `DemodulatorMgr::loadInstance()`, call `run()`, set active 6. Restore center frequency and view state 7. Set active demodulator diff --git a/docs/design/modem-system.md b/docs/design/modem-system.md index 143e7ba..29ef6d9 100644 --- a/docs/design/modem-system.md +++ b/docs/design/modem-system.md @@ -186,7 +186,7 @@ Settings that change the liquid-dsp constellation size take effect in place via | DSB | `ModemDSB` | `src/modules/modem/analog/ModemDSB.cpp` | 5400 | | I/Q | `ModemIQ` | `src/modules/modem/analog/ModemIQ.cpp` | 48000 | -Note: `ModemFMStereo` and `ModemIQ` inherit directly from `Modem`, not from `ModemAnalog`. They are listed here because they produce analog audio output, but they do not use `ModemAnalog`'s resampling infrastructure. Both return `"analog"` from `getType()`, which is how `DemodulatorThread` dispatches them as analog modems despite the non-standard inheritance. +Note: `ModemFMStereo` and `ModemIQ` inherit directly from `Modem`, not from `ModemAnalog`. They are listed here because they produce analog audio output, but they do not use `ModemAnalog`'s resampling infrastructure. Both return `"analog"` from `getType()`, which is how `DemodulatorThread` dispatches them as analog modems despite the non-standard inheritance. `ModemFMStereo` registers under the factory key `"FMS"` (and `getName()` returns `"FMS"`), so its UI/menu name is `"FM Stereo"` while the key is `"FMS"`. ### Digital (12, conditional on `ENABLE_DIGITAL_LAB`) diff --git a/docs/design/sdr-device-layer.md b/docs/design/sdr-device-layer.md index 3f124a4..cd0de9b 100644 --- a/docs/design/sdr-device-layer.md +++ b/docs/design/sdr-device-layer.md @@ -94,17 +94,17 @@ Represents a discovered or manually defined SDR device. | Property | Type | Description | |----------|------|-------------| -| `name` | `string` | Display name (from SoapySDR `label` or `device` field) | +| `name` | `string` | Display name (from SoapySDR `label` or `device` field); set during enumeration | | `serial` | `string` | Device serial number (setter exists but is never called) | -| `driver` | `string` | SoapySDR driver name | -| `hardware` | `string` | Hardware revision (populated from `getHardwareInfo()`) | +| `driver` | `string` | SoapySDR driver name; set during enumeration | +| `hardware` | `string` | Hardware revision (set during enumeration, populated from `getHardwareInfo()`) | | `tuner` | `string` | Tuner chip type (setter exists but is never called) | | `manufacturer` | `string` | Device manufacturer (setter exists but is never called) | | `product` | `string` | Product name (setter exists but is never called) | ### Device ID -`getDeviceId()` returns the device's display name (`getName()`). Note: earlier designs intended to include serial, remote address, or factory/params in the ID, but the current implementation simply returns the name field. +`getDeviceId()` returns the device's display name (`getName()`). ### State @@ -160,7 +160,7 @@ SoapySDR devices are configured via key-value argument strings: | `label` | Human-readable device label | **Stream arguments** (stored in `streamArgs`): -`SDRThread` carries a `SoapySDR::Kwargs streamArgs` member, but CubicSDR does not populate specific stream argument keys — the map is available for driver-specific stream configuration. +`SDRThread` carries a `SoapySDR::Kwargs streamArgs` member that stores and applies driver-specific stream configuration (the set of possible arguments is declared by the device's `getStreamArgsInfo()`, not by `SDRThread`). When a device is selected in the UI, `SDRDevicesDialog::OnUseSelected()` queries `getStreamArgsInfo(SOAPY_SDR_RX, 0)`, populates the `streamArgs` map from the edited property values, persists them via `DeviceConfig::setStreamOpts()`, and pushes them through `CubicSDR::setStreamArgs()` into `SDRThread::setStreamArgs()`. `SDRThread` itself does not add keys; it applies the selections received from the dialog. **Device settings** (stored in `DeviceConfig::settings`): - Driver-specific settings (e.g., `bias_tee` for RTL-SDR) @@ -194,7 +194,7 @@ Manual devices are: - List of all discovered devices (local, remote, manual) - Device properties display (sample rates, gains, antennas) - Manual device add/remove -- Remote device add/remove +- Remote device add (via `CubicSDR::addRemote`); no UI exists to remove a remote device — `removeRemote` is declared but never invoked - Device activation (triggers `SDRThread` creation) ## Device Configuration diff --git a/docs/design/signal-flow.md b/docs/design/signal-flow.md index 60eccd0..30198d2 100644 --- a/docs/design/signal-flow.md +++ b/docs/design/signal-flow.md @@ -117,10 +117,12 @@ Manages RtAudio hardware output using a **controller/bound** pattern: | `pipeIQInputData` | `DemodulatorThreadInputQueue` | SDRPostThread | DemodulatorPreThread | 100 | | `pipeIQDemodData` | `DemodulatorThreadPostInputQueue` | DemodulatorPreThread | DemodulatorThread | 100 | | `pipeAudioData` | `AudioThreadInputQueue` | DemodulatorThread | AudioThread | 100 | -| `audioVisOutputQueue` | `DemodulatorThreadOutputQueue` | DemodulatorThread | ScopeVisualProcessor | default | -| `audioSinkOutputQueue` | `DemodulatorThreadOutputQueue` | DemodulatorThread | AudioSinkFileThread | default | +| `audioVisOutputQueue` | `DemodulatorThreadOutputQueue` | DemodulatorThread | ScopeVisualProcessor | 1 | +| `audioSinkOutputQueue` | `DemodulatorThreadOutputQueue` | DemodulatorThread | AudioSinkFileThread | 1000 | -Note: `audioVisOutputQueue` is a per-DemodulatorThread member that is bound at runtime to the global `pipeAudioVisualData` queue via `setOutputQueue("AudioVisualOutput", ...)`. `audioSinkOutputQueue` is bound dynamically only when WAV recording starts. +Note: `pipeAudioVisualData` is a per-DemodulatorThread member that is bound at runtime to the global `pipeAudioVisualData` queue via `setOutputQueue("AudioVisualOutput", ...)` (max 1). `audioSinkOutputQueue` is bound dynamically only when WAV recording starts, to the sink thread's input queue (max 1000). + +Note: `pipeAudioVisualData` and `pipeDemodIQVisualData` are created and wired only when the respective compile-time view features are enabled (`CUBICSDR_ENABLE_VIEW_SCOPE` for the scope, `CUBICSDR_ENABLE_VIEW_DEMOD` for the demod spectrum). Otherwise these pointers are `nullptr`. Note: `DemodulatorThreadOutputQueue` and `AudioThreadInputQueue` are both aliases for `ThreadBlockingQueue` (defined in `src/audio/AudioThread.h`). They carry the same data type; the distinct names reflect the queue's role in the pipeline. diff --git a/docs/design/threading.md b/docs/design/threading.md index 4b26211..44475b0 100644 --- a/docs/design/threading.md +++ b/docs/design/threading.md @@ -24,7 +24,7 @@ threadObject = new ThreadClass(...); t_stdThread = new std::thread(&ThreadClass::threadMain, threadObject); ``` -Exception: on macOS, `DemodulatorPreThread` and `DemodulatorThread` use `pthread_create` with ~2MB stack sizes (2048000 bytes) to control the stack size directly. +Exception: on macOS, `DemodulatorInstance::run()` creates `DemodulatorPreThread` and `DemodulatorThread` via `pthread_create` with ~2MB stack sizes (2048000 bytes) to control the stack size directly. The demodulator thread `.cpp` files install only the `pthread_setschedparam` priority logic, not the stack size. ## Thread Inventory @@ -51,7 +51,7 @@ Exception: on macOS, `DemodulatorPreThread` and `DemodulatorThread` use `pthread ### Pattern 1: Queue-Based Data Flow (Primary) All data-carrying threads communicate via `ThreadBlockingQueue`: -- **Blocking push/pop** for critical data (demod pipeline, worker commands) +- **Blocking push / timed pop** for critical data (demod pipeline, worker commands) — pushes block indefinitely; pops use a short heartbeat timeout - **Non-blocking try_push/try_pop** for visualization (data loss acceptable) - Most queue items are `std::shared_ptr` (IQ data, audio data); worker command/result queues use value types @@ -125,7 +125,7 @@ Atomic variables signal parameter changes between UI and worker threads. Boolean ### Heartbeat Period -All IOThread subclasses use a consistent 50ms heartbeat timeout (`HEARTBEAT_CHECK_PERIOD_MICROS = 50 * 1000`) in their main loop `pop()` calls. This ensures the `stopping` atomic flag is checked at ~20Hz, enabling responsive shutdown without dedicated interrupt mechanisms. The constant is defined locally in each translation unit (not shared), which is a minor duplication but has no behavioral impact. +All IOThread subclasses that use a timed `pop()` in their main loop share a consistent 50ms heartbeat timeout (`HEARTBEAT_CHECK_PERIOD_MICROS = 50 * 1000`). This ensures the `stopping` atomic flag is checked at ~20Hz, enabling responsive shutdown without dedicated interrupt mechanisms. Threads that do not use a timed pop — `SDRThread` (blocking `readStream`), `SDREnumerator` (single-pass `run()`), and `RigThread` (uses `sleep_for(150ms)`) — rely on their own blocking/sleep behavior for shutdown. The constant is defined locally in each translation unit (not shared), which is a minor duplication but has no behavioral impact. ### SpinMutex @@ -188,7 +188,7 @@ An `AudioSinkFileThread` may also be started on-demand when recording is activat In `CubicSDR::OnExit()`: -1. `stopRig()` — calls `RigThread::terminate()` (sets atomic flag) then `isTerminated(1000)` to join (if rig is active) +1. `stopRig()` — calls `RigThread::terminate()` (sets atomic flag), then `isTerminated(1000)` and `join()`s and deletes the rig thread and `RigThread` objects (if rig is active) 2. `SDRThread::terminate()` — stops producing IQ data (waited up to 3s) 3. `SDRPostThread::terminate()` — stops channelizing (waited up to 3s) 4. `DemodulatorMgr::terminateAll()` — terminates all demodulator instances (queues flushed inside each `DemodulatorInstance::terminate()`) @@ -236,7 +236,7 @@ On macOS, threads are assigned scheduling priorities: | Audio Thread (controller) | `SCHED_RR` | max - 1 | | Audio Sink Thread | `SCHED_RR` | max - 1 | -Note: SDR Thread has `SCHED_FIFO` priority code but the entire `#ifdef __APPLE__` block is commented out on all platforms. +Note: SDR Thread has `SCHED_FIFO` priority code but the entire `#ifdef __APPLE__` block is commented out (as is the main wxWidgets thread's `SCHED_RR` block in `CubicSDR.cpp`). Windows and Linux use default thread priorities. diff --git a/docs/design/visual-data-pipeline.md b/docs/design/visual-data-pipeline.md index c247021..3f9dc2d 100644 --- a/docs/design/visual-data-pipeline.md +++ b/docs/design/visual-data-pipeline.md @@ -47,7 +47,7 @@ Two distribution strategies handle different multicast patterns: **`VisualDataReDistributor`** — Deep-copy dispatch via `ReBuffer` pool. Each output gets its own copy of the data, allocated from a pre-pooled buffer to avoid per-frame allocation. Used when consumers modify or consume the data independently. -Both `VisualDataDistributor` and `VisualDataReDistributor` are defined inline in `VisualProcessor.h`. +Both `VisualDataDistributor` and `VisualDataReDistributor` are defined inline in `VisualProcessor.h`. They are currently **declared-but-unused scaffolding**: neither is instantiated in the live pipeline. Actual distribution is done by `FFTDataDistributor`, `SpectrumVisualProcessor`, and `ScopeVisualProcessor`, which dispatch to outputs via the base `VisualProcessor::distribute()`. ## FFTDataDistributor (`src/process/FFTDataDistributor.h`) @@ -107,10 +107,10 @@ pipeIQDataIn → fftDistrib → fftQueue → wproc → pipeFFTDataOut The thread loop: 1. Sleep ~10ms between iterations -2. `fftDistrib.run()` — packages IQ data into FFT-ready batches (rate-limited by `FFT_DISTRIBUTOR_BUFFER_IN_SECONDS = 0.250s`) +2. `fftDistrib.run()` — packages IQ data into FFT-ready batches (input buffer sized by `FFT_DISTRIBUTOR_BUFFER_IN_SECONDS = 0.250s`) 3. `wproc.run()` — executes FFT processing in a tight loop until input is drained (one FFT per iteration) -This thread bridges IQ data to the waterfall display by running the FFT distributor and processor in a tight loop. The FFT distributor batches incoming IQ packets into FFT-sized chunks, and the processor executes one FFT per batch. Note: while `FFTDataDistributor` supports multiple outputs, `FFTVisualDataThread` attaches only one (`fftQueue`). The multi-consumer distribution (to both waterfall and spectrum) happens at the `SDRPostThread` level, which attaches separate queues to each consumer. +This thread bridges IQ data to the waterfall display by running the FFT distributor and processor in a tight loop. The FFT distributor batches incoming IQ packets into FFT-sized chunks (its execution pace is rate-limited separately by `linesPerSecond`/`lineRateAccum`), and the processor executes one FFT per batch. Note: while `FFTDataDistributor` supports multiple outputs, `FFTVisualDataThread` attaches only one (`fftQueue`). The multi-consumer distribution (to both waterfall and spectrum) happens at the `SDRPostThread` level, which attaches separate queues to each consumer. ## ScopeVisualProcessor @@ -123,7 +123,7 @@ Processes demodulated audio for scope/spectrum display. - `SCOPE_MODE_Y` — Single-channel time waveform - `SCOPE_MODE_2Y` — Dual-channel overlaid waveforms - `SCOPE_MODE_XY` — Lissajous figure (phase display) -- Spectrum mode — FFT of demodulated audio (default 1024 points); controlled by a separate boolean flag (`renderData->spectrum`) rather than a fourth `ScopeMode` enum value +- Spectrum mode — FFT of demodulated audio; FFT size defaults to 1024 (`DEFAULT_SCOPE_FFT_SIZE`), producing 512 output spectrum points (`fftSize/2`); controlled by a separate boolean flag (`renderData->spectrum`) Uses `try_pop` (non-blocking) instead of blocking pop, since audio data arrives at a fixed rate and stale data should be dropped. diff --git a/docs/design/visual-rendering.md b/docs/design/visual-rendering.md index 7e43628..c56d8b1 100644 --- a/docs/design/visual-rendering.md +++ b/docs/design/visual-rendering.md @@ -48,7 +48,7 @@ The primary display canvas. Handles: - `WF_DRAG_BANDWIDTH_LEFT` / `WF_DRAG_BANDWIDTH_RIGHT` — resize demodulator bandwidth - `WF_DRAG_RANGE` — create new demodulator by range selection - **Zoom:** Mouse wheel adjusts `mouseZoom`, which smoothly animates to the target zoom level -- **Frequency nudge:** Arrow keys shift center frequency by half/full bandwidth +- **Frequency nudge:** Arrow keys shift center frequency by half a bandwidth; Shift adds 10× the shift - **Visual gain:** Shift+Up/Down adjusts `scaleMove` (drives visual gain animation toward target scale factor) **Drag state machine:** @@ -187,7 +187,7 @@ Shared OpenGL context providing drawing primitives. All canvases share this cont | Method | Purpose | |--------|---------| | `BeginDraw(r, g, b)` | Clear color+depth buffers, load identity modelview | -| `EndDraw()` | Finalize frame (currently empty — was `glFlush()`) | +| `EndDraw()` | Finalize frame — currently empty (no-op) | | `DrawDemod(demod, color, center_freq=-1, srate=0)` | Draw demodulator bandwidth indicator with label | | `DrawDemodInfo(demod, color, center_freq=-1, srate=0, centerline=false)` | Draw demodulator info label (frequency, type, bandwidth) | | `DrawFreqSelector(uxPos, color, w=0, center_freq=-1, srate=0)` | Draw frequency selection marker (center line + BW edges) | @@ -210,8 +210,7 @@ glLoadIdentity() - `glBegin(GL_LINES)` / `glVertex3f()` — immediate mode for demod lines - `glBegin(GL_QUADS)` / `glVertex3f()` — immediate mode for filled regions - `glColor4f()` / `glColor3f()` — immediate mode color setting -- `glEnable(GL_LINE_SMOOTH)` — antialiased lines for borders and demod edges -- `glLineWidth()` — variable width for demod edge emphasis +- `glLineWidth()` — variable width for range-selector emphasis (`DrawRangeSelector`) **Blend modes used:** @@ -250,7 +249,7 @@ Context for the fine-tuning bar canvas. Renders per-digit frequency display with - `DrawBegin()` — clears with theme background, disables texturing - `Draw(r, g, b, a, p1, p2)` — draws a horizontal tuning bar between positions `p1`–`p2` (normalized 0–1), split at y=0 with additive blending gradient (dim at edges, full at center) -- `DrawTuner(freq, count, displayPos, displayWidth)` — renders a frequency value as individual digits, each in its own column with vertical grid lines at 25% alpha. Font size adapts to viewport via two independent paths: width-based (≥500px → 32px, ≥300px → 24px, else → 18px) and height-based (≥28px → 18px, ≥24px → 16px, else → 12px); whichever threshold is met first determines the size +- `DrawTuner(freq, count, displayPos, displayWidth)` — renders a frequency value as individual digits, each in its own column with vertical grid lines at 25% alpha. Font size adapts to viewport via two sequential paths: width-based first (`<300px → 18px`, `<500px → 24px`, else → 32px), then height-based overrides (`<18px → 12px`, `<24px → 16px`, `<28px → 18px`; height ≥ 28px leaves the width-selected size unchanged) - `static DrawTunerDigitBox(index, count, displayPos, displayWidth, color)` — draws a red highlight box around a single digit position via `GL_LINE_STRIP` (note: `color` parameter is currently ignored; highlight is always red) - `GetTunerDigitIndex(mPos, count, displayPos, displayWidth)` — hit-test utility converting mouse position to digit index - `DrawTunerBarIndexed(start, end, count, displayPos, displayWidth, color, alpha, top, bottom)` — draws a colored bar for a range of digit indices on the top/bottom half with additive blending (note: `alpha` parameter is currently ignored; hardcoded to 0.6) @@ -477,8 +476,6 @@ OnPaint(): │ ├── Draw frequency selector // hover/active frequency marker │ - ├── Draw frequency/bandwidth info // text readout at cursor position - │ └── SwapBuffers() ``` @@ -645,7 +642,7 @@ Arrow keys control frequency and display in the WaterfallCanvas: | UP | Shift held | Increase visual gain: `scaleMove = 1.0` | | DOWN | Shift held | Decrease visual gain: `scaleMove = -1.0` | -All keyboard controls are **momentary** — on key-up, `freqMove`, `scaleMove`, and `zoom` reset to their default (0, 0, 1.0). Frequency movement has velocity decay: `freqMove -= freqMove * 0.2` per frame, stopping at 0.01. +Keyboard controls are **momentary**: on key-up, `scaleMove` resets to 0 and `zoom` resets to 1.0 (with UP/DOWN key-up re-arming `mouseZoom` to 0.95/1.05). `freqMove` is not hard-reset on key-up; it stops by velocity decay — `freqMove -= freqMove * 0.2` per frame, snapping to 0 at 0.01. ### SpectrumCanvas Interactions @@ -653,7 +650,7 @@ All keyboard controls are **momentary** — on key-up, `freqMove`, `scaleMove`, |-------|--------| | Left-drag | Horizontal pan: `moveCenterFrequency(deltaX * bandwidth)` | | Right-drag | Visual gain: `updateScaleFactorFromYMove(deltaMouseY)`, clamped to [0.25, 10.0] (requires `scaleFactorEnabled`) | -| Right-click (no vertical drag) | Animate scale factor back to 1.0, toggle peak hold on spectrum/demod processors (only triggers when `originDeltaMouseY == 0`) | +| Right-click (no vertical drag) | Animate scale factor back to 1.0 and reset the peak-hold accumulator on spectrum/demod processors (only triggers when `originDeltaMouseY == 0`; the on/off peak-hold toggle is done elsewhere) | | Mouse wheel | Forwarded to `WaterfallCanvas::OnMouseWheelMoved()` for coordinated zoom | | 'B' key | Toggle dB display mode | @@ -664,7 +661,7 @@ The spectrum canvas tracks cumulative bandwidth change (`bwChange`) and resets t | Input | Action | |-------|--------| | Left-drag | Panel slide with inertia: `dragAccel = 4.0 * deltaMouseX`, momentum-based snap | -| Mouse wheel | Not wired in event table (handler exists but `EVT_MOUSEWHEEL` is absent from the event table) | +| Mouse wheel | No action (handler exists but is not registered in the event table) | Panel position animates with spring-like behavior: `ctr += (ctrTarget - ctr) * 0.2`. Panels use 3D perspective projection with `lookat(0, 0, -1.205, ...)` and rotation via `atan2(pos, 1.2)` for a card-flip effect. The two panels (scope + spectrum) are separated by `panelSpacing = 0.4f` (total interval = `panelWidth * 2.0 + panelSpacing`).