diff --git a/data_acq_and_processing/common_cpp/ipc/src/shared_types.cpp b/data_acq_and_processing/common_cpp/ipc/src/shared_types.cpp index 00408fe..fa45072 100644 --- a/data_acq_and_processing/common_cpp/ipc/src/shared_types.cpp +++ b/data_acq_and_processing/common_cpp/ipc/src/shared_types.cpp @@ -127,12 +127,26 @@ void write_trace_block(BinaryWriter& writer, const SweepTraceBlock& trace) { } } +// Guard a wire-supplied element count before reserve(): a torn ring slot or buggy +// producer can present a count near 2^32, and reserve() of that many elements would +// request gigabytes and crash the process (length_error/bad_alloc/OOM-kill). The +// payload itself is already bounded by the ring slot size, so any count whose +// minimum encoding cannot fit in the bytes still available is structurally invalid. +void require_count_fits(std::uint32_t count, std::size_t min_bytes_each, BinaryReader& reader) { + if (min_bytes_each != 0U + && static_cast(count) * min_bytes_each > reader.remaining_bytes()) { + throw std::runtime_error("Declared element count exceeds remaining payload bytes"); + } +} + [[nodiscard]] auto read_trace_block(BinaryReader& reader) -> SweepTraceBlock { SweepTraceBlock trace{}; trace.combo.input_pos = reader.read(); trace.combo.output_pos = reader.read(); const auto point_count = reader.read(); + // Each point encodes frequency (4B) + s11 (8B) + s21 (8B) = 20 bytes. + require_count_fits(point_count, 20U, reader); trace.frequency_hz.reserve(point_count); trace.s11.reserve(point_count); trace.s21.reserve(point_count); @@ -183,6 +197,8 @@ void write_trace_collection(BinaryWriter& writer, std::uint32_t magic, const Raw collection.monotonic_ns = reader.read(); const auto trace_count = reader.read(); + // Each trace block is at least 12 bytes (combo 8B + point_count 4B, zero points). + require_count_fits(trace_count, 12U, reader); collection.traces.reserve(trace_count); for (std::uint32_t index = 0; index < trace_count; ++index) { collection.traces.push_back(read_trace_block(reader)); diff --git a/data_acq_and_processing/preprocessing/data_preprocessor/src/data_preprocessor.cpp b/data_acq_and_processing/preprocessing/data_preprocessor/src/data_preprocessor.cpp index e7fbe8b..8aa84de 100644 --- a/data_acq_and_processing/preprocessing/data_preprocessor/src/data_preprocessor.cpp +++ b/data_acq_and_processing/preprocessing/data_preprocessor/src/data_preprocessor.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include #include #include @@ -138,13 +140,24 @@ void DataPreprocessor::run(const std::atomic& stop_requested) { std::vector serialized_raw{}; ipc::RawSweepCollection raw_collection{}; + std::uint64_t error_count = 0; while (!stop_requested.load(std::memory_order_relaxed)) { - if (!try_pop_raw_collection(serialized_raw, &raw_collection)) { - continue; + try { + if (!try_pop_raw_collection(serialized_raw, &raw_collection)) { + continue; + } + const auto preprocessed_collection = preprocess_collection(raw_collection); + publish_preprocessed_collection(preprocessed_collection); + } catch (const std::exception& exc) { + // A single malformed or transiently-bad collection must not kill the + // long-running preprocessor: drop it and keep serving the next sweep. + // Logging is throttled so a persistent error cannot flood the log. + if (error_count % 100 == 0) { + std::cerr << "data_preprocessor: dropped collection after error (count=" + << (error_count + 1) << "): " << exc.what() << '\n'; + } + ++error_count; } - - const auto preprocessed_collection = preprocess_collection(raw_collection); - publish_preprocessed_collection(preprocessed_collection); } } diff --git a/data_acq_and_processing/processing/data_processor/src/data_processor.cpp b/data_acq_and_processing/processing/data_processor/src/data_processor.cpp index f8d95ff..ffe4c59 100644 --- a/data_acq_and_processing/processing/data_processor/src/data_processor.cpp +++ b/data_acq_and_processing/processing/data_processor/src/data_processor.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -61,71 +62,84 @@ void DataProcessor::run(const std::atomic& stop_requested) { const std::size_t history_limit = replay_history_limit(config_); std::uint64_t last_replayed_revision = live_config_loader_.revision(); std::uint64_t last_applied_history_command_seq = 0; + std::uint64_t error_count = 0; while (!stop_requested.load(std::memory_order_relaxed)) { - const auto live_config_raw = live_config_loader_.refresh_if_needed(); - const auto live_config = resolve_effective_live_config(live_config_raw); - const auto live_revision = live_config_loader_.revision(); - auto& processor = resolve_processor(live_config); + try { + const auto live_config_raw = live_config_loader_.refresh_if_needed(); + const auto live_config = resolve_effective_live_config(live_config_raw); + const auto live_revision = live_config_loader_.revision(); + auto& processor = resolve_processor(live_config); - if (live_revision != last_replayed_revision) { - if (live_config.history_command_seq > last_applied_history_command_seq) { - if (live_config.history_command == HistoryCommand::RemoveLast) { - if (!preprocessed_history.empty()) { - preprocessed_history.pop_back(); + if (live_revision != last_replayed_revision) { + if (live_config.history_command_seq > last_applied_history_command_seq) { + if (live_config.history_command == HistoryCommand::RemoveLast) { + if (!preprocessed_history.empty()) { + preprocessed_history.pop_back(); + } + } else if (live_config.history_command == HistoryCommand::ClearAll) { + preprocessed_history.clear(); } - } else if (live_config.history_command == HistoryCommand::ClearAll) { - preprocessed_history.clear(); + last_applied_history_command_seq = live_config.history_command_seq; } - last_applied_history_command_seq = live_config.history_command_seq; - } - if (!live_config.reprocess_current_result) { - // Socket-fed speed updates should affect only future preprocessed collections, - // not replay the current history entry. - } else if (should_replay_entire_history(live_config)) { - for (std::size_t index = 0; index < preprocessed_history.size(); ++index) { + if (!live_config.reprocess_current_result) { + // Socket-fed speed updates should affect only future preprocessed collections, + // not replay the current history entry. + } else if (should_replay_entire_history(live_config)) { + for (std::size_t index = 0; index < preprocessed_history.size(); ++index) { + const auto replay_result = process_collection( + preprocessed_history[index], + std::span(preprocessed_history.data(), index), + processor, + live_config + ); + publish_result_collection(replay_result, results_ring_); + publish_locator(replay_result, live_config); + } + } else if (!preprocessed_history.empty()) { const auto replay_result = process_collection( - preprocessed_history[index], - std::span(preprocessed_history.data(), index), + preprocessed_history.back(), + std::span(preprocessed_history.data(), preprocessed_history.size() - 1U), processor, live_config ); publish_result_collection(replay_result, results_ring_); publish_locator(replay_result, live_config); } - } else if (!preprocessed_history.empty()) { - const auto replay_result = process_collection( + last_replayed_revision = live_revision; + } + + if (preprocessed_ring_.pop(bytes)) { + auto preprocessed = ipc::deserialize_preprocessed_collection(bytes); + preprocessed_history.push_back(std::move(preprocessed)); + while (preprocessed_history.size() > history_limit) { + preprocessed_history.erase(preprocessed_history.begin()); + } + + const auto result_collection = process_collection( preprocessed_history.back(), std::span(preprocessed_history.data(), preprocessed_history.size() - 1U), processor, live_config ); - publish_result_collection(replay_result, results_ring_); - publish_locator(replay_result, live_config); - } - last_replayed_revision = live_revision; - } - - if (preprocessed_ring_.pop(bytes)) { - auto preprocessed = ipc::deserialize_preprocessed_collection(bytes); - preprocessed_history.push_back(std::move(preprocessed)); - while (preprocessed_history.size() > history_limit) { - preprocessed_history.erase(preprocessed_history.begin()); + publish_result_collection(result_collection, results_ring_); + publish_locator(result_collection, live_config); + continue; } - const auto result_collection = process_collection( - preprocessed_history.back(), - std::span(preprocessed_history.data(), preprocessed_history.size() - 1U), - processor, - live_config - ); - publish_result_collection(result_collection, results_ring_); - publish_locator(result_collection, live_config); - continue; + std::this_thread::sleep_for(std::chrono::milliseconds(config_.runtime.idle_sleep_ms)); + } catch (const std::exception& exc) { + // A single bad collection (torn ring slot, decode/processing error) must + // not kill the long-running processor: drop it and keep going. Throttle + // logging and pause briefly so a persistent error cannot busy-spin/flood. + if (error_count % 100 == 0) { + std::cerr << "data_processor: dropped collection after error (count=" + << (error_count + 1) << "): " << exc.what() << '\n'; + } + ++error_count; + std::this_thread::sleep_for(std::chrono::milliseconds(config_.runtime.idle_sleep_ms)); } - - std::this_thread::sleep_for(std::chrono::milliseconds(config_.runtime.idle_sleep_ms)); } } diff --git a/data_acq_and_processing/processing/locator/src/tcp_server.cpp b/data_acq_and_processing/processing/locator/src/tcp_server.cpp index e8f119c..e7fa61f 100644 --- a/data_acq_and_processing/processing/locator/src/tcp_server.cpp +++ b/data_acq_and_processing/processing/locator/src/tcp_server.cpp @@ -131,9 +131,15 @@ ClientQueue::ClientQueue(std::size_t capacity) : capacity_(std::max auto ClientQueue::try_push(std::vector packet) -> bool { { std::lock_guard guard(mutex_); - if (closed_ || queue_.size() >= capacity_) { + if (closed_) { return false; } + // Latest-wins backpressure: never block or disconnect a slow client. When the + // queue is full, drop the oldest queued packet(s) so the client always advances + // toward the freshest result. Bounded memory; freshness over completeness. + while (queue_.size() >= capacity_) { + queue_.pop_front(); + } queue_.push_back(std::move(packet)); } not_empty_.notify_one(); @@ -195,10 +201,9 @@ void ClientSession::enqueue(std::vector packet) { if (stop_requested_.load(std::memory_order_acquire)) { return; } - if (!queue_.try_push(std::move(packet))) { - log_warning("disconnecting client " + peer_name_ + " after outbound queue overflow"); - request_stop(); - } + // try_push only fails when the queue is closed (session already shutting down); a + // full queue now drops its oldest entry instead of disconnecting a slow client. + (void)queue_.try_push(std::move(packet)); } void ClientSession::request_stop() { @@ -424,7 +429,15 @@ void TcpServer::acceptor_loop() { &peer_len ); if (client_fd < 0) { - if (errno == EINTR) { + if (errno == EINTR || errno == ECONNABORTED) { + continue; + } + if (errno == EMFILE || errno == ENFILE || errno == ENOBUFS || errno == ENOMEM) { + // Transient resource exhaustion (often our own finished sessions + // still holding fds): reap them, back off briefly, and keep + // accepting. The acceptor must never die and silently stop serving. + reap_finished_clients(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); continue; } // Listening socket closed during shutdown produces EBADF/EINVAL; bail. diff --git a/deploy/install-daemon.sh b/deploy/install-daemon.sh new file mode 100755 index 0000000..d6c522d --- /dev/null +++ b/deploy/install-daemon.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# Install and enable the radar headless daemon (systemd system service), plus a +# passwordless sudoers rule that lets the service user stop/start it (start.sh +# stops the daemon before launching the interactive GUI). +# +# Run once as root, from anywhere inside the project: +# sudo bash deploy/install-daemon.sh +# +# Re-running is safe: it overwrites the unit/sudoers with current paths. +set -euo pipefail + +if [[ "${EUID}" -ne 0 ]]; then + echo "Must run as root: sudo bash $0" >&2 + exit 1 +fi + +PROJECT_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +SERVICE_USER="${SUDO_USER:-root}" +START_SH="${PROJECT_ROOT}/start.sh" +SERVICE_NAME="radar.service" +UNIT_PATH="/etc/systemd/system/${SERVICE_NAME}" +SUDOERS_PATH="/etc/sudoers.d/radar" +SYSTEMCTL="$(command -v systemctl)" + +if [[ -z "${SYSTEMCTL}" ]]; then + echo "systemctl not found; this installer requires systemd." >&2 + exit 1 +fi +if [[ ! -x "${START_SH}" ]]; then + echo "start.sh not found or not executable: ${START_SH}" >&2 + exit 1 +fi +if [[ "${SERVICE_USER}" == "root" ]]; then + echo "Run via sudo from your normal login user (got SUDO_USER=root)." >&2 + echo "Example: sudo bash deploy/install-daemon.sh" >&2 + exit 1 +fi + +echo "[install] project_root = ${PROJECT_ROOT}" +echo "[install] service_user = ${SERVICE_USER}" + +# --- systemd unit ----------------------------------------------------------- +# No network-online dependency: the appliance runs offline and the radar is on +# USB/loopback. Restart=on-failure self-heals crashes but leaves an intentional +# `systemctl stop` (done by start.sh for GUI handover) deactivated. +cat > "${UNIT_PATH}" < "${TMP_SUDO}" <mock latching in hardware deployments. +- Disk/fd/thread exhaustion over days of uptime: child stdout/stderr logs grow with no rotation (can fill the SD card and corrupt everything), partial-open in MultiDeviceVnaController leaks a libusb context + RX thread on every forever-retry, and locator client sessions are only reaped on new accept() so flapping clients leak fds+threads. Cap/rotate logs, free partial opens, and reap sessions from publish(). +- Stale /dev/shm and orphaned children defeat restart: no production code ever unlinks rings, so a geometry change wedges the pipeline in a boot loop (C++ throws 'geometry mismatch') while Python silently truncates and diverges; a kill -9'd GUI orphans C++ children that keep holding the radar/port/rings and the next start spawns a conflicting second set. Clean rings on headless start and reap pre-existing pipeline processes via pidfile/process-group. + +## Issues (ranked) + +### #1 [HIGH] Crashed C++ pipeline child is never restarted and the failure is invisible; systemd Restart never fires because the GUI parent stays alive +- **subsystem:** py_orchestration / cross_cutting +- **location:** python_app/orchestration/process_supervisor.py:289-317; python_app/gui/controllers/app_window_pipeline_mixin.py:289 +- **impact:** collect_exit_reports() is the only place a child death is observed; on exit it logs one ERROR, sets status='error', and pops the handle from self._processes. There is no watchdog and no respawn anywhere. If sweep_orchestrator/data_preprocessor/data_processor crashes (device hiccup, OOM, segfault) acquisition/processing never resume. systemd Restart=on-failure is on the parent only, and the parent stays healthy, so it never fires. On a headless Pi the appliance silently produces no data until the next reboot. Merges PS-001 and the cross_cutting child-crash finding. +- **fix:** In headless mode treat an unexpected child exit as recoverable: from _poll_rings respawn the crashed stage with bounded retry/backoff (track restart counts per name); after exhausting retries either expose a hard 'pipeline degraded' state or os._exit(non-zero) after logging to stderr so systemd Restart=on-failure performs a clean full recovery. Add a heartbeat that detects 'expected running but all acquisition children dead'. + +### #2 [CRITICAL] Headless auto-start failures are swallowed (exit code stays 0), leaving an idle zombie daemon that never auto-restarts +- **subsystem:** deploy_daemon +- **location:** python_app/gui/app_window.py:503 (and :490); python_app/gui/controllers/app_window_pipeline_mixin.py:151-155; app_window.py:289-292 +- **impact:** In headless mode the auto-start chain catches Exception and calls _show_exception/_show_error, both of which return immediately when RADAR_SYSTEM_HEADLESS=1. The Qt loop keeps running and the process exit code stays 0. So a missing device at boot, a busy SHM ring, or a producer that fails to spawn leaves the daemon alive doing nothing; systemd sees a healthy Type=simple process and Restart=on-failure NEVER fires. The appliance silently produces no data with no self-healing. +- **fix:** In headless mode propagate fatal auto-start/apply failures into a non-zero process exit (QApplication.exit(1) / os._exit(1) after logging to stderr) so systemd restarts the unit. Add a watchdog: if the pipeline is not producing data within N seconds of headless auto-start, exit non-zero so Restart=on-failure self-heals. + +### #3 [CRITICAL] Per-collection exception in the preprocessor run loop crashes the daemon with no auto-respawn +- **subsystem:** cpp_preprocess +- **location:** data_acq_and_processing/preprocessing/data_preprocessor/src/data_preprocessor.cpp:136-149 +- **impact:** run() calls try_pop_raw_collection -> deserialize_raw_collection, preprocess_collection, publish_preprocessed_collection with no try/catch around the per-collection body. Any of these throws on normal-but-imperfect input (torn/stale ring slot magic/length errors, S21/S11 axis or point-count mismatch, slot-too-small, bad_alloc). The throw unwinds to main()'s catch (main.cpp:90) -> exit 1, and the supervisor does not respawn it (see rank 1). One malformed or transiently-mismatched collection permanently stops all preprocessing and every downstream result. +- **fix:** Wrap the per-iteration body (pop+deserialize, preprocess, publish) in try/catch inside run(); on std::exception log (with collection_id when available) and continue, treating one bad collection as a recoverable drop. Keep only truly unrecoverable conditions (ring not open) fatal. Optionally add a consecutive-failure counter that exits only past a threshold. + +### #4 [MEDIUM] Unbounded reserve() on a wire-supplied u32 count during raw deserialize causes OOM crash +- **subsystem:** cpp_preprocess +- **location:** data_acq_and_processing/common_cpp/ipc/src/shared_types.cpp:136-138 (and :186) +- **impact:** read_trace_collection/read_trace_block do reserve(trace_count) and reserve(point_count) using raw uint32 counts read straight from the wire BEFORE any bytes-available check. A torn/corrupt ring slot or buggy producer can present a count near 2^32; reserve(4e9) of vector requests ~32GB and throws length_error/bad_alloc or trips the OOM killer on a 1-4GB Pi, killing the daemon (compounds rank 3). The payload_size<=slot_size_bytes check bounds the buffer but not the declared element count. +- **fix:** Before reserving, cap counts against reader.remaining_bytes(): require point_count*bytes_per_point (>=12B/point: 4 freq + 8 complex) <= remaining_bytes() and trace_count <= remaining_bytes()/min_trace_bytes; throw a descriptive runtime_error if exceeded (then caught by rank-3's loop guard) rather than reserving blindly. + +### #5 [HIGH] push() torn-write race: reader copies a slot the producer is mid-overwriting; no post-copy sequence re-check (C++ pop and Python reader) +- **subsystem:** ipc_shm / py_orchestration +- **location:** data_acq_and_processing/common_cpp/ipc/src/shm_ring.cpp:303 (pop check l.325); python_app/orchestration/shm/ring_reader.py:46-65 +- **impact:** push() writes payload_size, sets slot->sequence = write_seq+1, THEN memcpy's the payload, and only afterward publishes write_seq. A reader sitting on the same physical slot index can observe the NEW sequence (passing the sequence==read_seq+1 check) yet copy a mix of old+new payload bytes. The single fence between memcpy and write_seq.store does not protect a reader already inside the slot, and neither the C++ pop nor the Python ShmRingReader re-validates the sequence after copying. On a Pi where the C++ producer outruns the 50ms GUI poll, a lapping producer yields torn payloads that crash/garble deserialize/decode. Merges SHM-001 and the Python ring-reader race. +- **fix:** Seqlock-style publish: write payload+size FIRST, then publish slot->sequence with a release store; readers re-read the slot sequence AFTER copying (acquire) and discard+resync if it changed or if the writer advanced past read_seq+capacity. Equivalently use a per-slot odd/even generation counter. Also sanity-bound payload_size <= slot_size_bytes in the Python reader before slicing. + +### #6 [HIGH] backend_mode='auto' silently and permanently latches to synthetic mock data when the device is absent +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/multi_device_service.py:79-83 (open), :69/:109 (latch) +- **impact:** When MultiDeviceLibreVnaService.open() fails in 'auto' mode it sets _using_mock_backend=True and swallows the error; the matrix factory passes backend_mode=config.radar.driver_mode, so driver_mode='auto' makes _open_radar_with_retry succeed immediately with synthetic data and never enter the wait-for-device loop. The flag latches permanently (open/recover short-circuit), so even after the real VNA is plugged in the service emits fabricated S-parameters forever. A headless box that boots before the VNA is connected silently records/serves completely fake radar with no operator-visible error. +- **fix:** Do not let 'auto' fall back to mock for a hardware producer meant to wait for the device. Either require driver_mode in {native,mock} for matrix producers (reject auto), or on auto-fallback emit a loud throttled WARNING and re-attempt native on every open() without latching, or treat native open failure as retryable so _open_radar_with_retry keeps waiting. + +### #7 [HIGH] Native LibreVNA sweep uses a single 1500ms deadline for the entire multi-point sweep, guaranteeing timeouts and reconnect churn on large sweeps +- **subsystem:** cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/device_drivers/radar/librevna/librevna_minimal_driver_lifecycle.cpp:285 (used at :290) +- **impact:** acquire_native computes deadline = now + 1500ms once before the receive loop and reuses it across all sweep.points. Real dwell is ~points/IFBW seconds (e.g. 1001 points @1kHz IFBW ~= 1s + USB latency); at low IFBW/high points the sweep exceeds 1500ms. Once the shared deadline passes mid-sweep, wait_for_packet throws 'Timeout' (retryable), so acquire_sweep tears down and reconnects (lifecycle.cpp:153-160), retries the same too-short window up to 3 times, then rethrows -> no collection published and process exits 1. Native acquisition is effectively broken for any sweep longer than 1.5s, manifesting as reconnect churn then a crash. +- **fix:** Derive the deadline from configured sweep size (base + points/IFBW + margin) or extend it as progress is made (advance on each new datapoint, with an overall hard cap and a per-gap stall timeout). Never share one fixed wall-clock budget across an unbounded number of points. + +### #8 [HIGH] Transient device read/decode errors crash sweep_orchestrator with exit 1 and it is never restarted +- **subsystem:** cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/src/sweep_orchestrator.cpp:127 (main catch main.cpp:165-168) +- **impact:** run() lets any exception propagate to main -> exit 1. Many recoverable-in-spirit faults are fatal and not in the retryable list: a single corrupted USB frame ('Invalid LibreVNA packet CRC', transport.cpp:333); a cable replug yielding a retryable bulk error followed by non-retryable 'No compatible LibreVNA USB device found' on reconnect (transport.cpp:122); a K209 socket timeout treated as a hard read failure. The supervisor never respawns sweep_orchestrator (see rank 1), so any transient device fault permanently stops acquisition on the headless box. +- **fix:** Add a bounded backoff-based supervised retry around the acquisition loop that, on recoverable device errors (timeouts, NACK, transient USB/socket, transient device-not-found after replug), closes/reopens drivers and continues in continuous mode rather than exiting. Reserve exit-1 for genuinely fatal/config errors; distinguish exit codes and/or enable supervisor respawn for the orchestrator. + +### #9 [MEDIUM] C++ sweep_orchestrator open_all() has no wait-for-device retry; absent device at boot kills the daemon +- **subsystem:** py_hardware / cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/src/sweep_orchestrator.cpp:111-114 (open path lifecycle.cpp:102-114) +- **impact:** run() calls lifecycle_guard.open_all() exactly once; LibreVnaMinimalDriver::open() calls open_native() once and throws on failure with no retry, unwinding to main -> exit 1. For the 'librevna' and 'compact_m_k209' C++ paths, a device absent at boot (the normal Pi cold-boot race) terminates acquisition immediately. The native acquire path has only a small bounded in-loop reconnect (no unbounded wait), so a device unplugged longer than kNativeAcquireMaxAttempts also exits. This is the same robustness asymmetry the Python matrix producer was fixed to avoid. +- **fix:** Add a wait-for-device retry around open_all() mirroring matrix_raw_producer._open_radar_with_retry: loop open_all() with capped exponential backoff while !should_stop(stop_requested), logging throttled failures, proceeding only once open succeeds. Extend native acquire reconnect to keep retrying (interruptible by stop_requested) in continuous mode instead of giving up after kNativeAcquireMaxAttempts. + +### #10 [HIGH] kamil_adc producer has no open/reconnect retry and silently exits 0 on a partial sweep +- **subsystem:** py_hardware / cross_cutting +- **location:** python_app/scripts/kamil_adc_raw_producer.py:59-90 (acquire :79, partial-sweep break :89-90) +- **impact:** Unlike matrix_raw_producer, this opens radar/switches once with no retry; KamilAdcService.open() failure (collector not ready, TTY not created within startup_timeout_s, USB CDC-ACM not yet enumerated) propagates and the process exits. In the loop, radar.acquire() raising a TTY-closed/process-exited RuntimeError on a USB unplug or collector death is not caught, so one transient hiccup terminates the producer with no reconnect. Worse, len(traces)!=len(combos) breaks the loop and returns 0 even when stop was NOT requested, silently stopping continuous acquisition with no log. As a supervised child, systemd cannot restart it. Merges KAMIL-NO-WAIT and the cross_cutting kamil finding. +- **fix:** Mirror the matrix producer: wrap open()+acquire/switch in a reconnect-forever loop with capped backoff, interruptible by stop_requested (relaunch collector+reader on failure). Replace the unconditional break on incomplete traces with a check that exits only when stop_requested is set, otherwise reconnect+log. + +### #11 [HIGH] MultiDeviceVnaController partial-open leaks the master USB handle + RX thread on every failed open, unbounded under retry-forever +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/librevna_multi_device_driver/controller.py:48-57 (close :67-75) +- **impact:** self._all_devices is assigned only after ALL connection opens succeed. If the master opens but a slave is absent (common boot case: one of three USB VNAs not yet enumerated), the except calls close(), which iterates the still-empty _all_devices and frees nothing. The opened master connection (USBContext + claimed handle + running 'librevna-usb-rx' daemon thread) leaks. matrix_raw_producer retries FOREVER with 1-10s backoff, so every retry leaks one context+handle+thread (~6/min) until RLIMIT_NOFILE/pthread limits crash the producer, defeating the wait-forever design. +- **fix:** Track opened devices incrementally so close() can free a partial open: append each LibreVnaUsbBulkConnection to self._all_devices (and set _master_device) as it is constructed, or in the except explicitly close the master and any constructed slaves before re-raising. Verify with lsof/thread count that a repeated open-failure loop holds fd/thread count flat. + +### #12 [MEDIUM] push() silently overwrites unread data on overflow yet returns success; drops are invisible to producers and consumers +- **subsystem:** ipc_shm +- **location:** data_acq_and_processing/common_cpp/ipc/src/shm_ring.cpp:298-310 (callers sweep_orchestrator.cpp:86, data_preprocessor.cpp:128, data_processor.cpp:29) +- **impact:** When the ring is full, push() advances read_seq, increments dropped, overwrites the oldest unread slot, and still returns true. Callers treat only false (slot-too-small) as an error, so whenever the consumer is slower than the producer (heavy processing or a stalled GUI tap) unread collections are silently discarded with no log and no backpressure. dropped_count() exists but is never read anywhere in the tree, so loss is invisible on the headless daemon and detection-critical sweeps can vanish. +- **fix:** Surface drops: periodically log dropped_count() deltas, or change push() to return {Queued, Overwrote, TooLarge} so callers WARN on Overwrote. For the primary raw->preprocessed->results path consider a blocking/backpressure push variant so detection data is never silently dropped. + +### #13 [MEDIUM] Stale /dev/shm rings are reused on restart; geometry change wedges the pipeline (C++ throws) while Python silently truncates and diverges +- **subsystem:** cross_cutting / ipc_shm +- **location:** data_acq_and_processing/sweep_orchestrator/src/main.cpp:139; shm_ring.cpp:199; python_app/orchestration/shm/ring_writer.py:35-40; deploy/install-daemon.sh:54 +- **impact:** No production code unlinks rings (unlink_ring/cleanup_known_shm only run under start.sh --clean-shm, which the unit's 'start.sh --headless --skip-build' does not pass). After a SIGKILL/OOM/crash the ring files persist with their last write_seq/read_seq and the restarted side ATTACHES. (1) If capacity/slot_size_bytes change between runs, the C++ side throws 'geometry mismatch' and the producer/processor dies every start -- an unrecoverable boot loop. (2) The Python writer instead silently truncates+reinitializes a size-mismatched stale ring, so a C++ reader mapped to the old size reads garbage. Stale seq counters also cause first-read mis-sequencing. Merges RS-03 and SHM-003. +- **fix:** Make restart self-healing: have the ring owner (orchestrator) unlink_ring() each ring name at startup before open_or_create, OR add --clean-shm to the unit ExecStart / a systemd ExecStartPre that clears radar_* shm objects. On geometry mismatch, unlink+recreate instead of throwing. Document a single owner per ring responsible for create+unlink, and align Python/C++ mismatch behavior. + +### #14 [HIGH] Acceptor thread permanently exits on EMFILE/ENFILE; locator reports running but never accepts again +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/locator/src/tcp_server.cpp:431 +- **impact:** In acceptor_loop any accept() error other than EINTR breaks the loop and the acceptor thread exits for good. Transient/recoverable errors (ECONNABORTED, EMFILE/ENFILE on fd-limit, ENOBUFS/ENOMEM) all permanently stop accepting. running_ stays true, is_running() keeps returning true, and data_processor keeps publish()ing into a server that can never get a new client. After a GUI/client restart it can never reconnect, with no log and no exit, until the whole daemon restarts. Compounds with the session fd leak (rank 17) which itself triggers EMFILE here. +- **fix:** Distinguish fatal vs transient accept() errors: on EINTR/ECONNABORTED continue; on EMFILE/ENFILE/ENOBUFS/ENOMEM log a warning, sleep ~100ms, and continue so the acceptor recovers once fds free up; only break when running_ is false or the fd is genuinely closed (EBADF/EINVAL). Optionally keep one reserved fd to accept-and-close under EMFILE. + +### #15 [MEDIUM] Uncaught exception in the data_processor live loop crashes the headless daemon +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/data_processor/src/data_processor.cpp:111 (publish throw :30, main catch main.cpp:87) +- **impact:** run() calls deserialize_preprocessed_collection, process_collection, and publish_result_collection (which throws when a serialized result exceeds the results ring slot) with no per-iteration try/catch. Any throw propagates to main -> exit 1, taking down the whole processing+locator stage. A single oversized/edge-case result (e.g. a large bscan replay table) or one corrupt preprocessed frame is a hard outage rather than a dropped frame, and the supervisor does not respawn it (rank 1). +- **fix:** Wrap the per-iteration body (deserialize, process, publish_result_collection, publish_locator) in try/catch that logs and continues to the next ring item. Reserve fatal exit for truly unrecoverable conditions (ring detached). For publish, log-and-drop oversized results instead of throwing. + +### #16 [MEDIUM] Child stdout/stderr log files grow without rotation; long-lived daemon can exhaust the SD card and wedge the system +- **subsystem:** py_orchestration / recent_changes / cross_cutting +- **location:** python_app/orchestration/process_supervisor.py:158-164 +- **impact:** _spawn opens runtime/logs/{name}.out.log and .err.log in 'wb' (truncate only at spawn) and hands the fds to each child. There is zero rotation/size cap anywhere in the repo. The always-on data_processor and a continuously-logging producer (per-sweep logging, repeated reconnect warnings while a device is absent, locator per-malformed-packet warnings) run for days/weeks between reboots and grow .out/.err without bound. A full rootfs on a Pi corrupts SQLite/NPZ writes and SHM/config writes and can wedge the whole system -- including the very logs needed to diagnose it. Merges PS-002, RS-06, and the recent_changes data_processor log finding. +- **fix:** Do not redirect children to plain truncating files for a long-lived daemon: pipe output through a size-bounded RotatingFileHandler-style writer, run children under systemd-journald, or periodically rotate/cap (size + count). At minimum cap each file and rotate the always-on data_processor log on a size limit; extend throttling to all hot-path warnings. + +### #17 [MEDIUM] Non-draining/dead locator client leaks fd + 2 threads forever; sessions are only reaped on new accept() +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/locator/src/tcp_server.cpp:200 (write_all :38-59; reap only at :439) +- **impact:** The latest-wins change keeps a full-queue client instead of request_stop() on overflow. With a blocking writer (write_all loops on send() with no SO_SNDTIMEO) and reaping only inside acceptor_loop, a client whose TCP window goes to zero (peer alive but not reading) blocks the writer thread forever; the reader stays blocked in recv() (peer never closes) so exited_ is never set and the session is never reaped -- leaking one fd + two threads per stuck client. Separately, normally-finished sessions also linger in clients_ until the next accept(), so with a fixed/flapping client set zombies accumulate and every publish() wastes work iterating them; this eventually triggers EMFILE -> rank 14. Merges LOC-001 and the recent_changes reap finding. +- **fix:** Set SO_SNDTIMEO on accepted sockets and treat send timeout as fatal -> request_stop(), so a stuck peer is torn down. Set exited_ when BOTH loops finish so a writer-only death is reapable. Call reap_finished_clients() from publish()/broadcast_packet() (try-lock, join outside the mutex) or a periodic timer so clients_ is bounded regardless of new connections. Tune TCP keepalive (KEEPIDLE/INTVL/CNT). + +### #18 [HIGH] NaN/Infinity float fields round-trip into run_config.json and abort every C++ consumer at boot +- **subsystem:** py_config_models +- **location:** python_app/models/run_config_codec.py:113-122 (write path config_writer.py:46) +- **impact:** All float fields are coerced with bare float(); Python json.loads accepts NaN/Infinity and float('nan')/('inf') also arise from stray strings. validate_gpr_model uses float(rel_perm) <= 0.0, always False for NaN, so a NaN permittivity passes. ConfigWriter.write() calls json.dumps with default allow_nan=True, emitting literal NaN/Infinity into run_config.json; the C++ nlohmann parser (run_config.cpp:431, default flags) throws parse_error. The moment a profile with any non-finite numeric is saved, every spawned C++ process fails to load config and exits at startup -- the appliance silently never acquires while the JSON looks valid to an operator. +- **fix:** Reject non-finite numbers at decode and encode time: add a _read_float helper that does float(...) then raises ValueError if not math.isfinite, and use it for every float() in run_config_codec.py. Independently pass allow_nan=False to json.dumps in config_writer.py:46, live_processing_config.py:142, and profile_io_mixin.py:31 so a stray NaN fails loudly in Python. + +### #19 [MEDIUM] Abrupt GUI SIGKILL orphans C++ children holding the radar/rings/port; next start spawns a conflicting second pipeline +- **subsystem:** cross_cutting +- **location:** python_app/orchestration/process_supervisor.py:77-83 +- **impact:** Clean shutdown relies on closeEvent -> _stop_all_processes. If the GUI is killed abruptly (kill -9, OOM-killer, crash skipping closeEvent), the Popen children reparent to init and keep running, still holding the USB radar handle, locator TCP port, and SHM rings. is_running()/is_processor_running() consult only the in-memory _processes dict (empty in a fresh process), so the new instance does not detect orphans and spawns a second full pipeline; two processes then contend for the same device and rings. The start.sh flock and 'systemctl stop' only cover the systemd-managed case; a kill -9'd interactive launch or any non-cgroup kill leaves orphans uncovered. +- **fix:** Detect/reap pre-existing pipeline processes at startup independent of in-memory state: write child PIDs to a runtime pidfile and kill stale ones on start, or scan for known binary names, or under systemd use KillMode=control-group and launch children in a dedicated process group killed on supervisor start. Combine with rank-13 ring cleanup. + +### #20 [HIGH] Headless boot pip install hangs/fails forever on an offline appliance, causing a crash-restart loop +- **subsystem:** deploy_daemon +- **location:** /home/europa/Documents/radar_system/start.sh:354 (failure exit :167-169) +- **impact:** main() calls ensure_python_dependencies() even in --headless mode. If the import probe fails for any reason (partially-upgraded wheel, .pyc/.so mismatch after an OS update, corrupted .venv, a new dep in requirements.txt), the daemon runs pip install. On an offline appliance pip cannot reach PyPI, blocks on DNS/connect retries (delaying the unit), then exits non-zero -> with Restart=on-failure/RestartSec=3 this becomes a crash-restart loop that never starts the radar. The headless guard at :347-353 only skips sudo/system steps, not the more likely network block. +- **fix:** In headless mode treat missing dependencies as a hard, fast failure: if the import probe fails, log a clear error and exit non-zero immediately, or gate the pip-install branch behind ((HEADLESS == 0)). Provisioning should only happen during the documented interactive launch. Optionally set PIP_NO_INDEX defensively so any accidental install fails fast instead of hanging. + +### #21 [HIGH] Daemon runs --skip-build with no validation that build/bin binaries exist and are current +- **subsystem:** deploy_daemon +- **location:** /home/europa/Documents/radar_system/deploy/install-daemon.sh:54 (skips start.sh:360-362) +- **impact:** ExecStart passes --skip-build, so the boot daemon never builds. The default librevna producer is the native build/bin/sweep_orchestrator. If that binary is missing, stale (built against a changed C++/SHM layout), or wiped by git clean/partial update, the daemon either fails to spawn the producer (silenced per rank 1/2) or runs a producer whose SHM ring format mismatches the reader -> silent no-data or corrupt data. There is no pre-flight check that required binaries exist and are newer than sources. +- **fix:** Add a fast headless pre-flight that verifies the required build/bin binaries exist and are executable (no full rebuild) and aborts with a non-zero exit if missing or older than their sources, so Restart/operator notice fires. Alternatively run make -q and fail fast on a stale tree rather than trusting --skip-build. + +### #22 [HIGH] systemd unit has no boot ordering or device-readiness gate, racing USB/local-fs at boot +- **subsystem:** deploy_daemon +- **location:** /home/europa/Documents/radar_system/deploy/install-daemon.sh:46 +- **impact:** The generated unit has an empty [Unit] section (no After=/Wants=/Requires=) and Type=simple. WantedBy=multi-user.target only sets the install target, not startup ordering against device/filesystem readiness. On a Pi the USB radar enumerates asynchronously after udev settles and the .venv/project may live on a not-yet-ready mount, so the daemon can start before the device node exists and hit 'device not found' (then silently idle per rank 2 or churn per rank 7). Type=simple also marks the service 'started' the instant exec begins, so readiness cannot be relied upon. +- **fix:** Add ordering: After=local-fs.target systemd-udev-settle.service and Wants=systemd-udev-settle.service (or a device-specific BindsTo=/After=dev-...device via a udev SYSTEMD_WANTS tag); if the project mount is non-root add RequiresMountsFor=${PROJECT_ROOT}. Consider Type=notify with sd_notify(READY=1) once the pipeline is actually producing. + +### #23 [MEDIUM] Headless daemon writes all logs/errors only to an offscreen Qt widget; nothing reaches journald +- **subsystem:** cross_cutting +- **location:** python_app/gui/app_window.py:365 (widget app_window_ui_mixin.py:181; unit deploy/install-daemon.sh:46) +- **impact:** In --headless mode every GUI-side message (startup errors, pipeline-start failures, reader-poll exceptions, child-crash exit reports, 'Status: error') is rendered via _append_log_entry into a QTextEdit on the offscreen platform. Nothing is written to stdout/stderr/journald (no logging/StreamHandler in the GUI process; the unit sets no StandardOutput/SyslogIdentifier). The widget is capped at 1200 in-memory blocks, so older errors scroll away and are lost on exit. 'journalctl -u radar.service' shows no GUI diagnostics, making a headless box undebuggable when acquisition silently stops. +- **fix:** In headless mode also route _append_log_entry (at least WARN/ERROR) to Python logging with a StreamHandler to stderr (captured by journald) and/or a rotating file under runtime/logs. Set SyslogIdentifier and StandardError=journal in the unit. Keep the widget for GUI mode. + +### #24 [LOW] SIGTERM/SIGINT handler runs full Qt teardown inline from C signal context; re-entrant and reentrancy-unsafe +- **subsystem:** py_gui_lifecycle / cross_cutting +- **location:** python_app/gui/main.py:42 (closeEvent app_window.py:512) +- **impact:** _request_shutdown directly calls window.close() -> closeEvent (which terminates C++ children with multi-second waits and closes mmaps) and app.quit() from signal context. Python delivers handlers between bytecodes on the main thread, so a second SIGTERM (systemd escalation or a double Ctrl-C) arriving during the blocking teardown re-enters _request_shutdown -> closeEvent recursively on half-torn-down state (readers None, supervisor map mutated mid-iteration), corrupting teardown ordering or raising inside the handler. There is no closeEvent re-entry guard and no signal de-arming. This is the daemon's normal shutdown path. Merges RS-001 and the cross_cutting signal-safety finding. +- **fix:** Make the handler async-signal-safe: only set a flag / write a self-pipe (signal.set_wakeup_fd + QSocketNotifier) or QTimer.singleShot(0, window.close) to schedule teardown on the next event-loop iteration, and immediately reset handlers to SIG_IGN/SIG_DFL so a repeat signal cannot re-enter. Add a self._closing guard at the top of closeEvent that returns early if teardown is in progress. + +### #25 [LOW] pop() throws on oversized payload_size and reader trusts payload_size before bounds-checking the mapping +- **subsystem:** ipc_shm +- **location:** data_acq_and_processing/common_cpp/ipc/src/shm_ring.cpp:331-338 (Python ring_reader.py:56,63) +- **impact:** pop() throws runtime_error when slot->payload_size > slot_size_bytes -- reachable from a torn write (rank 5) or stale/corrupt ring (rank 13) -- and the throw propagates up the preprocessor/processor run loops, killing the daemon (compounds rank 3/15). The Python reader does NOT validate payload_size at all before slicing, so a torn/corrupt size runs off the slot into adjacent slots/header and decode_* mis-parses. The size is also read non-atomically relative to the producer's write of it (rank 5), so even in normal wrap the size can belong to a different generation than the copied bytes. Merges SHM-006 and SHM-007. +- **fix:** Make pop() treat an over-size payload as a corrupt slot it skips: log, advance read_seq past it (resync), and return false instead of throwing. Validate payload_size <= slot_size_bytes in the Python reader and reject/resync otherwise. Combine with rank-5's post-copy sequence re-validation so a size/payload pair is accepted only if the slot sequence is unchanged across the read. + +### #26 [LOW] Blocking device I/O makes SIGTERM/SIGINT shutdown hang up to the full I/O timeout (~20s for K209) +- **subsystem:** cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/src/sweep_orchestrator.cpp:117 (and :149) +- **impact:** The signal handler only sets g_stop_requested and the run loop checks it between sweeps/combos. Every per-combo step blocks in non-interruptible device I/O: native VNA sweep up to ~1500ms in libusb_bulk_transfer, DeviceInfo wait up to 2s, remote K209 ::recv up to 20000ms. On a headless Pi a SIGTERM during a stalled read is ignored for the full timeout, and a wedged device that keeps timing-out-and-retrying can effectively never honor stop, forcing the supervisor's force-kill. Clean shutdown / switch-to-safe-state is not guaranteed. +- **fix:** Make the stop flag observable inside blocking waits: pass stop_requested into the driver acquire path (or a self-pipe/eventfd woken by the handler), check it inside wait_for_packet/wait_for_ack/pump_usb and recv_exact/send_all loops, and keep per-call USB/socket timeouts short and re-loop so SIGTERM is honored within a few hundred ms. + +### #27 [MEDIUM] Oversized serialized collection makes push() return false and is escalated to a fatal crash; tap-ring failure aborts the primary path +- **subsystem:** cpp_acquisition / cpp_preprocess +- **location:** data_acq_and_processing/sweep_orchestrator/src/sweep_orchestrator.cpp:84-91 (also data_preprocessor.cpp:126-134) +- **impact:** push() returns false only when payload > slot_size_bytes; overflow is handled internally by overwrite-oldest. publish_collection/publish_preprocessed_collection throw on false -> exit 1. slot_size_bytes is fixed at create time but serialized size scales with run_combos x sweep.points x per-point bytes, so growing combo/point count past the slot makes EVERY collection too large and the first publish crashes the daemon at startup with no recovery. Worse, a too-small raw_tap/preprocessed_tap slot crashes the whole producer even though the primary ring already accepted the data -- a debug/GUI tap takes down the real data path. Merges SO-005 and PREP-003. +- **fix:** Validate worst-case serialized size against slot_size_bytes at open/startup and fail fast with a clear config error there. At runtime, log-and-drop (increment an oversize counter) on a too-large payload instead of throwing, and make tap pushes strictly best-effort so a tap failure can never abort the primary path. + +### #28 [LOW] ConfigWriter.write performs a non-atomic write_text of run_config.json consumed by spawning C++ children +- **subsystem:** py_config_models +- **location:** python_app/orchestration/config_writer.py:43-47 +- **impact:** write() does output_path.write_text(json.dumps(...)) directly (no temp+rename), unlike sibling writers that use temp+replace. The supervisor reads this file (_read_radar_model :222) and every C++ child reads it via --config at startup. An interrupted write (power loss mid-write, or a child reading during a restart rewrite) yields a truncated/empty file -> json.loads raises and start() aborts opaquely, or a child fails to parse. On crash mid-write the on-disk file is left corrupt and persists across reboot, so the boot daemon fails to start the pipeline every boot until manually repaired. +- **fix:** Write atomically: dump to output_path.with_suffix('.json.tmp'), flush+os.fsync, then os.replace() onto the destination (matching ProcessingLiveConfigWriter). Optionally fsync the parent dir for power-loss durability. + +### #29 [MEDIUM] Respawn opens child log files in truncate mode, destroying the prior child's crash log before it is reported +- **subsystem:** py_orchestration +- **location:** python_app/orchestration/process_supervisor.py:163-164 +- **impact:** _spawn always opens stdout/stderr with open(path,'wb') (truncate) and early-returns only if the existing handle is still alive. When a process has crashed but its exit has not yet been collected (collect_exit_reports removes it, but start() can run before the next 50ms poll, e.g. single-capture restart or operator re-Start), the next _spawn reopens 'wb' and erases the crashed child's stdout/stderr -- the diagnostic evidence of why it died is gone before anyone reads it, undermining the exit-report mechanism. Same loss across a parent restart for the previous boot's final crash log. +- **fix:** Before truncating, if a stale (exited, uncollected) entry exists for this name, fold its tail into an exit report first or roll the existing log to {name}.out.log.prev/.err.log.prev. Alternatively open in append mode with a session delimiter (paired with rotation from rank 16) so the crash log survives respawn. + +### #30 [MEDIUM] Singular OSL calibration points silently substitute degenerate coefficients; a bad calibration loads and is used +- **subsystem:** cpp_preprocess +- **location:** data_acq_and_processing/preprocessing/calibration_master/src/channel_bundle_support.cpp:221-226 +- **impact:** solve_osl_coefficients initializes source_match=0, reflection_tracking=1 and only overwrites them when norm(open_delta-short_delta) > 1e-12. When open/short standards are nearly equal (a degenerate capture, common with a flaky USB VNA on a Pi) the point keeps the degenerate coefficients, making S11 correction at that frequency reduce to measured-minus-directivity with no real correction. There is no count, log, or threshold on fallbacks, and 1e-12 on a float magnitude-squared rarely trips for ill-conditioned-but-nonzero denominators. A largely-degenerate calibration loads successfully and produces systematically wrong S11 with no operator-visible indication. +- **fix:** Track the fraction of fallback points per combo; throw at load time if it exceeds a small threshold so a bad bundle is rejected at startup instead of silently used. Use a relative (not just absolute) conditioning check on the denominator and log which combos/frequencies were degenerate. + +### #31 [MEDIUM] libusb retry path does full libusb_exit/init churn per recovery; a recoverable USB glitch becomes a fatal device-not-found +- **subsystem:** cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/device_drivers/radar/librevna/librevna_minimal_driver_lifecycle.cpp:157-159 (transport.cpp:111,171) +- **impact:** On a retryable native error, acquire_sweep calls close_native() (libusb_exit) then open_native() (libusb_init), destroying and recreating the entire libusb context and re-enumerating all USB devices per transient stall. On a Pi this re-enumeration is slow and racy right after a replug: the kernel may not have re-bound the device, so find_matching_device_handle returns null and open_native throws the non-retryable 'No compatible LibreVNA USB device found', turning a recoverable glitch into a fatal exit (compounds rank 8). Repeated init/exit cycling also stresses libusb on a long-running daemon. +- **fix:** Keep the libusb_context alive across retries; only release/reclaim the interface and reopen the handle, not the whole context. On reconnect retry device discovery with a short bounded backoff (a few hundred ms, a few attempts) to absorb re-enumeration latency, and classify 'device not found immediately after a transient error' as retryable. + +### #32 [MEDIUM] multi_device recover() uses blocking time.sleep and ignores stop_requested, delaying SIGTERM shutdown by seconds per failed acquisition +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/multi_device_service.py:114-139 (controller :169) +- **impact:** recover() sleeps through _REOPEN_BACKOFF_SECONDS (0.25+0.5+1.0=1.75s) with no stop hook, and _acquire_native_collection_with_recovery calls recover() up to recovery_attempts+1 (default 4) per acquire_collection(). One failed acquisition can block ~4 x (1.75s + open/close) before stop_requested is re-checked. On device removal + SIGTERM the producer can take tens of seconds (with USB re-enumeration) to exit, risking systemd TimeoutStopSec SIGKILL and an unclean shutdown; the signal handler only sets a threading.Event these C-level/sleep sections never observe. +- **fix:** Thread the stop Event into recover() and _acquire_native_collection_with_recovery; use stop_event.wait(delay) instead of time.sleep and bail out of both the backoff and recovery-attempt loops the moment stop is set. Cap total recovery wall-time per acquire_collection() so shutdown stays well under TimeoutStopSec. + +### #33 [LOW] stop() force-kills on a shared 2s deadline, drops handles without exit reports, and may orphan device-I/O grandchildren +- **subsystem:** py_orchestration +- **location:** python_app/orchestration/process_supervisor.py:230-261 +- **impact:** _stop_processes terminates all named processes against a single shared 2.0s deadline, kills stragglers, then _drop_exited() removes handles with NO ProcessExitReport. (1) The librevna_multi/sn9000/kamil producers are launched as 'python -m python_app.scripts...'; SIGTERM/SIGKILL to that python parent does not necessarily kill device-I/O grandchildren/threads, so a hung device thread can be orphaned holding the VNA/USB device and make the NEXT start() fail to acquire it. (2) Any abnormal exit during stop (e.g. processor segfault on teardown) is silently swallowed, so recurring shutdown crashes are invisible. +- **fix:** Use start_new_session=True (process group) on Popen for the python producer commands and os.killpg on stop so grandchildren die. Give each process its own kill deadline rather than a shared 2s budget. Before _drop_exited, capture exit codes and log abnormal stop-time exits (or route through collect_exit_reports). + +### #34 [LOW] Latest socket vlc speed never expires; a dropped client's last speed is used indefinitely as live motion +- **subsystem:** recent_changes / cpp_processing_locator +- **location:** data_acq_and_processing/processing/locator/src/tcp_server.cpp:280 (read :414-420) +- **impact:** reader_loop stores any finite inbound vlc into a shared atomic that latest_socket_speed() returns forever until the next value or process restart; there is no timestamp/TTL and the value is not reset when the contributing client disconnects. If the speed feed (odometer/positioning) drops or freezes, the GPR pipeline keeps consuming the last speed as if live, silently producing migration/positioning results from stale motion with no indication the feed died. +- **fix:** Store (value, monotonic timestamp) and have latest_socket_speed() return nullopt once older than a configured staleness window so the processor falls back to manual speed or flags missing data. Optionally reset the slot to NaN when the last contributing client disconnects. + +### #35 [LOW] GPIO control-button watcher leaks line/chip/pipe fds and an orphaned thread on partial start failure, wedging the button until reboot +- **subsystem:** py_config_models / recent_changes +- **location:** python_app/gui/control_button.py:87-94 (mixin app_window_control_button_mixin.py:55-60) +- **impact:** start() opens the GPIO line, then os.pipe(), then starts the daemon thread, with no rollback. If os.pipe() (fd exhaustion) or Thread.start() fails after _line.open() succeeded, start() raises with the GPIO chip+line fds (and possibly pipe fds) still open; the mixin's except only logs and _control_button_watcher stays None, so _stop_control_button_watcher can never release them. The kernel line stays claimed (consumer='radar_control_button'), so a later retry/restart hits EBUSY and the button silently never works again until reboot. The _run loop also leaks fds on any select/read error. Merges the two GPIO-watcher leak reports. +- **fix:** Wrap start()'s body in try/except that calls _line.close() and _close_stop_pipe() before re-raising, and close/release in a finally in _run (or have the failed handler trigger stop). Guard start() against double-start. Alternatively assign self._control_button_watcher before start() (or in finally) and call _stop_control_button_watcher() in the except path. + +### #36 [LOW] Non-positive/out-of-range ring capacity, slot_size, and sweep points pass Python validation and crash C++ at boot +- **subsystem:** py_config_models +- **location:** python_app/models/run_config_validation.py:49-50; run_config_codec.py:118 +- **impact:** load_ring_payload coerces capacity/slot_size_bytes with bare int() and no range check, and radar.sweep.points is int()-coerced with no check. A profile with capacity 0/-1, negative slot_size, or points<=0 is accepted and written to run_config.json. The C++ side throws ('Ring capacity must be > 0', 'Value out of uint32 range', 'radar.sweep.points must be > 0') and every pipeline process aborts at startup -- a recurring boot-time crash with no acquisition until the config is hand-edited. Python int(100.5)=100 also accepts a fractional points value that C++ number_to_u32 rejects, so a profile that loads in the GUI still fails in C++. Merges CFG ring and points validation findings. +- **fix:** In Python enforce capacity > 0, slot_size_bytes > 0 (with a uint32 upper bound and an overflow-safe cap on capacity*slot_size), and radar.sweep.points > 0; reject fractional points (require integral input) and validate stop_hz >= start_hz, mirroring the C++ contracts so failures surface in the GUI/save path. + +### #37 [LOW] Explicit JSON null in numeric/bool config fields is silently coerced or hard-fails instead of using the default +- **subsystem:** py_config_models +- **location:** python_app/models/run_config_codec.py:32 (and the int()/float()/bool() call sites) +- **impact:** _read_str guards strings against null (payload.get returns None for explicit null; str(None)='None'), but numeric/bool fields still use int()/float()/bool() directly. With explicit null, int(None)/float(None) raise TypeError (bypassing the intended default fallback) and bool(None)=False silently overrides a True default -- e.g. multi_device.force_external_reference (default True) and control_button.active_low (default True). A profile with 'force_external_reference': null quietly disables the external reference and 'active_low': null flips the button edge polarity. +- **fix:** Generalize null-as-missing handling: add _read_int/_read_float/_read_bool helpers mirroring _read_str that treat None as 'use default', and apply them wherever int()/float()/bool() wrap payload.get(). This prevents the TypeError on null numerics and stops null from silently flipping a True default to False. + +### #38 [LOW] Empty S11 calibration/reference paths silently disable correction with no operator warning +- **subsystem:** cpp_preprocess +- **location:** data_acq_and_processing/preprocessing/calibration_master/src/channel_bundle_support.cpp:303-305 (and :386-388) +- **impact:** S11CalibrationBundle::load returns early (correction disabled) when all open/short/load paths are empty, and S11ReferenceBundle::load returns early when path is empty; apply() then passes S11 through uncorrected and validate_combos skips validation when not enabled. A config typo resolving an S11 path to empty (or a missing key defaulting to '') silently disables one-port S11 correction: the box boots, runs headless, and emits uncorrected S11 with no error or warning until measurement quality is questioned much later. +- **fix:** Distinguish intentionally-disabled from misconfigured: require an explicit s11.calibration.enabled flag to disable (no-op when flagged), but when a path is expected and resolves empty/missing, throw at load so startup fails loudly. At minimum log a clear WARNING to stderr (visible in the per-process log). + +### #39 [LOW] _poll_rings exception handler dedups by (type,str), permanently silencing distinct recurring reader failures +- **subsystem:** py_gui_lifecycle +- **location:** python_app/gui/controllers/app_window_pipeline_mixin.py:319-324 +- **impact:** When _poll_rings raises, it logs once per unique (type, message) and suppresses every identical exception thereafter. A persistent reader fault (SHM ring detached after a producer crash, repeated 'Result ring reader is not initialised') is logged once then silently swallowed every 50ms forever; the status label is set to error only via collect_exit_reports, not here, so the operator sees no continuing signal that polling/rendering is dead -- the screen simply stops updating. There is also no recovery attempt (readers are never reset/reconnected). +- **fix:** Keep dedup for log spam but still set the status label to error on a repeated reader error, periodically re-log (every N seconds or count), and trigger a reader-reconnect or pipeline-stop path so a wedged reader is surfaced and recovered rather than failing silently. + +### #40 [MEDIUM] Blocking hardware capture and time.sleep drain loops run on the GUI/event-loop thread, freezing the headless daemon and starving signal delivery +- **subsystem:** py_gui_lifecycle +- **location:** python_app/gui/controllers/app_window_control_button_mixin.py:73 (drains app_window_pipeline_mixin.py:417, snapshot_mixin.py:236) +- **impact:** _on_control_button_pressed -> _capture_tmp_reference runs entirely on the Qt main thread: it calls _stop_run() (with time.sleep drain loops) and capture_reference_set() which opens the device and acquires median_sweep_count sweeps synchronously. During this the event loop is blocked, so the 50ms _poll_rings stops draining SHM rings (rings fill/overwrite), the headless keepalive timer that delivers Unix signals stops firing, and queued button signals stall. The bounded drain loops (~0.6s stop, ~0.35s clear, ~1.2s snapshot) compound this on the closeEvent path, widening the signal-reentrancy window (rank 24). A physical button press produces a multi-second total UI/daemon stall and delays SIGTERM. Merges the capture-on-GUI-thread and drain-loop findings. +- **fix:** Run capture off the GUI thread (QThread/worker, results marshaled via queued signal) and guard re-entrant presses with a busy flag. Convert the bounded drain loops to event-loop-friendly waits (QEventLoop+QTimer or a worker) so signals and the keepalive timer keep firing, or aggressively cap/avoid blocking drains on the closeEvent path. + +### #41 [LOW] RF switches are not driven to a safe/default state on shutdown or crash; a transient ioctl failure is fatal +- **subsystem:** cpp_acquisition +- **location:** data_acq_and_processing/sweep_orchestrator/device_drivers/switches/h7992_minimal_driver.cpp:136 (hmc349a :119; hot-loop switch_to sweep_orchestrator.cpp:156-157) +- **impact:** open() drives switches to default_position, but close_native() only releases the GPIO fds and never returns the lines to the safe position, so on exit (clean SIGTERM, crash, or exit-1) the RF front-end is left in an undefined electrical state between runs. Worse, switch_to() in the hot loop is unguarded: a single GPIO_V2_LINE_SET_VALUES ioctl failure throws, is in no retry path, and kills the whole daemon, leaving the matrix switches in whatever state they were last commanded. +- **fix:** In close_native() command lines to default_position before closing fds so the RF path is left known-safe; ensure the DriverLifecycleGuard destructor also drives switches safe. Wrap per-combo switch_to() in the same recoverable-error handling as device reads so a transient ioctl failure retries/reconnects instead of crashing. + +### #42 [LOW] USBTransport.disconnect() closes the libusb handle/context even when the RX-thread join times out (use-after-free hazard) +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/librevna_driver/transport/usb.py:155-174 +- **impact:** disconnect() sets the stop event, joins the RX thread with a 1.0s timeout, then unconditionally releaseInterface()/close()es the handle and closes the context even if the join TIMED OUT and the thread is still inside a blocking bulkRead. Closing the USBContext/handle out from under a live RX thread is a use-after-free / libusb-state-corruption hazard that on a Pi can hang or crash during reconnect; under retry-forever, any RX thread that fails to exit within 1s raises the odds of an orphaned daemon thread referencing a closed context. +- **fix:** After join(timeout=1.0) check rx_thread.is_alive(); if still alive, log a hard fault and either retry the join with a longer bound or skip closing the handle/context (deliberate leak is safer than closing under a live thread). Better: ensure the 100ms bulkRead + stop_event check guarantees exit, and assert the join succeeded before closing. + +### #43 [LOW] GpioOutputLines/GpioLineEventWatcher close() can raise from os.close and leave the second fd open +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/switch_drivers/gpio_uapi.py:182-185 (watcher :318-321) +- **impact:** close() calls _close_line_fd() then _close_chip_fd() sequentially with no exception isolation. If os.close(line_fd) raises (EINTR, or EIO/ENODEV when a USB GPIO expander is yanked on a Pi), the exception propagates and _close_chip_fd() never runs, leaking the chip fd; the chip fd is also never reset to -1, so a later reopen overwrites/leaks it. Over many switch open/close cycles in a long-running daemon this slowly exhausts fds. +- **fix:** Make close() best-effort and idempotent: wrap each os.close in try/finally (or contextlib.suppress(OSError)) so both _close_line_fd and _close_chip_fd always run and always reset their fd to -1 even when close() errors. Apply to both classes. + +### #44 [LOW] matrix producer's radar.close() in finally can block SIGTERM-driven shutdown on a hung device +- **subsystem:** recent_changes / py_hardware +- **location:** python_app/scripts/matrix_raw_producer.py:154-157 +- **impact:** On SIGTERM the loop breaks and finally calls radar.close() under suppress(Exception). For SN9000 (VISA/TCP) or LibreVNA (libusb), close() can issue a blocking transport teardown that hangs when the device is unresponsive -- exactly the failure this producer tolerates -- and suppress() does not bound time. With the supervisor's ~2s pre-SIGKILL budget, a hung close() means force-kill; and if acquire_collection() is mid-blocking-read when the signal arrives, the Python handler cannot interrupt the C-level call, so stop_requested is observed only after it returns, delaying clean exit up to the device timeout and risking SIGKILL mid-sweep (partial device state). +- **fix:** Bound device teardown: run radar.close() with a watchdog/timeout (timer thread or hard deadline) so a hung transport cannot delay exit, and ensure the driver's blocking acquire uses a finite transport timeout so stop_requested is checked at bounded intervals. + +### #45 [LOW] KamilAdcService.open()/_wait_for_tty busy-polls and ignores the stop Event, delaying shutdown during the boot startup window +- **subsystem:** py_hardware +- **location:** python_app/hardware_full/kamil_adc_service.py:419-430 +- **impact:** _wait_for_tty polls with time.sleep(0.05) up to startup_timeout_s with no reference to the producer's stop_requested Event. If the collector is slow to create the TTY (or never does) at boot and the operator sends SIGTERM during this window, the producer cannot interrupt the wait and must block until startup_timeout_s elapses before unwinding. Combined with the lack of an open() retry loop (rank 10), startup is the least responsive phase to a stop request, adding to worst-case TimeoutStopSec pressure. +- **fix:** Accept an optional stop Event in open()/_wait_for_tty and break the poll loop promptly when set (event.wait(0.05) instead of time.sleep). Have kamil_adc_raw_producer pass its stop_requested Event through so shutdown is immediate in all phases. + +### #46 [LOW] Crashed-child exit reporting reads both 16KB log tails on the GUI thread every 50ms; stop() blocks the GUI for seconds +- **subsystem:** py_orchestration +- **location:** python_app/orchestration/process_supervisor.py:240-253 (tails :299-300) +- **impact:** collect_exit_reports (called every 50ms from the GUI QTimer) does a 16KB seek+read on each exited child's stdout AND stderr (SD-card I/O from the UI loop). More significantly, _stop_processes blocks the calling thread up to ~2s (terminate deadline) + up to 1s per force-killed process; stop_all on three stuck children freezes the GUI ~3-5s. On the headless box the GUI is the supervising loop, so during a stop the 50ms ring poll stalls, exit reports are not collected, and any added watchdog is starved. +- **fix:** Move process termination/wait off the GUI thread (worker thread or QProcess async finished signals) or cap the total stop budget. Read log tails lazily only when actually building an ERROR report, not on every 50ms poll for every exited process. + +### #47 [LOW] load_channel_traces silently collapses duplicate combos via insert_or_assign, hiding bundle corruption +- **subsystem:** cpp_preprocess +- **location:** data_acq_and_processing/preprocessing/calibration_master/src/channel_bundle_support.cpp:142 +- **impact:** load_channel_traces builds traces_by_combo with insert_or_assign for every trace. If a calibration/reference bundle contains two traces for the same ComboKey (a generation bug or a partially-overwritten/corrupted bundle), the second silently overwrites the first. The operator believes a calibration is loaded for that combo when it is actually an arbitrary last-wins duplicate, potentially the wrong standard; this passes all combo-coverage validation and is undetectable at runtime. +- **fix:** Use insert() and check the bool result; on a duplicate ComboKey throw a descriptive runtime_error ('duplicate combo X in bundle') so a malformed bundle is rejected at load time. + +### #48 [LOW] Stale-but-running locator keeps emitting sts=1 with a fresh timestamp; clients cannot tell processing has stalled +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/locator/src/payload_builder.cpp:184 +- **impact:** build_payload_json hardcodes sts=1 and a fresh wall-clock tim on every packet. The snapshot-on-connect sends the last cached packet to new clients, and publish() is only driven by data_processor frames. If the processor loop stalls or exits, the locator keeps the last cached packet, and any newly connecting client receives a packet that always claims sts=1 with stale observations -- there is no liveness/heartbeat or staleness indication, so a downstream consumer cannot distinguish live data from a frozen pipeline. +- **fix:** Carry a real status/age signal: stamp packets with the source frame time so consumers can detect staleness, or emit a heartbeat with sts reflecting whether a fresh result was produced within a recent window. At minimum do not re-send a stale cached snapshot to a new client without marking it stale. + +### #49 [MEDIUM] Tap/overflow rings have two concurrent plain-store writers to read_seq -> lost-update race +- **subsystem:** ipc_shm +- **location:** data_acq_and_processing/common_cpp/ipc/src/shm_ring.cpp:299 (Python ring_reader.py:59,64,95) +- **impact:** The ring is single-producer/single-consumer, but on the tap rings (raw_tap, preprocessed_tap) BOTH sides write read_seq concurrently: the C++ producer advances read_seq on overflow (plain store read_seq+1) while the GUI ShmRingReader advances read_seq on every pop and drop_all. These plain stores clobber each other: the producer can rewind read_seq from a faster consumer's higher value back to R+1, so already-consumed slots are re-read (duplicate payloads) or the over-full check is computed against a rewound read_seq, corrupting full/empty accounting. Manifests as duplicated/garbled GUI frames and unbounded apparent backlog. +- **fix:** Make read_seq advancement a CAS loop on both the producer overflow path and all consumers, or redesign so the producer never touches read_seq for overflow (advance only write_seq with separate dropped accounting; consumers detect lapping via the per-slot sequence check). At minimum the producer's overflow store must be a compare_exchange so it never moves read_seq backward. + +### #50 [LOW] open_existing() never validates capacity/slot_size against mapped size; slot_header() can compute out-of-bounds offsets (SIGSEGV) +- **subsystem:** ipc_shm +- **location:** data_acq_and_processing/common_cpp/ipc/src/shm_ring.cpp:216 (slot math :349-353) +- **impact:** open_existing() only checks st_size >= sizeof(Header), magic, and version; it trusts header->capacity and slot_size_bytes verbatim. slot_header()/slot_payload() then compute sizeof(Header)+index*slot_stride and read slot_size_bytes past it. A stale/truncated/corrupt file (a crash during ftruncate left a short file, or a different-geometry ring with a matching magic) makes the computed slot address point outside the mmap -> SIGSEGV or reading adjacent memory. push()/pop() dereference slot fields with no bounds check -- a hard crash on a headless Pi. +- **fix:** In open_existing() compute expected_size = sizeof(Header) + (sizeof(SlotHeader)+slot_size_bytes)*capacity from header fields and require st_size >= expected_size (and capacity>0, slot_size_bytes>0, no multiplication overflow) before returning, else throw. Apply the same expected-size check in open_or_create()'s EEXIST branch (it currently only compares equality, not actual file size). + +### #51 [MEDIUM] stop_headless_service: a sudo failure aborts the entire GUI launch under set -euo pipefail +- **subsystem:** deploy_daemon +- **location:** /home/europa/Documents/radar_system/start.sh:327 +- **impact:** 'sudo systemctl stop ${SERVICE_NAME}' runs as a bare command under set -euo pipefail. If the sudoers rule is absent/mismatched (SERVICE_USER differs from the login user, install under a different account, or systemctl path moved so the NOPASSWD absolute-path match fails), sudo prompts for a password in a possibly TTY-less context or returns non-zero. Unguarded, a non-zero return makes set -e abort start.sh entirely -- the operator cannot launch the GUI at all, and the still-running daemon keeps owning the radar/SHM/locator port. +- **fix:** Make the stop best-effort and non-fatal: 'sudo -n systemctl stop "${SERVICE_NAME}" || echo WARN...' then verify with systemctl is-active and only hard-fail if still active. Use sudo -n to avoid hanging for a password on a TTY-less invocation. In sudoers allow both /usr/bin/systemctl and /bin/systemctl (or a unit-scoped path) so a relocated binary still matches. + +### #52 [LOW] Locator reader recv()/writer have no SO_RCVTIMEO/SO_SNDTIMEO; a stalled client pins the reader thread and the vlc-update path +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/locator/src/tcp_server.cpp:63 +- **impact:** read_exact loops on recv() with no timeout. A client that sends a valid 8-byte header advertising a payload then sends nothing leaves reader_loop blocked in recv() indefinitely. The reader is the only path that updates latest_socket_speed_, so a stalled/slow first client can prevent fresh vlc speed updates, and the session is not reaped (exited_ unset) until external teardown. A buggy/hostile LAN client can hold a reader thread per connection. Shutdown still works (request_stop -> shutdown unblocks recv), so this is a steady-state hang. +- **fix:** Set SO_RCVTIMEO on accepted sockets and treat EAGAIN/EWOULDBLOCK in read_exact as a check-stop-flag-and-retry (or slow-client disconnect after N timeouts). Re-check stop_requested_ between recv() calls so a stalled reader notices teardown even before shutdown(). + +### #53 [LOW] Malformed JSON types (array/object where a scalar is expected) raise TypeError, defeating ValueError-only config error handling +- **subsystem:** py_config_models +- **location:** python_app/models/run_config_validation.py:23 +- **impact:** Every coercion in load_switch_payload/load_control_button_payload/load_ring_payload and the radar/laser/locator sections uses bare int()/float()/str() on the raw JSON value. A wrong-type field (radar_port:[1,2], pin:{}, capacity:[..]) raises TypeError, not ValueError. The codec and gui_profile_codec emit clean ValueError for bad shape, so callers/tests that catch ValueError as the canonical bad-config signal let TypeError escape uncaught. The GUI load path catches broad Exception so it survives, but any non-GUI consumer doing 'except ValueError' crashes instead of reporting a config error. +- **fix:** Route all scalar reads through typed helpers (like gui_profile_codec's _optional_int/_optional_float/_optional_string with isinstance checks that raise ValueError), or wrap the int()/float() calls so a non-scalar value raises ValueError with the field name, making the malformed-input contract uniform. + +### #54 [LOW] install-daemon.sh aborts if SUDO_USER is unset (root login / sudo -i / cloud-init), blocking first-boot provisioning +- **subsystem:** deploy_daemon +- **location:** /home/europa/Documents/radar_system/deploy/install-daemon.sh:18 +- **impact:** SERVICE_USER = ${SUDO_USER:-root}; installing from a real root shell, serial console, sudo -i, or cloud-init (SUDO_USER unset) makes SERVICE_USER=root and the script exits demanding a normal login user. On a fresh Pi image, first-boot provisioning is frequently done as root with no SUDO_USER, so the documented one-shot install fails and the daemon is never installed. There is also no validation that SERVICE_USER exists, is in plugdev, or can read the .venv/project tree, so a mismatched user yields a daemon that cannot execute its own venv. +- **fix:** Accept an explicit RADAR_SERVICE_USER arg/env and fall back to the owner of PROJECT_ROOT (stat -c %U) rather than failing when SUDO_USER is empty. Validate the chosen user exists (id), is in plugdev, and owns/can read the .venv and project tree, failing with an actionable message otherwise. + +### #55 [LOW] Socket-supplied vlc speed never triggers reprocessing of the current result and is read non-atomically w.r.t. live config +- **subsystem:** cpp_processing_locator +- **location:** data_acq_and_processing/processing/data_processor/src/data_processor.cpp:166 (reprocess gate :71) +- **impact:** resolve_effective_live_config() overlays latest_socket_speed() onto gpr_speed_m_s every tick, but the reprocess/replay branch is gated solely on the file revision counter, which changes only when the live-config FILE changes. A new vlc value over the socket therefore does not reprocess the current result -- it only affects the next preprocessed frame popped from the ring. If the radar is paused (no new frames), a speed update from the client has no visible effect until motion resumes. Matches an inline comment so may be intended, but the on-wire speed control silently does nothing while idle. +- **fix:** If live speed should affect the current/last result, track a dirty flag when latest_socket_speed() changes value and OR it into the reprocess condition (respecting reprocess_current_result). If the current behavior is intended, document it explicitly so it is not mistaken for a bug during field debugging. + +### #56 [LOW] Watcher pressed/failed slots can execute after the watcher is stopped because signals are never disconnected on teardown +- **subsystem:** py_gui_lifecycle +- **location:** python_app/gui/controllers/app_window_control_button_mixin.py:80 +- **impact:** _stop_control_button_watcher calls watcher.stop() and sets the field None but never disconnects watcher.pressed/failed. A queued cross-thread pressed emitted just before stop() can still be delivered after stop() returns and after the watcher is dereferenced. closeEvent stops the watcher first but then continues multi-second teardown while the loop is not spinning, so in resume-after-close or non-close stop paths a queued press can re-trigger _capture_tmp_reference against an already torn-down pipeline (supervisor stopped, readers None). +- **fix:** In _stop_control_button_watcher, disconnect watcher.pressed/failed from their slots before/after stop() and consider watcher.deleteLater(). Re-check self._supervisor/self._closing state at the top of _on_control_button_pressed. + +### #57 [LOW] parse_combos_from_text raises uncaught/opaque ValueError on non-numeric combo tokens with no count cap +- **subsystem:** py_config_models +- **location:** python_app/models/run_config_validation.py:97 +- **impact:** parse_combos_from_text splits on ',' and ':' then int()s each side with no guard. A token like 'a:0', '0:', or '0:x' raises ValueError('invalid literal for int') exposing the raw failure rather than a combo-context message, and an empty side raises without saying which field is wrong. Reached from the GUI switches text box (caught broadly, so no crash) but the operator gets an opaque Python error. There is also no cap on parsed combos, so a pathological pasted string allocates one ComboModel per token, unbounded before downstream Cartesian expansion. +- **fix:** Wrap the int() conversions in try/except ValueError and re-raise with the offending pair/side (e.g. 'Invalid combo {pair!r}: input/output must be integers'), reject empty sides explicitly, and cap the combo count to a sane maximum to bound resource use. diff --git a/docs/run_config.md b/docs/run_config.md index 183250f..839949f 100644 --- a/docs/run_config.md +++ b/docs/run_config.md @@ -232,6 +232,42 @@ Use mock switches on a laptop without GPIO: "driver_mode": "mock" ``` +## `control_button` + +Optional physical GPIO push-button that triggers a runtime action on press. +The watcher runs in both GUI and headless modes (it is attached to the main +window, which both launch paths build). On press it reuses the existing +"Capture Tmp Reference" flow: stop the pipeline, capture a fresh tmp S21 +reference with the current sweep settings, then restart the pipeline if it had +been running. + +```json +"control_button": { + "enabled": true, + "gpio_chip": "/dev/gpiochip0", + "pin": 26, + "active_low": true, + "bias": "", + "debounce_ms": 50, + "action": "capture_tmp_reference" +} +``` + +| Field | Meaning | +| --- | --- | +| `enabled` | Master switch. When `false` (default) no GPIO line is opened, so non-Pi hosts are unaffected. | +| `gpio_chip` | Linux GPIO chip path, usually `/dev/gpiochip0`. | +| `pin` | BCM line offset of the button. `26` is physical pin 37, with GND on physical pin 39. | +| `active_low` | `true` for a button wired to GND with the internal pull-up: the line idles high and a press is detected on the falling edge. `false` mirrors this for a button wired to 3V3 with a pull-down (rising edge). | +| `bias` | Internal bias override: `pull_up`, `pull_down`, or `disabled`. Empty (default) derives the bias from `active_low`. | +| `debounce_ms` | Hardware debounce period applied by the kernel, in milliseconds. | +| `action` | Action to run on press. Currently only `capture_tmp_reference`. | + +Occupied BCM lines (native switches) are `17`, `22`, `23`, `27`; pick a free +line such as `16`, `20`, `21`, or `26` for the button. A failure to open the +line (missing chip, line already in use) is logged as a warning and never +aborts startup. + ## `run` Runtime behavior and combo selection. diff --git a/locator_test_client.py b/locator_test_client.py index ad284c5..47ad0ad 100644 --- a/locator_test_client.py +++ b/locator_test_client.py @@ -6,7 +6,7 @@ import threading import time from typing import Any, Dict, Tuple -HOST = "127.0.0.1" +HOST = "192.168.8.2" PORT = 8888 CLIENT_DEVICE_ID = 0 MIN_TEST_VLC = 5.0 diff --git a/python_app/gui/app_window.py b/python_app/gui/app_window.py index 71010c1..2108a89 100644 --- a/python_app/gui/app_window.py +++ b/python_app/gui/app_window.py @@ -12,13 +12,15 @@ import html import json import os from pathlib import Path +import sys import traceback from PyQt6.QtCore import QTimer from PyQt6.QtGui import QTextCursor -from PyQt6.QtWidgets import QMainWindow, QMessageBox +from PyQt6.QtWidgets import QApplication, QMainWindow, QMessageBox from python_app.gui.controllers.app_window_config_mixin import AppWindowConfigMixin +from python_app.gui.controllers.app_window_control_button_mixin import AppWindowControlButtonMixin from python_app.gui.controllers.app_window_pipeline_mixin import AppWindowPipelineMixin from python_app.gui.controllers.app_window_plot_mixin import AppWindowPlotMixin from python_app.gui.controllers.app_window_preprocess_mixin import AppWindowPreprocessMixin @@ -47,6 +49,7 @@ class AppWindow( AppWindowPlotMixin, AppWindowPipelineMixin, AppWindowSnapshotMixin, + AppWindowControlButtonMixin, QMainWindow, ): """Top-level window coordinating GUI state and acquisition runtime.""" @@ -64,6 +67,7 @@ class AppWindow( self._init_history_state() self._init_runtime_limits() self._init_polling_timer() + self._init_control_button_state() self._bootstrap_ui_runtime() def _init_paths(self, project_root: Path) -> None: @@ -234,6 +238,9 @@ class AppWindow( self._pipeline_metrics.set_log_sink(self._log) self._timer.start() self._maybe_auto_start_pipeline() + self._start_control_button_watcher() + if self._is_truthy_env("RADAR_SYSTEM_HEADLESS"): + self._install_headless_watchdog() def _resolve_startup_profile_path(self) -> Path: """Resolve active profile path from session-state or root fallback path.""" @@ -278,7 +285,7 @@ class AppWindow( if self._is_truthy_env("RADAR_SYSTEM_AUTO_APPLY_RADAR"): QTimer.singleShot(500, self._auto_apply_radar_then_start) else: - QTimer.singleShot(500, self._start_run) + QTimer.singleShot(500, self._auto_start_pipeline_step) def _auto_apply_radar_then_start(self) -> None: """Apply current radar settings then start the pipeline (headless boot).""" @@ -287,8 +294,59 @@ class AppWindow( except Exception as exc: # noqa: BLE001 self._log_exception("Auto apply-radar failed", exc, level="WARN") # Hand control back to the event loop so widget updates from - # _apply_radar_settings can flush before _start_run takes over. - QTimer.singleShot(100, self._start_run) + # _apply_radar_settings can flush, then wait one second before the + # start takes over so the device settles after apply-radar. + QTimer.singleShot(1000, self._auto_start_pipeline_step) + + def _auto_start_pipeline_step(self) -> None: + """Run the launcher-requested pipeline start. + + In headless mode a start that does not bring the pipeline up is fatal: we + exit non-zero so `systemd Restart=on-failure` restarts the unit instead of + leaving an idle daemon producing nothing. (The producer itself waits for + the device forever, so a live-but-deviceless producer counts as running.) + """ + self._start_run() + if self._is_truthy_env("RADAR_SYSTEM_HEADLESS") and not self._supervisor.is_running(): + self._headless_fatal("Headless auto-start did not bring the pipeline up") + + def _install_headless_watchdog(self) -> None: + """Self-heal a headless daemon: if a managed pipeline process crashes (exits + without us stopping it), exit non-zero so the service restarts clean. + + Intentional stops drop processes from the supervisor first, so a normal + stop/start or tmp-reference transition never trips this. + """ + self._headless_watchdog = QTimer(self) + self._headless_watchdog.setInterval(2000) + self._headless_watchdog.timeout.connect(self._headless_watchdog_tick) + self._headless_watchdog.start() + + def _headless_watchdog_tick(self) -> None: + """Escalate any unexpected managed-process exit to a fatal headless restart.""" + crashed = [ + report + for report in self._supervisor.collect_exit_reports() + if not report.expected_clean_exit + ] + if crashed: + names = ", ".join(report.name for report in crashed) + details = "\n\n".join(report.format() for report in crashed) + self._headless_fatal(f"Pipeline process exited unexpectedly: {names}", details=details) + + def _headless_fatal(self, reason: str, *, details: str | None = None) -> None: + """Log loudly to stderr and exit non-zero so systemd restarts the service. + + Headless deployments have no operator and the in-app log only reaches an + offscreen widget, so a dead pipeline would otherwise go unnoticed. + """ + self._log_error(reason, details=details) + print(f"[radar] FATAL (headless): {reason}", file=sys.stderr, flush=True) + if details: + print(details, file=sys.stderr, flush=True) + app = QApplication.instance() + if app is not None: + app.exit(1) @staticmethod def _is_truthy_env(name: str) -> bool: @@ -507,6 +565,8 @@ class AppWindow( def closeEvent(self, event) -> None: # noqa: N802 """Ensure workers and dialogs are closed before window destruction.""" try: + # 0) Stop the GPIO button watcher so a late press cannot start work. + self._stop_control_button_watcher() self._resume_pipeline_after_capture = False # 1) Abort active capture first (releases exclusive hardware resources). self._abort_capture_sequence(resume_pipeline=False) diff --git a/python_app/gui/control_button.py b/python_app/gui/control_button.py new file mode 100644 index 0000000..1da5446 --- /dev/null +++ b/python_app/gui/control_button.py @@ -0,0 +1,131 @@ +"""Background watcher that turns a physical GPIO button press into a Qt signal. + +A single :class:`GpioLineEventWatcher` is polled on a dedicated thread via +:func:`select.select`, woken either by a GPIO edge or by a self-pipe used for +clean shutdown. Because the watcher is a ``QObject``, its ``pressed`` signal is +delivered through the event loop on the thread that owns it (the GUI thread), +so connected slots may touch widgets exactly as a button click would. +""" + +from __future__ import annotations + +import os +import select +import threading + +from PyQt6.QtCore import QObject, pyqtSignal + +from python_app.hardware_full.switch_drivers.gpio_uapi import ( + GPIO_V2_LINE_EVENT_FALLING_EDGE, + GPIO_V2_LINE_EVENT_RISING_EDGE, + GPIO_V2_LINE_FLAG_BIAS_DISABLED, + GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN, + GPIO_V2_LINE_FLAG_BIAS_PULL_UP, + GPIO_V2_LINE_FLAG_EDGE_FALLING, + GPIO_V2_LINE_FLAG_EDGE_RISING, + GpioLineEventWatcher, +) + +_BIAS_FLAGS = { + "pull_up": GPIO_V2_LINE_FLAG_BIAS_PULL_UP, + "pull_down": GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN, + "disabled": GPIO_V2_LINE_FLAG_BIAS_DISABLED, +} + + +class ControlButtonWatcher(QObject): + """Monitor a GPIO push-button on a background thread and emit ``pressed``. + + The press is detected on a single edge (falling for active-low wiring, + rising otherwise), so a normal push produces exactly one ``pressed`` signal. + ``failed`` reports an unrecoverable watcher error as a human-readable string. + """ + + pressed = pyqtSignal() + failed = pyqtSignal(str) + + def __init__( + self, + *, + chip: str, + pin: int, + active_low: bool = True, + bias: str = "", + debounce_ms: int = 50, + parent: QObject | None = None, + ) -> None: + """Configure the watcher; the GPIO line stays closed until :meth:`start`.""" + super().__init__(parent) + active_low = bool(active_low) + # Active-low wiring idles high and falls on press; active-high is the mirror. + self._press_edge = ( + GPIO_V2_LINE_EVENT_FALLING_EDGE if active_low else GPIO_V2_LINE_EVENT_RISING_EDGE + ) + edge_flag = ( + GPIO_V2_LINE_FLAG_EDGE_FALLING if active_low else GPIO_V2_LINE_FLAG_EDGE_RISING + ) + self._line = GpioLineEventWatcher( + chip, + int(pin), + edge_flags=edge_flag, + bias_flags=self._resolve_bias_flags(bias, active_low), + debounce_us=max(0, int(debounce_ms)) * 1000, + consumer="radar_control_button", + ) + self._thread: threading.Thread | None = None + self._stop_read_fd = -1 + self._stop_write_fd = -1 + + @staticmethod + def _resolve_bias_flags(bias: str, active_low: bool) -> int: + """Return GPIO bias flags, defaulting to the bias that matches the wiring.""" + name = (bias or "").strip().lower() + if name in _BIAS_FLAGS: + return _BIAS_FLAGS[name] + return GPIO_V2_LINE_FLAG_BIAS_PULL_UP if active_low else GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN + + def start(self) -> None: + """Open the GPIO line and begin watching for presses on a background thread.""" + self._line.open() + self._stop_read_fd, self._stop_write_fd = os.pipe() + self._thread = threading.Thread( + target=self._run, name="control-button-watcher", daemon=True + ) + self._thread.start() + + def stop(self) -> None: + """Signal the watcher thread to exit and release the GPIO line and pipe.""" + if self._stop_write_fd >= 0: + try: + os.write(self._stop_write_fd, b"\x00") + except OSError: + pass + if self._thread is not None: + self._thread.join(timeout=2.0) + self._thread = None + self._close_stop_pipe() + self._line.close() + + def _run(self) -> None: + """Block on the GPIO line until an edge fires or shutdown is requested.""" + line_fd = self._line.fileno() + try: + while True: + readable, _, _ = select.select([line_fd, self._stop_read_fd], [], []) + if self._stop_read_fd in readable: + return + if line_fd in readable and self._line.read_event() == self._press_edge: + self.pressed.emit() + except Exception as exc: # noqa: BLE001 + self.failed.emit(str(exc)) + + def _close_stop_pipe(self) -> None: + """Close both ends of the self-pipe used to wake the watcher thread.""" + for attr in ("_stop_read_fd", "_stop_write_fd"): + fd = getattr(self, attr) + if fd >= 0: + try: + os.close(fd) + except OSError: + pass + setattr(self, attr, -1) diff --git a/python_app/gui/controllers/app_window_control_button_mixin.py b/python_app/gui/controllers/app_window_control_button_mixin.py new file mode 100644 index 0000000..75b86ee --- /dev/null +++ b/python_app/gui/controllers/app_window_control_button_mixin.py @@ -0,0 +1,90 @@ +"""Mixin wiring a physical GPIO control button to a runtime action. + +Both GUI and headless launches construct :class:`AppWindow`, so attaching the +watcher here makes the button behave identically in both modes. The button +reuses the existing "Capture Tmp Reference" flow (stop the pipeline, capture a +fresh tmp reference, then restart the pipeline if it had been running), so no +acquisition logic is duplicated for the hardware trigger. +""" + +from __future__ import annotations + +from python_app.gui.control_button import ControlButtonWatcher +from python_app.models.run_config_schema import ControlButtonModel + + +class AppWindowControlButtonMixin: + """Start and stop a background GPIO button watcher bound to a runtime action.""" + + def _init_control_button_state(self) -> None: + """Initialize the watcher handle before the watcher is started.""" + self._control_button_watcher: ControlButtonWatcher | None = None + + def _start_control_button_watcher(self) -> None: + """Open the configured GPIO button line and begin watching for presses. + + Any failure is logged and swallowed: the same build runs on developer + machines and non-Pi hosts where the GPIO chip is absent, and a missing + button must never abort startup. + """ + config = getattr(self._defaults_config, "control_button", None) + if config is None or not config.enabled: + return + if config.pin < 0 or not config.gpio_chip: + self._log_warning( + "Control button enabled but gpio_chip/pin are unset; watcher not started." + ) + return + if config.action != ControlButtonModel.ACTION_CAPTURE_TMP_REFERENCE: + self._log_warning( + f"Control button action '{config.action}' is not supported; watcher not started." + ) + return + + try: + watcher = ControlButtonWatcher( + chip=config.gpio_chip, + pin=config.pin, + active_low=config.active_low, + bias=config.bias, + debounce_ms=config.debounce_ms, + parent=self, + ) + watcher.pressed.connect(self._on_control_button_pressed) + watcher.failed.connect(self._on_control_button_failed) + watcher.start() + except Exception as exc: # noqa: BLE001 + self._log_exception("Failed to start GPIO control button watcher", exc, level="WARN") + return + + self._control_button_watcher = watcher + self._log( + "GPIO control button watcher started: " + f"chip={config.gpio_chip}, pin={config.pin}, active_low={config.active_low}, " + f"debounce_ms={config.debounce_ms}, action={config.action}" + ) + + def _on_control_button_pressed(self) -> None: + """Run the configured action for a physical press on the Qt main thread. + + Delivered as a queued signal from the watcher thread, so this executes + on the GUI thread exactly like a click on "Capture Tmp Reference". + """ + self._log("GPIO control button pressed: capturing tmp reference.") + self._capture_tmp_reference() + + def _on_control_button_failed(self, message: str) -> None: + """Log an unrecoverable watcher error reported from the background thread.""" + self._log_warning("GPIO control button watcher stopped", details=message) + + def _stop_control_button_watcher(self) -> None: + """Stop the watcher and release its GPIO line during shutdown.""" + watcher = getattr(self, "_control_button_watcher", None) + if watcher is None: + return + try: + watcher.stop() + except Exception as exc: # noqa: BLE001 + self._log_warning("Error stopping GPIO control button watcher", details=str(exc)) + finally: + self._control_button_watcher = None diff --git a/python_app/gui/controllers/app_window_pipeline_mixin.py b/python_app/gui/controllers/app_window_pipeline_mixin.py index 208732c..e807022 100644 --- a/python_app/gui/controllers/app_window_pipeline_mixin.py +++ b/python_app/gui/controllers/app_window_pipeline_mixin.py @@ -6,7 +6,11 @@ import time from python_app.gui.runtime.constraints import validate_processing_mode_constraints -from python_app.gui.runtime.history import build_run_history_signature, record_result_history +from python_app.gui.runtime.history import ( + build_processor_run_signature, + build_run_history_signature, + record_result_history, +) from python_app.hardware_full.kamil_adc_service import apply_kamil_adc_laser_control from python_app.hardware_full.single_radar_service import create_single_radar_service from python_app.models.dataset_model import ComboKey, ResultCollection, SweepCollection @@ -57,7 +61,8 @@ class AppWindowPipelineMixin: processor_was_running = self._supervisor.is_processor_running() config = self._build_config() run_signature = self._build_run_history_signature(config) - if self._processor_requires_restart(run_signature): + processor_signature = self._build_processor_run_signature(config) + if self._processor_requires_restart(processor_signature): self._log("Restarting data_processor because stable run settings changed") self._stop_all_processes() processor_was_running = False @@ -122,7 +127,7 @@ class AppWindowPipelineMixin: self._raw_reader = ShmRingReader(config.rings.raw_tap.name) self._pre_reader = ShmRingReader(config.rings.preprocessed_tap.name) self._result_reader = ShmRingReader(config.rings.results.name) - self._processor_run_signature = run_signature + self._processor_run_signature = processor_signature self._single_capture_active = single_capture self._single_capture_start_ns = None self._single_capture_seen_raw = False @@ -167,8 +172,8 @@ class AppWindowPipelineMixin: config = self._build_config() if config.radar.driver_mode == "native": self._refresh_radar_limits_from_device() - run_signature = self._build_run_history_signature(config) - if processor_only_running and self._processor_requires_restart(run_signature): + processor_signature = self._build_processor_run_signature(config) + if processor_only_running and self._processor_requires_restart(processor_signature): self._stop_all_processes() self._reset_runtime_history() self._history_run_signature = None @@ -492,9 +497,13 @@ class AppWindowPipelineMixin: self._result_history.extend(result_tail) def _build_run_history_signature(self, config: RunConfigModel) -> tuple[object, ...]: - """Build signature used to decide when history should be reset.""" + """Build signature used to decide when display history should be reset.""" return build_run_history_signature(config) + def _build_processor_run_signature(self, config: RunConfigModel) -> tuple[object, ...]: + """Build signature used to decide when the data_processor must be restarted.""" + return build_processor_run_signature(config) + def _validate_processing_mode_constraints(self, config: RunConfigModel) -> None: """Validate processing-mode constraints for run start.""" validate_processing_mode_constraints( diff --git a/python_app/gui/controllers/app_window_preprocess_mixin.py b/python_app/gui/controllers/app_window_preprocess_mixin.py index a27cd69..7804791 100644 --- a/python_app/gui/controllers/app_window_preprocess_mixin.py +++ b/python_app/gui/controllers/app_window_preprocess_mixin.py @@ -142,11 +142,19 @@ class AppWindowPreprocessMixin: ) self._selected_preprocess_sets["s21_reference"] = TMP_REFERENCE_SET_NAME self._selected_preprocess_radar_key = radar_key - self._processor_run_signature = None + # Do NOT restart data_processor for a reference change: it does not consume + # the S21 reference (the data_preprocessor does), and restarting it would tear + # down the locator server it hosts and drop every connected client. The + # preprocessor reloads the new reference when acquisition restarts below; the + # processor keeps running. We only re-baseline the on-screen display history. self._history_run_signature = None - if self._supervisor.is_processor_running(): - self._stop_all_processes() self._reset_runtime_history() + # Tell the still-running data_processor to drop its accumulated background/ + # history so the new reference takes effect cleanly (no old/new-reference + # blend) without restarting the process. Reuses the live-config clear_all + # command, applied on the processor's next poll. + if self._supervisor.is_processor_running(): + self._write_live_processing_config(history_command="clear_all", bump_history_seq=True) self._refresh_preprocess_summary_labels() if collection.traces: self._draw_single_trace( diff --git a/python_app/gui/main.py b/python_app/gui/main.py index 1364525..93d30ac 100644 --- a/python_app/gui/main.py +++ b/python_app/gui/main.py @@ -2,6 +2,7 @@ from __future__ import annotations +from contextlib import suppress import os from pathlib import Path import signal @@ -63,7 +64,12 @@ def main() -> int: _install_unix_signal_handlers(app, window) else: window.showMaximized() - return app.exec() + exit_code = app.exec() + # Release hardware, SHM readers and child processes before exiting so that a + # systemd restart (after a headless fatal exit) starts from a clean slate. + with suppress(Exception): + window.close() + return exit_code if __name__ == "__main__": diff --git a/python_app/gui/runtime/history.py b/python_app/gui/runtime/history.py index 2fb2866..6bfffe5 100644 --- a/python_app/gui/runtime/history.py +++ b/python_app/gui/runtime/history.py @@ -59,12 +59,18 @@ def remove_last_aligned_histories( return retained_raw, retained_preprocessed, retained_results -def build_run_history_signature( +def build_processor_run_signature( config: RunConfigModel, ) -> tuple[object, ...]: - """Build deterministic signature to detect run-settings changes (excluding live processing params).""" + """Build signature of settings that change the data SHAPE the data_processor parses. + + Excludes the preprocess set names on purpose: the data_processor does not consume + calibration/reference sets (those are applied upstream by the data_preprocessor), so + changing a set must NOT restart the processor. Restarting it would also tear down the + locator TCP server it hosts and drop every connected client. Only genuine shape changes + (radar model, sweep, switch layout, combos) require a processor restart. + """ combos_signature = tuple((int(combo.input), int(combo.output)) for combo in config.combos) - preprocess_signature = tuple(preprocess_asset_model(config, key).set_name for key in PREPROCESS_ASSET_KEYS) sweep_points_signature: object = "adc" if config.is_kamil_adc else int(config.radar.sweep.points) return ( str(config.radar.model), @@ -85,11 +91,23 @@ def build_run_history_signature( str(config.output_switch.driver), int(config.output_switch.positions), bool(config.output_switch.invert_logic), - preprocess_signature, combos_signature, ) +def build_run_history_signature( + config: RunConfigModel, +) -> tuple[object, ...]: + """Build full signature for GUI display-history reset (processor shape + preprocess sets). + + Display history still resets when the reference/calibration set changes (the on-screen + B-scan would otherwise mix old- and new-reference frames), even though the processor + process itself is intentionally kept alive across that change. + """ + preprocess_signature = tuple(preprocess_asset_model(config, key).set_name for key in PREPROCESS_ASSET_KEYS) + return build_processor_run_signature(config) + (preprocess_signature,) + + def _tail_occurrence_key(history: list[THistoryCollection], index: int) -> tuple[int, int]: """Return `(collection_id, occurrence_from_tail)` for the item at `index`.""" collection_id = int(history[index].collection_id) diff --git a/python_app/hardware_full/librevna_multi_device_driver/controller.py b/python_app/hardware_full/librevna_multi_device_driver/controller.py index 898bd30..648df35 100644 --- a/python_app/hardware_full/librevna_multi_device_driver/controller.py +++ b/python_app/hardware_full/librevna_multi_device_driver/controller.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Iterator, Sequence +from contextlib import suppress from dataclasses import replace from typing import Optional import threading @@ -46,12 +47,15 @@ class MultiDeviceVnaController: self._is_closed = False try: + # Register each device the moment it opens so a partial open (e.g. a + # slave that fails after the master is up) is fully released by close(). + # Otherwise the master USB handle and its RX thread leak on every retry. self._master_device = LibreVnaUsbBulkConnection(master_serial_number) - self._slave_devices = [ - LibreVnaUsbBulkConnection(slave_serial_number) - for slave_serial_number in slave_serial_numbers - ] - self._all_devices = [self._master_device, *self._slave_devices] + self._all_devices.append(self._master_device) + for slave_serial_number in slave_serial_numbers: + connection = LibreVnaUsbBulkConnection(slave_serial_number) + self._slave_devices.append(connection) + self._all_devices.append(connection) except Exception: self.close() raise @@ -65,14 +69,20 @@ class MultiDeviceVnaController: self.close() def close(self) -> None: - """Stop sweeping and close every opened device transport.""" + """Stop sweeping and close every opened device transport. + + Resilient to a half-open or already-broken controller: failing to idle or + close one device must not prevent the others from being released. + """ if self._is_closed: return self._is_closed = True - self._send_idle_to_all_devices() + with suppress(Exception): + self._send_idle_to_all_devices() for device_connection in self._all_devices: - device_connection.close() + with suppress(Exception): + device_connection.close() def stop_continuous_sweep(self) -> None: """Stop the currently running sweep without closing device transports.""" diff --git a/python_app/hardware_full/multi_device_service.py b/python_app/hardware_full/multi_device_service.py index c9e71d4..30f5b8b 100644 --- a/python_app/hardware_full/multi_device_service.py +++ b/python_app/hardware_full/multi_device_service.py @@ -76,11 +76,14 @@ class MultiDeviceLibreVnaService: slave_serial_numbers=self.slave_serials, force_external_reference=self.force_external_reference, ) - except Exception: - if self.backend_mode == "native": - raise - self._using_mock_backend = True - self._controller = None + except Exception as exc: + # Never silently latch to synthetic data: a deployed appliance must wait + # for the real device, not record fakes. Synthetic data requires an + # explicit backend_mode='mock' (selected in __post_init__); both 'auto' + # and 'native' re-raise so the producer's wait-for-device retry keeps + # trying until the hardware appears. + logger.warning("Multi-device open failed (backend_mode=%s): %s", self.backend_mode, exc) + raise def close(self) -> None: """Close native device transports; never raises. diff --git a/python_app/hardware_full/switch_drivers/gpio_uapi.py b/python_app/hardware_full/switch_drivers/gpio_uapi.py index 12de1fd..3d4264b 100644 --- a/python_app/hardware_full/switch_drivers/gpio_uapi.py +++ b/python_app/hardware_full/switch_drivers/gpio_uapi.py @@ -1,4 +1,4 @@ -"""Minimal Linux GPIO v2 UAPI wrapper for output-only line control.""" +"""Minimal Linux GPIO v2 UAPI wrapper for output lines and input edge events.""" from __future__ import annotations @@ -12,7 +12,18 @@ from typing import Sequence GPIO_MAX_NAME_SIZE = 32 GPIO_V2_LINES_MAX = 64 GPIO_V2_LINE_NUM_ATTRS_MAX = 10 +GPIO_V2_LINE_FLAG_INPUT = 1 << 2 GPIO_V2_LINE_FLAG_OUTPUT = 1 << 3 +GPIO_V2_LINE_FLAG_EDGE_RISING = 1 << 4 +GPIO_V2_LINE_FLAG_EDGE_FALLING = 1 << 5 +GPIO_V2_LINE_FLAG_BIAS_PULL_UP = 1 << 8 +GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN = 1 << 9 +GPIO_V2_LINE_FLAG_BIAS_DISABLED = 1 << 10 + +GPIO_V2_LINE_ATTR_ID_DEBOUNCE = 3 + +GPIO_V2_LINE_EVENT_RISING_EDGE = 1 +GPIO_V2_LINE_EVENT_FALLING_EDGE = 2 _IOC_NRBITS = 8 _IOC_TYPEBITS = 8 @@ -96,6 +107,19 @@ class GpioV2LineValues(ctypes.Structure): ] +class GpioV2LineEvent(ctypes.Structure): + """ctypes mapping of `gpio_v2_line_event`.""" + + _fields_ = [ + ("timestamp_ns", ctypes.c_uint64), + ("id", ctypes.c_uint32), + ("offset", ctypes.c_uint32), + ("seqno", ctypes.c_uint32), + ("line_seqno", ctypes.c_uint32), + ("padding", ctypes.c_uint32 * 6), + ] + + GPIO_V2_GET_LINE_IOCTL = _iowr(0xB4, 0x07, GpioV2LineRequest) GPIO_V2_LINE_SET_VALUES_IOCTL = _iowr(0xB4, 0x0F, GpioV2LineValues) @@ -198,3 +222,112 @@ class GpioOutputLines: if self._chip_fd >= 0: os.close(self._chip_fd) self._chip_fd = -1 + + +class GpioLineEventWatcher: + """Watch a single GPIO input line for edge events via Linux GPIO v2 UAPI. + + The line file descriptor returned by the kernel becomes readable whenever a + requested edge occurs; each read yields exactly one ``gpio_v2_line_event``. + Callers drive the wait loop themselves (e.g. with :func:`select.select`) + using :meth:`fileno`, which keeps this wrapper free of any threading or + polling policy. + """ + + def __init__( + self, + chip: str, + offset: int, + *, + edge_flags: int, + bias_flags: int = 0, + debounce_us: int = 0, + consumer: str = "radar_input", + ) -> None: + """Build an input line request descriptor for edge detection.""" + if not chip: + raise ValueError("gpio chip path must not be empty") + if offset < 0: + raise ValueError("GPIO offset must be non-negative") + if not edge_flags: + raise ValueError("at least one edge flag is required") + + self._chip = chip + self._offset = int(offset) + self._flags = GPIO_V2_LINE_FLAG_INPUT | int(edge_flags) | int(bias_flags) + self._debounce_us = max(0, int(debounce_us)) + self._consumer = (consumer or "radar_input").encode("ascii", errors="ignore")[: GPIO_MAX_NAME_SIZE - 1] + + self._chip_fd = -1 + self._line_fd = -1 + + def open(self) -> None: + """Open GPIO chip and request the configured input line with edge events.""" + if self._line_fd >= 0: + return + + try: + self._chip_fd = os.open(self._chip, os.O_RDONLY | os.O_CLOEXEC) + except OSError as exc: + raise RuntimeError(f"Failed to open GPIO chip '{self._chip}': {exc}") from exc + + request = GpioV2LineRequest() + request.offsets[0] = ctypes.c_uint32(self._offset).value + request.num_lines = ctypes.c_uint32(1).value + request.config.flags = ctypes.c_uint64(self._flags).value + request.consumer = self._consumer + + if self._debounce_us > 0: + config_attr = request.config.attrs[0] + config_attr.attr.id = ctypes.c_uint32(GPIO_V2_LINE_ATTR_ID_DEBOUNCE).value + config_attr.attr.value = ctypes.c_uint64(self._debounce_us).value + config_attr.mask = ctypes.c_uint64(1).value # applies to line index 0 + request.config.num_attrs = ctypes.c_uint32(1).value + + try: + fcntl.ioctl(self._chip_fd, GPIO_V2_GET_LINE_IOCTL, request) + except OSError as exc: + self._close_chip_fd() + raise RuntimeError(f"Failed to request GPIO line on '{self._chip}': {exc}") from exc + + if request.fd < 0: + self._close_chip_fd() + raise RuntimeError(f"GPIO line request returned invalid fd for '{self._chip}'") + + self._line_fd = int(request.fd) + + def fileno(self) -> int: + """Return the line file descriptor for use with poll/select.""" + if self._line_fd < 0: + raise RuntimeError("GPIO line request is not open") + return self._line_fd + + def read_event(self) -> int: + """Read one queued edge event and return its event id (rising/falling).""" + if self._line_fd < 0: + raise RuntimeError("GPIO line request is not open") + + size = ctypes.sizeof(GpioV2LineEvent) + data = os.read(self._line_fd, size) + if len(data) < size: + raise RuntimeError(f"Short GPIO event read: expected {size} bytes, got {len(data)}") + + event = GpioV2LineEvent.from_buffer_copy(data) + return int(event.id) + + def close(self) -> None: + """Close line request and chip file descriptors.""" + self._close_line_fd() + self._close_chip_fd() + + def _close_line_fd(self) -> None: + """Close line file descriptor if currently open.""" + if self._line_fd >= 0: + os.close(self._line_fd) + self._line_fd = -1 + + def _close_chip_fd(self) -> None: + """Close chip file descriptor if currently open.""" + if self._chip_fd >= 0: + os.close(self._chip_fd) + self._chip_fd = -1 diff --git a/python_app/models/run_config_codec.py b/python_app/models/run_config_codec.py index a3a2b3d..1be7b60 100644 --- a/python_app/models/run_config_codec.py +++ b/python_app/models/run_config_codec.py @@ -12,7 +12,12 @@ from python_app.models.run_config_schema import ( PreprocessNotchModel, RunConfigModel, ) -from python_app.models.run_config_validation import load_ring_payload, load_switch_payload, validate_gpr_model +from python_app.models.run_config_validation import ( + load_control_button_payload, + load_ring_payload, + load_switch_payload, + validate_gpr_model, +) def _as_dict(value: Any, context: str) -> dict[str, Any]: @@ -82,6 +87,7 @@ def run_config_from_dict(payload: dict[str, Any]) -> RunConfigModel: switches_payload = _as_dict(payload.get("switches"), "switches") port1_payload = _as_dict(switches_payload.get("port1"), "switches.port1") port2_payload = _as_dict(switches_payload.get("port2"), "switches.port2") + control_button_payload = _as_dict(payload.get("control_button"), "control_button") run_payload = _as_dict(payload.get("run"), "run") preprocess_payload = _as_dict(payload.get("preprocess"), "preprocess") gpr_payload = _as_dict(payload.get("gpr"), "gpr") @@ -245,6 +251,7 @@ def run_config_from_dict(payload: dict[str, Any]) -> RunConfigModel: load_switch_payload(port1_payload, model.output_switch) load_switch_payload(port2_payload, model.input_switch) + load_control_button_payload(control_button_payload, model.control_button) model.apply_device_model_constraints() model.runtime.settling_ms = int(run_payload.get("settling_ms", model.runtime.settling_ms)) @@ -477,6 +484,15 @@ def run_config_to_dict(model: RunConfigModel) -> dict[str, Any]: "invert_logic": model.input_switch.invert_logic, }, }, + "control_button": { + "enabled": model.control_button.enabled, + "gpio_chip": model.control_button.gpio_chip, + "pin": model.control_button.pin, + "active_low": model.control_button.active_low, + "bias": model.control_button.bias, + "debounce_ms": model.control_button.debounce_ms, + "action": model.control_button.action, + }, "run": { "settling_ms": model.runtime.settling_ms, "idle_sleep_ms": model.runtime.idle_sleep_ms, diff --git a/python_app/models/run_config_schema.py b/python_app/models/run_config_schema.py index 49daa5c..f00b149 100644 --- a/python_app/models/run_config_schema.py +++ b/python_app/models/run_config_schema.py @@ -126,6 +126,27 @@ class SwitchModel: invert_logic: bool = False +@dataclass(slots=True) +class ControlButtonModel: + """Physical GPIO push-button that triggers a runtime action on press. + + Default wiring: the button sits between the GPIO line and GND with the + internal pull-up enabled, so the line idles high and a press drives it low + (``active_low``). The watcher reacts to the press edge only, so one push + yields one action. Disabled by default so non-Pi hosts never touch GPIO. + """ + + ACTION_CAPTURE_TMP_REFERENCE = "capture_tmp_reference" + + enabled: bool = False + gpio_chip: str = "/dev/gpiochip0" + pin: int = -1 + active_low: bool = True + bias: str = "" + debounce_ms: int = 50 + action: str = ACTION_CAPTURE_TMP_REFERENCE + + @dataclass(slots=True) class RingEndpointModel: """Shared-memory ring endpoint description.""" @@ -266,6 +287,7 @@ class RunConfigModel: preprocess: PreprocessModel = field(default_factory=PreprocessModel) gpr: GprModel = field(default_factory=GprModel) combos: list[ComboModel] = field(default_factory=list) + control_button: ControlButtonModel = field(default_factory=ControlButtonModel) LIBREVNA_MODEL = "librevna" LIBREVNA_MULTI_MODEL = "librevna_multi" diff --git a/python_app/models/run_config_validation.py b/python_app/models/run_config_validation.py index 6885f75..4a23bd6 100644 --- a/python_app/models/run_config_validation.py +++ b/python_app/models/run_config_validation.py @@ -4,7 +4,13 @@ from __future__ import annotations from typing import Any -from python_app.models.run_config_schema import ComboModel, GprModel, RingEndpointModel, SwitchModel +from python_app.models.run_config_schema import ( + ComboModel, + ControlButtonModel, + GprModel, + RingEndpointModel, + SwitchModel, +) def load_switch_payload( payload: dict[str, Any], @@ -23,6 +29,20 @@ def load_switch_payload( target.invert_logic = bool(payload.get("invert_logic", target.invert_logic)) +def load_control_button_payload( + payload: dict[str, Any], + target: ControlButtonModel, +) -> None: + """Populate control-button model from payload preserving defaults.""" + target.enabled = bool(payload.get("enabled", target.enabled)) + target.gpio_chip = str(payload.get("gpio_chip", target.gpio_chip)) + target.pin = int(payload.get("pin", target.pin)) + target.active_low = bool(payload.get("active_low", target.active_low)) + target.bias = str(payload.get("bias", target.bias)) + target.debounce_ms = int(payload.get("debounce_ms", target.debounce_ms)) + target.action = str(payload.get("action", target.action)) + + def load_ring_payload(payload: dict[str, Any], target: RingEndpointModel) -> None: """Populate ring endpoint model from payload preserving defaults.""" target.name = str(payload.get("name", target.name)) diff --git a/python_app/orchestration/config_writer.py b/python_app/orchestration/config_writer.py index edd35aa..7c7c99d 100644 --- a/python_app/orchestration/config_writer.py +++ b/python_app/orchestration/config_writer.py @@ -43,7 +43,10 @@ class ConfigWriter: def write(self, config: RunConfigModel, output_path: Path) -> Path: """Write run configuration JSON file.""" output_path.parent.mkdir(parents=True, exist_ok=True) - output_path.write_text(json.dumps(config.to_dict(), indent=2), encoding="utf-8") + # allow_nan=False: a stray NaN/Infinity must fail loudly here in Python + # rather than serialize to a non-standard token that aborts every C++ + # consumer at startup with an opaque JSON parse error. + output_path.write_text(json.dumps(config.to_dict(), indent=2, allow_nan=False), encoding="utf-8") return output_path diff --git a/python_app/orchestration/process_supervisor.py b/python_app/orchestration/process_supervisor.py index 9b3a629..883e464 100644 --- a/python_app/orchestration/process_supervisor.py +++ b/python_app/orchestration/process_supervisor.py @@ -20,7 +20,9 @@ class ManagedProcess: name: str command: list[str] allow_clean_exit: bool - handle: subprocess.Popen[str] + handle: subprocess.Popen[bytes] + stdout_path: Path + stderr_path: Path @dataclass(slots=True) @@ -141,30 +143,52 @@ class ProcessSupervisor: self._stop_processes(["sweep_orchestrator", "data_preprocessor", "data_processor"]) def _spawn(self, name: str, command: list[str], *, allow_clean_exit: bool) -> None: - """Spawn one process unless same process is already alive.""" + """Spawn one process unless same process is already alive. + + Child stdout/stderr are redirected to per-process log files rather than + captured pipes: a long-running child (e.g. an acquisition producer waiting + for its device) would otherwise fill the OS pipe buffer once nobody drains + it and block on write. Files never back-pressure the child, and they keep + a persistent log we can read for exit reports and tail for diagnostics. + """ existing = self._processes.get(name) if existing is not None and existing.handle.poll() is None: return + logs_dir = self._project_root / "python_app/runtime/logs" + logs_dir.mkdir(parents=True, exist_ok=True) + stdout_path = logs_dir / f"{name}.out.log" + stderr_path = logs_dir / f"{name}.err.log" + + stdout_file = open(stdout_path, "wb") + stderr_file = open(stderr_path, "wb") try: handle = subprocess.Popen( command, cwd=self._project_root, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, + stdout=stdout_file, + stderr=stderr_file, ) except OSError as exc: + stdout_file.close() + stderr_file.close() command_text = shlex.join(command) raise RuntimeError( f"Failed to spawn {name} with command `{command_text}` from `{self._project_root}`: " f"{type(exc).__name__}: {exc}" ) from exc + finally: + # The child holds its own dup'd fds; the parent's copies are not needed. + stdout_file.close() + stderr_file.close() + self._processes[name] = ManagedProcess( name=name, command=command, allow_clean_exit=allow_clean_exit, handle=handle, + stdout_path=stdout_path, + stderr_path=stderr_path, ) def _acquisition_command(self, config_path: Path) -> list[str]: @@ -236,6 +260,25 @@ class ProcessSupervisor: for name in exited_names: self._processes.pop(name, None) + @staticmethod + def _read_log_tail(path: Path, max_bytes: int = 16384) -> str: + """Return the trailing `max_bytes` of a child log file, decoded best-effort. + + Bounded so a large/long-lived log never produces an enormous exit report. + """ + try: + with open(path, "rb") as handle: + handle.seek(0, 2) + size = handle.tell() + if size > max_bytes: + handle.seek(-max_bytes, 2) + else: + handle.seek(0) + data = handle.read() + except OSError: + return "" + return data.decode("utf-8", errors="replace").strip() + def _is_alive(self, name: str) -> bool: """Return `True` when named process handle exists and is running.""" process = self._processes.get(name) @@ -253,12 +296,8 @@ class ProcessSupervisor: if return_code is None: continue - stderr = "" - stdout = "" - if process.handle.stdout is not None: - stdout = process.handle.stdout.read().strip() - if process.handle.stderr is not None: - stderr = process.handle.stderr.read().strip() + stdout = self._read_log_tail(process.stdout_path) + stderr = self._read_log_tail(process.stderr_path) reports.append( ProcessExitReport( diff --git a/python_app/scripts/matrix_raw_producer.py b/python_app/scripts/matrix_raw_producer.py index 6d0cd7d..5a622b5 100644 --- a/python_app/scripts/matrix_raw_producer.py +++ b/python_app/scripts/matrix_raw_producer.py @@ -17,27 +17,64 @@ from python_app.storage.npz.serialize import RAW_MAGIC, serialize_trace_collecti logger = logging.getLogger(__name__) -# Maximum number of acquisitions allowed to fail in a row before we give up and -# let the supervisor restart the whole process. Picked high enough to survive -# transient USB stalls (each retry triggers a full reset cycle of ~1-2s) but -# bounded so a permanently broken device does not loop forever. -_MAX_CONSECUTIVE_ACQUIRE_FAILURES = 20 -# Cooldown applied between a failed acquire and the next reset attempt. Stops -# us from busy-spinning when the device keeps refusing to come back. -_ACQUIRE_FAILURE_COOLDOWN_S = 1.0 +# The producer waits for the matrix radar forever: a device that is absent at boot +# or disappears mid-run must never kill the producer, only make it wait. Reconnect +# uses a capped exponential backoff so a long absence does not busy-spin, and every +# wait is interruptible by SIGINT/SIGTERM (stop_requested) for a prompt clean exit. +_OPEN_RETRY_MIN_S = 1.0 +_OPEN_RETRY_MAX_S = 10.0 +# Throttle open-failure logging during a long wait so a permanently absent device +# does not flood the process log: log the first failure, then every Nth attempt. +_OPEN_RETRY_LOG_EVERY = 30 -def _reset_radar_service( - config: RunConfigModel, previous: MatrixRadarService | None -) -> MatrixRadarService: - """Close `previous` (best-effort) and return a freshly opened+configured service.""" +def _open_radar_with_retry( + config: RunConfigModel, + previous: MatrixRadarService | None, + stop_requested: threading.Event, +) -> MatrixRadarService | None: + """Open+configure the matrix radar, retrying forever until success or stop. + + Used for both the initial open and every in-loop reconnect, so a device that is + absent at boot or disappears mid-run never kills the producer — it just waits. + Returns the opened service, or ``None`` if a stop was requested before any device + became available. Backoff is capped and every wait is interruptible by SIGTERM. + """ if previous is not None: with suppress(Exception): previous.close() - radar = create_matrix_radar_service(config) - radar.open() - radar.configure(config.radar.sweep) - return radar + + attempt = 0 + delay = _OPEN_RETRY_MIN_S + while not stop_requested.is_set(): + radar = create_matrix_radar_service(config) + try: + radar.open() + radar.configure(config.radar.sweep) + except Exception as exc: # noqa: BLE001 — waiting for the device is the point + with suppress(Exception): + radar.close() # drop any partial open before the next attempt + attempt += 1 + if attempt == 1 or attempt % _OPEN_RETRY_LOG_EVERY == 0: + logger.warning( + "Matrix radar not available (attempt %d); retrying every up to %.0fs " + "until the device is present: %s", + attempt, + _OPEN_RETRY_MAX_S, + exc, + ) + if stop_requested.wait(delay): + with suppress(Exception): + radar.close() + return None + delay = min(delay * 2.0, _OPEN_RETRY_MAX_S) + continue + + if attempt > 0: + logger.info("Matrix radar opened after %d attempt(s).", attempt + 1) + return radar + + return None def main() -> int: @@ -75,52 +112,26 @@ def main() -> int: ) radar: MatrixRadarService | None = None - consecutive_failures = 0 try: - radar = _reset_radar_service(config, previous=None) + radar = _open_radar_with_retry(config, previous=None, stop_requested=stop_requested) + if radar is None: + return 0 # asked to stop before a device became available collection_id = 1 while not stop_requested.is_set(): collection_start = time.monotonic() try: - if radar is None: - radar = _reset_radar_service(config, previous=None) collection = radar.acquire_collection(collection_id=collection_id) - except Exception as exc: # noqa: BLE001 — top-level recovery is the point - consecutive_failures += 1 - if consecutive_failures > _MAX_CONSECUTIVE_ACQUIRE_FAILURES: - logger.error( - "Matrix radar acquisition failed %d times in a row; giving up. " - "Last error: %s", - consecutive_failures - 1, - exc, - ) - raise + except Exception as exc: # noqa: BLE001 — reconnect forever, never give up logger.warning( - "Matrix radar acquisition failed (%d/%d), resetting service: %s", - consecutive_failures, - _MAX_CONSECUTIVE_ACQUIRE_FAILURES, + "Matrix radar acquisition failed; reconnecting and waiting for the device: %s", exc, exc_info=True, ) - # Cooldown gives slow USB stacks (and the device firmware) time - # to settle before the next open() attempt. - if stop_requested.wait(_ACQUIRE_FAILURE_COOLDOWN_S): - break - try: - radar = _reset_radar_service(config, previous=radar) - except Exception as reset_exc: # noqa: BLE001 - logger.warning( - "Matrix radar reset (%d/%d) failed, will retry: %s", - consecutive_failures, - _MAX_CONSECUTIVE_ACQUIRE_FAILURES, - reset_exc, - exc_info=True, - ) - radar = None + radar = _open_radar_with_retry(config, previous=radar, stop_requested=stop_requested) + if radar is None: + break # stop requested while waiting to reconnect continue - consecutive_failures = 0 - payload = serialize_trace_collection(collection, RAW_MAGIC) if not raw_writer.push(payload): raise RuntimeError( diff --git a/run_config.json b/run_config.json index 47c8cd7..97c2748 100644 --- a/run_config.json +++ b/run_config.json @@ -39,6 +39,15 @@ "invert_logic": false } }, + "control_button": { + "enabled": true, + "gpio_chip": "/dev/gpiochip0", + "pin": 26, + "active_low": true, + "bias": "", + "debounce_ms": 50, + "action": "capture_tmp_reference" + }, "run": { "settling_ms": 0, "idle_sleep_ms": 2, diff --git a/start.sh b/start.sh index 5a12d47..5a37096 100755 --- a/start.sh +++ b/start.sh @@ -10,6 +10,11 @@ REQUIREMENTS_FILE="${PROJECT_ROOT}/requirements.txt" PYTHON_CMD="" PROFILE_PATH="" +# Single-instance coordination: the headless daemon and the interactive GUI +# must never run at once (they share the radar, SHM rings and locator port). +SERVICE_NAME="radar.service" +LOCK_FILE="/tmp/radar_system.lock" + SKIP_BUILD=0 BUILD_ONLY=0 CLEAN_SHM=0 @@ -154,6 +159,15 @@ ensure_python_dependencies() { return fi + if ((HEADLESS == 1)); then + # Headless = unattended (often offline) appliance: never attempt a network + # pip install that could hang or crash-loop the service. Provisioning is a + # one-time interactive step. Fail fast with a clear, actionable message. + echo "Python dependencies are missing and headless mode does not provision them." >&2 + echo "Run an interactive './start.sh' once (online) to create the venv, then retry." >&2 + exit 1 + fi + echo "[start.sh] Installing Python dependencies into virtual environment..." "${VENV_PIP}" install --upgrade pip "${VENV_PIP}" install -r "${REQUIREMENTS_FILE}" @@ -311,11 +325,58 @@ run_producer_only() { exec "${PYTHON_CMD}" -m python_app.scripts.kamil_adc_raw_producer --config "${PROFILE_PATH}" } +stop_headless_service() { + # Stop the headless daemon (if running) so it releases the radar, SHM rings + # and locator port before the interactive GUI/producer takes over. Relies on + # the passwordless sudoers rule installed by deploy/install-daemon.sh. Safe + # no-op when systemd or the unit is absent (is-active is false -> skip). + command -v systemctl >/dev/null 2>&1 || return 0 + if systemctl is-active --quiet "${SERVICE_NAME}"; then + echo "[start.sh] Stopping ${SERVICE_NAME} so the GUI can take over the hardware..." + # Non-fatal: a sudo/systemctl failure must not abort the GUI launch under + # `set -e`. The single-instance lock below still prevents a real conflict. + if ! sudo systemctl stop "${SERVICE_NAME}"; then + echo "[start.sh] WARNING: failed to stop ${SERVICE_NAME}; relying on the instance lock." >&2 + fi + fi +} + +verify_cpp_binaries() { + # Guard the daemon's --skip-build path: refuse to run against a tree whose C++ + # binaries were never built, instead of failing obscurely at spawn time. + local missing=0 bin + for bin in data_processor data_preprocessor; do + if [[ ! -x "${PROJECT_ROOT}/build/bin/${bin}" ]]; then + echo "Required binary missing or not executable: build/bin/${bin}" >&2 + missing=1 + fi + done + if ((missing == 1)); then + echo "Build the C++ binaries first: run ./start.sh without --skip-build, or 'make all'." >&2 + exit 1 + fi +} + +acquire_single_instance_lock() { + # Fail fast if another instance already owns the hardware, instead of + # surfacing a cryptic 'shm ring busy' / 'port in use' later. fd 9 survives + # the exec into Python, so the lock is held for the whole app lifetime. + exec 9>"${LOCK_FILE}" + if ! flock -n 9; then + echo "[start.sh] Another radar instance already holds ${LOCK_FILE}; aborting." >&2 + echo "[start.sh] Stop it first: sudo systemctl stop ${SERVICE_NAME} (or close the other run)." >&2 + exit 1 + fi +} + main() { parse_args "$@" resolve_profile_path check_environment - if ((KAMIL_ADC_MODE == 0)); then + # Skip first-time provisioning (build headers, USB udev rule) in headless + # mode: the daemon runs unattended at boot as a non-root user and must not + # block on sudo. A fresh machine is provisioned by one interactive launch. + if ((KAMIL_ADC_MODE == 0 && HEADLESS == 0)); then ensure_system_dependencies ensure_usb_access_rules fi @@ -329,11 +390,21 @@ main() { build_cpp_binaries fi + verify_cpp_binaries + if ((BUILD_ONLY == 1)); then echo "[start.sh] Build completed." exit 0 fi + # Interactive GUI/producer launch: stop the headless daemon first so it + # frees the radar, SHM and locator port. The daemon itself runs --headless + # and skips this, so it never stops itself. + if ((HEADLESS == 0)); then + stop_headless_service + fi + acquire_single_instance_lock + if ((PRODUCER_ONLY == 1)); then run_producer_only fi