From 804e5800dd4a9a436eeb707c24db369fff0558c8 Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:08:19 +0200 Subject: [PATCH 1/6] Fix log-stream byte offsets: CRLF mid-line duplication, split UTF-8 chars, lines=0 Read output-file content in binary so offsets are real byte positions: text-mode tell() cookies plus newline translation delivered a trailing \r as \n twice when a CRLF log was polled mid-line, injecting spurious blank lines into the stream viewer. Hold back an incomplete trailing UTF-8 sequence so a character split across polls isn't rendered as replacement characters, and make lines=0 mean "no history, stream from the end" instead of returning the whole file. Also simplifies the health endpoint and hoists the re/sqlite3 imports. Co-Authored-By: Claude Fable 5 --- WebATM/server/routes.py | 120 ++++++++++++++++++++----------------- tests/test_routes_files.py | 70 ++++++++++++++++++++++ 2 files changed, 135 insertions(+), 55 deletions(-) diff --git a/WebATM/server/routes.py b/WebATM/server/routes.py index 09d2395..43f8a53 100644 --- a/WebATM/server/routes.py +++ b/WebATM/server/routes.py @@ -7,6 +7,8 @@ import json import os +import re +import sqlite3 import time from pathlib import Path @@ -129,6 +131,32 @@ def by_name(entry): return sorted(folders, key=by_name) + sorted(files, key=by_name) +def _split_incomplete_utf8(data): + """Split off an incomplete trailing UTF-8 sequence from ``data``. + + A log poll can catch the writer mid-character; the held-back bytes are + re-read whole on the next poll instead of being decoded as replacement + characters twice. + + Args: + data (bytes): Chunk read up to end-of-file. + + Returns: + tuple[bytes, bytes]: ``(complete, held_back)``; ``held_back`` is empty + unless ``data`` ends inside a multi-byte character. + """ + for i in range(1, min(3, len(data)) + 1): + byte = data[-i] + if byte >= 0xC0: # lead byte of a 2-4 byte sequence + seq_len = 2 if byte < 0xE0 else 3 if byte < 0xF0 else 4 + if seq_len > i: + return data[:-i], data[-i:] + break + if byte < 0x80: # ASCII, nothing pending + break + return data, b"" + + def get_webpack_assets(): """Read the webpack manifest and build script tags in load order. @@ -142,13 +170,11 @@ def get_webpack_assets(): list[str]: HTML ``'] with open(manifest_path) as f: @@ -171,7 +197,6 @@ def get_webpack_assets(): except Exception as e: logger.info(f"Error reading webpack manifest: {e}") - # Fallback to single bundle.js return [''] @@ -271,14 +296,12 @@ def update_server_config(): set_bluesky_proxy, ) - # Every (re)connect gets a completely fresh proxy: recreating the - # ZMQ client is the reliable way to shed any half-dead connection - # state. Only the Socket.IO wiring carries over. The old proxy is - # replaced in place (never deleted) so concurrent requests always - # find a usable current_app.bluesky_proxy. The swap-and-connect + # Every (re)connect gets a fresh proxy — recreating the ZMQ client + # is the reliable way to shed half-dead connection state; only the + # Socket.IO wiring carries over. The old proxy is replaced in place + # so concurrent requests always find a usable proxy, and the swap # runs under connect_lock so the integrated auto-start (or another - # concurrent connect request) can never revive the proxy this - # request is tearing down. + # connect request) can't revive the proxy being torn down. with connect_lock: old_proxy = getattr(current_app, "bluesky_proxy", None) if old_proxy is not None: @@ -516,15 +539,11 @@ def search_navdata(): # (this also strips any FTS syntax the user might type) and turn # each into a prefix term so "heath" matches "Heathrow" and "kse" # matches "KSEA". Multiple tokens are implicitly AND-ed. - import re - tokens = re.findall(r"[A-Za-z0-9]+", query) if not tokens: return jsonify({"success": True, "results": []}) match_expr = " ".join(f"{t}*" for t in tokens) - import sqlite3 - # Open read-only so a concurrent rebuild can't be corrupted. conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) try: @@ -609,38 +628,20 @@ def status_check(): sections, or 503 with the error on failure. """ try: - hostname = getattr(current_app.bluesky_proxy, "server_ip", None) - listening, _ = probe_bluesky_ports(hostname) - port_11000_listening = 11000 in listening - port_11001_listening = 11001 in listening - bluesky_running = bool(listening) - - # Additional check: if we have a proxy connection, see if it's receiving data - proxy_running = False - proxy_connected = False - has_active_nodes = False - if hasattr(current_app, "bluesky_proxy"): - proxy_running = getattr(current_app.bluesky_proxy, "running", False) - proxy_connected = getattr( - current_app.bluesky_proxy, "is_connected", False - ) - tracked_nodes = getattr(current_app.bluesky_proxy, "tracked_nodes", []) - has_active_nodes = len(tracked_nodes) > 0 - - # Get session information from session manager - session_info = session_manager.get_session_info() + proxy = getattr(current_app, "bluesky_proxy", None) + listening, _ = probe_bluesky_ports(getattr(proxy, "server_ip", None)) response_data = { "status": "healthy", "bluesky_server": { - "ports_accessible": bluesky_running, - "port_11000": port_11000_listening, - "port_11001": port_11001_listening, - "proxy_running": proxy_running, - "proxy_connected": proxy_connected, - "has_active_nodes": has_active_nodes, + "ports_accessible": bool(listening), + "port_11000": 11000 in listening, + "port_11001": 11001 in listening, + "proxy_running": getattr(proxy, "running", False), + "proxy_connected": getattr(proxy, "is_connected", False), + "has_active_nodes": len(getattr(proxy, "tracked_nodes", [])) > 0, }, - "session_info": session_info, + "session_info": session_manager.get_session_info(), "timestamp": time.time(), } @@ -1000,23 +1001,24 @@ def download_output_file(filepath): def get_output_file_content(filepath): """Read output-file content (GET /api/bluesky/output/content/). - Supports log streaming: with ``offset`` > 0 the file is read - incrementally from that byte offset to the end; with offset 0 the + Supports log streaming with plain byte offsets: with ``offset`` > 0 + the file is read from that byte offset to the end; with offset 0 the last ``lines`` lines are tailed for the initial load. If the file shrank below the offset (truncated/rewritten between polls), the stream restarts with a tail load instead of silently skipping the new content. Query parameters: - ``offset``: byte offset to read from (0 = tail mode). - - ``lines``: maximum lines for the initial tail load (default 200). + - ``lines``: maximum lines for the initial tail load (default 200); + 0 or negative skips history and streams from the current end. Args: filepath (str): Path of the file relative to the output directory; validated against traversal. Returns: - JSON with ``content``, the new ``offset``, ``total_size`` and - ``filename``, or a 400/403/404/500 error payload. + JSON with ``content``, the new byte ``offset``, ``total_size`` + and ``filename``, or a 400/403/404/500 error payload. """ try: resolved_path, error = _validate_output_path(filepath) @@ -1028,23 +1030,31 @@ def get_output_file_content(filepath): file_size = resolved_path.stat().st_size # A file smaller than the poller's offset was truncated or - # rewritten (e.g. a re-run scenario logging to the same name). - # The offset points into the old contents, so restart with a - # tail load instead of pinning the stream at end-of-file, which - # would silently skip everything the new file already holds. + # rewritten (e.g. a re-run scenario logging to the same name); + # restart with a tail load instead of pinning at end-of-file. if offset > file_size: offset = 0 - with open(resolved_path, errors="replace") as f: + # Read in binary so offsets are real byte positions. Text-mode + # tell() returns opaque cookies whose newline translation also + # delivered a trailing \r as \n twice when a CRLF log was caught + # mid-line (a spurious blank line in the stream viewer). + with open(resolved_path, "rb") as f: if offset > 0: - # Incremental read from offset to end. f.seek(offset) - content = f.read() - else: + data = f.read() + elif max_lines > 0: # Initial (or post-truncation) load: tail the last N lines. - content = "".join(f.readlines()[-max_lines:]) + data = b"".join(f.readlines()[-max_lines:]) + else: + f.seek(0, os.SEEK_END) + data = b"" new_offset = f.tell() + data, held_back = _split_incomplete_utf8(data) + new_offset -= len(held_back) + content = data.decode("utf-8", errors="replace").replace("\r\n", "\n") + return jsonify( { "success": True, diff --git a/tests/test_routes_files.py b/tests/test_routes_files.py index d0a7261..d4e6b03 100644 --- a/tests/test_routes_files.py +++ b/tests/test_routes_files.py @@ -314,6 +314,76 @@ def test_truncated_file_restarts_stream(self, client): assert "second run" in body["content"] assert body["offset"] == log.stat().st_size + def test_crlf_log_caught_mid_line_does_not_duplicate_newline(self, client): + # A CRLF log polled between the \r and the \n used to deliver that + # line ending twice: text-mode reading translated the trailing \r to + # \n in one poll, then the next poll delivered the real \n again — + # injecting a spurious blank line into the stream viewer. + output_dir = client.base_path / "output" + output_dir.mkdir(exist_ok=True) + log = output_dir / "run.log" + log.write_bytes(b"line one\r\nline two\r") + + initial = client.get("/api/bluesky/output/content/run.log").get_json() + assert initial["offset"] == log.stat().st_size + assert "line one\nline two" in initial["content"] + + # The writer completes the \r\n and adds another line. + with log.open("ab") as f: + f.write(b"\nline three\r\n") + resp = client.get( + f"/api/bluesky/output/content/run.log?offset={initial['offset']}" + ).get_json() + assert "line three" in resp["content"] + assert resp["offset"] == log.stat().st_size + + # What the client renders across both polls holds no blank line. + combined = initial["content"] + resp["content"] + assert "\n\n" not in combined + assert combined.replace("\r", "") == "line one\nline two\nline three\n" + + def test_multibyte_char_split_across_polls_is_held_back(self, client): + # A UTF-8 character split by the poll must be re-read whole on the + # next poll, not rendered as two replacement characters. + output_dir = client.base_path / "output" + output_dir.mkdir(exist_ok=True) + log = output_dir / "run.log" + payload = "altitude café".encode() + log.write_bytes(payload[:-2]) # cut inside the 2-byte "é" + + initial = client.get("/api/bluesky/output/content/run.log").get_json() + assert initial["content"] == "altitude caf" + assert "�" not in initial["content"] + + with log.open("ab") as f: + f.write(payload[-2:] + b"\n") + resp = client.get( + f"/api/bluesky/output/content/run.log?offset={initial['offset']}" + ).get_json() + assert resp["content"] == "é\n" + + def test_lines_zero_skips_history(self, client): + # lines=0 used to return the entire file ([-0:] slices everything); + # it now means "no history — stream from the current end". + output_dir = client.base_path / "output" + output_dir.mkdir(exist_ok=True) + log = output_dir / "run.log" + log.write_text("a\nb\nc\n") + + body = client.get("/api/bluesky/output/content/run.log?lines=0").get_json() + assert body["content"] == "" + assert body["offset"] == log.stat().st_size + + def test_lines_limits_initial_tail(self, client): + output_dir = client.base_path / "output" + output_dir.mkdir(exist_ok=True) + log = output_dir / "run.log" + log.write_text("a\nb\nc\n") + + body = client.get("/api/bluesky/output/content/run.log?lines=2").get_json() + assert body["content"] == "b\nc\n" + assert body["offset"] == log.stat().st_size + class TestFileStatusConfigured: def test_filestatus_after_configuration(self, client): From ddec75a3c1b1d21348dc6bd97398bfeae237100a Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:08:28 +0200 Subject: [PATCH 2/6] Ignore double-clicks and suspend zoom during map drawing modes The second click of a double-click (originalEvent.detail > 1) is a repeat of the first, not a deliberate placement: it dropped duplicate vertices, instantly finished two-click shapes with degenerate geometry, and created aircraft with a meaningless heading. All drawing modes now skip it and disable MapLibre's double-click zoom while drawing (restored on stop). Aircraft creation also rejects a heading click on the exact spawn position (e.g. both clicks snapped to the same navaid), mirroring the circle tool's zero-radius guard. Co-Authored-By: Claude Fable 5 --- .../src/ui/map/BaseDrawingManager.test.ts | 21 +++++++++- frontend/src/ui/map/BaseDrawingManager.ts | 10 +++++ .../aircraft/AircraftCreationManager.test.ts | 42 ++++++++++++++++++- .../map/aircraft/AircraftCreationManager.ts | 29 ++++++++++--- .../ui/map/routes/RouteDrawingManager.test.ts | 1 + .../ui/map/shapes/ShapeDrawingManager.test.ts | 3 +- 6 files changed, 95 insertions(+), 11 deletions(-) diff --git a/frontend/src/ui/map/BaseDrawingManager.test.ts b/frontend/src/ui/map/BaseDrawingManager.test.ts index 537e038..291782b 100644 --- a/frontend/src/ui/map/BaseDrawingManager.test.ts +++ b/frontend/src/ui/map/BaseDrawingManager.test.ts @@ -16,6 +16,7 @@ function createFakeMap() { return { handlers, canvas, + doubleClickZoom: { enable: vi.fn(), disable: vi.fn() }, on: vi.fn((event: string, handler: (e: unknown) => void) => { (handlers[event] ??= []).push(handler); }), @@ -69,8 +70,8 @@ describe('BaseDrawingManager', () => { let snapper: { snap: ReturnType; highlight: ReturnType; clearHighlight: ReturnType }; let manager: TestDrawingManager; - const clickEvent = (lat: number, lng: number) => - ({ lngLat: { lat, lng }, preventDefault: vi.fn() }) as unknown as MapMouseEvent; + const clickEvent = (lat: number, lng: number, detail = 1) => + ({ lngLat: { lat, lng }, originalEvent: { detail }, preventDefault: vi.fn() }) as unknown as MapMouseEvent; function createManager( finishOnEnter = false, @@ -118,6 +119,22 @@ describe('BaseDrawingManager', () => { expect(manager.points).toEqual([{ lat: 50, lng: 5 }]); }); + it('ignores the second click of a double-click', () => { + manager.start(); + map.fire('click', clickEvent(52, 4)); + map.fire('click', clickEvent(52, 4, 2)); + expect(manager.points).toEqual([{ lat: 52, lng: 4 }]); + }); + + it('suspends double-click zoom while drawing and restores it after', () => { + manager.start(); + expect(map.doubleClickZoom.disable).toHaveBeenCalledTimes(1); + expect(map.doubleClickZoom.enable).not.toHaveBeenCalled(); + + manager.stop(); + expect(map.doubleClickZoom.enable).toHaveBeenCalledTimes(1); + }); + it('mousemove highlights navaids and reports the cursor position', () => { manager.start(); map.fire('mousemove', clickEvent(51, 3)); diff --git a/frontend/src/ui/map/BaseDrawingManager.ts b/frontend/src/ui/map/BaseDrawingManager.ts index 443445f..2f6861f 100644 --- a/frontend/src/ui/map/BaseDrawingManager.ts +++ b/frontend/src/ui/map/BaseDrawingManager.ts @@ -102,6 +102,10 @@ export abstract class BaseDrawingManager { // aircraft placement) so two tools never consume the same clicks. claimDrawing(this, () => this.cancelDrawing()); + // A double-click during a draw is two placement clicks, not a zoom + // request; restored in suspendMapInteraction(). + map.doubleClickZoom.disable(); + map.getCanvas().style.cursor = DRAWING_CURSOR; this.onDrawingEnabled(); @@ -138,6 +142,7 @@ export abstract class BaseDrawingManager { // listener and the navaid highlight must be released regardless. const map = this.mapDisplay.getMap(); if (map) { + map.doubleClickZoom.enable(); map.getCanvas().style.cursor = ''; if (this.mapClickHandler) map.off('click', this.mapClickHandler); if (this.mapRightClickHandler) map.off('contextmenu', this.mapRightClickHandler); @@ -158,6 +163,11 @@ export abstract class BaseDrawingManager { private onMapClick(e: MapMouseEvent): void { if (!this.drawingMode) return; + // The second click of a double-click is a repeat of the first, not a + // deliberate placement: it would drop a duplicate vertex, or instantly + // finish a two-click shape (box/circle) with degenerate geometry. + if (e.originalEvent.detail > 1) return; + // Snap to a nearby navaid when enabled, else use the raw click. const snapped = this.navaidSnapper.snap(e); const point = snapped diff --git a/frontend/src/ui/map/aircraft/AircraftCreationManager.test.ts b/frontend/src/ui/map/aircraft/AircraftCreationManager.test.ts index f57cb44..13d6471 100644 --- a/frontend/src/ui/map/aircraft/AircraftCreationManager.test.ts +++ b/frontend/src/ui/map/aircraft/AircraftCreationManager.test.ts @@ -23,6 +23,7 @@ function createFakeMap() { const layers: Record = {}; return { handlers, + doubleClickZoom: { enable: vi.fn(), disable: vi.fn() }, getCanvas: () => ({ style: { cursor: '' } }), on: vi.fn((event: string, handler: (e: unknown) => void) => { (handlers[event] ??= []).push(handler); @@ -50,8 +51,8 @@ function createFakeMap() { }; } -function clickEvent(lat: number, lng: number): MapMouseEvent { - return { lngLat: { lat, lng } } as unknown as MapMouseEvent; +function clickEvent(lat: number, lng: number, detail = 1): MapMouseEvent { + return { lngLat: { lat, lng }, originalEvent: { detail } } as unknown as MapMouseEvent; } function creationData(id: string): AircraftCreationData { @@ -151,6 +152,43 @@ describe('AircraftCreationManager map drawing', () => { expect(app.sendCommand).not.toHaveBeenCalled(); }); + it('a double-click places the position once instead of completing the draw', () => { + draw.startAircraftDrawing(creationData('AC1')); + // A double-click delivers two click events; the second carries + // detail=2 and must not be treated as the heading click. + map.fire('click', clickEvent(52, 4)); + map.fire('click', clickEvent(52, 4, 2)); + + expect(app.sendCommand).not.toHaveBeenCalled(); + expect(bannerVisible()).toBe(true); + + // The draw is still live: a real heading click completes it. + map.fire('click', clickEvent(53, 4)); + expect(app.sendCommand).toHaveBeenCalledWith('CRE AC1,B738,52,4,0,10000,250'); + }); + + it('ignores a heading click on the exact spawn position', () => { + draw.startAircraftDrawing(creationData('AC1')); + // Both clicks snapping to the same navaid yields identical points, + // whose bearing is meaningless. + map.fire('click', clickEvent(52, 4)); + map.fire('click', clickEvent(52, 4)); + + expect(app.sendCommand).not.toHaveBeenCalled(); + + map.fire('click', clickEvent(53, 4)); + expect(app.sendCommand).toHaveBeenCalledWith('CRE AC1,B738,52,4,0,10000,250'); + }); + + it('suspends double-click zoom while drawing and restores it after', () => { + draw.startAircraftDrawing(creationData('AC1')); + expect(map.doubleClickZoom.disable).toHaveBeenCalledTimes(1); + expect(map.doubleClickZoom.enable).not.toHaveBeenCalled(); + + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + expect(map.doubleClickZoom.enable).toHaveBeenCalledTimes(1); + }); + it('Escape also cancels after the position click', () => { draw.startAircraftDrawing(creationData('AC1')); map.fire('click', clickEvent(52, 4)); diff --git a/frontend/src/ui/map/aircraft/AircraftCreationManager.ts b/frontend/src/ui/map/aircraft/AircraftCreationManager.ts index 5ec3ba4..f54788c 100644 --- a/frontend/src/ui/map/aircraft/AircraftCreationManager.ts +++ b/frontend/src/ui/map/aircraft/AircraftCreationManager.ts @@ -76,10 +76,8 @@ export class AircraftCreationManager { * Invoked by AircraftCreationForm after the modal closes. */ private startAircraftDrawing(data: AircraftCreationData): void { - // Restart cleanly if a previous draw is still active - otherwise the - // old map handlers stay attached (their references get overwritten - // below, so they could never be removed again) and a single click - // would fire twice, completing the draw instantly with heading 0. + // Restart cleanly if a previous draw is still active - stale handlers + // would double-fire each click and could never be removed again. if (this.aircraftDrawingMode) { this.stopAircraftDrawing(); } @@ -131,8 +129,11 @@ export class AircraftCreationManager { // the same map clicks. claimDrawing(this, () => this.stopAircraftDrawing()); - // Match the crosshair cursor used by the console map picker and the - // shape/route drawing modes so every drawing mode looks the same. + // A double-click during the draw is two placement clicks, not a zoom + // request; restored in disableAircraftMapDrawing(). + map.doubleClickZoom.disable(); + + // Crosshair cursor, matching the other drawing modes. map.getCanvas().style.cursor = DRAWING_CURSOR; this.aircraftMapClickHandler = (e: MapMouseEvent) => { @@ -171,6 +172,7 @@ export class AircraftCreationManager { // listener and the drawing claim must be released regardless. const map = this.mapDisplay.getMap(); if (map) { + map.doubleClickZoom.enable(); // Restore MapLibre's default cursor when leaving drawing mode. map.getCanvas().style.cursor = ''; if (this.aircraftMapClickHandler) map.off('click', this.aircraftMapClickHandler); @@ -198,12 +200,27 @@ export class AircraftCreationManager { private handleAircraftMapClick(e: MapMouseEvent): void { if (!this.aircraftDrawingMode) return; + // The second click of a double-click is a repeat of the first, not a + // deliberate placement - without this, double-clicking the position + // would instantly create the aircraft with a meaningless heading. + if (e.originalEvent.detail > 1) return; + // Snap both clicks to a nearby navaid when enabled: the first click sets // the spawn position, the second sets the heading/direction (aim at a // known navaid for a precise heading). let point: [number, number] = [e.lngLat.lng, e.lngLat.lat]; const snapped = this.navaidSnapper.snap(e); if (snapped) point = [snapped.lng, snapped.lat]; + + // A heading click on the exact spawn position (e.g. both clicks + // snapped to the same navaid) has no direction; drop it and keep + // waiting, mirroring the circle tool's zero-radius guard. + const [position] = this.aircraftDrawingPoints; + if (position && position[0] === point[0] && position[1] === point[1]) { + this.updateDrawingBanner('Click a point away from the aircraft to set its heading'); + return; + } + this.aircraftDrawingPoints.push(point); if (this.aircraftDrawingPoints.length === 1) { diff --git a/frontend/src/ui/map/routes/RouteDrawingManager.test.ts b/frontend/src/ui/map/routes/RouteDrawingManager.test.ts index 3b64b4f..447f56a 100644 --- a/frontend/src/ui/map/routes/RouteDrawingManager.test.ts +++ b/frontend/src/ui/map/routes/RouteDrawingManager.test.ts @@ -110,6 +110,7 @@ describe('RouteDrawingManager finish hand-off to the constraints modal', () => { off(ev: string, fn: unknown) { handlers.get(ev)?.delete(fn); }, + doubleClickZoom: { enable: () => undefined, disable: () => undefined }, getCanvas: () => ({ style: {} as CSSStyleDeclaration }), getSource: () => undefined, addSource: () => undefined, diff --git a/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts b/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts index 9dc7842..fbf878e 100644 --- a/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts +++ b/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts @@ -103,6 +103,7 @@ class FakeMap { removeLayer(id: string): void { this.layers.delete(id); } + doubleClickZoom = { enable: (): void => {}, disable: (): void => {} }; getCanvas(): { style: { cursor: string } } { return { style: { cursor: '' } }; } @@ -158,7 +159,7 @@ describe('ShapeDrawingManager create validation and finish', () => { } function mapClick(lat: number, lng: number): void { - fakeMap.handlers.get('click')?.({ lngLat: { lat, lng } } as MapMouseEvent); + fakeMap.handlers.get('click')?.({ lngLat: { lat, lng }, originalEvent: { detail: 1 } } as MapMouseEvent); } function mapRightClick(): void { From 42991ea3c636eeef493e65a7568d381b4a3bc751 Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:08:37 +0200 Subject: [PATCH 3/6] Fix aircraft selection races: sync empty-map hit test, route broadcast handoff The empty-map-click unselect deferred its hit test by 50 ms, letting the select-triggered flyTo (or a data update) move the aircraft off the clicked point and misread an aircraft click as an empty-map click, instantly unselecting the aircraft that was just clicked. The hit test is now synchronous with the click dispatch. When selection switches directly between aircraft, the old aircraft's route broadcast is now toggled off; before, it kept streaming ROUTEDATA that got re-interpreted as an implicit selection stealing the new selection back. Unsolicited ROUTEDATA only becomes an implicit selection while nothing is selected. Drops the now-unused sendExplicitPos wrapper. Co-Authored-By: Claude Fable 5 --- frontend/src/core/App.ts | 8 +- .../AircraftInteractionManager.test.ts | 88 ++++++++++++++- .../aircraft/AircraftInteractionManager.ts | 106 ++++++++---------- 3 files changed, 133 insertions(+), 69 deletions(-) diff --git a/frontend/src/core/App.ts b/frontend/src/core/App.ts index 0e68621..e7c34c1 100644 --- a/frontend/src/core/App.ts +++ b/frontend/src/core/App.ts @@ -422,10 +422,12 @@ export class App { // Check if we just sent an explicit POS command for this aircraft const isExplicitPosResponse = this.aircraftInteractionManager?.wasLastExplicitPosFor(data.acid) ?? false; - // If receiving ROUTEDATA for an aircraft that is not currently selected, - // treat it as an implicit selection ONLY if it's not a response to our explicit POS + // Unsolicited ROUTEDATA (a route broadcast enabled outside this + // client) becomes an implicit selection, but only while nothing + // is selected: it must never steal an active selection (e.g. a + // still-streaming broadcast of a previously selected aircraft). const currentSelection = this.stateManager.getState().selectedAircraft; - if (data.acid && data.acid !== currentSelection && !isExplicitPosResponse) { + if (data.acid && !currentSelection && !isExplicitPosResponse) { logger.info('App', '🛰️ Unsolicited ROUTEDATA received for', data.acid, '- treating as implicit selection'); this.stateManager.setSelectedAircraft(data.acid); } diff --git a/frontend/src/ui/map/aircraft/AircraftInteractionManager.test.ts b/frontend/src/ui/map/aircraft/AircraftInteractionManager.test.ts index 948a043..e256c3a 100644 --- a/frontend/src/ui/map/aircraft/AircraftInteractionManager.test.ts +++ b/frontend/src/ui/map/aircraft/AircraftInteractionManager.test.ts @@ -1,8 +1,11 @@ // @vitest-environment happy-dom /** - * Tests for AircraftInteractionManager panel-event handling: cleanup on - * destroy() (the document-level listeners used to outlive the map) and - * the unselect path (stop follow mode, toggle the route broadcast off). + * Tests for AircraftInteractionManager panel-event handling — cleanup on + * destroy() (the document-level listeners used to outlive the map), the + * unselect path (stop follow mode, toggle the route broadcast off) — and + * the empty-map-click unselect, which must hit-test synchronously (a + * deferred check used to misread aircraft clicks as empty-map clicks once + * the camera or the aircraft had moved off the clicked point). */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { AircraftInteractionManager } from './AircraftInteractionManager'; @@ -16,13 +19,23 @@ import type { AircraftData } from '../../../data/types'; const stubMap = () => ({ on: vi.fn(), once: vi.fn((_event: string, cb: () => void) => cb()), - getLayer: vi.fn(() => undefined), - queryRenderedFeatures: vi.fn(() => []), + getLayer: vi.fn(() => ({ id: 'aircraft-points' })), + queryRenderedFeatures: vi.fn((): unknown[] => []), getZoom: vi.fn(() => 8), easeTo: vi.fn(), flyTo: vi.fn(), getCanvas: vi.fn(() => ({ style: {} })), }); +type StubMap = ReturnType; + +// The generic (non-layer) handler registered for a map event. +const mapHandler = (map: StubMap, event: string): ((e: unknown) => void) => { + const call = map.on.mock.calls.find( + (c: unknown[]) => c[0] === event && typeof c[1] === 'function' + ); + if (!call) throw new Error(`no generic ${event} handler registered`); + return call[1] as (e: unknown) => void; +}; const panelEvent = (type: 'aircraft-single-click' | 'aircraft-double-click' | 'aircraft-unselect', aircraftId: string) => document.dispatchEvent(new CustomEvent(type, { detail: { aircraftId } })); @@ -44,12 +57,14 @@ describe('AircraftInteractionManager', () => { let manager: AircraftInteractionManager; let stateManager: StateManager; let sendCommand: ReturnType; + let map: StubMap; beforeEach(() => { sendCommand = vi.fn(); stateManager = new StateManager(); + map = stubMap(); manager = new AircraftInteractionManager( - { getMap: () => stubMap() } as unknown as MapDisplay, + { getMap: () => map } as unknown as MapDisplay, stateManager, { sendCommand } as unknown as SocketManager, ); @@ -86,4 +101,65 @@ describe('AircraftInteractionManager', () => { expect(sendCommand).toHaveBeenCalledWith('POS KL123'); vi.useRealTimers(); }); + + it('empty-map click unselects, stops following and toggles the route off', () => { + vi.useFakeTimers(); + stateManager.updateAircraftData(aircraft(['KL123'])); + stateManager.setSelectedAircraft('KL123'); + // Panel double-click starts follow mode once the animations finish + panelEvent('aircraft-double-click', 'KL123'); + vi.advanceTimersByTime(200); + expect(manager.getFollowingAircraft()).toBe('KL123'); + sendCommand.mockClear(); + + map.queryRenderedFeatures.mockReturnValue([]); + mapHandler(map, 'click')({ point: { x: 10, y: 10 } }); + + expect(stateManager.getState().selectedAircraft).toBeNull(); + expect(manager.getFollowingAircraft()).toBeNull(); + expect(sendCommand).toHaveBeenCalledWith('POS KL123'); + vi.useRealTimers(); + }); + + it('does not unselect when the click hit an aircraft, even if it later moves off the point', () => { + vi.useFakeTimers(); + stateManager.updateAircraftData(aircraft(['KL123'])); + stateManager.setSelectedAircraft('KL123'); + + // At click time the point is on the aircraft; the select-triggered + // flyTo (or a data update) then moves it away. The old deferred + // re-query saw the empty point and spuriously unselected. + map.queryRenderedFeatures.mockReturnValueOnce([{ properties: { entity_id: 'KL123' } }]); + mapHandler(map, 'click')({ point: { x: 10, y: 10 } }); + vi.advanceTimersByTime(1000); + + expect(stateManager.getState().selectedAircraft).toBe('KL123'); + vi.useRealTimers(); + }); + + it("toggles the old aircraft's route broadcast off when selection switches between aircraft", () => { + stateManager.updateAircraftData(aircraft(['KL123', 'KL456'])); + stateManager.setSelectedAircraft('KL123'); + sendCommand.mockClear(); + + stateManager.setSelectedAircraft('KL456'); + expect(sendCommand).toHaveBeenCalledWith('POS KL123'); + + // Plain unselect paths send their own toggle; the subscription + // must not add a second one (which would toggle the route back on). + sendCommand.mockClear(); + stateManager.setSelectedAircraft(null); + expect(sendCommand).not.toHaveBeenCalled(); + }); + + it('ignores empty-map clicks while a drawing tool is active', () => { + stateManager.updateAircraftData(aircraft(['KL123'])); + stateManager.setSelectedAircraft('KL123'); + manager.setDrawingToolActiveCheck(() => true); + + map.queryRenderedFeatures.mockReturnValue([]); + mapHandler(map, 'click')({ point: { x: 10, y: 10 } }); + + expect(stateManager.getState().selectedAircraft).toBe('KL123'); + }); }); diff --git a/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts b/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts index 81de6af..d5e8fcb 100644 --- a/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts +++ b/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts @@ -47,7 +47,7 @@ export class AircraftInteractionManager { // Document-level listeners and state subscriptions, released in destroy() // (map listeners die with the map, but these would outlive it). private documentListeners = new ListenerRegistry(); - private unsubscribeAircraftData: (() => void) | null = null; + private stateUnsubscribers: (() => void)[] = []; constructor( mapDisplay: MapDisplay, @@ -103,33 +103,29 @@ export class AircraftInteractionManager { } }); - // Click on empty map - unselect aircraft + // Click on empty map - unselect aircraft. The aircraft layer handlers + // fire in this same synchronous dispatch, so the hit test must be + // synchronous too: deferring it (the old 50ms setTimeout) let the + // select-triggered flyTo or a data update move the aircraft off the + // clicked point, misreading an aircraft click as an empty-map click + // and instantly unselecting the aircraft that was just clicked. this.map.on('click', (e: MapMouseEvent) => { - // Small delay to let aircraft-specific click handlers run first - setTimeout(() => { - if (!this.map) return; + // With a drawing tool active, an empty-map click is a point + // placement, not a request to unselect the aircraft. + if (this.isDrawingToolActive && this.isDrawingToolActive()) { + return; + } - // If a drawing tool is active, an empty-map click is a point - // placement, not a request to unselect the aircraft. - if (this.isDrawingToolActive && this.isDrawingToolActive()) { - return; - } + if (this.queryAircraftAt(e.point).length > 0) return; - const features = this.queryAircraftAt(e.point); - const clickedOnAircraft = features.length > 0; - - // If click was not on an aircraft, unselect and stop following - if (!clickedOnAircraft) { - logger.debug('AircraftInteractionManager', 'Empty map click - unselecting aircraft'); - const currentSelection = this.stateManager.getState().selectedAircraft; - if (currentSelection) { - // Send POS command to toggle route visibility off - this.requestRouteData(currentSelection); - } - this.stopFollowing(); - this.stateManager.setSelectedAircraft(null); - } - }, 50); + logger.debug('AircraftInteractionManager', 'Empty map click - unselecting aircraft'); + const currentSelection = this.stateManager.getState().selectedAircraft; + if (currentSelection) { + // Send POS command to toggle route visibility off + this.requestRouteData(currentSelection); + } + this.stopFollowing(); + this.stateManager.setSelectedAircraft(null); }); // Stop following on user drag @@ -160,27 +156,16 @@ export class AircraftInteractionManager { private setup2DLayerHandlers(): void { if (!this.map) return; - // Single click on aircraft - select and zoom - this.map.on('click', 'aircraft-points', (e) => { - if (!e.features || e.features.length === 0) return; - - const aircraftId = e.features[0].properties?.entity_id || e.features[0].properties?.callsign; - if (aircraftId) { - logger.debug('AircraftInteractionManager', 'MAP 2D SINGLE CLICK:', aircraftId); - this.handleMapAircraftClick(aircraftId, false); - } - }); - - // Double click on aircraft - select, zoom, and follow - this.map.on('dblclick', 'aircraft-points', (e) => { - if (!e.features || e.features.length === 0) return; - - const aircraftId = e.features[0].properties?.entity_id || e.features[0].properties?.callsign; - if (aircraftId) { - logger.debug('AircraftInteractionManager', 'MAP 2D DOUBLE CLICK:', aircraftId); - this.handleMapAircraftClick(aircraftId, true); - } - }); + for (const [event, isDoubleClick] of [['click', false], ['dblclick', true]] as const) { + this.map.on(event, 'aircraft-points', (e) => { + const properties = e.features?.[0]?.properties; + const aircraftId = properties?.entity_id || properties?.callsign; + if (aircraftId) { + logger.debug('AircraftInteractionManager', `MAP 2D ${isDoubleClick ? 'DOUBLE' : 'SINGLE'} CLICK:`, aircraftId); + this.handleMapAircraftClick(aircraftId, isDoubleClick); + } + }); + } logger.debug('AircraftInteractionManager', '2D layer click handlers set up'); } @@ -226,11 +211,22 @@ export class AircraftInteractionManager { */ private setupStateSubscriptions(): void { // Follow mode tracks each aircraft data update - this.unsubscribeAircraftData = this.stateManager.subscribe('aircraftData', (newData) => { + this.stateUnsubscribers.push(this.stateManager.subscribe('aircraftData', (newData) => { if (newData) { this.updateFollowing(newData); } - }); + })); + + // When selection switches directly from one aircraft to another + // (map or panel click - no unselect step in between), toggle the + // old aircraft's route broadcast off. Otherwise it keeps streaming + // ROUTEDATA, which wastes bandwidth and used to get re-interpreted + // as an "implicit selection" that stole the new selection back. + this.stateUnsubscribers.push(this.stateManager.subscribe('selectedAircraft', (newAircraft, oldAircraft) => { + if (oldAircraft && newAircraft && oldAircraft !== newAircraft) { + this.requestRouteData(oldAircraft); + } + })); logger.debug('AircraftInteractionManager', 'State subscriptions set up'); } @@ -468,16 +464,6 @@ export class AircraftInteractionManager { (Date.now() - this.lastExplicitPosTime) < 1000; } - /** - * Public wrapper around requestRouteData() used by other subsystems - * (e.g. RouteDrawingManager) that need BlueSky to re-broadcast ROUTEDATA - * for an aircraft. Marks the POS as explicit so the resulting ROUTEDATA - * is processed on the normal (non-unsolicited) path in App.onRouteData. - */ - public sendExplicitPos(aircraftId: string): void { - this.requestRouteData(aircraftId); - } - /** * Register a predicate that reports whether any interactive drawing tool * (route, shape, aircraft placement) is in progress. When true, empty-map @@ -509,8 +495,8 @@ export class AircraftInteractionManager { public destroy(): void { this.stopFollowing(); this.documentListeners.removeAll(); - this.unsubscribeAircraftData?.(); - this.unsubscribeAircraftData = null; + this.stateUnsubscribers.forEach(unsub => unsub()); + this.stateUnsubscribers = []; logger.debug('AircraftInteractionManager', 'AircraftInteractionManager destroyed'); } } From 4b824f72d64dccc758c570bfbd28e54fbd0aa65c Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:08:47 +0200 Subject: [PATCH 4/6] Stop dropping speed-only route constraints in ADDWPT commands BlueSky resolves an empty positional arg to "no constraint", so a speed-only waypoint constraint now emits an empty altitude slot (,,,) instead of discarding the speed with a warning. Command building is collapsed into one arg-list path, and the private unit-label helpers move to DataProcessor.altitudeUnitLabel alongside the existing speedUnitLabel. Co-Authored-By: Claude Fable 5 --- frontend/src/data/DataProcessor.test.ts | 7 + frontend/src/data/DataProcessor.ts | 17 +++ .../map/routes/RouteConstraintsModal.test.ts | 38 +++++ .../ui/map/routes/RouteConstraintsModal.ts | 136 +++++------------- 4 files changed, 99 insertions(+), 99 deletions(-) diff --git a/frontend/src/data/DataProcessor.test.ts b/frontend/src/data/DataProcessor.test.ts index da53f5d..47fccbd 100644 --- a/frontend/src/data/DataProcessor.test.ts +++ b/frontend/src/data/DataProcessor.test.ts @@ -85,6 +85,13 @@ describe('DataProcessor compact map labels', () => { expect(DataProcessor.formatAltitudeLabel(304.8, 'ft')).toBe('1000ft'); expect(DataProcessor.formatAltitudeLabel(3000.4, 'm')).toBe('3000m'); }); + + it('altitudeUnitLabel returns the display suffix for each unit', () => { + expect(DataProcessor.altitudeUnitLabel('ft')).toBe('ft'); + expect(DataProcessor.altitudeUnitLabel('m')).toBe('m'); + expect(DataProcessor.altitudeUnitLabel('km')).toBe('km'); + expect(DataProcessor.altitudeUnitLabel('fl')).toBe('FL'); + }); }); describe('DataProcessor.convertVerticalSpeed', () => { diff --git a/frontend/src/data/DataProcessor.ts b/frontend/src/data/DataProcessor.ts index ccae190..c523a9b 100644 --- a/frontend/src/data/DataProcessor.ts +++ b/frontend/src/data/DataProcessor.ts @@ -121,6 +121,23 @@ export class DataProcessor { } } + /** + * Altitude unit suffix for labels (e.g. "ft", "FL"). + */ + static altitudeUnitLabel(unit: AltitudeUnit): string { + switch (unit) { + case 'm': + return 'm'; + case 'km': + return 'km'; + case 'fl': + return 'FL'; + case 'ft': + default: + return 'ft'; + } + } + /** * Unit suffix used in compact map labels (e.g. "250kt"). */ diff --git a/frontend/src/ui/map/routes/RouteConstraintsModal.test.ts b/frontend/src/ui/map/routes/RouteConstraintsModal.test.ts index 79aacf4..54be384 100644 --- a/frontend/src/ui/map/routes/RouteConstraintsModal.test.ts +++ b/frontend/src/ui/map/routes/RouteConstraintsModal.test.ts @@ -143,4 +143,42 @@ describe('RouteConstraintsModal', () => { expect(sendCommand).toHaveBeenCalledWith('ADDWPT KL204 53.000000,5.000000,20000,300'); expect(onCancel).not.toHaveBeenCalled(); }); + + it('a speed-only constraint keeps an empty altitude slot instead of dropping the speed', async () => { + const modal = makeModal(); + modal.show('KL204', [{ lat: 52, lng: 4 }], 'ft', 'knots'); + + (document.getElementById('route-constraints-bulk-spd') as HTMLInputElement).value = '300'; + + (document.getElementById('submit-route-constraints-btn') as HTMLButtonElement).click(); + await vi.waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1)); + + expect(sendCommand).toHaveBeenCalledWith('ADDWPT KL204 52.000000,4.000000,,300'); + }); + + it('an altitude-only constraint omits the speed argument', async () => { + const modal = makeModal(); + modal.show('KL204', [{ lat: 52, lng: 4 }], 'ft', 'knots'); + + (document.getElementById('route-constraints-bulk-alt') as HTMLInputElement).value = '20000'; + + (document.getElementById('submit-route-constraints-btn') as HTMLButtonElement).click(); + await vi.waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1)); + + expect(sendCommand).toHaveBeenCalledWith('ADDWPT KL204 52.000000,4.000000,20000'); + }); + + it('converts constraints from the captured units to feet and knots', async () => { + const modal = makeModal(); + modal.show('KL204', [{ lat: 52, lng: 4 }], 'fl', 'km/h'); + + (document.getElementById('route-constraints-bulk-alt') as HTMLInputElement).value = '100'; + (document.getElementById('route-constraints-bulk-spd') as HTMLInputElement).value = '555.6'; + + (document.getElementById('submit-route-constraints-btn') as HTMLButtonElement).click(); + await vi.waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1)); + + // FL100 -> 10000 ft; 555.6 km/h -> 300 kt + expect(sendCommand).toHaveBeenCalledWith('ADDWPT KL204 52.000000,4.000000,10000,300'); + }); }); diff --git a/frontend/src/ui/map/routes/RouteConstraintsModal.ts b/frontend/src/ui/map/routes/RouteConstraintsModal.ts index 385c440..55a0b3f 100644 --- a/frontend/src/ui/map/routes/RouteConstraintsModal.ts +++ b/frontend/src/ui/map/routes/RouteConstraintsModal.ts @@ -19,13 +19,9 @@ interface ActiveRoute { } /** - * RouteConstraintsModal - Owns the per-waypoint constraint modal and the - * ADDWPT command build/send pipeline. - * - * The manager hands us a snapshot {acid, points, altUnit, spdUnit} when it - * finishes drawing; we collect constraints from the user and send one - * ADDWPT command per waypoint. We notify the manager via onComplete/onCancel - * so it can drop its drawing state. + * RouteConstraintsModal - Per-waypoint constraint modal plus the ADDWPT + * command build/send pipeline. The drawing manager hands over a finished + * route snapshot and is notified back via onComplete/onCancel. */ export class RouteConstraintsModal { private app: App; @@ -34,9 +30,8 @@ export class RouteConstraintsModal { private active: ActiveRoute | null = null; private constraintRows: WaypointConstraint[] = []; - // True while submit() is sending its command sequence. Blocks re-entrant - // submits (a double-click would send every ADDWPT twice) and stops a - // mid-send modal close from reporting the route as cancelled. + // Blocks re-entrant submits (double-click) and stops a mid-send modal + // close from reporting the route as cancelled. private sending = false; constructor(app: App, onComplete: () => void, onCancel: () => void) { @@ -57,11 +52,10 @@ export class RouteConstraintsModal { cancelBtn.addEventListener('click', () => modalManager.close(MODAL_ID)); } - // ModalManager owns the other close paths (X button, backdrop click, - // Escape). Treat any close while a route is still pending as a - // cancel; submit() clears `active` before closing so a successful - // submission doesn't double-report, and a close while the commands - // are already being sent is not a cancel either. + // ModalManager owns the other close paths (X, backdrop, Escape). + // Any close while a route is still pending counts as a cancel; + // submit() clears `active` before closing so success doesn't + // double-report as a cancel. modalManager.on(MODAL_ID, (event) => { if (event === 'close' && this.active && !this.sending) { this.active = null; @@ -89,14 +83,13 @@ export class RouteConstraintsModal { const table = document.getElementById('route-constraints-table') as HTMLTableElement | null; if (!table) { logger.error('RouteConstraintsModal', 'route-constraints-table not found in DOM'); - // If the table is missing we still fire-and-forget the submit so - // the user's clicks aren't silently dropped. - this.submit(); + // Submit unconstrained so the drawn route isn't silently dropped. + void this.submit(); return; } - const altUnitLabel = this.altUnitLabel(altUnit); - const spdUnitLabel = this.speedUnitLabel(spdUnit); + const altUnitLabel = DataProcessor.altitudeUnitLabel(altUnit); + const spdUnitLabel = DataProcessor.speedUnitLabel(spdUnit); const altHeader = document.getElementById('route-constraints-alt-header'); if (altHeader) altHeader.textContent = `Altitude (${altUnitLabel})`; @@ -208,10 +201,9 @@ export class RouteConstraintsModal { } /** - * Build and send the ADDWPT commands (one per waypoint) sequentially, - * with a small delay between them: sendCommand() resolves right after - * socket.emit(), and back-to-back sends race on the proxy's shared ZMQ - * socket, crashing BlueSky with msgpack "ExtraData" errors. + * Send the ADDWPT commands sequentially with a small delay between them: + * back-to-back sends race on the proxy's shared ZMQ socket and crash + * BlueSky with msgpack "ExtraData" errors. */ private async submit(): Promise { if (this.sending) return; @@ -226,9 +218,8 @@ export class RouteConstraintsModal { const consoleInstance = this.app.getConsole(); const acid = this.active.acid; - // Delay in ms between successive stack commands. 50 ms per waypoint - // stays snappy for small routes (10 wpts ~= 0.5 s) while still giving - // the proxy/ZMQ pipeline room to serialize sends cleanly. + // 50 ms stays snappy for small routes while giving the proxy/ZMQ + // pipeline room to serialize sends cleanly. const COMMAND_INTERVAL_MS = 50; logger.info( @@ -257,9 +248,8 @@ export class RouteConstraintsModal { } } - // No POS refresh needed afterwards: BlueSky auto-broadcasts - // ROUTEDATA after every ADDWPT, and an extra POS races with that - // broadcast and shows stale data. + // No POS refresh afterwards: BlueSky auto-broadcasts ROUTEDATA + // after every ADDWPT, and an extra POS races with it. } catch (err) { logger.error('RouteConstraintsModal', 'Error sending route commands:', err); alert('Error sending route commands: ' + (err as Error).message); @@ -268,8 +258,7 @@ export class RouteConstraintsModal { if (submitBtn) submitBtn.disabled = false; } - // Clear `active` before closing so the close event isn't treated as - // a cancel of a still-pending route. + // Clear `active` before closing so the close event isn't a cancel. this.active = null; modalManager.close(MODAL_ID); this.onComplete(); @@ -280,81 +269,30 @@ export class RouteConstraintsModal { } /** - * Build one ADDWPT command per waypoint. BlueSky's ADDWPT takes optional - * alt/spd arguments, so omitting them cleanly represents "no constraint". - * - * Format: - * ADDWPT , (no constraints) - * ADDWPT ,, (alt only) - * ADDWPT ,,, (alt + spd) - * - * If a speed is specified without an altitude, we still need an altitude - * placeholder in the positional args; we leave that combination out and - * warn so the user adds an altitude too. + * Build one ADDWPT command per waypoint: + * ADDWPT ,[,][,] + * BlueSky resolves an empty positional arg to "no constraint", so a + * speed-only constraint keeps an empty altitude slot: ,,,. */ private generateCommands(): string[] { if (!this.active) return []; const { acid, points, altUnit, spdUnit } = this.active; return points.map((pt, i) => { - const row = this.constraintRows[i] || { alt: null, spd: null }; - - const hasAlt = row.alt !== null && !isNaN(row.alt); - const hasSpd = row.spd !== null && !isNaN(row.spd); - - const latlon = `${pt.lat.toFixed(6)},${pt.lng.toFixed(6)}`; - - if (!hasAlt && !hasSpd) { - return `ADDWPT ${acid} ${latlon}`; - } - - const altFt = hasAlt - ? String( - Math.round( - DataProcessor.altitudeToFeet(row.alt as number, altUnit) - ) - ) - : ''; - - if (!hasSpd) { - return `ADDWPT ${acid} ${latlon},${altFt}`; + const { alt, spd } = this.constraintRows[i] ?? { alt: null, spd: null }; + const hasAlt = alt !== null && !isNaN(alt); + const hasSpd = spd !== null && !isNaN(spd); + + const args = [`${pt.lat.toFixed(6)},${pt.lng.toFixed(6)}`]; + if (hasAlt || hasSpd) { + args.push(hasAlt + ? String(Math.round(DataProcessor.altitudeToFeet(alt, altUnit))) + : ''); } - - const spdKts = String( - Math.round(DataProcessor.speedToKnots(row.spd as number, spdUnit)) - ); - - if (!hasAlt) { - // BlueSky ADDWPT positional args require alt before spd; warn - // once and emit just the position (spd will be ignored). - logger.warn( - 'RouteConstraintsModal', - `WP${i + 1}: speed provided without altitude; speed will be ignored` - ); - return `ADDWPT ${acid} ${latlon}`; + if (hasSpd) { + args.push(String(Math.round(DataProcessor.speedToKnots(spd, spdUnit)))); } - - return `ADDWPT ${acid} ${latlon},${altFt},${spdKts}`; + return `ADDWPT ${acid} ${args.join(',')}`; }); } - - private altUnitLabel(u: AltitudeUnit): string { - switch (u) { - case 'm': return 'm'; - case 'km': return 'km'; - case 'fl': return 'FL'; - case 'ft': - default: return 'ft'; - } - } - - private speedUnitLabel(u: SpeedUnit): string { - switch (u) { - case 'm/s': return 'm/s'; - case 'km/h': return 'km/h'; - case 'mph': return 'mph'; - case 'knots': - default: return 'kt'; - } - } } From 237f7ce85e5336671bdf0f6d20813a8b412a190b Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:08:59 +0200 Subject: [PATCH 5/6] Recover from missing 3D aircraft models instead of rendering nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A saved model no longer in the catalog is now reset to Auto in storage and state — not just displayed as Auto — so the renderer stops forcing the missing file (re-picking Auto in an already-Auto select fires no change event to recover). usableModelPath gains a default-model fallback tier for when a globally forced model fails to load: the configured fallback path IS the forced path in that case and couldn't serve as the fallback. Extracts isKnownModelSelection for the shared catalog check. Co-Authored-By: Claude Fable 5 --- frontend/src/data/aircraftModels.test.ts | 17 +++++ frontend/src/data/aircraftModels.ts | 16 ++++- .../aircraft/Aircraft3DCustomLayer.test.ts | 68 +++++++++++++++++-- .../ui/map/aircraft/Aircraft3DCustomLayer.ts | 15 ++-- .../src/ui/panels/left/DisplayOptionsPanel.ts | 16 ++++- 5 files changed, 117 insertions(+), 15 deletions(-) diff --git a/frontend/src/data/aircraftModels.test.ts b/frontend/src/data/aircraftModels.test.ts index a03c3c9..d86195b 100644 --- a/frontend/src/data/aircraftModels.test.ts +++ b/frontend/src/data/aircraftModels.test.ts @@ -5,6 +5,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { fetchAircraftModels, + isKnownModelSelection, populateModelSelect, resetAircraftModelsCache, } from './aircraftModels'; @@ -92,3 +93,19 @@ describe('populateModelSelect', () => { expect(select.value).toBe(AUTO_MODEL_SENTINEL); }); }); + +describe('isKnownModelSelection', () => { + it('accepts the Auto sentinel regardless of catalog', () => { + expect(isKnownModelSelection([], AUTO_MODEL_SENTINEL)).toBe(true); + expect(isKnownModelSelection(MODELS, AUTO_MODEL_SENTINEL)).toBe(true); + }); + + it('accepts a model present in the catalog', () => { + expect(isKnownModelSelection(MODELS, 'B747.glb')).toBe(true); + }); + + it('rejects a model missing from the catalog', () => { + expect(isKnownModelSelection(MODELS, 'GONE.glb')).toBe(false); + expect(isKnownModelSelection([], 'A320.glb')).toBe(false); + }); +}); diff --git a/frontend/src/data/aircraftModels.ts b/frontend/src/data/aircraftModels.ts index 75b0d58..e98d642 100644 --- a/frontend/src/data/aircraftModels.ts +++ b/frontend/src/data/aircraftModels.ts @@ -64,6 +64,18 @@ export function resetAircraftModelsCache(): void { catalogPromise = null; } +/** + * Whether `selected` is a valid model choice against the given catalog: + * either the Auto sentinel or a model file present in the catalog. + */ +export function isKnownModelSelection( + models: AircraftModelOption[], + selected: string +): boolean { + return selected === AUTO_MODEL_SENTINEL + || models.some(m => m.filename === selected); +} + /** * Rebuild a model