From 7f019cec4e25a418a7a13cce646e64ccde76bf0b Mon Sep 17 00:00:00 2001 From: Mark Allyn Date: Sun, 19 Apr 2026 08:39:43 -0700 Subject: [PATCH] Add security and reliability and add stubs for non essential controls --- CLAUDE.md | 22 ++++++++ DESIGN.md | 103 ++++++++++++++++++++++++++++++++++++- src/settings.h | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 258 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 98a7cc1..ddfc035 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -396,6 +396,16 @@ SUMMARY OF Controls: │ m │ Calibrator stretch │ │ │ ✓ │ │ │ │ └─────┴─────────────────────────────────────┴───────┴──────────┴──────────────┴────────────┴─────────┴─────┘ +Table for general controls not implimented on the keyboard in the table above: + +1. Intensity +2. Focus +3. Astignetism +4. Gain +5. rain clutter +6. water wave clutter +7. graticule light intensity + SUMMARY of target handling: The traffic cop handles anything that is coming from the simulator as well as the raspberry pi's @@ -413,6 +423,18 @@ It can run as a separate thread. It will not write data to anywhere except when CONTROLS +Every control listed above — both keyboard controls and the 7 general operator controls — +shall have a corresponding default constant in settings.h. This allows any startup value +to be changed at compile time without touching scope or rendering code. + +The 7 general controls (Intensity, Focus, Astigmatism, Gain, Rain Clutter, Wave Clutter, +Graticule Intensity) are physical encoder controls not yet purchased. Their placeholder +default constants are defined in settings.h. KnobPanel (Thread 3) will compile and run +from the start but will idle without ever acquiring Mutex A until real hardware is wired +in. SharedRenderState holds the default values unchanged; Thread 1 reads and applies them +every frame. No feature flags or conditional compilation are needed — the code path is +complete end-to-end, always at the compile-time default. + Things to note about the keyboard type controls. The letter on the keyboard are temporary. When I get around to making the operators panel, this all will go away. diff --git a/DESIGN.md b/DESIGN.md index f7ed757..b282b52 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -144,7 +144,7 @@ Shared PPI behavior: | `TargetBuffer` | 2,4 | Mutex B — target data handoff between Thread 2 (traffic cop) and Thread 4 (simulator) | | `TrafficCop` | 2 | Polls Simulator and RPi receivers each beam update; writes targets to SharedRenderState | | `Simulator` | 4 | Runs fake targets independently; returns data to TrafficCop when polled | -| `KnobPanel` | 3 | Future hardware stub — writes to SharedRenderState under Mutex A | +| `KnobPanel` | 3 | Future hardware stub — idles without acquiring Mutex A until Arduino hardware is wired; SharedRenderState holds compile-time defaults from settings.h | | `RPiReceiver` | 2 | Stub — one instance per Raspberry Pi; called by TrafficCop | --- @@ -198,6 +198,88 @@ shaders/ --- +## Robustness and Safety + +### Input Rate Limiting (the mad-child problem) +GLFW key callbacks can fire hundreds of times per second under key-mashing or hardware +encoder noise. Each control variable carries a `last_event_time` timestamp alongside +its value. Any input arriving within `MIN_INPUT_INTERVAL_MS` of the previous accepted +input for that control is silently discarded. The `KEY_MAX_STEP` / `KEY_GONIO_MAX_STEP` +constants cap how far a single accepted input can move a value. Together these make it +impossible to slam a bearing to an extreme in under a second regardless of how fast the +input fires. + +### Control Variable Clamping +Every state variable that a control modifies has companion `MIN_*` and `MAX_*` constants +in `settings.h`. The clamp is applied **at the point of write** using `std::clamp()`, in +every code path: keyboard callback, KnobPanel, and any future source. Thread 1 reads +already-clean values and applies a second `std::clamp()` before passing to the shader as +a second line of defense. No validation logic is needed at the shader boundary. + +### Incoming Data Validation +`RPiReceiver::parseFrame()` and `Simulator::poll()` both return `std::optional`. +A return of `std::nullopt` means the frame was malformed or out of bounds. `TrafficCop` +discards nullopt silently — no exception, no abort, no log spam. Fields validated: +bearing (0–360), range (0–radar max), amplitude (0–1), altitude (0–60,000 ft), timestamp +(not stale, not future), target count (truncated to `MAX_SIMULTANEOUS_TARGETS`), and +frame byte length (rejected above `MAX_RPI_FRAME_BYTES`). + +### Array Bounds Safety +- All fixed-size target storage uses `std::array` — never a raw C array. +- Array views passed between functions use `std::span` (C++20) — size always travels with the pointer. +- Ring buffers for phosphor persistence use modulo indexing (`index % CAPACITY`), never raw pointer arithmetic. +- `static_assert` guards validate that `MAX_TARGETS` and similar constants are within sane limits at compile time. +- Bounds-checked access (`.at()`) used in debug builds; validated index used in release. + +### Thread Safety Rules +1. **Snapshot pattern**: Thread 1 acquires Mutex A, copies the entire `SharedRenderState` struct, releases the lock immediately, then renders from the local copy. No OpenGL calls are ever made while holding a mutex (Core Guideline CP.22). +2. **Always `std::scoped_lock`**: Mutexes are never locked/unlocked manually. `std::scoped_lock` releases on all exit paths including exceptions (SEI CERT CON51). +3. **Lock ordering**: If code ever needs both Mutex A and Mutex B simultaneously, A must be acquired first. Documented here as a project-wide invariant to prevent deadlock (SEI CERT CON53). +4. **Atomic simple flags**: Boolean toggles that are written by one thread and read by another (`prf_high`, `goniometer_mode`) use `std::atomic` — no mutex overhead for single-variable reads. +5. **KnobPanel idle guarantee**: Until hardware is connected, KnobPanel's thread loop sleeps and never calls `lock()` on Mutex A. Zero contention on that mutex from Thread 3. + +### RAII Requirements +Every resource that is opened must be wrapped in an RAII holder so that it closes on any exit path: + +| Resource | RAII mechanism | +|---|---| +| OpenGL VAO / VBO / texture / shader | Thin `GLHandle` wrapper; destructor calls `glDelete*()` | +| GLFW window | `unique_ptr` | +| FreeType `FT_Library` / `FT_Face` | Small RAII structs; destructor calls `FT_Done_*` | +| Mutex locks | `std::scoped_lock` or `std::lock_guard` — never bare lock/unlock | +| Worker threads | `std::jthread` (C++20) — auto-joins on destruction; no detached threads | +| File handles (shader source) | `std::ifstream` — RAII by default | + +No raw `new` or `delete` anywhere in the codebase. No `malloc`/`free`. + +### SEI CERT C++ Compliance — Key Rules +| Rule | Enforcement | +|---|---| +| CON50 | Never destroy a mutex while locked — `std::scoped_lock` makes this structurally impossible | +| CON51 | Release locks on exceptions — `std::scoped_lock` handles automatically | +| CON53 | Consistent lock ordering (A before B) — documented invariant | +| ARR50 | Array indices always validated before use or bounded by `std::array::at()` | +| MEM51 | All resources managed by RAII wrappers; no raw `new`/`delete` | +| ERR50 | No `abort()` or `exit()` in normal operation; bad data is dropped, not fatal | +| INT30 | Bearing arithmetic uses `fmod()`, not unsigned wraparound subtraction | +| FLP30 | No floating-point loop counters; sweep angle is a double accumulator | +| OOP50 | No virtual method calls in constructors or destructors | + +### C++ Core Guidelines Compliance — Key Rules +| Guideline | Enforcement | +|---|---| +| I.6 / I.7 | `Expects()` / `Ensures()` (GSL) at function entry/exit in debug builds | +| I.12 | `gsl::not_null` for the active scope pointer in ScopeManager | +| R.1 | All resource management through RAII handles (see table above) | +| CP.20 | `std::scoped_lock` for all mutex acquisition — no bare lock/unlock | +| CP.21 | `std::scoped_lock(mutexA, mutexB)` if both must be held simultaneously | +| CP.22 | No OpenGL calls while holding any mutex — snapshot pattern enforces this | +| Bounds.1 | No pointer arithmetic; `std::span` for array views | +| Bounds.2 | Array access only via `.at()` or pre-validated index | +| Bounds.4 | No `sprintf`, `strcpy`, `gets`; use `std::string` and `std::format` (C++20) | + +--- + ## Key Design Notes 1. **ScopeManager** sits in Thread 1 and holds the active scope pointer. The GLFW @@ -237,8 +319,25 @@ shaders/ file that needs a tunable value includes this file. No magic numbers anywhere else in the codebase. Add candidate variables here before coding begins; values can be refined during debugging and appearance work. + Sections defined: phosphor colors (P1/P7), sweep parameters, graticule + geometry and colors, cursor, noise floor, graticule swap animation timing, + key-hold acceleration, auto-advance timer, window/layout geometry, text + colors/sizes, **general operator control defaults** (Intensity, Focus, + Astigmatism, Gain, Rain Clutter, Wave Clutter, Graticule Intensity), + **per-scope default state** (initial bearing, range, cursor, calibrator + scale for each scope), and **PAR geometry** (non-linear scale breakpoint, + runway heading, glide path angle, scan widths). -10. **MarineAScope graticule swap** is a state machine with four states: +10. **General operator controls** (Intensity, Focus, Astigmatism, Gain, Rain + Clutter, Wave Clutter, Graticule Intensity) are placeholders for physical + encoders not yet purchased. Each has a `DEFAULT_*` constant in settings.h + and a corresponding variable in `SharedRenderState`. KnobPanel (Thread 3) + has a stub write path for each. The thread starts and idles, but never + calls `lock()` on Mutex A, so it imposes zero contention. When hardware + arrives, only KnobPanel changes — Thread 1 and the shaders are already + fully wired to consume the values. + +11. **MarineAScope graticule swap** is a state machine with four states: NORMAL → SLIDING_OUT → BARE_CRT → SLIDING_IN → NORMAL. The u and d keys are blocked during the animation. The new range value is latched when the key is pressed but not applied to the scope state until SLIDING_IN completes. diff --git a/src/settings.h b/src/settings.h index 39b2d47..46a87e8 100644 --- a/src/settings.h +++ b/src/settings.h @@ -179,3 +179,138 @@ * GRATICULE_LABEL_COLOR_G Green component * GRATICULE_LABEL_COLOR_B Blue component */ + +/* + * ================================================================ + * INPUT RATE LIMITING + * ================================================================ + * Minimum milliseconds between accepted inputs for any single + * control. Inputs arriving faster than this are silently discarded. + * Protects against key-mashing and encoder bounce. + * + * MIN_INPUT_INTERVAL_MS General controls (bearing, range, cursor) + * MIN_INPUT_INTERVAL_GONIO_MS Goniometer — kept slower for precise null hunting + */ + +/* + * ================================================================ + * GENERAL OPERATOR CONTROLS — Default, Minimum, and Maximum Values + * ================================================================ + * These controls will eventually be driven by physical encoders + * wired through KnobPanel (Thread 3). Until hardware is installed, + * KnobPanel idles without ever acquiring Mutex A, and + * SharedRenderState holds these defaults unchanged. Thread 1 reads + * and applies them every frame regardless of source. + * + * Clamping is applied at the point of write using std::clamp(). + * All values normalized 0.0–1.0 unless noted otherwise. + * + * DEFAULT_INTENSITY CRT beam intensity + * MIN_INTENSITY / MAX_INTENSITY + * + * DEFAULT_FOCUS CRT focus sharpness + * MIN_FOCUS / MAX_FOCUS + * + * DEFAULT_ASTIGMATISM CRT astigmatism correction offset + * MIN_ASTIGMATISM / MAX_ASTIGMATISM + * + * DEFAULT_GAIN Receiver gain — scales target return amplitude + * MIN_GAIN / MAX_GAIN + * + * DEFAULT_RAIN_CLUTTER Rain clutter suppression (0 = full clutter shown) + * MIN_RAIN_CLUTTER / MAX_RAIN_CLUTTER + * + * DEFAULT_WAVE_CLUTTER Sea/wave clutter suppression level + * MIN_WAVE_CLUTTER / MAX_WAVE_CLUTTER + * + * DEFAULT_GRATICULE_INTENSITY Incandescent backlight brightness of graticule glass + * MIN_GRATICULE_INTENSITY / MAX_GRATICULE_INTENSITY + */ + +/* + * ================================================================ + * SCOPE DEFAULT STATE — Initial control values at startup + * ================================================================ + * Each scope initializes its runtime state from these constants. + * Change a value here and recompile to adjust the startup condition + * without touching scope implementation code. + * + * MIN_*/MAX_* companions are the hard clamp limits applied at every + * write — keyboard, KnobPanel, or any other source. + * + * --- Marine A-Scope --- + * MARINE_A_DEFAULT_BEARING_DEG Initial antenna bearing, degrees True + * MARINE_A_MIN_BEARING_DEG (0.0) + * MARINE_A_MAX_BEARING_DEG (359.9) + * MARINE_A_DEFAULT_MAX_RANGE_MI Initial max range index (0=2mi, 1=4mi, 2=6mi) + * + * --- Chain Home A-Scope --- + * CHAIN_HOME_DEFAULT_GONIO_AZ_DEG Initial goniometer azimuth, degrees + * CHAIN_HOME_MIN_GONIO_AZ_DEG (0.0) + * CHAIN_HOME_MAX_GONIO_AZ_DEG (359.9) + * CHAIN_HOME_DEFAULT_GONIO_EL_DEG Initial goniometer elevation, degrees + * CHAIN_HOME_MIN_GONIO_EL_DEG (0.0) + * CHAIN_HOME_MAX_GONIO_EL_DEG (90.0) + * CHAIN_HOME_DEFAULT_PRF_HIGH true = 25 Hz, false = 12.5 Hz + * CHAIN_HOME_DEFAULT_CALIBRATOR Initial trace scale factor (1.0 = nominal) + * CHAIN_HOME_MIN_CALIBRATOR Minimum scale (prevents trace from collapsing) + * CHAIN_HOME_MAX_CALIBRATOR Maximum scale (prevents trace from stretching off screen) + * + * --- Marine PPI Scope --- + * MARINE_PPI_DEFAULT_MAX_RANGE_MI Initial max range index (0=2mi, 1=4mi, 2=6mi) + * MARINE_PPI_DEFAULT_CURSOR_BRG Initial cursor bearing, degrees True + * MARINE_PPI_DEFAULT_CURSOR_RNG Initial cursor range in miles + * MARINE_PPI_DEFAULT_BEARING_OFFSET Initial antenna bearing offset (0 = True North) + * MARINE_PPI_MIN_BEARING_OFFSET (-180.0) + * MARINE_PPI_MAX_BEARING_OFFSET (180.0) + * + * --- ATC PPI Scope --- + * ATC_PPI_DEFAULT_MAX_RANGE_MI Initial max range index (0=5mi, 1=10mi, 2=15mi, 3=20mi) + * ATC_PPI_DEFAULT_CURSOR_BRG Initial cursor bearing, degrees True + * ATC_PPI_DEFAULT_CURSOR_RNG Initial cursor range in miles + * ATC_PPI_DEFAULT_BEARING_OFFSET Initial antenna bearing offset (0 = True North) + * ATC_PPI_MIN_BEARING_OFFSET (-180.0) + * ATC_PPI_MAX_BEARING_OFFSET (180.0) + */ + +/* + * ================================================================ + * INCOMING DATA VALIDATION LIMITS + * ================================================================ + * Applied in RPiReceiver::parseFrame() and Simulator::poll(). + * Any target whose fields fall outside these bounds is discarded + * (returns std::nullopt). The exhibit continues; no crash, no assert. + * + * TARGET_MAX_BEARING_DEG Upper bound for target bearing (360.0) + * TARGET_MIN_BEARING_DEG Lower bound for target bearing (0.0) + * TARGET_MAX_RANGE_MI Hard ceiling; per-scope max is enforced separately + * TARGET_MIN_RANGE_MI Must be non-negative (0.0) + * TARGET_MAX_AMPLITUDE Maximum normalized signal amplitude (1.0) + * TARGET_MIN_AMPLITUDE Minimum (0.0) + * TARGET_MAX_ALTITUDE_FT Sanity ceiling for ATC/PAR targets (60000.0) + * TARGET_MIN_ALTITUDE_FT (0.0) + * TARGET_MAX_STALE_SEC Discard targets older than this many seconds + * + * MAX_SIMULTANEOUS_TARGETS Hard array size cap; frames claiming more + * targets than this are truncated, not rejected + * MAX_RPI_FRAME_BYTES Maximum accepted frame size from any RPi; + * larger frames are discarded as malformed + */ + +/* + * ================================================================ + * PAR GEOMETRY + * ================================================================ + * Constants for the Precision Approach Radar non-linear display + * scale and runway geometry at BLI Runway 16/34. + * + * PAR_NONLINEAR_BREAKPOINT_MI Range at which non-linear scale transitions (5.0) + * Inner 0–5 miles occupies PAR_NONLINEAR_INNER_FRAC + * of horizontal width; outer 5–10 miles fills the rest + * PAR_NONLINEAR_INNER_FRAC Fraction of display width used by inner 5 miles (0.70) + * + * PAR_RUNWAY_HEADING_DEG True heading of active runway 34 approach (340.0) + * PAR_GLIDE_PATH_ANGLE_DEG Standard glide path angle in degrees (3.0) + * PAR_AZIMUTH_SCAN_WIDTH_DEG Total lateral scan width, degrees either side (10.0) + * PAR_ELEVATION_SCAN_MAX_DEG Upper elevation limit of elevation scan (7.0) + */