From 3e2fe721d367d34ea099f9bfab58cbbe0d304962 Mon Sep 17 00:00:00 2001 From: Jeremy Gallant <8975765+philon-@users.noreply.github.com> Date: Fri, 12 Sep 2025 17:21:13 +0200 Subject: [PATCH] Improvements to node filtering and storage (#839) * Improves node filtering and voltage display Ensures voltage values are always displayed as positive. Enhances node filtering logic to handle unknown or undefined values, preventing nodes from being unexpectedly hidden. Updates UI labels for clarity. Improves Nodes page responsiveness by debouncing node updates and optimizing selector usage, preventing unnecessary re-renders. Adds validation warnings for key conflicts between nodes. * Update packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update packages/web/src/core/stores/nodeDBStore/nodeValidation.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Revert copilot suggestion * Review changes --------- Co-authored-by: philon- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../web/public/i18n/locales/en/nodes.json | 4 - packages/web/public/i18n/locales/en/ui.json | 2 +- .../NodeDetailsDialog/NodeDetailsDialog.tsx | 5 +- packages/web/src/components/Sidebar.tsx | 5 +- packages/web/src/components/UI/Slider.tsx | 8 +- .../generic/Filter/useFilterNode.ts | 88 ++++++++--- packages/web/src/core/stores/index.ts | 14 +- .../web/src/core/stores/messageStore/index.ts | 2 +- .../web/src/core/stores/nodeDBStore/index.ts | 88 +++++++---- ...deDBStore.test.ts => nodeDBStore.test.tsx} | 137 +++++++++++++++++- .../core/stores/nodeDBStore/nodeValidation.ts | 53 ++++++- .../core/stores/utils/bindStoreToDevice.ts | 101 +++++++++++++ packages/web/src/pages/Map/index.tsx | 37 +++-- packages/web/src/pages/Nodes/index.tsx | 44 ++++-- 14 files changed, 488 insertions(+), 100 deletions(-) rename packages/web/src/core/stores/nodeDBStore/{nodeDBStore.test.ts => nodeDBStore.test.tsx} (79%) create mode 100644 packages/web/src/core/stores/utils/bindStoreToDevice.ts diff --git a/packages/web/public/i18n/locales/en/nodes.json b/packages/web/public/i18n/locales/en/nodes.json index b63e90c4..3968ffc1 100644 --- a/packages/web/public/i18n/locales/en/nodes.json +++ b/packages/web/public/i18n/locales/en/nodes.json @@ -46,11 +46,7 @@ "connectionStatus": { "direct": "Direct", "away": "away", - "unknown": "-", "viaMqtt": ", via MQTT" - }, - "lastHeardStatus": { - "never": "Never" } }, diff --git a/packages/web/public/i18n/locales/en/ui.json b/packages/web/public/i18n/locales/en/ui.json index 32957c10..205da2d1 100644 --- a/packages/web/public/i18n/locales/en/ui.json +++ b/packages/web/public/i18n/locales/en/ui.json @@ -182,7 +182,7 @@ "label": "Unknown number of hops" }, "showUnheard": { - "label": "Never heard" + "label": "Unknown last heard" }, "language": { "label": "Language", diff --git a/packages/web/src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx b/packages/web/src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx index 3489cca1..e2eaa0a7 100644 --- a/packages/web/src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx +++ b/packages/web/src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx @@ -174,7 +174,10 @@ export const NodeDetailsDialog = ({ { key: "voltage", label: t("nodeDetails.voltage"), - value: node.deviceMetrics?.voltage, + value: + typeof node.deviceMetrics?.voltage === "number" + ? Math.abs(node.deviceMetrics?.voltage) + : undefined, format: (val: number) => `${val.toFixed(2)}V`, }, ]; diff --git a/packages/web/src/components/Sidebar.tsx b/packages/web/src/components/Sidebar.tsx index d16bc09d..8cc117d8 100644 --- a/packages/web/src/components/Sidebar.tsx +++ b/packages/web/src/components/Sidebar.tsx @@ -205,7 +205,10 @@ export const Sidebar = ({ children }: SidebarProps) => { } deviceMetrics={{ batteryLevel: myNode.deviceMetrics?.batteryLevel, - voltage: myNode.deviceMetrics?.voltage, + voltage: + typeof myNode.deviceMetrics?.voltage === "number" + ? Math.abs(myNode.deviceMetrics?.voltage) + : undefined, }} /> )} diff --git a/packages/web/src/components/UI/Slider.tsx b/packages/web/src/components/UI/Slider.tsx index de370372..814196f3 100644 --- a/packages/web/src/components/UI/Slider.tsx +++ b/packages/web/src/components/UI/Slider.tsx @@ -50,6 +50,8 @@ export function Slider({ onValueCommit?.(newValue); }; + const thumbIds = currentValue.map((_, idx) => `${internalId}-thumb-${idx}`); // Unique IDs for each thumb, pregenerated to please the linter + return ( - {currentValue.map((_) => ( + {currentValue.map((_, idx) => ( ))} diff --git a/packages/web/src/components/generic/Filter/useFilterNode.ts b/packages/web/src/components/generic/Filter/useFilterNode.ts index 275c67b1..46a43a16 100644 --- a/packages/web/src/components/generic/Filter/useFilterNode.ts +++ b/packages/web/src/components/generic/Filter/useFilterNode.ts @@ -67,25 +67,33 @@ export function useFilterNode() { ...filterOverrides, }; - if (!node.user) { - return false; - } - const nodeName = filterState.nodeName.toLowerCase(); - if ( - nodeName && - !( - node.user?.shortName.toLowerCase().includes(nodeName) || - node.user?.longName.toLowerCase().includes(nodeName) || - node.num.toString().includes(nodeName) || - numberToHexUnpadded(node.num).includes(nodeName.replace(/!/g, "")) - ) - ) { - return false; + if (nodeName) { + const short = node.user?.shortName?.toLowerCase() ?? ""; + const long = node.user?.longName?.toLowerCase() ?? ""; + const numStr = node.num.toString(); + const hex = numberToHexUnpadded(node.num); + + if ( + !short.includes(nodeName) && + !long.includes(nodeName) && + !numStr.includes(nodeName) && + !hex.includes(nodeName.replace(/!/g, "")) + ) { + return false; + } } const hops = node.hopsAway ?? 7; - if (hops < filterState.hopsAway[0] || hops > filterState.hopsAway[1]) { + if ( + (node.hopsAway === undefined && + !shallowEqualArray( + filterState.hopsAway, + defaultFilterValues.hopsAway, + )) || // If hops are unknown, hide node if state is not default + hops < filterState.hopsAway[0] || + hops > filterState.hopsAway[1] + ) { return false; } @@ -96,8 +104,13 @@ export function useFilterNode() { return false; } - const secondsAgo = Date.now() / 1000 - (node.lastHeard ?? 0); + const secondsAgo = Math.max(0, Date.now() / 1000 - (node.lastHeard ?? 0)); if ( + (node.lastHeard === 0 && + !shallowEqualArray( + filterState.lastHeard, + defaultFilterValues.lastHeard, + )) || // If lastHeard is unknown (0), hide node if state is not default secondsAgo < filterState.lastHeard[0] || (secondsAgo > filterState.lastHeard[1] && filterState.lastHeard[1] !== defaultFilterValues.lastHeard[1]) @@ -128,6 +141,8 @@ export function useFilterNode() { const snr = node.snr ?? -20; if ( + (node.snr === undefined && + !shallowEqualArray(filterState.snr, defaultFilterValues.snr)) || (snr < filterState.snr[0] && filterState.snr[0] !== defaultFilterValues.snr[0]) || (snr > filterState.snr[1] && @@ -138,6 +153,11 @@ export function useFilterNode() { const channelUtilization = node.deviceMetrics?.channelUtilization ?? 0; if ( + (node.deviceMetrics?.channelUtilization === undefined && + !shallowEqualArray( + filterState.channelUtilization, + defaultFilterValues.channelUtilization, + )) || channelUtilization < filterState.channelUtilization[0] || channelUtilization > filterState.channelUtilization[1] ) { @@ -146,6 +166,11 @@ export function useFilterNode() { const airUtilTx = node.deviceMetrics?.airUtilTx ?? 0; if ( + (node.deviceMetrics?.airUtilTx === undefined && + !shallowEqualArray( + filterState.airUtilTx, + defaultFilterValues.airUtilTx, + )) || airUtilTx < filterState.airUtilTx[0] || airUtilTx > filterState.airUtilTx[1] ) { @@ -154,14 +179,24 @@ export function useFilterNode() { const batt = node.deviceMetrics?.batteryLevel ?? 101; if ( + (node.deviceMetrics?.batteryLevel === undefined && + !shallowEqualArray( + filterState.batteryLevel, + defaultFilterValues.batteryLevel, + )) || batt < filterState.batteryLevel[0] || batt > filterState.batteryLevel[1] ) { return false; } - const voltage = node.deviceMetrics?.voltage ?? 0; + const voltage = Math.abs(node.deviceMetrics?.voltage ?? 0); if ( + (node.deviceMetrics?.voltage === undefined && + !shallowEqualArray( + filterState.voltage, + defaultFilterValues.voltage, + )) || voltage < filterState.voltage[0] || (voltage > filterState.voltage[1] && filterState.voltage[1] !== defaultFilterValues.voltage[1]) @@ -170,14 +205,25 @@ export function useFilterNode() { } const role: Protobuf.Config.Config_DeviceConfig_Role = - node.user.role ?? Protobuf.Config.Config_DeviceConfig_Role.CLIENT; - if (!filterState.role.includes(role)) { + node.user?.role ?? Protobuf.Config.Config_DeviceConfig_Role.CLIENT; + if ( + (node.user?.role === undefined && + !shallowEqualArray(filterState.role, defaultFilterValues.role)) || + !filterState.role.includes(role) + ) { return false; } const hwModel: Protobuf.Mesh.HardwareModel = - node.user.hwModel ?? Protobuf.Mesh.HardwareModel.UNSET; - if (!filterState.hwModel.includes(hwModel)) { + node.user?.hwModel ?? Protobuf.Mesh.HardwareModel.UNSET; + if ( + (node.user?.hwModel === undefined && + !shallowEqualArray( + filterState.hwModel, + defaultFilterValues.hwModel, + )) || + !filterState.hwModel.includes(hwModel) + ) { return false; } diff --git a/packages/web/src/core/stores/index.ts b/packages/web/src/core/stores/index.ts index 445e2da5..077188b7 100644 --- a/packages/web/src/core/stores/index.ts +++ b/packages/web/src/core/stores/index.ts @@ -2,6 +2,7 @@ import { useDeviceContext } from "@core/hooks/useDeviceContext"; import { type Device, useDeviceStore } from "@core/stores/deviceStore"; import { type MessageStore, useMessageStore } from "@core/stores/messageStore"; import { type NodeDB, useNodeDBStore } from "@core/stores/nodeDBStore"; +import { bindStoreToDevice } from "@core/stores/utils/bindStoreToDevice"; export { CurrentDeviceContext, @@ -30,13 +31,11 @@ export { } from "@core/stores/sidebarStore"; // Define hooks to access the stores -export const useNodeDB = (): NodeDB => { - const { deviceId } = useDeviceContext(); - const nodeDB = useNodeDBStore( - (s) => s.getNodeDB(deviceId) ?? s.addNodeDB(deviceId), - ); - return nodeDB; -}; +export const useNodeDB = bindStoreToDevice( + useNodeDBStore, + (s, deviceId): NodeDB => s.getNodeDB(deviceId) ?? s.addNodeDB(deviceId), +); + export const useDevice = (): Device => { const { deviceId } = useDeviceContext(); @@ -45,6 +44,7 @@ export const useDevice = (): Device => { ); return device; }; + export const useMessages = (): MessageStore => { const { deviceId } = useDeviceContext(); diff --git a/packages/web/src/core/stores/messageStore/index.ts b/packages/web/src/core/stores/messageStore/index.ts index 32a209cc..3c86ac40 100644 --- a/packages/web/src/core/stores/messageStore/index.ts +++ b/packages/web/src/core/stores/messageStore/index.ts @@ -393,7 +393,7 @@ const persistOptions: PersistOptions< PrivateMessageStoreState, MessageStorePersisted > = { - name: "meshtastic-MessageStore-store", + name: "meshtastic-message-store", storage: createStorage(), version: CURRENT_STORE_VERSION, partialize: (s): MessageStorePersisted => ({ diff --git a/packages/web/src/core/stores/nodeDBStore/index.ts b/packages/web/src/core/stores/nodeDBStore/index.ts index 9e4e8dc3..627e0b34 100644 --- a/packages/web/src/core/stores/nodeDBStore/index.ts +++ b/packages/web/src/core/stores/nodeDBStore/index.ts @@ -6,7 +6,11 @@ import { createStorage } from "@core/stores/utils/indexDB.ts"; import { Protobuf, type Types } from "@meshtastic/core"; import { produce } from "immer"; import { create as createStore, type StateCreator } from "zustand"; -import { type PersistOptions, persist } from "zustand/middleware"; +import { + type PersistOptions, + persist, + subscribeWithSelector, +} from "zustand/middleware"; import type { NodeError, NodeErrorType, ProcessPacketParams } from "./types.ts"; const CURRENT_STORE_VERSION = 0; @@ -102,7 +106,8 @@ function nodeDBFactory( // Validation failed and error has been set inside validateIncomingNode return; } - nodeDB.nodeMap.set(node.num, next); + + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(node.num, next); }), ), @@ -113,7 +118,9 @@ function nodeDBFactory( if (!nodeDB) { throw new Error(`No nodeDB found (id: ${id})`); } - nodeDB.nodeMap.delete(nodeNum); + const updated = new Map(nodeDB.nodeMap); + updated.delete(nodeNum); + nodeDB.nodeMap = updated; }), ), @@ -147,7 +154,10 @@ function nodeDBFactory( if (!nodeDB) { throw new Error(`No nodeDB found (id: ${id})`); } - nodeDB.nodeErrors.set(nodeNum, { node: nodeNum, error }); + nodeDB.nodeErrors = new Map(nodeDB.nodeErrors).set(nodeNum, { + node: nodeNum, + error, + }); }), ), @@ -158,7 +168,9 @@ function nodeDBFactory( if (!nodeDB) { throw new Error(`No nodeDB found (id: ${id})`); } - nodeDB.nodeErrors.delete(nodeNum); + const updated = new Map(nodeDB.nodeErrors); + updated.delete(nodeNum); + nodeDB.nodeErrors = updated; }), ), @@ -181,16 +193,21 @@ function nodeDBFactory( throw new Error(`No nodeDB found (id: ${id})`); } const node = nodeDB.nodeMap.get(data.from); + const nowSec = Math.floor(Date.now() / 1000); // lastHeard is in seconds(!) + if (node) { - node.lastHeard = data.time > 0 ? data.time : Date.now(); // fallback to now if time is 0 or negative - node.snr = data.snr; - nodeDB.nodeMap.set(data.from, node); + const updated = { + ...node, + lastHeard: data.time > 0 ? data.time : nowSec, + snr: data.snr, + }; + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(data.from, updated); } else { - nodeDB.nodeMap.set( + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set( data.from, create(Protobuf.Mesh.NodeInfoSchema, { num: data.from, - lastHeard: data.time > 0 ? data.time : Date.now(), // fallback to now if time is 0 or negative, + lastHeard: data.time > 0 ? data.time : nowSec, // fallback to now if time is 0 or negative, snr: data.snr, }), ); @@ -208,9 +225,8 @@ function nodeDBFactory( const current = nodeDB.nodeMap.get(user.from) ?? create(Protobuf.Mesh.NodeInfoSchema); - current.user = user.data; - current.num = user.from; - nodeDB.nodeMap.set(user.from, current); + const updated = { ...current, user: user.data, num: user.from }; + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(user.from, updated); }), ), @@ -224,9 +240,12 @@ function nodeDBFactory( const current = nodeDB.nodeMap.get(position.from) ?? create(Protobuf.Mesh.NodeInfoSchema); - current.position = position.data; - current.num = position.from; - nodeDB.nodeMap.set(position.from, current); + const updated = { + ...current, + position: position.data, + num: position.from, + }; + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(position.from, updated); }), ), @@ -259,22 +278,25 @@ function nodeDBFactory( }; const setErrorProxy = (nodeNum: number, err: NodeErrorType) => { - mergedErrors.set(nodeNum, { error: err } as NodeError); + mergedErrors.set(nodeNum, { + node: nodeNum, + error: err, + }); }; - for (const [nodeNum, newNode] of newDB.nodeMap) { + for (const [num, newNode] of newDB.nodeMap) { const next = validateIncomingNode( newNode, setErrorProxy, getNodesProxy, ); if (next) { - mergedNodes.set(nodeNum, next); + mergedNodes.set(num, next); } - const err = newDB.getNodeError(nodeNum); - if (err && !oldDB.hasNodeError(nodeNum)) { - mergedErrors.set(nodeNum, err); + const err = newDB.getNodeError(num); + if (err && !oldDB.hasNodeError(num)) { + mergedErrors.set(num, err); } } @@ -297,7 +319,10 @@ function nodeDBFactory( const node = nodeDB.nodeMap.get(nodeNum); if (node) { - node.isFavorite = isFavorite; + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(nodeNum, { + ...node, + isFavorite: isFavorite, + }); } }), ), @@ -312,7 +337,10 @@ function nodeDBFactory( const node = nodeDB.nodeMap.get(nodeNum); if (node) { - node.isIgnored = isIgnored; + nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(nodeNum, { + ...node, + isIgnored: isIgnored, + }); } }), ), @@ -392,7 +420,7 @@ export const nodeDBInitializer: StateCreator = ( const nodeDB = nodeDBFactory(id, get, set); set( produce((draft) => { - draft.nodeDBs.set(id, nodeDB); + draft.nodeDBs = new Map(draft.nodeDBs).set(id, nodeDB); // Enforce retention limit evictOldestEntries(draft.nodeDBs, NODEDB_RETENTION_NUM); @@ -404,7 +432,9 @@ export const nodeDBInitializer: StateCreator = ( removeNodeDB: (id) => { set( produce((draft) => { - draft.nodeDBs.delete(id); + const updated = new Map(draft.nodeDBs); + updated.delete(id); + draft.nodeDBs = updated; }), ); }, @@ -472,7 +502,7 @@ console.debug( ); export const useNodeDBStore = persistNodes - ? createStore( - persist(nodeDBInitializer, persistOptions), + ? createStore( + subscribeWithSelector(persist(nodeDBInitializer, persistOptions)), ) - : createStore()(nodeDBInitializer); + : createStore(subscribeWithSelector(nodeDBInitializer)); diff --git a/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts b/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.tsx similarity index 79% rename from packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts rename to packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.tsx index da2b4192..420a20b1 100644 --- a/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts +++ b/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.tsx @@ -1,7 +1,6 @@ -/** biome-ignore-all lint/suspicious/noExplicitAny: */ -/** biome-ignore-all lint/style/noNonNullAssertion: */ import { create } from "@bufbuild/protobuf"; import { Protobuf } from "@meshtastic/core"; +import { act, render, screen } from "@testing-library/react"; import { toByteArray } from "base64-js"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -18,6 +17,14 @@ vi.mock("idb-keyval", () => ({ }), })); +let deviceIdForTests = 1; +vi.mock("@core/hooks/useDeviceContext", () => ({ + useDeviceContext: () => ({ deviceId: deviceIdForTests }), + __setDeviceId: (id: number) => { + deviceIdForTests = id; + }, +})); + // import a fresh copy of the store module (because the store is created at import time) async function freshStore(persist = false) { vi.resetModules(); @@ -33,8 +40,9 @@ async function freshStore(persist = false) { }, })); - const mod = await import("./index.ts"); - return mod; + const storeMod = await import("./index.ts"); + const { useNodeDB } = await import("../index.ts"); + return { ...storeMod, useNodeDB }; } function makeNode(num: number, extras: Record = {}) { @@ -97,7 +105,7 @@ describe("NodeDB store", () => { expect(db.getNode(50)?.snr).toBe(9); db.processPacket({ from: 50, time: 0, snr: 9 } as any); - expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now(), -1); // within 10ms + expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now() / 1000, -1); // within 1s, note lastHeard is in seconds expect(db.getNode(50)?.snr).toBe(9); }); @@ -336,9 +344,9 @@ describe("NodeDB – merge semantics, PKI checks & extras", () => { expect(n5.user?.publicKey).toEqual(keyOld); // keep old PK expect(n5.user?.longName).toBe("old-5"); - // error flagged + // error not flagged; dropped silently const err = newDB!.getNodeError(5); - expect(String(err!.error)).toMatch(/MISMATCH|PK/i); + expect(err).toBeUndefined(); }); it("old key empty, new key present, store new node", async () => { @@ -451,3 +459,118 @@ describe("NodeDB – merge semantics, PKI checks & extras", () => { expect(newDB.getMyNode().num).toBe(4242); }); }); + +describe("NodeDB deviceContext & debounce", () => { + beforeEach(() => { + idbMem.clear(); + vi.clearAllMocks(); + }); + + it("useNodeDB resolves per-device DB and switches with deviceId", async () => { + const { useNodeDBStore, useNodeDB } = await freshStore(); + + // device 1 + deviceIdForTests = 1; + const st = useNodeDBStore.getState(); + const db1 = st.addNodeDB(1); + db1.addNode({ num: 10 } as any); + + function Comp() { + const len = useNodeDB((db) => db.getNodesLength(), { + debounce: 0, + equality: (a, b) => a === b, + }); + return
{len}
; + } + + const { rerender } = render(); + expect(screen.getByTestId("len").textContent).toBe("1"); + + // switch to device 2 and add nodes + deviceIdForTests = 2; + const db2 = st.addNodeDB(2); + db2.addNode({ num: 20 } as any); + db2.addNode({ num: 21 } as any); + db2.addNode({ num: 22 } as any); + + // re-render so the hook re-subscribes with the new deviceId + await act(async () => { + rerender(); + }); + + expect(screen.getByTestId("len").textContent).toBe("3"); + }); + + it("useNodeDB selector re-renders only when the selected slice changes", async () => { + const { useNodeDBStore, useNodeDB } = await freshStore(); + deviceIdForTests = 1; + + const st = useNodeDBStore.getState(); + const db = st.addNodeDB(1); + + let renders = 0; + function Comp() { + const len = useNodeDB((d) => d.getNodesLength(), { + debounce: 0, + equality: (a, b) => a === b, + }); + renders++; + return
{len}
; + } + + render(); + expect(screen.getByTestId("len").textContent).toBe("0"); + expect(renders).toBe(1); + + // mutate something unrelated to length + db.setNodeError(999, "X" as any); + await act(() => Promise.resolve()); + expect(screen.getByTestId("len").textContent).toBe("0"); + expect(renders).toBe(1); // no re-render + + // now actually change the slice + db.addNode({ num: 1 } as any); + await act(() => Promise.resolve()); + expect(screen.getByTestId("len").textContent).toBe("1"); + expect(renders).toBe(2); + }); + + it("useNodeDB debounce coalesces rapid updates", async () => { + vi.useFakeTimers(); + const { useNodeDBStore, useNodeDB } = await freshStore(); + deviceIdForTests = 1; + + const st = useNodeDBStore.getState(); + const db = st.addNodeDB(1); + + let renders = 0; + function Comp() { + const len = useNodeDB((d) => d.getNodesLength(), { + debounce: 50, + equality: (a, b) => a === b, + }); + renders++; + return
{len}
; + } + + render(); + + // burst of updates within the debounce window + db.addNode({ num: 1 } as any); + db.addNode({ num: 2 } as any); + db.addNode({ num: 3 } as any); + + await act(() => { + vi.advanceTimersByTime(49); + }); + expect(renders).toBe(1); // not yet + + await act(() => { + vi.advanceTimersByTime(2); + }); + expect(screen.getByTestId("len").textContent).toBe("3"); + expect(renders).toBe(2); // single coalesced re-render + + vi.useRealTimers(); + }); +}); diff --git a/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts b/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts index 52975660..1bd30ec9 100644 --- a/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts +++ b/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts @@ -1,5 +1,28 @@ import type { NodeErrorType } from "@core/stores"; import type { Protobuf } from "@meshtastic/core"; +import { fromByteArray } from "base64-js"; + +export function equalKey( + a?: Uint8Array | null, + b?: Uint8Array | null, +): boolean { + if (!a || !b) { + return false; + } + if (a === b) { + return true; + } + const len = a.byteLength; + if (len !== b.byteLength) { + return false; + } + for (let i = 0; i < len; i++) { + if (a[i] !== b[i]) { + return false; + } + } + return true; +} // Validates a new incoming node against existing nodes. // If valid, returns a node to store, else returns undefined. @@ -26,6 +49,12 @@ export function validateIncomingNode( ); if (nodesWithSameKey.length > 0) { // This is a potential impersonation attempt. + + console.warn( + `Node ${num} rejected: Public key already claimed by another node. Key:`, + fromByteArray(newNode.user?.publicKey ?? new Uint8Array()), + ); + setNodeError(num, "DUPLICATE_PKI"); return undefined; // drop newNode entirely } @@ -41,7 +70,7 @@ export function validateIncomingNode( // A public key is considered matching if the incoming key equals // the existing key, OR if the existing key is empty. const isKeyMatchingOrExistingEmpty = - oldNode.user?.publicKey === newNode.user?.publicKey || + equalKey(oldNode.user?.publicKey, newNode.user?.publicKey) || oldNode.user?.publicKey === undefined || oldNode.user?.publicKey.length === 0; @@ -49,14 +78,34 @@ export function validateIncomingNode( // Keys match or existing key was empty: trust the incoming node data completely. // This allows for legitimate updates to user info and other fields. return newNode; - } else { + } else if ( + newNode.user?.publicKey !== undefined && + newNode.user?.publicKey.length > 0 + ) { + console.warn( + `Node ${num} rejected: existing key does not match incoming key. Old key:`, + fromByteArray(oldNode.user?.publicKey ?? new Uint8Array()), + "New key:", + fromByteArray(newNode.user?.publicKey ?? new Uint8Array()), + ); + // Keys do not match and existing key was not empty: potential impersonation attempt. setNodeError(num, "MISMATCH_PKI"); return oldNode; // drop newNode fields and return old + } else { + // Incoming node has no public key: ignore the new node entirely. + console.warn( + `Node ${num} rejected: incoming node has no public key, but existing does.`, + ); + return oldNode; // drop newNode fields and return old } } else { // Multiple existing nodes with the same node number // This should never happen, but if it does, we drop the new node entirely. + console.warn( + `Node ${num} rejected: Multiple existing nodes with this node number.`, + ); + setNodeError(num, "DUPLICATE_PKI"); return undefined; // drop newNode entirely } diff --git a/packages/web/src/core/stores/utils/bindStoreToDevice.ts b/packages/web/src/core/stores/utils/bindStoreToDevice.ts new file mode 100644 index 00000000..82dfc7d9 --- /dev/null +++ b/packages/web/src/core/stores/utils/bindStoreToDevice.ts @@ -0,0 +1,101 @@ +import { useDeviceContext } from "@core/hooks/useDeviceContext"; +import { useCallback, useMemo, useRef, useSyncExternalStore } from "react"; +import type { StoreApi, UseBoundStore } from "zustand"; +import { shallow } from "zustand/shallow"; + +type GenericEqualityFn = (a: T, b: T) => boolean; + +type DebounceOpts = { + debounce?: number; // 0/undefined = no debounce + equality?: GenericEqualityFn; // default: shallow + fireImmediately?: boolean; // default: true +}; + +type StoreWithSelector = UseBoundStore> & { + getState(): S; + subscribe: ( + selector: (state: S) => U, + listener: (next: U, prev: U) => void, + options?: { equalityFn?: GenericEqualityFn; fireImmediately?: boolean }, + ) => () => void; +}; + +export function bindStoreToDevice( + store: StoreWithSelector, + resolveDB: (state: S, deviceId: number) => DB, +) { + // Overloads: + function useBound(): DB; + function useBound(selector: (db: DB) => T, opts?: DebounceOpts): T; + + // Implementation: + function useBound( + selector?: (db: DB) => T, + opts?: DebounceOpts, + ): DB | T { + const { deviceId } = useDeviceContext(); + + // Build the store-level selector + const storeSelector = useCallback( + (state: S) => { + const db = resolveDB(state, deviceId); + return selector ? selector(db) : db; + }, + [deviceId, resolveDB, selector], + ); + + type Selected = ReturnType; + + const wait = opts?.debounce ?? 0; + const fireImmediately = opts?.fireImmediately ?? true; + const equality: GenericEqualityFn = + (opts?.equality as GenericEqualityFn) ?? + (shallow as unknown as GenericEqualityFn); + + const snapRef = useRef(storeSelector(store.getState())); + snapRef.current = storeSelector(store.getState()); // this ensures rerenders with a new selector (new deviceId) see the right initial value + + const timerRef = useRef | undefined>( + undefined, + ); + + const subscribe = useMemo(() => { + return (onChange: () => void) => { + const unsubscribe = store.subscribe( + storeSelector, + (next: Selected, prev: Selected) => { + const emit = () => { + snapRef.current = next; + onChange(); + }; + + if (equality(next, prev)) { + return; + } + if (wait > 0) { + if (timerRef.current) { + clearTimeout(timerRef.current); + } + timerRef.current = setTimeout(emit, wait); + } else { + emit(); + } + }, + { equalityFn: equality, fireImmediately }, + ); + + return () => { + if (timerRef.current) { + clearTimeout(timerRef.current); + } + unsubscribe(); + }; + }; + }, [store, storeSelector, equality, wait, fireImmediately]); + + const getSnapshot = () => snapRef.current; + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); + } + + return useBound; +} diff --git a/packages/web/src/pages/Map/index.tsx b/packages/web/src/pages/Map/index.tsx index b7b2217a..69d40a78 100644 --- a/packages/web/src/pages/Map/index.tsx +++ b/packages/web/src/pages/Map/index.tsx @@ -11,11 +11,19 @@ import { cn } from "@core/utils/cn.ts"; import type { Protobuf } from "@meshtastic/core"; import { bbox, lineString } from "@turf/turf"; import { FunnelIcon, MapPinIcon } from "lucide-react"; -import { useCallback, useDeferredValue, useMemo, useState } from "react"; +import { + useCallback, + useDeferredValue, + useMemo, + useRef, + useState, +} from "react"; import { Marker, Popup, useMap } from "react-map-gl/maplibre"; import { NodeDetail } from "../../components/PageComponents/Map/NodeDetail.tsx"; import { Avatar } from "../../components/UI/Avatar.tsx"; +const NODEDB_DEBOUNCE_MS = 250; + type NodePosition = { latitude: number; longitude: number; @@ -31,7 +39,19 @@ const convertToLatLng = (position?: { const MapPage = () => { const { waypoints } = useDevice(); - const { getNodes, hasNodeError } = useNodeDB(); + const { nodes: validNodes, hasNodeError } = useNodeDB( + (db) => ({ + // only nodes with a position + nodes: db.getNodes((n): n is Protobuf.Mesh.NodeInfo => + Boolean(n.position?.latitudeI), + ), + hasNodeError: db.hasNodeError, + // include the Map reference so error badges update when nodeErrors changes + _errorsRef: db.nodeErrors, + }), + { debounce: NODEDB_DEBOUNCE_MS }, + ); + const { nodeFilter, defaultFilterValues, isFilterDirty } = useFilterNode(); const { default: map } = useMap(); @@ -39,14 +59,6 @@ const MapPage = () => { const [selectedNode, setSelectedNode] = useState(null); - const validNodes = useMemo( - () => - getNodes((node): node is Protobuf.Mesh.NodeInfo => - Boolean(node.position?.latitudeI), - ), - [getNodes], - ); - const [filterState, setFilterState] = useState( () => defaultFilterValues, ); @@ -57,6 +69,8 @@ const MapPage = () => { [validNodes, deferredFilterState, nodeFilter], ); + const hasFitBoundsOnce = useRef(false); + const handleMarkerClick = useCallback( (node: Protobuf.Mesh.NodeInfo, event: { originalEvent: MouseEvent }) => { event?.originalEvent?.stopPropagation(); @@ -76,7 +90,7 @@ const MapPage = () => { // Get the bounds of the map based on the nodes furtherest away from center const getMapBounds = useCallback(() => { - if (!map || validNodes.length === 0) { + if (hasFitBoundsOnce.current || !map || validNodes.length === 0) { return; } @@ -108,6 +122,7 @@ const MapPage = () => { if (center) { map.easeTo(center); } + hasFitBoundsOnce.current = true; }, [map, validNodes]); // Generate all markers diff --git a/packages/web/src/pages/Nodes/index.tsx b/packages/web/src/pages/Nodes/index.tsx index 7f3448ad..cb7d0b6e 100644 --- a/packages/web/src/pages/Nodes/index.tsx +++ b/packages/web/src/pages/Nodes/index.tsx @@ -26,12 +26,13 @@ import { useCallback, useDeferredValue, useEffect, - useMemo, useState, } from "react"; import { useTranslation } from "react-i18next"; import { base16 } from "rfc4648"; +const NODEDB_DEBOUNCE_MS = 250; + export interface DeleteNoteDialogProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -41,7 +42,7 @@ const NodesPage = (): JSX.Element => { const { t } = useTranslation("nodes"); const { currentLanguage } = useLang(); const { hardware, connection, setDialogOpen } = useDevice(); - const { getNodes, hasNodeError } = useNodeDB(); + const { setNodeNumDetails } = useAppStore(); const { nodeFilter, defaultFilterValues, isFilterDirty } = useFilterNode(); @@ -57,11 +58,21 @@ const NodesPage = (): JSX.Element => { ); const deferredFilterState = useDeferredValue(filterState); - const filteredNodes = useMemo( - () => getNodes((node) => nodeFilter(node, deferredFilterState)), - [deferredFilterState, getNodes, nodeFilter], + // stable predicate so the selector identity doesn’t thrash + const predicate = useCallback( + (node: Protobuf.Mesh.NodeInfo) => nodeFilter(node, deferredFilterState), + [nodeFilter, deferredFilterState], ); + // subscribe to actual data (nodes array) and to nodeErrors ref for badge updates + const { nodes: filteredNodes, hasNodeError } = useNodeDB( + (db) => ({ + nodes: db.getNodes(predicate, false), + hasNodeError: db.hasNodeError, + _errorsRef: db.nodeErrors, // include the Map ref so UI also re-renders on error changes + }), + { debounce: NODEDB_DEBOUNCE_MS }, + ); const handleTraceroute = useCallback( (traceroute: Types.PacketMetadata) => { setSelectedTraceroute(traceroute); @@ -103,7 +114,7 @@ const NodesPage = (): JSX.Element => { } connection.events.onPositionPacket.subscribe(handleLocation); return () => { - connection.events.onPositionPacket.subscribe(handleLocation); + connection.events.onPositionPacket.unsubscribe(handleLocation); }; }, [connection, handleLocation]); @@ -125,6 +136,15 @@ const NodesPage = (): JSX.Element => { .match(/.{1,2}/g) ?.join(":") ?? t("unknown.shortName"); + const shortName = + node.user?.shortName ?? + numberToHexUnpadded(node.num).slice(-4).toUpperCase(); + const longName = + node.user?.longName ?? + t("fallbackName", { + last4: shortName, + }); + return { id: node.num, isFavorite: node.isFavorite, @@ -132,12 +152,12 @@ const NodesPage = (): JSX.Element => { { content: ( ), - sortValue: node.user?.shortName ?? "", // Non-sortable column + sortValue: shortName, // Non-sortable column }, { content: ( @@ -148,10 +168,10 @@ const NodesPage = (): JSX.Element => { }} className="cursor-pointer underline ml-2 whitespace-break-spaces" > - {node.user?.longName ?? numberToHexUnpadded(node.num)} + {longName} ), - sortValue: node.user?.longName ?? numberToHexUnpadded(node.num), + sortValue: longName, }, { content: ( @@ -164,7 +184,7 @@ const NodesPage = (): JSX.Element => { ? t("unit.hop.plural") : t("unit.hops_one") } ${t("nodesTable.connectionStatus.away")}` - : t("nodesTable.connectionStatus.unknown")} + : t("unknown.longName")} {node?.viaMqtt === true ? t("nodesTable.connectionStatus.viaMqtt") : ""} @@ -176,7 +196,7 @@ const NodesPage = (): JSX.Element => { content: ( {node.lastHeard === 0 ? ( -

{t("nodesTable.lastHeardStatus.never")}

+ t("unknown.longName") ) : (