diff --git a/WebATM/bluesky_client.py b/WebATM/bluesky_client.py index 9855141..a71c0c5 100644 --- a/WebATM/bluesky_client.py +++ b/WebATM/bluesky_client.py @@ -17,6 +17,7 @@ """ import threading +import traceback from collections import defaultdict, deque from collections.abc import Callable @@ -147,10 +148,10 @@ def seqidx2id(seqidx): def safe_decode(data): """Decode bytes to a readable string without raising. - Attempts UTF-8 decoding first and returns the result only if it consists - entirely of printable ASCII characters; otherwise falls back to ASCII - decoding, and finally to an uppercase hexadecimal representation. Non-bytes - input is converted with ``str()``. + Attempts UTF-8 decoding and returns the result only if it consists + entirely of printable ASCII characters; otherwise falls back to an + uppercase hexadecimal representation. Non-bytes input is converted with + ``str()``. Args: data (bytes | object): Value to decode or stringify. @@ -160,22 +161,12 @@ def safe_decode(data): """ if isinstance(data, bytes): try: - # First try utf-8 decoding decoded = data.decode("utf-8") - # Check if the decoded string contains only printable ASCII characters - if all(32 <= ord(c) <= 126 for c in decoded): - return decoded - else: - # Contains non-printable characters, use hex representation - return data.hex().upper() except UnicodeDecodeError: - try: - # Try ASCII decoding - decoded = data.decode("ascii") - return decoded - except UnicodeDecodeError: - # Unable to decode as text, use hex representation - return data.hex().upper() + return data.hex().upper() + if all(32 <= ord(c) <= 126 for c in decoded): + return decoded + return data.hex().upper() return str(data) @@ -232,16 +223,11 @@ def emit(self, *args, **kwargs): *args (Any): Positional arguments forwarded to each callback. **kwargs (Any): Keyword arguments forwarded to each callback. """ - callbacks_snapshot = self.callbacks[ - : - ] # Make a copy to avoid concurrency issues - for callback in callbacks_snapshot: + for callback in self.callbacks[:]: try: callback(*args, **kwargs) except Exception as e: logger.warning(f"Signal {self.name}: Error in callback {callback}: {e}") - import traceback - traceback.print_exc() @@ -289,11 +275,7 @@ def emit(self, topic: str, *args, **kwargs): callback(*args, **kwargs) except Exception as e: logger.warning(f"Subscriber {topic}: Error in callback {callback}: {e}") - logger.debug(f"Subscriber {topic}: Error type: {type(e).__name__}") - logger.debug(f"Subscriber {topic}: Args: {args}") - logger.debug(f"Subscriber {topic}: Kwargs: {kwargs}") - import traceback - + logger.debug(f"Subscriber {topic}: Args: {args} Kwargs: {kwargs}") traceback.print_exc() @@ -711,31 +693,21 @@ def _process_data_message(self, msg): else: self.subscriber.emit(topic, data) # Pass as single argument elif topic == "ECHO": - # ECHO expects: text, flags, sender_id (can be called with varying args) - # Always include sender_id from message header to identify which node sent the echo - if isinstance(data, (list, tuple)): - # Ensure we always pass sender_id from message header - if len(data) >= 3: - # Data already contains [text, flags, sender_id] - self.subscriber.emit(topic, *data) - elif len(data) == 2: - # Data is [text, flags] - add sender_id from header - self.subscriber.emit(topic, data[0], data[1], sender_id) - elif len(data) == 1: - # Data is [text] - add default flags and sender_id from header - self.subscriber.emit(topic, data[0], 0, sender_id) - else: - # Empty list - send empty text with sender_id from header - self.subscriber.emit(topic, "", 0, sender_id) - elif isinstance(data, dict): + # ECHO handlers expect (text, flags, sender_id). Normalize + # the payload — [text], [text, flags], [text, flags, + # sender_id], a dict, or a bare string — filling missing + # flags with 0 and the sender from the message header. + if isinstance(data, dict): text = data.get("text", "") flags = data.get("flags", 0) - # Use sender_id from data if available, otherwise from message header - data_sender_id = data.get("sender_id", sender_id) - self.subscriber.emit(topic, text, flags, data_sender_id) + echo_sender = data.get("sender_id", sender_id) else: - # Simple string or other data - add defaults and sender_id from header - self.subscriber.emit(topic, str(data), 0, sender_id) + if not isinstance(data, (list, tuple)): + data = [str(data)] + text = data[0] if len(data) > 0 else "" + flags = data[1] if len(data) > 1 else 0 + echo_sender = data[2] if len(data) > 2 else sender_id + self.subscriber.emit(topic, text, flags, echo_sender) elif topic == "STATECHANGE": # STATECHANGE follows BlueSky's shared-state format: # [action_type, {"simstate": , ...}] @@ -973,20 +945,10 @@ def delnode(self, node_id): return self.send("DELNODE", node_id, target_server) def on_node_added_request_data(self, node_id): - """When a new node is announced, request the initial/current state of all - subscribed shared states.""" - logger.info("A new node has been added! request topics") - - # TODO: fix request - # Request all BlueSky topics we want to receive add #STACK - # topics = ['RESET', 'REQUEST', 'PLOT', 'SHOWDIALOG', 'SIMINFO', - # 'SIMSETTINGS', 'TRAILS', 'ROUTEDATA', 'ACDATA', 'DEFWPT', - # 'POLY', 'STACKCMDS'] - + """When a new node is announced, request the current state of the + subscribed shared states (shapes and the command dictionary).""" topics = ["POLY", "STACKCMDS"] - - logger.debug( + logger.info( f"Requesting topics {topics} from all nodes (triggered by new node {safe_decode(node_id)})" ) self.send("REQUEST", topics) - self.send("REQUEST", topics) diff --git a/WebATM/proxy/core.py b/WebATM/proxy/core.py index 27e9b44..c42f0b6 100644 --- a/WebATM/proxy/core.py +++ b/WebATM/proxy/core.py @@ -196,10 +196,6 @@ def _on_actnode_changed(self, node_id): """Callback when active node changes.""" return self.node_mgr._on_actnode_changed(node_id) - def _emit_active_node_poly_data(self): - """Emit POLY and POLYLINE data for the currently active node.""" - return self.node_mgr._emit_active_node_poly_data() - def _on_node_added(self, node_id): """Callback when a new node is discovered.""" return self.node_mgr._on_node_added(node_id) diff --git a/WebATM/proxy/handlers/shapes.py b/WebATM/proxy/handlers/shapes.py index 4924937..0aa4e61 100644 --- a/WebATM/proxy/handlers/shapes.py +++ b/WebATM/proxy/handlers/shapes.py @@ -335,7 +335,9 @@ def on_poly_received(data, *args, **kwargs): # sets (not just this message's shapes). active_node_id = proxy._get_safe_active_node() if sender_id and active_node_id and sender_id == active_node_id: - if proxy.socketio: + # Same guard as every other emit site; a client connecting later + # gets the stored sets from its initial_data/connect envelopes. + if proxy.socketio and proxy.connected_clients > 0: proxy.socketio.emit( "poly", proxy.poly_data_by_node.get(sender_id, {"polys": {}}) ) diff --git a/WebATM/server/socket_handlers.py b/WebATM/server/socket_handlers.py index 824b39e..bc82197 100644 --- a/WebATM/server/socket_handlers.py +++ b/WebATM/server/socket_handlers.py @@ -15,6 +15,27 @@ logger = get_logger() +def _tracked_binary_node_id(proxy, node_id): + """Map a frontend hex node ID to the tracked node's binary ID. + + Args: + proxy (BlueSkyProxy): The current BlueSky proxy. + node_id (str): Hex-string node ID as sent by the frontend. + + Returns: + bytes | None: The original binary node ID, or None when the node is + not tracked. + """ + node_data = proxy.tracked_nodes.get(node_id) + if node_data is None: + logger.debug( + f"Could not find node ID for: {node_id} " + f"(available: {list(proxy.tracked_nodes.keys())})" + ) + return None + return node_data.get("node_id") + + def register_socket_handlers(socketio, session_manager): """Register all Socket.IO event handlers. @@ -29,8 +50,8 @@ def on_connect(auth): """Handle a new web client connection (``connect`` event). Creates and tracks a session, increments the connected-client - counter, and sends the ``initial_data`` snapshot and the active - node's shapes. + counter, and sends this client the ``initial_data`` snapshot and the + active node's shape envelopes. Args: auth: Socket.IO auth payload (unused). @@ -52,11 +73,16 @@ def on_connect(auth): ) try: - emit("initial_data", current_app.bluesky_proxy.get_current_data()) - # Shapes created before this client connected. node_info is NOT - # sent here: it would show "Connected (No Data)" before the user + snapshot = current_app.bluesky_proxy.get_current_data() + emit("initial_data", snapshot) + # Complete shape envelopes for this client only. A reconnecting + # browser needs them to prune shapes deleted while it was away + # (initial_data only ever adds shapes); other clients are already + # in sync, so this must not broadcast. node_info is NOT sent + # here: it would show "Connected (No Data)" before the user # connects; it flows naturally once data arrives. - current_app.bluesky_proxy._emit_active_node_poly_data() + emit("poly", snapshot["poly_data"] or {"polys": {}}) + emit("polyline", snapshot["polyline_data"] or {"polys": {}}) except Exception as e: logger.info(f"Error sending initial data to {session_id}: {e}") @@ -64,11 +90,9 @@ def on_connect(auth): def on_disconnect(reason): """Handle a web client disconnect (``disconnect`` event). - Removes the session from the session manager and decrements the - connected-client counter. The counter is only decremented for - connections whose session was actually tracked, keeping it - symmetric with ``on_connect`` (a connection rejected there never - incremented it). + Removes the session and decrements the connected-client counter — + but only for connections whose session was actually tracked, keeping + the counter symmetric with ``on_connect``. Args: reason: Disconnect reason supplied by Flask-SocketIO. @@ -107,10 +131,6 @@ def on_command(data): def on_set_active_node(data): """Switch the active simulation node (``set_active_node`` event). - The frontend sends hex-string node IDs; the handler looks up the - original binary ID in the proxy's tracked nodes before delegating to - ``actnode``. - Args: data (dict): Payload with the hex-string ``node_id``. """ @@ -118,15 +138,10 @@ def on_set_active_node(data): if not node_id: return - node_data = current_app.bluesky_proxy.tracked_nodes.get(node_id) - if node_data is None: - logger.debug( - f"Could not find node ID for: {node_id} " - f"(available: {list(current_app.bluesky_proxy.tracked_nodes.keys())})" - ) + binary_node_id = _tracked_binary_node_id(current_app.bluesky_proxy, node_id) + if binary_node_id is None: return - binary_node_id = node_data.get("node_id") logger.info(f"Setting active node to: {node_id} (binary: {binary_node_id})") try: current_app.bluesky_proxy.actnode(binary_node_id) @@ -158,7 +173,7 @@ def on_add_nodes(data): if server_id and isinstance(server_id, str): server_id = server_id.encode() current_app.bluesky_proxy.addnodes(count, server_id=server_id) - logger.info(f"Added {count} nodes to server {server_id}") + logger.info(f"Requested {count} new node(s) on server {server_id}") except Exception as e: logger.info(f"Error adding nodes: {e}") @@ -166,11 +181,9 @@ def on_add_nodes(data): def on_del_node(data): """Terminate a single simulation node (``del_node`` event). - The frontend sends hex-string node IDs; the handler looks up the - original binary ID in the proxy's tracked nodes before delegating to - ``delnode``, which sends a DELNODE message to the owning server. The - node's removal flows back through the normal node-removed pipeline - (tracked-nodes cleanup, active-node failover, ``node_info`` emission). + Sends a DELNODE message to the owning server; the node's removal + flows back through the normal node-removed pipeline (tracked-nodes + cleanup, active-node failover, ``node_info`` emission). Args: data (dict): Payload with the hex-string ``node_id``. @@ -179,15 +192,10 @@ def on_del_node(data): if not node_id: return - node_data = current_app.bluesky_proxy.tracked_nodes.get(node_id) - if node_data is None: - logger.debug( - f"Could not find node ID for: {node_id} " - f"(available: {list(current_app.bluesky_proxy.tracked_nodes.keys())})" - ) + binary_node_id = _tracked_binary_node_id(current_app.bluesky_proxy, node_id) + if binary_node_id is None: return - binary_node_id = node_data.get("node_id") logger.info( f"Requesting node termination: {node_id} (binary: {binary_node_id})" ) diff --git a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts new file mode 100644 index 0000000..0dbd834 --- /dev/null +++ b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts @@ -0,0 +1,142 @@ +/** + * Tests for Aircraft3DCustomLayer.updateAircraft removal handling. + * + * Focus: an empty aircraft batch (the last aircraft was deleted) must still + * fall through to the removal loop so the deleted aircraft's 3D mesh is torn + * down. A previous early-return on `id.length === 0` left it on the map as a + * ghost until the next non-empty tick or a full reset. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { AircraftData, DisplayOptions } from '../../../data/types'; + +// Base class: only the members updateAircraft touches, plus a settable scene. +vi.mock('../rendering/CustomLayer3D', () => ({ + CustomLayer3D: class { + id: string; + scene: unknown = undefined; + camera = {}; + renderer = { capabilities: { getMaxAnisotropy: () => 16 } }; + map = null; + constructor(id: string) { + this.id = id; + } + createTransformMatrix() { + return {}; + } + isGlobeProjection() { + return false; + } + }, +})); + +// Stateful fake fleet: tracks live ids and records remove() calls. +const fleetState = new Map(); +const removeCalls: string[] = []; +vi.mock('./Aircraft3DFleet', () => ({ + Aircraft3DFleet: class { + constructor(_deps: unknown) {} + get(id: string) { + return fleetState.get(id); + } + create(id: string, _data: unknown, modelPath: string) { + fleetState.set(id, { modelPath }); + } + remove(id: string) { + removeCalls.push(id); + fleetState.delete(id); + } + update() {} + forEach(cb: (entry: unknown, id: string) => void) { + fleetState.forEach((entry, id) => cb(entry, id)); + } + refreshPending() {} + prunePending() {} + reapplyAllTransforms() {} + }, +})); + +vi.mock('./Aircraft3DModelLoader', () => ({ + Aircraft3DModelLoader: class { + constructor(_opts: unknown) {} + hasFailed() { + return false; + } + load() {} + clearCache() {} + clearAll() {} + }, +})); + +vi.mock('./Aircraft3DTransforms', () => ({ + Aircraft3DTransforms: class { + constructor(_deps: unknown) {} + updateSceneOrigin() { + return false; + } + updateMeshTransform() {} + }, +})); + +import { Aircraft3DCustomLayer } from './Aircraft3DCustomLayer'; + +function makeLayer(): Aircraft3DCustomLayer { + const layer = new Aircraft3DCustomLayer( + { selectedAircraftModel: 'auto' } as DisplayOptions, + null + ); + // Mark the scene ready so updateAircraft processes instead of queuing. + (layer as unknown as { scene: object }).scene = {}; + return layer; +} + +function batch(ids: string[]): AircraftData { + return { + id: ids, + lat: ids.map(() => 52), + lon: ids.map(() => 4), + alt: ids.map(() => 1000), + trk: ids.map(() => 90), + actype: ids.map(() => 'A320'), + inconf: ids.map(() => false), + } as AircraftData; +} + +describe('Aircraft3DCustomLayer.updateAircraft removal', () => { + beforeEach(() => { + fleetState.clear(); + removeCalls.length = 0; + }); + + it('removes a mesh when its aircraft disappears from a non-empty batch', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1', 'AC2'])); + expect(fleetState.has('AC1')).toBe(true); + + layer.updateAircraft(batch(['AC2'])); + + expect(removeCalls).toContain('AC1'); + expect(fleetState.has('AC1')).toBe(false); + expect(fleetState.has('AC2')).toBe(true); + }); + + it('clears the last aircraft when an empty batch arrives (no ghost)', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1'])); + expect(fleetState.has('AC1')).toBe(true); + + layer.updateAircraft(batch([])); + + expect(removeCalls).toContain('AC1'); + expect(fleetState.size).toBe(0); + }); + + it('ignores a batch with no id array without throwing', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1'])); + + expect(() => layer.updateAircraft({} as AircraftData)).not.toThrow(); + // The existing aircraft is left untouched (guarded before removal). + expect(fleetState.has('AC1')).toBe(true); + expect(removeCalls).not.toContain('AC1'); + }); +}); diff --git a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts index 1632b65..baf072a 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts @@ -114,7 +114,10 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { return; } - if (!aircraftData.id || aircraftData.id.length === 0) { + // An empty batch (last aircraft deleted) must still fall through to + // the removal loop below, or the deleted aircraft's mesh lingers as + // a ghost — only bail when the id array is missing entirely. + if (!aircraftData.id) { return; } @@ -157,8 +160,6 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { lon: aircraftData.lon[i], alt: aircraftData.alt[i], hdg: aircraftData.trk[i], - selected: false, - inconf: aircraftData.inconf ? aircraftData.inconf[i] : false, actype, }; diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts index bc407fa..f2c9cf4 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts @@ -10,7 +10,7 @@ import { Aircraft3DFleet, type Aircraft3DFleetDeps } from './Aircraft3DFleet'; import type { AircraftMeshData } from './Aircraft3DTransforms'; const DATA: AircraftMeshData = { - lat: 52, lon: 4, alt: 1000, hdg: 90, selected: false, inconf: false, actype: 'A320', + lat: 52, lon: 4, alt: 1000, hdg: 90, actype: 'A320', }; /** A model whose single mesh carries an array of materials. */ diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts index 5950855..cf59e00 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts @@ -15,7 +15,6 @@ export interface Aircraft3DMesh { mesh: THREE.Object3D; data: AircraftMeshData; modelPath: string; - lastUpdate: number; currentGroup: 'mercator' | 'globe'; /** Plays the GLB's baked animation clips; absent for static models. */ mixer?: THREE.AnimationMixer; @@ -124,7 +123,6 @@ export class Aircraft3DFleet { mesh, data, modelPath, - lastUpdate: Date.now(), currentGroup: isGlobe ? 'globe' : 'mercator', mixer, }); @@ -155,7 +153,6 @@ export class Aircraft3DFleet { this.applyTransform(aircraftMesh.mesh, data, aircraftMesh.currentGroup === 'globe'); aircraftMesh.data = data; - aircraftMesh.lastUpdate = Date.now(); } /** diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts index a55ed3e..fdb0f40 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts @@ -32,7 +32,7 @@ function acData(positions: Array<[number, number]>): AircraftData { } function meshData(lat: number, lon: number): AircraftMeshData { - return { lat, lon, alt: 3000, hdg: 90, selected: false, inconf: false, actype: 'A320' }; + return { lat, lon, alt: 3000, hdg: 90, actype: 'A320' }; } describe('Aircraft3DTransforms scene origin vs invalid coordinates', () => { diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts index e3c1878..a1581a9 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts @@ -21,8 +21,6 @@ export interface AircraftMeshData { lon: number; alt: number; hdg: number; - selected: boolean; - inconf: boolean; actype: string; } diff --git a/tests/test_bluesky_client.py b/tests/test_bluesky_client.py index b194a15..fe8cc6e 100644 --- a/tests/test_bluesky_client.py +++ b/tests/test_bluesky_client.py @@ -383,6 +383,61 @@ def test_poly_single_action_sets_context_and_unwraps(self): assert received == [(b"U", payload)] assert client.context.sender_id == b"NODE\x81" + def test_echo_list_payload_gets_sender_from_header(self): + # BlueSky sends ECHO as [text, flags]; the header sender must be + # appended so the handler knows which node answered. + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", ["Roger", 4])) + + assert received == [("Roger", 4, b"NODE\x81")] + + def test_echo_dict_payload_prefers_its_own_sender(self): + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message( + self._frame("ECHO", {"text": "hi", "flags": 1, "sender_id": "n2"}) + ) + + assert received == [("hi", 1, "n2")] + + def test_echo_bare_string_payload_gets_defaults(self): + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", "plain")) + + assert received == [("plain", 0, b"NODE\x81")] + + def test_echo_oversized_payload_still_delivers(self): + # A payload with more than three items used to be splatted straight + # into the handler, raising TypeError inside the subscriber and + # silently dropping the echo. Extra items are now ignored. + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", ["msg", 2, "n3", "extra"])) + + assert received == [("msg", 2, "n3")] + def test_poly_collected_multi_action_dispatches_each_pair(self): # The POLY publisher collects actions between send ticks, so one # message can carry several (action, payload) pairs — e.g. an update @@ -399,3 +454,43 @@ def test_poly_collected_multi_action_dispatches_each_pair(self): client._process_data_message(self._frame("POLY", [b"U", update, b"D", delete])) assert received == [(b"U", update), (b"D", delete)] + + +class TestNodeDiscovery: + """_process_subscription_message turns raw (un)subscribe frames into node + discovery, which triggers exactly one REQUEST for the shared states.""" + + @staticmethod + def _subscribe_frame(sender): + from WebATM.bluesky_client import MSG_SUBSCRIBE + + return [bytes([MSG_SUBSCRIBE]) + sender] + + def test_new_node_requests_shared_state_once(self, monkeypatch): + # Each REQUEST broadcast makes every node re-send its POLY shapes and + # the full STACKCMDS command dictionary, so a duplicate send doubles + # that traffic for no benefit. + client = BlueSkyClient() + sent = [] + monkeypatch.setattr( + client, + "send", + lambda topic, data="", to_group="": sent.append((topic, data, to_group)), + ) + node = b"S\x01\x02\x03\x81" + + client._process_subscription_message(self._subscribe_frame(node)) + + assert node in client.nodes + assert sent == [("REQUEST", ["POLY", "STACKCMDS"], "")] + + def test_rediscovered_node_is_not_reannounced(self, monkeypatch): + client = BlueSkyClient() + sent = [] + monkeypatch.setattr(client, "send", lambda *a, **k: sent.append(a)) + node = b"S\x01\x02\x03\x81" + + client._process_subscription_message(self._subscribe_frame(node)) + client._process_subscription_message(self._subscribe_frame(node)) + + assert len(sent) == 1 diff --git a/tests/test_socket_handlers.py b/tests/test_socket_handlers.py index 50cfabf..e647eb1 100644 --- a/tests/test_socket_handlers.py +++ b/tests/test_socket_handlers.py @@ -45,6 +45,56 @@ def test_disconnect_untracked_session_does_not_decrement(self, sio): assert app.bluesky_proxy.connected_clients == 1 +class TestConnectShapeEnvelopes: + def test_connect_sends_scoped_shape_envelopes(self, sio): + """Every new connection gets complete poly/polyline envelopes (empty + here — no active node) so a reconnecting browser can prune shapes + deleted while it was away.""" + app, socketio, client = sio + received = {pkt["name"]: pkt["args"] for pkt in client.get_received()} + assert received["poly"][0] == {"polys": {}} + assert received["polyline"][0] == {"polys": {}} + + def test_connect_sends_active_node_shapes_to_new_client(self, sio): + """The connecting client receives the active node's stored shapes.""" + app, socketio, client = sio + proxy = app.bluesky_proxy + + class FakeClient: + act_id = b"\x01\x02" + + proxy.bluesky_client = FakeClient() + proxy.running = True + proxy.was_connected = True + node_id = FakeClient.act_id.hex() + proxy.tracked_nodes[node_id] = {"node_id": FakeClient.act_id} + shapes = {"polys": {"zone1": {"lat": [52.0], "lon": [4.0]}}} + proxy.poly_data_by_node[node_id] = shapes + + second = socketio.test_client(app) + try: + received = {pkt["name"]: pkt["args"] for pkt in second.get_received()} + assert received["poly"][0] == shapes + assert received["polyline"][0] == {"polys": {}} + finally: + second.disconnect() + + def test_connect_does_not_broadcast_shapes_to_other_clients(self, sio): + """A client connecting must not re-send (or clear) shape data on + every other connected client.""" + app, socketio, client = sio + client.get_received() # drain this client's own connect envelopes + + second = socketio.test_client(app) + try: + assert second.is_connected() + events = [pkt["name"] for pkt in client.get_received()] + assert "poly" not in events + assert "polyline" not in events + finally: + second.disconnect() + + class TestCommandEvent: def test_command_returns_result(self, sio): app, socketio, client = sio