From 18db66bce327819dea9c0920f0e697cf71736363 Mon Sep 17 00:00:00 2001 From: thatsatechnique <28403172+thatsatechnique@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:52:32 -0800 Subject: [PATCH] =?UTF-8?q?fix(ook):=20harden=20for=20upstream=20review=20?= =?UTF-8?q?=E2=80=94=20tests,=20cleanup,=20CSS=20extraction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add kill_all() handler for OOK process cleanup on global reset - Fix stop_ook() to close pipes and join parser thread (prevents hangs) - Add ook.css with CSS classes, replace inline styles in ook.html - Register ook.css in lazy-load style map (INTERCEPT_MODE_STYLE_MAP) - Fix frontend frequency min=24 to match backend validation - Add 22 unit tests for decode_ook_frame, ook_parser_thread, and routes Co-Authored-By: Claude Opus 4.6 --- app.py | 9 +- routes/ook.py | 25 ++- static/css/modes/ook.css | 110 +++++++++++++ templates/index.html | 3 +- templates/partials/modes/ook.html | 41 +++-- tests/test_ook.py | 252 ++++++++++++++++++++++++++++++ 6 files changed, 413 insertions(+), 27 deletions(-) create mode 100644 static/css/modes/ook.css create mode 100644 tests/test_ook.py diff --git a/app.py b/app.py index 420f8a5..0c2c7f0 100644 --- a/app.py +++ b/app.py @@ -832,7 +832,7 @@ def health_check() -> Response: def kill_all() -> Response: """Kill all decoder, WiFi, and Bluetooth processes.""" global current_process, sensor_process, wifi_process, adsb_process, ais_process, acars_process - global vdl2_process, morse_process, radiosonde_process + global vdl2_process, morse_process, radiosonde_process, ook_process global aprs_process, aprs_rtl_process, dsc_process, dsc_rtl_process, bt_process # Import modules to reset their state @@ -896,6 +896,13 @@ def kill_all() -> Response: with morse_lock: morse_process = None + # Reset OOK state + with ook_lock: + if ook_process: + safe_terminate(ook_process) + unregister_process(ook_process) + ook_process = None + # Reset APRS state with aprs_lock: aprs_process = None diff --git a/routes/ook.py b/routes/ook.py index 87c209d..acedd0a 100644 --- a/routes/ook.py +++ b/routes/ook.py @@ -232,20 +232,39 @@ def start_ook() -> Response: return jsonify({'status': 'error', 'message': str(e)}), 500 +def _close_pipe(pipe_obj) -> None: + """Close a subprocess pipe, suppressing errors.""" + if pipe_obj is not None: + with contextlib.suppress(Exception): + pipe_obj.close() + + @ook_bp.route('/ook/stop', methods=['POST']) def stop_ook() -> Response: global ook_active_device with app_module.ook_lock: if app_module.ook_process: - stop_event = getattr(app_module.ook_process, '_stop_parser', None) + proc = app_module.ook_process + stop_event = getattr(proc, '_stop_parser', None) + parser_thread = getattr(proc, '_parser_thread', None) + + # Signal parser thread to stop if stop_event: stop_event.set() - safe_terminate(app_module.ook_process) - unregister_process(app_module.ook_process) + # Close pipes so parser thread unblocks from readline() + _close_pipe(getattr(proc, 'stdout', None)) + _close_pipe(getattr(proc, 'stderr', None)) + + safe_terminate(proc) + unregister_process(proc) app_module.ook_process = None + # Join parser thread with timeout + if parser_thread: + parser_thread.join(timeout=0.5) + if ook_active_device is not None: app_module.release_sdr_device(ook_active_device) ook_active_device = None diff --git a/static/css/modes/ook.css b/static/css/modes/ook.css new file mode 100644 index 0000000..beb0489 --- /dev/null +++ b/static/css/modes/ook.css @@ -0,0 +1,110 @@ +/* OOK Signal Decoder Styles */ + +.ook-presets { + display: flex; + flex-wrap: wrap; + gap: 4px; +} + +.ook-preset-add { + margin-top: 6px; + display: flex; + gap: 4px; +} + +.ook-preset-add input { + width: 80px; + background: var(--bg-tertiary, #111); + border: 1px solid var(--border-color, #222); + border-radius: 3px; + color: var(--text-dim); + font-family: var(--font-mono); + font-size: 11px; + padding: 3px 6px; +} + +.ook-preset-hint { + font-size: 9px; + color: var(--text-muted, #555); +} + +.ook-encoding-btns { + display: flex; + gap: 4px; +} + +.ook-encoding-btns .preset-btn { + flex: 1; +} + +.ook-timing-grid { + display: grid; + grid-template-columns: 1fr 1fr; + gap: 8px; +} + +.ook-timing-presets { + display: flex; + flex-wrap: wrap; + gap: 4px; + margin-top: 4px; +} + +.ook-status-row { + display: flex; + align-items: center; + gap: 8px; + font-size: 12px; + color: var(--text-dim); +} + +.ook-status-dot { + width: 8px; + height: 8px; + border-radius: 50%; + background: var(--text-dim); +} + +.ook-warning { + font-size: 11px; + color: #ffaa00; + line-height: 1.5; +} + +.ook-command-display { + display: none; + margin-top: 8px; +} + +.ook-command-label { + font-size: 10px; + color: var(--text-muted, #555); + text-transform: uppercase; + letter-spacing: 1px; +} + +.ook-command-text { + margin: 0; + padding: 6px 8px; + background: var(--bg-deep, #0a0a0a); + border: 1px solid var(--border-color, #1a2e1a); + border-radius: 4px; + font-family: var(--font-mono); + font-size: 10px; + color: var(--text-dim); + white-space: pre-wrap; + word-break: break-all; + line-height: 1.5; +} + +.ook-dedup-label { + display: flex; + align-items: center; + gap: 8px; + cursor: pointer; +} + +.ook-dedup-label input[type="checkbox"] { + width: auto; + margin: 0; +} diff --git a/templates/index.html b/templates/index.html index 8538606..4545891 100644 --- a/templates/index.html +++ b/templates/index.html @@ -86,7 +86,8 @@ morse: "{{ url_for('static', filename='css/modes/morse.css') }}", radiosonde: "{{ url_for('static', filename='css/modes/radiosonde.css') }}", meteor: "{{ url_for('static', filename='css/modes/meteor.css') }}", - system: "{{ url_for('static', filename='css/modes/system.css') }}" + system: "{{ url_for('static', filename='css/modes/system.css') }}", + ook: "{{ url_for('static', filename='css/modes/ook.css') }}" }; window.INTERCEPT_MODE_STYLE_LOADED = {}; window.INTERCEPT_MODE_STYLE_PROMISES = {}; diff --git a/templates/partials/modes/ook.html b/templates/partials/modes/ook.html index eb2919d..228b107 100644 --- a/templates/partials/modes/ook.html +++ b/templates/partials/modes/ook.html @@ -13,16 +13,15 @@

Frequency

- +
- -
+ +
-
- +
+
@@ -44,16 +43,14 @@

Modulation

-
+
+ style="background: var(--accent); color: #000;">PWM + onclick="OokMode.setEncoding('ppm')">PPM + onclick="OokMode.setEncoding('manchester')">Manchester

@@ -67,7 +64,7 @@

Pulse widths in microseconds for the flex decoder.

-
+
@@ -95,7 +92,7 @@
-
+
- 

+            

         

     

 
diff --git a/tests/test_ook.py b/tests/test_ook.py
new file mode 100644
index 0000000..4026c5b
--- /dev/null
+++ b/tests/test_ook.py
@@ -0,0 +1,252 @@
+"""Tests for OOK signal decoder utilities and route handlers."""
+
+from __future__ import annotations
+
+import io
+import json
+import queue
+import threading
+
+import pytest
+
+from utils.ook import decode_ook_frame, ook_parser_thread
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+def _login_session(client) -> None:
+    """Mark the Flask test session as authenticated."""
+    with client.session_transaction() as sess:
+        sess['logged_in'] = True
+        sess['username'] = 'test'
+        sess['role'] = 'admin'
+
+
+# ---------------------------------------------------------------------------
+# decode_ook_frame
+# ---------------------------------------------------------------------------
+
+class TestDecodeOokFrame:
+    def test_valid_hex_returns_bits_and_hex(self):
+        result = decode_ook_frame('aa55')
+        assert result is not None
+        assert result['hex'] == 'aa55'
+        assert result['bits'] == '1010101001010101'
+        assert result['byte_count'] == 2
+        assert result['bit_count'] == 16
+
+    def test_strips_0x_prefix(self):
+        result = decode_ook_frame('0xaa55')
+        assert result is not None
+        assert result['hex'] == 'aa55'
+
+    def test_strips_0X_uppercase_prefix(self):
+        result = decode_ook_frame('0Xff')
+        assert result is not None
+        assert result['hex'] == 'ff'
+        assert result['bits'] == '11111111'
+
+    def test_strips_spaces(self):
+        result = decode_ook_frame('aa 55')
+        assert result is not None
+        assert result['hex'] == 'aa55'
+
+    def test_invalid_hex_returns_none(self):
+        assert decode_ook_frame('zzzz') is None
+
+    def test_empty_string_returns_none(self):
+        assert decode_ook_frame('') is None
+
+    def test_just_0x_prefix_returns_none(self):
+        assert decode_ook_frame('0x') is None
+
+    def test_single_byte(self):
+        result = decode_ook_frame('48')
+        assert result is not None
+        assert result['bits'] == '01001000'
+        assert result['byte_count'] == 1
+
+    def test_hello_ascii(self):
+        """'Hello' in hex is 48656c6c6f."""
+        result = decode_ook_frame('48656c6c6f')
+        assert result is not None
+        assert result['hex'] == '48656c6c6f'
+        assert result['byte_count'] == 5
+        assert result['bit_count'] == 40
+
+
+# ---------------------------------------------------------------------------
+# ook_parser_thread
+# ---------------------------------------------------------------------------
+
+class TestOokParserThread:
+    def _run_parser(self, json_lines, encoding='pwm', deduplicate=False):
+        """Feed JSON lines to parser thread and collect output events."""
+        raw = '\n'.join(json.dumps(line) for line in json_lines) + '\n'
+        stdout = io.BytesIO(raw.encode('utf-8'))
+        output_queue = queue.Queue()
+        stop_event = threading.Event()
+
+        t = threading.Thread(
+            target=ook_parser_thread,
+            args=(stdout, output_queue, stop_event, encoding, deduplicate),
+        )
+        t.start()
+        t.join(timeout=2)
+
+        events = []
+        while not output_queue.empty():
+            events.append(output_queue.get_nowait())
+        return events
+
+    def test_parses_codes_field_list(self):
+        events = self._run_parser([{'codes': ['aa55']}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['hex'] == 'aa55'
+        assert frames[0]['bits'] == '1010101001010101'
+        assert frames[0]['inverted'] is False
+
+    def test_parses_codes_field_string(self):
+        events = self._run_parser([{'codes': 'ff00'}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['hex'] == 'ff00'
+
+    def test_parses_code_field(self):
+        events = self._run_parser([{'code': 'abcd'}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['hex'] == 'abcd'
+
+    def test_parses_data_field(self):
+        events = self._run_parser([{'data': '1234'}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['hex'] == '1234'
+
+    def test_strips_brace_bit_count_prefix(self):
+        """rtl_433 sometimes prefixes with {N} bit count."""
+        events = self._run_parser([{'codes': ['{16}aa55']}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['hex'] == 'aa55'
+
+    def test_deduplication_suppresses_consecutive_identical(self):
+        events = self._run_parser(
+            [{'codes': ['aa55']}, {'codes': ['aa55']}, {'codes': ['aa55']}],
+            deduplicate=True,
+        )
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+
+    def test_deduplication_allows_different_frames(self):
+        events = self._run_parser(
+            [{'codes': ['aa55']}, {'codes': ['ff00']}, {'codes': ['aa55']}],
+            deduplicate=True,
+        )
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 3
+
+    def test_no_code_field_emits_ook_raw(self):
+        events = self._run_parser([{'model': 'unknown', 'id': 42}])
+        raw_events = [e for e in events if e.get('type') == 'ook_raw']
+        assert len(raw_events) == 1
+
+    def test_rssi_extracted_from_snr(self):
+        events = self._run_parser([{'codes': ['aa55'], 'snr': 12.3}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+        assert frames[0]['rssi'] == 12.3
+
+    def test_encoding_passed_through(self):
+        events = self._run_parser([{'codes': ['aa55']}], encoding='manchester')
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert frames[0]['encoding'] == 'manchester'
+
+    def test_timestamp_present(self):
+        events = self._run_parser([{'codes': ['aa55']}])
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert 'timestamp' in frames[0]
+        assert len(frames[0]['timestamp']) > 0
+
+    def test_invalid_json_skipped(self):
+        """Non-JSON lines should be silently skipped."""
+        raw = b'not json\n{"codes": ["aa55"]}\n'
+        stdout = io.BytesIO(raw)
+        output_queue = queue.Queue()
+        stop_event = threading.Event()
+
+        t = threading.Thread(
+            target=ook_parser_thread,
+            args=(stdout, output_queue, stop_event),
+        )
+        t.start()
+        t.join(timeout=2)
+
+        events = []
+        while not output_queue.empty():
+            events.append(output_queue.get_nowait())
+        frames = [e for e in events if e.get('type') == 'ook_frame']
+        assert len(frames) == 1
+
+
+# ---------------------------------------------------------------------------
+# Route handlers
+# ---------------------------------------------------------------------------
+
+class TestOokRoutes:
+    @pytest.fixture
+    def client(self):
+        import app as app_module
+        from routes import register_blueprints
+
+        app_module.app.config['TESTING'] = True
+        if 'ook' not in app_module.app.blueprints:
+            register_blueprints(app_module.app)
+        with app_module.app.test_client() as c:
+            yield c
+
+    def test_status_returns_not_running(self, client):
+        _login_session(client)
+        resp = client.get('/ook/status')
+        assert resp.status_code == 200
+        data = resp.get_json()
+        assert data['running'] is False
+
+    def test_stop_when_not_running(self, client):
+        _login_session(client)
+        resp = client.post('/ook/stop')
+        assert resp.status_code == 200
+        data = resp.get_json()
+        assert data['status'] == 'not_running'
+
+    def test_start_validates_frequency(self, client):
+        _login_session(client)
+        resp = client.post('/ook/start',
+                           json={'frequency': 'invalid'},
+                           content_type='application/json')
+        assert resp.status_code == 400
+
+    def test_start_validates_encoding(self, client):
+        _login_session(client)
+        resp = client.post('/ook/start',
+                           json={'encoding': 'invalid_enc'},
+                           content_type='application/json')
+        assert resp.status_code == 400
+
+    def test_start_validates_timing_params(self, client):
+        _login_session(client)
+        resp = client.post('/ook/start',
+                           json={'short_pulse': 'not_a_number'},
+                           content_type='application/json')
+        assert resp.status_code == 400
+
+    def test_start_rejects_negative_frequency(self, client):
+        _login_session(client)
+        resp = client.post('/ook/start',
+                           json={'frequency': '-5'},
+                           content_type='application/json')
+        assert resp.status_code == 400