mirror of
https://github.com/cjcliffe/CubicSDR.git
synced 2026-10-08 00:00:23 -04:00
Initial agent evaluation
This commit is contained in:
@@ -0,0 +1,76 @@
|
||||
# Agent Session Log
|
||||
|
||||
## Session: Project Evaluation and Planning
|
||||
|
||||
**Date:** 2026-07-23
|
||||
**Model:** opencode/big-pickle
|
||||
|
||||
## Actions
|
||||
|
||||
1. Explored the CubicSDR codebase structure, technology stack, and key directories
|
||||
2. Analyzed code quality: TODO/FIXME markers, test coverage, memory safety, build system, documentation, git hygiene, vendored dependencies
|
||||
3. Generated `RECOMMENDATIONS.md` with evaluation summary and 10 priority-ranked recommendations
|
||||
4. Generated `PLAN.md` with detailed implementation plans for each recommendation
|
||||
5. Moved both files to `docs/` directory
|
||||
6. Created `AGENTS.md` in project root to direct agents to documentation
|
||||
7. Split `PLAN.md` into 10 individual plan files under `docs/plans/`, each self-contained with project context
|
||||
8. Updated `PLAN.md` to serve as an index linking to individual plans
|
||||
9. Removed numeric prefixes from plan filenames, ordering by priority in the index instead
|
||||
10. Updated `AGENTS.md` to reflect final file structure
|
||||
|
||||
## Files Created/Modified
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `RECOMMENDATIONS.md` | Created, then moved to `docs/` |
|
||||
| `PLAN.md` | Created, then moved to `docs/`, then rewritten as index |
|
||||
| `docs/RECOMMENDATIONS.md` | Created (moved from root) |
|
||||
| `docs/PLAN.md` | Created (moved from root, rewritten as index) |
|
||||
| `docs/plans/add-unit-tests.md` | Created |
|
||||
| `docs/plans/fix-gitignore.md` | Created |
|
||||
| `docs/plans/modernize-cmake.md` | Created |
|
||||
| `docs/plans/smart-pointers.md` | Created |
|
||||
| `docs/plans/split-appframe.md` | Created |
|
||||
| `docs/plans/remove-c4996-suppression.md` | Created |
|
||||
| `docs/plans/replace-reinterpret-cast.md` | Created |
|
||||
| `docs/plans/update-vendored-deps.md` | Created |
|
||||
| `docs/plans/add-ci-test-execution.md` | Created |
|
||||
| `docs/plans/resolve-todos.md` | Created |
|
||||
| `AGENTS.md` | Created |
|
||||
| `AGENT-LOG.md` | Created |
|
||||
|
||||
## Session: Documentation Review and Improvements
|
||||
|
||||
**Date:** 2026-07-23
|
||||
**Model:** opencode/big-pickle
|
||||
|
||||
### Actions
|
||||
|
||||
1. Read all 12 docs files to evaluate quality, completeness, and correctness
|
||||
2. Identified 9 issues: 3 critical (broken cross-references), 2 structural (missing CONTRIBUTING.md, no execution order), 4 minor (Python patterns, no versioning, thread semantics, rollback strategy)
|
||||
3. Fixed broken cross-references in `resolve-todos.md` (3 refs to `05-split-appframe.md`, `07-replace-reinterpret-cast.md`) and `add-ci-test-execution.md` (1 ref to `01-add-unit-tests.md`)
|
||||
4. Fixed `replace-reinterpret-cast.md` `writeToBuffer` helper: added note explaining why the write-side `reinterpret_cast` to `unsigned char*` is well-defined per C++ aliasing rules, distinguishing it from the 20 read-side violations
|
||||
5. Added "Recommended Execution Order" section to `PLAN.md` with 10-step sequence and risk rationale
|
||||
6. Created `CONTRIBUTING.md` referencing plans, code conventions, build/test instructions
|
||||
7. Removed irrelevant Python patterns (`*.pyc`, `__pycache__/`) from `fix-gitignore.md`
|
||||
8. Added `Last Updated: 2026-07-23` header to all 12 documentation files
|
||||
9. Added "Design Considerations: Join vs. Detach" section to `smart-pointers.md` covering thread shutdown semantics, exception safety, and RAII wrapper guidance
|
||||
10. Added "Rollback Strategy" section to `update-vendored-deps.md` with commit-per-dependency, pin-before-update, and TinyXML-2 feature branch guidance
|
||||
|
||||
### Files Modified
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `docs/RECOMMENDATIONS.md` | Added date header |
|
||||
| `docs/PLAN.md` | Added date header, added recommended execution order section |
|
||||
| `docs/plans/add-unit-tests.md` | Added date header |
|
||||
| `docs/plans/fix-gitignore.md` | Added date header, removed Python patterns |
|
||||
| `docs/plans/modernize-cmake.md` | Added date header |
|
||||
| `docs/plans/smart-pointers.md` | Added date header, added join vs. detach design section |
|
||||
| `docs/plans/split-appframe.md` | Added date header |
|
||||
| `docs/plans/remove-c4996-suppression.md` | Added date header |
|
||||
| `docs/plans/replace-reinterpret-cast.md` | Added date header, fixed writeToBuffer helper explanation |
|
||||
| `docs/plans/update-vendored-deps.md` | Added date header, added rollback strategy section |
|
||||
| `docs/plans/add-ci-test-execution.md` | Added date header, fixed broken cross-reference |
|
||||
| `docs/plans/resolve-todos.md` | Added date header, fixed 3 broken cross-references |
|
||||
| `CONTRIBUTING.md` | Created |
|
||||
@@ -0,0 +1,36 @@
|
||||
# Agent Instructions
|
||||
|
||||
This is a C++ Software-Defined Radio application (CubicSDR) built with wxWidgets, OpenGL, liquid-dsp, and SoapySDR.
|
||||
|
||||
## Project Documentation
|
||||
|
||||
- `docs/RECOMMENDATIONS.md` — Project evaluation, strengths, weaknesses, and priority-ranked improvement recommendations
|
||||
- `docs/PLAN.md` — Index of implementation plans with risk/effort estimates and execution order
|
||||
- `docs/plans/` — Individual implementation plans for each recommendation:
|
||||
- `add-unit-tests.md` — Test infrastructure and initial test coverage
|
||||
- `fix-gitignore.md` — Comprehensive .gitignore patterns
|
||||
- `modernize-cmake.md` — CMake 2.8 → 3.14+ modernization
|
||||
- `smart-pointers.md` — Replace raw new/delete with std::unique_ptr
|
||||
- `split-appframe.md` — Split 3,200-line AppFrame.cpp into 5 files
|
||||
- `remove-c4996-suppression.md` — Address unsafe CRT function usage
|
||||
- `replace-reinterpret-cast.md` — Fix undefined behavior in DataTree
|
||||
- `update-vendored-deps.md` — Update or replace third-party libraries
|
||||
- `add-ci-test-execution.md` — Run tests in CI pipeline
|
||||
- `resolve-todos.md` — Address 14 open TODO/FIXME markers
|
||||
|
||||
## Build
|
||||
|
||||
CMake-based. See `CMakeLists.txt` for build configuration. The project targets C++14 on Windows (MSVC), macOS, and Linux.
|
||||
|
||||
## Key Directories
|
||||
|
||||
- `src/` — Application source (sdr/, demod/, audio/, visual/, modules/modem/, util/)
|
||||
- `external/` — Vendored third-party libraries (lodepng, tinyxml, rtaudio, liquid-dsp, hamlib, cubicvr2)
|
||||
- `font/` — Bitmap fonts
|
||||
- `cmake/` — CMake helper modules
|
||||
|
||||
## Conventions
|
||||
|
||||
- Follow existing code style in each file
|
||||
- Do not add comments unless asked
|
||||
- SPDX license headers on source files: `// Copyright (c) Charles J. Cliffe // SPDX-License-Identifier: GPL-2.0+`
|
||||
@@ -0,0 +1,47 @@
|
||||
# Contributing to CubicSDR
|
||||
|
||||
## Getting Started
|
||||
|
||||
1. Read `docs/RECOMMENDATIONS.md` for a project overview, known issues, and improvement areas.
|
||||
2. Read `docs/PLAN.md` for the full list of improvement plans with risk/effort estimates and a recommended execution order.
|
||||
3. Pick a plan from `docs/plans/` that matches your interest and skill level.
|
||||
|
||||
## Picking Up a Plan
|
||||
|
||||
Each plan in `docs/plans/` includes:
|
||||
- **Current State** — what exists today
|
||||
- **Implementation Plan** — step-by-step instructions
|
||||
- **Files to Create/Modify** — exact files affected
|
||||
|
||||
Plans are ordered by dependency. Check the "Dependencies" column in `docs/PLAN.md` before starting — some plans require others to be completed first.
|
||||
|
||||
## Code Conventions
|
||||
|
||||
- Follow existing code style in each file (indentation, naming, braces).
|
||||
- Do not add comments unless asked.
|
||||
- SPDX license headers on all source files:
|
||||
```
|
||||
// Copyright (c) Charles J. Cliffe // SPDX-License-Identifier: GPL-2.0+
|
||||
```
|
||||
|
||||
## Build and Test
|
||||
|
||||
CubicSDR uses CMake. See the project README and wiki for build instructions.
|
||||
|
||||
Once the test infrastructure is in place (see `docs/plans/add-unit-tests.md`):
|
||||
```bash
|
||||
cmake -B build -DBUILD_TESTING=ON
|
||||
cmake --build build
|
||||
ctest --test-dir build --output-on-failure
|
||||
```
|
||||
|
||||
## Submitting Changes
|
||||
|
||||
1. Fork the repository and create a feature branch.
|
||||
2. Implement the plan, following the steps in the plan file.
|
||||
3. Verify the build compiles cleanly and tests pass.
|
||||
4. Submit a pull request referencing the plan (e.g., "Implements docs/plans/smart-pointers.md").
|
||||
|
||||
## Reporting Issues
|
||||
|
||||
The project has 14 open TODO/FIXME markers tracked in `docs/plans/resolve-todos.md`. If you encounter a bug or have a feature request, open an issue on the GitHub repository.
|
||||
@@ -0,0 +1,35 @@
|
||||
# CubicSDR Improvement Plans
|
||||
|
||||
Detailed implementation plans for each recommendation. See [RECOMMENDATIONS.md](RECOMMENDATIONS.md) for the full evaluation and summary.
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Plans
|
||||
|
||||
| Plan | Risk | Effort | Dependencies |
|
||||
|------|------|--------|-------------|
|
||||
| [Fix .gitignore](plans/fix-gitignore.md) | None | 5 min | None |
|
||||
| [Add Unit Tests](plans/add-unit-tests.md) | Low | 2-3 days | None |
|
||||
| [Replace reinterpret_cast Type Punning](plans/replace-reinterpret-cast.md) | Low | 1 day | None |
|
||||
| [Modernize CMake](plans/modernize-cmake.md) | Low-Medium | 1-2 days | None |
|
||||
| [Split AppFrame.cpp](plans/split-appframe.md) | Low | 1 day | None |
|
||||
| [Resolve Open TODOs](plans/resolve-todos.md) | Low | 1 day | split-appframe, replace-reinterpret-cast |
|
||||
| [Replace Raw new/delete with Smart Pointers](plans/smart-pointers.md) | Medium | 1 day | None |
|
||||
| [Remove MSVC C4996 Suppression](plans/remove-c4996-suppression.md) | Medium | 1-2 days | None |
|
||||
| [Add CI Test Execution](plans/add-ci-test-execution.md) | Low | 2 hours | add-unit-tests |
|
||||
| [Update Vendored Dependencies](plans/update-vendored-deps.md) | High | 3-5 days | None |
|
||||
|
||||
## Recommended Execution Order
|
||||
|
||||
Execute in this order to minimize risk and satisfy dependencies:
|
||||
|
||||
1. **Fix .gitignore** — Zero risk, immediate value, unblocks clean builds
|
||||
2. **Add Unit Tests** — Foundational; enables test-driven work on subsequent plans
|
||||
3. **Add CI Test Execution** — Locks in test infrastructure before code changes
|
||||
4. **Replace reinterpret_cast** — Low risk, standalone, improves correctness
|
||||
5. **Modernize CMake** — Low-medium risk, standalone, enables better build practices
|
||||
6. **Split AppFrame.cpp** — Low risk, standalone, reduces cognitive load for later work
|
||||
7. **Resolve TODOs** — Depends on AppFrame split and reinterpret_cast replacement
|
||||
8. **Replace Raw new/delete** — Medium risk; do after tests exist to catch regressions
|
||||
9. **Remove C4996 Suppression** — Medium risk; requires touching many files
|
||||
10. **Update Vendored Dependencies** — Highest risk; do last, requires extensive testing
|
||||
@@ -0,0 +1,105 @@
|
||||
# CubicSDR Project Evaluation
|
||||
|
||||
**Project:** CubicSDR v0.2.8
|
||||
**License:** GPL-2.0+
|
||||
**Language:** C++ (C++11/14)
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Architecture
|
||||
|
||||
- ~95 source files across well-organized modules: SDR I/O, demodulation, audio, visualization, and modem plugins
|
||||
- Threading model: Producer-consumer pattern with blocking queues for SDR -> demod -> audio pipeline
|
||||
- Build: Single monolithic CMakeLists.txt (1117 lines) supporting Windows/macOS/Linux
|
||||
- Vendored deps: lodepng, tinyxml, rtaudio, cubicvr2, hamlib, liquid-dsp in `external/`
|
||||
|
||||
## Key Strengths
|
||||
|
||||
- Clean modular source organization (`sdr/`, `demod/`, `audio/`, `visual/`, `modules/modem/`)
|
||||
- Proper thread safety with atomics, mutexes, lock_guard, shared_ptr throughout
|
||||
- Good SPDX license headers on all files
|
||||
- Cross-platform support (Win/Mac/Linux)
|
||||
|
||||
## Key Weaknesses
|
||||
|
||||
| Issue | Severity |
|
||||
|-------|----------|
|
||||
| **Zero test coverage** — no test framework, no test files, CI only builds | Critical |
|
||||
| **Monolithic files** — `AppFrame.cpp` is 3,202 lines | High |
|
||||
| **Memory safety** — raw `new`/`delete` for threads, 20 `reinterpret_cast`s in DataTree, suppressed MSVC C4996 warnings | High |
|
||||
| **Outdated CMake** — targets 2.8.12, uses `-std=c++0x` draft flag, deprecated patterns | Medium |
|
||||
| **Incomplete .gitignore** — missing IDE, OS, and build artifact patterns | Medium |
|
||||
| **Minimal docs** — no CONTRIBUTING, CHANGELOG, API docs, or Doxygen | Medium |
|
||||
| **14 open TODOs** in project code, including acknowledged bugs | Low-Medium |
|
||||
| **No input validation** on XML config files | Medium |
|
||||
|
||||
## Recommendations (Priority Order)
|
||||
|
||||
1. **Add unit tests** — Start with `DataTree`, `Timer`, `ThreadBlockingQueue`, frequency conversion utilities
|
||||
2. **Fix `.gitignore`** — Add `.vs/`, `*.obj`, `CMakeCache.txt`, `.DS_Store`, IDE files
|
||||
3. **Modernize CMake** — Bump to 3.10+, use `target_compile_features(cxx_std_14)`, replace deprecated variables
|
||||
4. **Replace raw `new`/`delete`** with `std::unique_ptr`/`std::shared_ptr` for thread objects in `CubicSDR.cpp`
|
||||
5. **Split `AppFrame.cpp`** — Extract menu handling, keyboard shortcuts, device management into separate files
|
||||
6. **Remove MSVC C4996 suppression** — Address the underlying unsafe CRT calls (`sprintf` -> `snprintf`, etc.)
|
||||
7. **Replace `reinterpret_cast` type punning** in `DataTree.h` with `memcpy`-based serialization (avoids UB)
|
||||
8. **Use git submodules or a package manager** for vendored external dependencies
|
||||
9. **Add CI test execution** after builds
|
||||
10. **Resolve or convert to tracked issues** the 14 open TODOs
|
||||
|
||||
## Detailed Findings
|
||||
|
||||
### TODO/FIXME Comments (14 in project code)
|
||||
|
||||
| File | Line | Comment | Severity |
|
||||
|------|------|---------|----------|
|
||||
| `CubicSDRDefs.h` | 49 | `TODO: Make the waterfall resolutions an option.` | Low |
|
||||
| `AppFrame.cpp` | 289 | `TODO: refactor these..` | Medium |
|
||||
| `AppFrame.cpp` | 2868 | `TODO: Move the stuff from there to here` | Medium |
|
||||
| `AppFrame.cpp` | 3084 | `TODO: Catch key-ups outside of original target` | Low |
|
||||
| `DataTree.h` | 304, 363 | `TODO: smarter way with templates?` (x2) | Medium |
|
||||
| `DataTree.cpp` | 143 | `TODO: code below returns a forced cast in (char*) beware...` | High |
|
||||
| `DataTree.cpp` | 171, 225 | `TODO: stack recursion optimization` (x2) | Low |
|
||||
| `DemodulatorThread.cpp` | 257 | `TODO: handle digital modems with audio output` | Medium |
|
||||
| `DemodulatorMgr.cpp` | 233 | `TODO: This is probably unnecessary and confusing` | Medium |
|
||||
| `SoapySDRThread.cpp` | 203, 215, 486 | Various TODOs about timing and bandwidth | Low |
|
||||
| `GainCanvas.cpp` | 291 | `TODO: if it not desirable, do not update in AGC mode` | Low |
|
||||
| `ScopeCanvas.cpp` | 132 | `TODO: find out why frontbuffer drawing has stopped working in wx 3.1.0?` | Medium |
|
||||
| `PrimaryGLContext.cpp` | 120 | `TODO: Better recording indicator...` | Low |
|
||||
| `BookmarkView.cpp` | 553 | `TODO: keys for other actions?` | Low |
|
||||
|
||||
### Memory Safety Concerns
|
||||
|
||||
- **`DataTree.h`** contains 20 `reinterpret_cast` operations for type punning — potential aliasing violations and undefined behavior. Use `memcpy` or `std::bit_cast` instead.
|
||||
- **`CubicSDR.cpp`** allocates thread objects (`std::thread*`) with raw `new` and manual `delete`. Any exception between allocation and deletion causes a leak. Should use `std::unique_ptr<std::thread>`.
|
||||
- **`IOThread.cpp`** uses bare `catch (...)` that silently swallows all exceptions.
|
||||
- MSVC C4996 warning is globally suppressed, hiding potential buffer overflow risks from unsafe CRT functions.
|
||||
|
||||
### Build System Issues
|
||||
|
||||
- `cmake_minimum_required(VERSION 2.8.12)` — should be 3.10+
|
||||
- Uses `ADD_DEFINITIONS(-std=c++0x)` instead of `target_compile_features()` — `c++0x` is the C++11 draft, not the finalized standard
|
||||
- Uses deprecated `CMAKE_CREATE_WIN32_EXE` variable
|
||||
- Uses global `include_directories()` instead of `target_include_directories()`
|
||||
- Hardcoded library paths: `link_directories(/usr/local/lib)`, ALSA paths at `/usr/include` and `/usr/lib`
|
||||
- Header/source file lists in CMakeLists.txt have some mismatches (`.cpp` files listed as headers, `.h` files listed as sources)
|
||||
|
||||
### Documentation
|
||||
|
||||
- README.md is minimal (45 lines) with no inline build instructions
|
||||
- No CONTRIBUTING.md, CHANGELOG.md, CODE_OF_CONDUCT.md, or architecture docs
|
||||
- No Doxygen or API documentation
|
||||
- Build instructions exist only on external wiki; user manual in separate repository
|
||||
|
||||
### CI/CD
|
||||
|
||||
- CircleCI configuration exists but only compiles — no test execution
|
||||
- Recent commits are macOS-focused (bundling, code signing)
|
||||
- Single branch (`master`), 36 tags from 0.1.0 to 0.2.7
|
||||
|
||||
### .gitignore
|
||||
|
||||
Currently only covers `build/`, `cmake_build/`, `dist/`. Missing entries for:
|
||||
- IDE files (`.vs/`, `*.suo`, `.idea/`, `*.xcworkspace`, `*.xcodeproj`)
|
||||
- OS files (`.DS_Store`, `Thumbs.db`, `desktop.ini`)
|
||||
- Compiled artifacts (`*.o`, `*.obj`, `*.dll`, `*.so`, `*.dylib`, `*.exe`, `*.lib`, `*.a`)
|
||||
- CMake generated files (`CMakeCache.txt`, `CMakeFiles/`, `cmake_install.cmake`, `Makefile`)
|
||||
- Package files (`*.deb`, `*.rpm`, `*.dmg`)
|
||||
@@ -0,0 +1,37 @@
|
||||
# Plan: Add CI Test Execution
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers adding test execution to the CircleCI pipeline.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
CircleCI config (`.circleci/config.yml`) only compiles the project. No tests are run.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
Depends on: [Add Unit Tests](add-unit-tests.md). Once tests exist:
|
||||
|
||||
1. Update `.circleci/config.yml` to add a test step after build:
|
||||
```yaml
|
||||
- run:
|
||||
name: Run tests
|
||||
command: |
|
||||
cd build
|
||||
ctest --output-on-failure
|
||||
```
|
||||
2. Consider adding a matrix of test configurations (Release/Debug).
|
||||
3. Add test result upload for CI visibility.
|
||||
4. Consider adding a separate "test" workflow that depends on "build".
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `.circleci/config.yml` | Add test execution step |
|
||||
|
||||
## Dependencies
|
||||
|
||||
This plan requires the test infrastructure from [Add Unit Tests](add-unit-tests.md) to be completed first.
|
||||
@@ -0,0 +1,99 @@
|
||||
# Plan: Add Unit Tests
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers adding unit test infrastructure and initial test coverage.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
- Zero test coverage. No test framework, no test files, no test targets in CMakeLists.txt.
|
||||
- CI (CircleCI) only builds — no test execution.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 1: Set up test infrastructure
|
||||
|
||||
1. Add a `tests/` directory at the project root.
|
||||
2. Add a test framework. **Recommendation: Catch2 v3** (header-only, single header, modern C++14, good assertion macros, CMake integration via `FetchContent` or bundled header).
|
||||
3. Add a `tests/CMakeLists.txt` with a `cubicsdr_tests` target.
|
||||
4. Add `add_subdirectory(tests)` to the root `CMakeLists.txt` (guarded by an option like `BUILD_TESTING`).
|
||||
5. Update CircleCI config to run `ctest` after build.
|
||||
|
||||
### Phase 2: Core utility tests (highest value, lowest effort)
|
||||
|
||||
**Test file: `tests/test_SpinMutex.cpp`**
|
||||
- `src/util/SpinMutex.h` — header-only, zero external dependencies (`<atomic>` only).
|
||||
- Tests:
|
||||
- Single-thread: `lock()` → `try_lock()` returns false → `unlock()` → `try_lock()` returns true.
|
||||
- Multi-thread: two threads incrementing a shared counter under the lock; verify final count equals sum of increments.
|
||||
- Verify `lock_guard<SpinMutex>` works (satisfies C++ Lockable concept).
|
||||
- Verify copy construction and assignment are deleted (compile-time).
|
||||
|
||||
**Test file: `tests/test_Gradient.cpp`**
|
||||
- `src/util/Gradient.h` + `src/util/Gradient.cpp` — self-contained, no project dependencies.
|
||||
- Tests:
|
||||
- Two-color gradient (black to white) produces linear interpolation [0.0 → 1.0].
|
||||
- Output array length matches requested `len`.
|
||||
- Values clamped to [0.0, 1.0] for edge cases.
|
||||
- `clear()` resets state; `generate()` after `clear()` produces empty arrays.
|
||||
- Multi-color gradient (3+ stops) produces correct interpolation.
|
||||
- Document bug: `generate()` with single color causes divide-by-zero (`colors.size() - 1 == 0`).
|
||||
|
||||
**Test file: `tests/test_ThreadBlockingQueue.cpp`**
|
||||
- `src/util/ThreadBlockingQueue.h` — depends only on `SpinMutex.h` (also header-only).
|
||||
- Tests:
|
||||
- Single-threaded: `push`/`pop` ordering, `size()` tracking, `empty()`/`full()` states, `flush()`.
|
||||
- Capacity: `set_max_num_items()`, `try_push()` returns false when full.
|
||||
- Multi-threaded producer-consumer: one thread pushes N items, another pops N items; verify all items received.
|
||||
- Timeout: `push` with short timeout returns false when queue is full.
|
||||
- `NON_BLOCKING_TIMEOUT` path: verify immediate fail when queue is full/empty.
|
||||
|
||||
**Test file: `tests/test_DataTree.cpp`**
|
||||
- `src/util/DataTree.h` + `src/util/DataTree.cpp` — depends on bundled `external/tinyxml/`.
|
||||
- Tests:
|
||||
- `DataElement::set()`/`get()` for all scalar types (char, int, float, double, etc.).
|
||||
- `DataElement::getDataType()` returns correct enum for each type.
|
||||
- `DataNode` tree building: `newChild()`, `child()` by name/index, `numChildren()`.
|
||||
- `DataNode` iteration: `hasAnother()`/`getNext()`/`rewind()`.
|
||||
- `DataTree::SaveToFileXML()` / `LoadFromFileXML()` round-trip.
|
||||
- `DataElement` vector types: `set()`/`get()` for `vector<int>`, `vector<float>`, etc.
|
||||
|
||||
**Test file: `tests/test_Timer.cpp`**
|
||||
- `src/util/Timer.h` + `src/util/Timer.cpp` — platform-specific (`<windows.h>` on Windows).
|
||||
- Tests (using `lockFramerate()` for determinism):
|
||||
- `start()` resets timer; `getMilliseconds()` returns 0 after start.
|
||||
- `lockFramerate(30.0)` → `update()` advances by ~33.33ms each call.
|
||||
- `paused(true)` freezes `getMilliseconds()` while `totalMilliseconds()` continues.
|
||||
- `getNumUpdates()` counts `update()` calls correctly.
|
||||
- `setMilliseconds()`/`setSeconds()` force specific values.
|
||||
|
||||
### Phase 3: Update CI
|
||||
|
||||
- Add `BUILD_TESTING=ON` to CircleCI build steps.
|
||||
- Add `ctest --test-dir build --output-on-failure` after build.
|
||||
- Consider adding a separate "test" job that depends on the "build" job.
|
||||
|
||||
## Files to Create/Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `tests/CMakeLists.txt` | Create |
|
||||
| `tests/test_SpinMutex.cpp` | Create |
|
||||
| `tests/test_Gradient.cpp` | Create |
|
||||
| `tests/test_ThreadBlockingQueue.cpp` | Create |
|
||||
| `tests/test_DataTree.cpp` | Create |
|
||||
| `tests/test_Timer.cpp` | Create |
|
||||
| `CMakeLists.txt` | Add `add_subdirectory(tests)` and `BUILD_TESTING` option |
|
||||
| `.circleci/config.yml` | Add test execution step |
|
||||
|
||||
## Testability Assessment
|
||||
|
||||
| Module | Self-Contained? | External Deps | Testability |
|
||||
|--------|----------------|---------------|-------------|
|
||||
| `SpinMutex.h` | Yes (header-only) | None (`<atomic>` only) | Excellent |
|
||||
| `Gradient.h/.cpp` | Yes | None (`<vector>` only) | Excellent |
|
||||
| `ThreadBlockingQueue.h` | Mostly | `SpinMutex.h` (in-project) | Excellent |
|
||||
| `DataTree.h/.cpp` | Mostly | `tinyxml.h` (bundled) | Good |
|
||||
| `Timer.h/.cpp` | Mostly | Platform APIs | Moderate |
|
||||
@@ -0,0 +1,88 @@
|
||||
# Plan: Fix .gitignore
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers adding comprehensive patterns to `.gitignore`.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
`.gitignore` only covers `build/`, `cmake_build/`, `dist/`. Missing IDE, OS, editor, build artifact, and package patterns.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
Replace the contents of `.gitignore` with comprehensive patterns:
|
||||
|
||||
```gitignore
|
||||
# Build directories
|
||||
build/
|
||||
cmake_build/
|
||||
dist/
|
||||
Testing/
|
||||
|
||||
# CMake generated files
|
||||
CMakeCache.txt
|
||||
CMakeFiles/
|
||||
cmake_install.cmake
|
||||
Makefile
|
||||
compile_commands.json
|
||||
_deps/
|
||||
|
||||
# IDE files
|
||||
.vs/
|
||||
*.suo
|
||||
*.user
|
||||
*.sln.docstates
|
||||
.idea/
|
||||
.vscode/
|
||||
*.code-workspace
|
||||
.project
|
||||
.cproject
|
||||
.settings/
|
||||
.classpath
|
||||
*.xcworkspace/
|
||||
*.xcodeproj/
|
||||
*.swp
|
||||
*.swo
|
||||
*~
|
||||
|
||||
# OS files
|
||||
.DS_Store
|
||||
Thumbs.db
|
||||
Desktop.ini
|
||||
|
||||
# Compiled artifacts
|
||||
*.o
|
||||
*.obj
|
||||
*.a
|
||||
*.lib
|
||||
*.so
|
||||
*.dll
|
||||
*.dylib
|
||||
*.exe
|
||||
*.out
|
||||
*.app/
|
||||
*.dSYM/
|
||||
|
||||
# MSVC artifacts
|
||||
*.pdb
|
||||
*.ilk
|
||||
*.exp
|
||||
*.log
|
||||
|
||||
# Package artifacts
|
||||
*.deb
|
||||
*.rpm
|
||||
*.dmg
|
||||
*.nsi
|
||||
|
||||
# Misc
|
||||
.cache/
|
||||
```
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `.gitignore` | Rewrite |
|
||||
@@ -0,0 +1,58 @@
|
||||
# Plan: Modernize CMake
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers modernizing the CMake build system from CMake 2.8 patterns to modern CMake 3.14+.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
- `cmake_minimum_required(VERSION 2.8.12)` — from 2013
|
||||
- Uses `-std=c++0x` (draft C++11) instead of proper standard setting
|
||||
- Global `include_directories()`, `ADD_DEFINITIONS()`, `link_libraries()` instead of target-specific
|
||||
- Deprecated `CMAKE_CREATE_WIN32_EXE`, `LINK_FLAGS`, `SOURCE_GROUP REGULAR_EXPRESSION`
|
||||
- Stray comma in `CMAKE_OSX_DEPLOYMENT_TARGET` assignment (line 303)
|
||||
- Uppercase CMake commands throughout
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 1: Minimum viable modernization
|
||||
|
||||
1. Bump `cmake_minimum_required(VERSION 3.14...3.28)` — enables modern policies while allowing newer CMake.
|
||||
2. Replace `ADD_DEFINITIONS( -std=c++0x -pthread )` (line 166) with:
|
||||
```cmake
|
||||
target_compile_features(CubicSDR PRIVATE cxx_std_14)
|
||||
find_package(Threads REQUIRED)
|
||||
target_link_libraries(CubicSDR PRIVATE Threads::Threads)
|
||||
```
|
||||
3. Fix the stray comma on line 303: `SET(CMAKE_OSX_DEPLOYMENT_TARGET, "10.9")` → `set(CMAKE_OSX_DEPLOYMENT_TARGET "10.9")`.
|
||||
4. Replace `set(CMAKE_CREATE_WIN32_EXE ...)` (line 708) with:
|
||||
```cmake
|
||||
set_target_properties(CubicSDR PROPERTIES WIN32_EXECUTABLE TRUE)
|
||||
target_link_options(CubicSDR PRIVATE /SUBSYSTEM:WINDOWS /ENTRY:"mainCRTStartup")
|
||||
```
|
||||
|
||||
### Phase 2: Target-specific commands
|
||||
|
||||
5. Replace all `include_directories(...)` with `target_include_directories(CubicSDR PRIVATE ...)`.
|
||||
6. Replace all `ADD_DEFINITIONS(...)` with `target_compile_definitions(CubicSDR PRIVATE ...)`.
|
||||
7. Replace `link_libraries(${HAMLIB_LIBRARY})` with `target_link_libraries(CubicSDR PRIVATE ${HAMLIB_LIBRARY})`.
|
||||
8. Replace `set_target_properties(... LINK_FLAGS ...)` with `target_link_options()`.
|
||||
|
||||
### Phase 3: Style normalization
|
||||
|
||||
9. Convert all uppercase CMake commands to lowercase (`SET` → `set`, `IF` → `if`, etc.).
|
||||
10. Remove empty `endif()` arguments: `endif( CMAKE_SIZEOF_VOID_P EQUAL 8 )` → `endif()`.
|
||||
|
||||
### Phase 4: Advanced modernization (optional, future)
|
||||
|
||||
11. Replace `include(${wxWidgets_USE_FILE})` with imported targets (requires wxWidgets 3.2+).
|
||||
12. Replace `SOURCE_GROUP(... REGULAR_EXPRESSION ...)` with `source_group(TREE ...)`.
|
||||
13. Consider bumping minimum to CMake 3.16+ for broader modern feature access.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `CMakeLists.txt` | All phases |
|
||||
@@ -0,0 +1,34 @@
|
||||
# Plan: Remove MSVC C4996 Suppression
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers removing the global MSVC C4996 warning suppression and addressing the underlying unsafe CRT function usage.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
Line 712 of `CMakeLists.txt`:
|
||||
```cmake
|
||||
ADD_DEFINITIONS(/wd"4996")
|
||||
```
|
||||
This globally suppresses all "This function or variable may be unsafe" warnings from MSVC CRT. This hides potential buffer overflow risks from functions like `sprintf`, `strcpy`, `strcat`, etc.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
1. Remove the `/wd4996` suppression from `CMakeLists.txt`.
|
||||
2. Build the project and catalog all C4996 warnings (file, line, function).
|
||||
3. For each occurrence, replace with the safe variant:
|
||||
- `sprintf` → `snprintf`
|
||||
- `strcpy` → `strncpy` or `std::string` operations
|
||||
- `strcat` → `strncat` or `std::string` operations
|
||||
- `sscanf` → use bounds-checked alternatives
|
||||
4. If any third-party/vendored code triggers C4996, suppress it locally with `#pragma warning(disable: 4996)` around just that code, not globally.
|
||||
5. Re-enable the warning globally and verify a clean build.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `CMakeLists.txt` | Remove `/wd4996` |
|
||||
| Various source files | Replace unsafe CRT functions (exact count TBD after unsuppressing) |
|
||||
@@ -0,0 +1,41 @@
|
||||
# Plan: Replace reinterpret_cast Type Punning in DataTree
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers replacing undefined-behavior `reinterpret_cast` type punning in `src/util/DataTree.h` with safe `std::memcpy`-based serialization.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
`src/util/DataTree.h` contains 20 `reinterpret_cast` operations (lines 209, 230, 306-398) that perform type-punning between `unsigned char` byte buffers and typed scalar values. This violates C++ strict aliasing rules and can cause undefined behavior.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
1. Create a helper template function in `DataTree.h` (or a new `DataTreeUtil.h`):
|
||||
```cpp
|
||||
template <typename T>
|
||||
T readFromBuffer(const std::vector<unsigned char>& buf, size_t offset = 0) {
|
||||
T value;
|
||||
std::memcpy(&value, buf.data() + offset, sizeof(T));
|
||||
return value;
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
void writeToBuffer(std::vector<unsigned char>& buf, const T& value) {
|
||||
const unsigned char* bytes = reinterpret_cast<const unsigned char*>(&value);
|
||||
buf.insert(buf.end(), bytes, bytes + sizeof(T));
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** The `writeToBuffer` helper retains a `reinterpret_cast` for the write side. Casting to `unsigned char*` (or `std::byte*`) is well-defined per [basic.lval]/11 — it is the one exception to strict aliasing. The goal here is to eliminate the **20 read-side** `reinterpret_cast`s that alias typed scalars through `unsigned char` buffers, which are the actual aliasing violations. The write-side cast is safe and idiomatic.
|
||||
2. Replace all 20 `reinterpret_cast` read operations (lines 306-398) with calls to `readFromBuffer<T>()`.
|
||||
3. Replace the 2 `reinterpret_cast` write operations (lines 209, 230) with calls to `writeToBuffer<T>()` (which uses a safe cast to `unsigned char*`).
|
||||
4. Add `#include <cstring>` to `DataTree.h` for `std::memcpy`.
|
||||
5. Build and verify no regressions.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `src/util/DataTree.h` | Add helper functions, replace all reinterpret_casts |
|
||||
@@ -0,0 +1,67 @@
|
||||
# Plan: Resolve Open TODOs
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers resolving the 14 TODO/FIXME markers in the project source code.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
14 TODO/FIXME markers in project source code:
|
||||
|
||||
| File | Line | Comment | Action |
|
||||
|------|------|---------|--------|
|
||||
| `CubicSDRDefs.h` | 49 | `TODO: Make the waterfall resolutions an option.` | Convert to GitHub issue |
|
||||
| `AppFrame.cpp` | 289 | `TODO: refactor these..` | Address during AppFrame split |
|
||||
| `AppFrame.cpp` | 2868 | `TODO: Move the stuff from there to here` | Address during AppFrame split |
|
||||
| `AppFrame.cpp` | 3084 | `TODO: Catch key-ups outside of original target` | Convert to GitHub issue |
|
||||
| `DataTree.h` | 304, 363 | `TODO: smarter way with templates?` | Convert to GitHub issue |
|
||||
| `DataTree.cpp` | 143 | `TODO: forced cast in (char*) beware...` | Address during reinterpret_cast fix |
|
||||
| `DataTree.cpp` | 171, 225 | `TODO: stack recursion optimization` | Convert to GitHub issue |
|
||||
| `DemodulatorThread.cpp` | 257 | `TODO: handle digital modems with audio output` | Convert to GitHub issue |
|
||||
| `DemodulatorMgr.cpp` | 233 | `TODO: This is probably unnecessary and confusing` | Investigate and either fix or remove |
|
||||
| `SoapySDRThread.cpp` | 203, 215, 486 | Various TODOs about timing and bandwidth | Convert to GitHub issues |
|
||||
| `GainCanvas.cpp` | 291 | `TODO: if not desirable, do not update in AGC mode` | Convert to GitHub issue |
|
||||
| `ScopeCanvas.cpp` | 132 | `TODO: find out why frontbuffer drawing stopped in wx 3.1.0?` | Investigate; may be fixed in newer wxWidgets |
|
||||
| `PrimaryGLContext.cpp` | 120 | `TODO: Better recording indicator...` | Convert to GitHub issue |
|
||||
| `BookmarkView.cpp` | 553 | `TODO: keys for other actions?` | Convert to GitHub issue |
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
1. For TODOs that will be addressed by other plans, resolve them during those tasks:
|
||||
- `AppFrame.cpp:289,2868` → resolve during [Split AppFrame.cpp](split-appframe.md)
|
||||
- `DataTree.cpp:143` → resolve during [Replace reinterpret_cast](replace-reinterpret-cast.md)
|
||||
|
||||
2. For the remaining 10 TODOs, create GitHub issues for each with:
|
||||
- The original TODO text
|
||||
- File/line reference
|
||||
- Description of the feature request or bug
|
||||
|
||||
3. Remove the TODO comments from the code, replacing with issue references:
|
||||
```cpp
|
||||
// See: https://github.com/cjcliffe/CubicSDR/issues/XXX
|
||||
```
|
||||
|
||||
4. For `DemodulatorMgr.cpp:233` ("probably unnecessary and confusing"), investigate the code and either fix the issue or remove the dead code.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `src/CubicSDRDefs.h` | Remove TODO, add issue reference |
|
||||
| `src/AppFrame.cpp` | Remove resolved TODOs during split |
|
||||
| `src/DataTree.h` | Remove TODOs, add issue references |
|
||||
| `src/DataTree.cpp` | Remove TODOs, add issue references |
|
||||
| `src/DemodulatorThread.cpp` | Remove TODO, add issue reference |
|
||||
| `src/DemodulatorMgr.cpp` | Investigate and resolve |
|
||||
| `src/SoapySDRThread.cpp` | Remove TODOs, add issue references |
|
||||
| `src/GainCanvas.cpp` | Remove TODO, add issue reference |
|
||||
| `src/ScopeCanvas.cpp` | Remove TODO, add issue reference |
|
||||
| `src/PrimaryGLContext.cpp` | Remove TODO, add issue reference |
|
||||
| `src/BookmarkView.cpp` | Remove TODO, add issue reference |
|
||||
|
||||
## Related Plans
|
||||
|
||||
- [Split AppFrame.cpp](split-appframe.md) — resolves 2 AppFrame TODOs
|
||||
- [Replace reinterpret_cast](replace-reinterpret-cast.md) — resolves 1 DataTree TODO
|
||||
@@ -0,0 +1,86 @@
|
||||
# Plan: Replace Raw new/delete with Smart Pointers
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers replacing raw `new`/`delete` allocations with `std::unique_ptr` and fixing memory leaks in `src/CubicSDR.cpp`.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
In `src/CubicSDR.cpp`:
|
||||
- 8 raw `new std::thread(...)` allocations (lines 390, 393, 397, 405, 617, 794, 843, 1164)
|
||||
- 6 raw `new WorkerThread()` allocations (lines 340, 355, 358, 374, 400, 1157)
|
||||
- 15 raw `delete` operations (lines 506-530, 775, 804, 1148, 1153, 1188, 1194)
|
||||
- 4 confirmed memory leaks:
|
||||
- `m_glContextAttributes` (line 225) — never deleted
|
||||
- `confName` wxString (line 562) — never freed
|
||||
- `modPath` wxString (line 579) — never freed
|
||||
- `appframe` (line 404) — never explicitly deleted
|
||||
- `t_SDREnum` overwritten (lines 617, 794) without joining/deleting the old thread
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 1: Declare smart pointer members
|
||||
|
||||
In `src/CubicSDR.h`, change declarations from:
|
||||
```cpp
|
||||
std::thread *t_SDR;
|
||||
SDRThread *sdrThread;
|
||||
```
|
||||
to:
|
||||
```cpp
|
||||
std::unique_ptr<std::thread> t_SDR;
|
||||
std::unique_ptr<SDRThread> sdrThread;
|
||||
```
|
||||
|
||||
Apply to all thread and worker object pointers:
|
||||
- `t_SDR`, `t_PostSDR`, `t_SpectrumVisual`, `t_DemodVisual`, `t_SDREnum`, `t_Rig`
|
||||
- `sdrThread`, `sdrPostThread`, `spectrumVisualThread`, `demodVisualThread`, `sdrEnum`, `rigThread`
|
||||
- `m_glContext` (PrimaryGLContext*)
|
||||
|
||||
### Phase 2: Update allocations
|
||||
|
||||
Replace all `new` calls with `std::make_unique`:
|
||||
```cpp
|
||||
// Before:
|
||||
t_SDR = new std::thread(&SDRThread::threadMain, sdrThread);
|
||||
// After:
|
||||
t_SDR = std::make_unique<std::thread>(&SDRThread::threadMain, sdrThread);
|
||||
```
|
||||
|
||||
### Phase 3: Remove all manual `delete` calls
|
||||
|
||||
Smart pointers handle cleanup automatically. Remove all 15 `delete` operations.
|
||||
|
||||
### Phase 4: Fix memory leaks
|
||||
|
||||
- `m_glContextAttributes`: wrap in `std::unique_ptr<wxGLContextAttrs>` or delete in `OnExit()`.
|
||||
- `confName`/`modPath` (lines 562, 579): change from raw `new wxString` to stack-allocated `wxString` or `std::unique_ptr`.
|
||||
|
||||
### Phase 5: Fix t_SDREnum overwrite
|
||||
|
||||
Before overwriting `t_SDREnum`, join the existing thread:
|
||||
```cpp
|
||||
if (t_SDREnum && t_SDREnum->joinable()) {
|
||||
t_SDREnum->join();
|
||||
}
|
||||
t_SDREnum = std::make_unique<std::thread>(...);
|
||||
```
|
||||
|
||||
### Design Considerations: Join vs. Detach
|
||||
|
||||
When replacing raw `new`/`delete` with smart pointers, each thread must be evaluated for correct shutdown semantics:
|
||||
|
||||
- **Join before delete** — Use when the thread's work must complete before destruction (e.g., `t_SDR`, `t_DemodVisual`). The current code joins before deleting, so `std::unique_ptr` with explicit `join()` preserves existing behavior.
|
||||
- **Detach** — Use when the thread is a background worker that should outlive the owning scope. If any thread is detached, it must not be held by a `unique_ptr` that joins on destruction — use a detached raw pointer or `std::thread` stored separately.
|
||||
- **Exception safety** — `std::unique_ptr` destructor calls `delete` (not `join`). For threads that must be joined, always call `join()` explicitly before the `unique_ptr` goes out of scope. Consider a RAII wrapper that joins in the destructor if this pattern is needed in multiple places.
|
||||
|
||||
Review each thread allocation in `CubicSDR.cpp` and document the chosen semantics (join or detach) in a code comment at the declaration site.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `src/CubicSDR.h` | Change pointer declarations to smart pointers |
|
||||
| `src/CubicSDR.cpp` | Update allocations, remove deletes, fix leaks |
|
||||
@@ -0,0 +1,44 @@
|
||||
# Plan: Split AppFrame.cpp
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers splitting the monolithic `AppFrame.cpp` (3,202 lines) into multiple compilation units.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
- `AppFrame.cpp`: 3,202 lines in a single file
|
||||
- `AppFrame.h`: 389 lines
|
||||
- Handles menus, keyboard, device management, UI layout, hamlib, sessions, idle handlers, and accessors
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
Split into 5 compilation units (same class, multiple `.cpp` files — no header changes needed):
|
||||
|
||||
| New File | Content | ~Lines |
|
||||
|----------|---------|--------|
|
||||
| `AppFrame.cpp` (kept) | Constructor, destructor, init*, make* factory methods, OnClose, OnNewWindow, splitter events, accessors, utilities | ~1,030 |
|
||||
| `AppFrame_Menus.cpp` | `OnMenu`, all 19 `actionOnMenu*` methods, `makeFileMenu`, `makeDisplayMenu`, `makeAudioSampleRateMenu`, `makeRecordingMenu`, `updateRecordingMenu`, `getSettingsLabel` | ~900 |
|
||||
| `AppFrame_Handlers.cpp` | `OnIdle`, all 12 `handle*` methods, `handleUpdateDeviceParams` | ~710 |
|
||||
| `AppFrame_Keyboard.cpp` | `OnGlobalKeyDown`, `OnGlobalKeyUp`, `gkNudge`, `toggleActiveDemodRecording`, `toggleAllActiveDemodRecording` | ~334 |
|
||||
| `AppFrame_Hamlib.cpp` | All `#ifdef USE_HAMLIB` methods: `makeRigMenu`, `enableRig`, `disableRig`, `setRigControlPort`, `dismissRigControlPortDialog`, `actionOnMenuRig`, `handleRigMenu` | ~301 |
|
||||
|
||||
### Steps
|
||||
|
||||
1. Create the 4 new `.cpp` files, each including `AppFrame.h`.
|
||||
2. Move the method implementations (not declarations) to the new files.
|
||||
3. Update `CMakeLists.txt` to add the new source files to `cubicsdr_sources`.
|
||||
4. Build and verify no regressions.
|
||||
5. This is a low-risk change — the header stays the same, only the implementation is split across compilation units.
|
||||
|
||||
## Files to Create/Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `src/AppFrame_Menus.cpp` | Create |
|
||||
| `src/AppFrame_Handlers.cpp` | Create |
|
||||
| `src/AppFrame_Keyboard.cpp` | Create |
|
||||
| `src/AppFrame_Hamlib.cpp` | Create |
|
||||
| `src/AppFrame.cpp` | Remove moved methods |
|
||||
| `CMakeLists.txt` | Add new source files |
|
||||
@@ -0,0 +1,72 @@
|
||||
# Plan: Use Git Submodules for Vendored Dependencies
|
||||
|
||||
CubicSDR is a cross-platform Software-Defined Radio application (C++14, wxWidgets, OpenGL). This plan covers updating and potentially converting vendored third-party libraries to git submodules for better version tracking and upstream updates.
|
||||
|
||||
See also: [RECOMMENDATIONS.md](../RECOMMENDATIONS.md) | [PLAN.md](../PLAN.md)
|
||||
|
||||
**Last Updated:** 2026-07-23
|
||||
|
||||
## Current State
|
||||
|
||||
The `external/` directory contains full copies of 8 third-party libraries with no version tracking:
|
||||
|
||||
| Dependency | Vendored Version | Latest Upstream | Status |
|
||||
|-----------|-----------------|-----------------|--------|
|
||||
| lodepng | 20180819 | 20240326+ | Outdated ~6 years |
|
||||
| TinyXML | 2.6.2 | N/A (abandoned) | Replace with TinyXML-2 |
|
||||
| RtAudio | 5.2.0 | 6.0.1+ | 2 major versions behind |
|
||||
| CubicVR2 Math | ~2013 | N/A | Unmaintained |
|
||||
| RS-232 | 0.21 | 0.21 | Unmaintained since 2015 |
|
||||
| wglext | ~2013 | Registry-generated | Self-declared obsolete |
|
||||
| liquid-dsp | 1.5.0 | 1.7.0+ | 2 versions behind |
|
||||
| hamlib | 4.x (early) | 4.6+ | Outdated |
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
This is a large, risky change. Recommend doing it incrementally:
|
||||
|
||||
### Phase 1: Update actively-maintained dependencies
|
||||
|
||||
1. **lodepng**: Update to latest (20240326+). This is a drop-in update — same API, just newer version.
|
||||
2. **liquid-dsp**: Update to 1.7.0+. Requires testing DSP behavior changes.
|
||||
3. **RtAudio**: Evaluate upgrading to 6.0.1+ (major API changes) or staying on 5.x latest.
|
||||
4. **hamlib**: Update pre-built DLLs to latest 4.6+ release.
|
||||
|
||||
### Phase 2: Replace abandoned dependencies
|
||||
|
||||
5. **TinyXML → TinyXML-2**: Different API. Requires updating all XML serialization code in `DataTree.cpp` and `AppConfig.cpp`. This is a significant refactor.
|
||||
6. **wglext**: Replace with headers generated from the Khronos XML registry, or remove if not actually used.
|
||||
|
||||
### Phase 3: Convert to git submodules (optional)
|
||||
|
||||
7. For each dependency that has an active GitHub repo, convert to a git submodule:
|
||||
```bash
|
||||
git submodule add https://github.com/lvandeve/lodepng.git external/lodepng
|
||||
```
|
||||
8. Document the pinned commit/tag for each submodule.
|
||||
9. Update CI to initialize submodules: `git submodule update --init --recursive`.
|
||||
|
||||
### Phase 4: Evaluate removing unmaintained deps
|
||||
|
||||
10. **CubicVR2 Math**: If only used for basic vec2/vec3/mat4 operations, consider replacing with `glm` (widely used, actively maintained, header-only).
|
||||
11. **RS-232**: Evaluate `asio` or platform-native serial APIs as alternatives.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Action |
|
||||
|------|--------|
|
||||
| `external/` directory | Update/replace libraries |
|
||||
| `src/util/DataTree.cpp` | Update if migrating TinyXML → TinyXML-2 |
|
||||
| `src/AppConfig.cpp` | Update if migrating TinyXML → TinyXML-2 |
|
||||
| `CMakeLists.txt` | Update include paths, link targets |
|
||||
| `.gitmodules` | Create if using submodules |
|
||||
|
||||
## Rollback Strategy
|
||||
|
||||
Each dependency update should be done in a separate commit to enable granular rollback:
|
||||
|
||||
1. **Commit per dependency** — Update one library at a time (e.g., "Update lodepng to 20240326"). If a build breaks or DSP behavior changes, `git revert` that single commit without affecting other updates.
|
||||
2. **Pin before updating** — Record the current vendored version in the commit message before replacing. Example: "Update liquid-dsp from 1.5.0 to 1.7.0" makes the diff in `git log` self-documenting.
|
||||
3. **Build and smoke-test after each update** — Compile on all three platforms (or at least the primary dev platform) and run a quick manual smoke test (open a device, verify demodulation) before moving to the next dependency.
|
||||
4. **TinyXML-2 migration is the highest-risk step** — The API change affects `DataTree.cpp` and `AppConfig.cpp`. Consider doing this in a feature branch with CI builds on all platforms before merging. If the migration fails, revert the branch merge.
|
||||
5. **Submodule conversion (Phase 3) should be a single atomic commit** — Converting multiple libraries to submodules in separate commits creates confusing history. Do it all at once after all updates are stable.
|
||||
Reference in New Issue
Block a user