From 58c0254e656a2b5cc17c0a2c73b6e949b87ae12e Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Tue, 21 Jul 2026 23:30:42 -0700 Subject: [PATCH] Fix device configuration streaming --- .../PageComponents/Settings/LoRa.tsx | 9 +- .../src/core/hooks/useWaitForConfig.test.tsx | 59 +++++++++++++ apps/web/src/core/hooks/useWaitForConfig.ts | 25 +++++- apps/web/src/pages/Settings/index.tsx | 17 +++- apps/web/src/validation/config/lora.test.ts | 12 +++ apps/web/src/validation/config/lora.ts | 9 ++ .../core/client/MeshClient.progress.test.ts | 62 ++++++++++++-- packages/sdk/src/core/client/MeshClient.ts | 83 ++++++++++++++++--- .../sdk/src/core/packet-codec/decodePacket.ts | 12 +-- packages/sdk/src/core/queue/Queue.ts | 25 ++++++ .../src/features/config/ConfigEditor.test.ts | 36 ++++++++ .../features/config/domain/ConfigEditor.ts | 11 ++- 12 files changed, 318 insertions(+), 42 deletions(-) create mode 100644 apps/web/src/core/hooks/useWaitForConfig.test.tsx diff --git a/apps/web/src/components/PageComponents/Settings/LoRa.tsx b/apps/web/src/components/PageComponents/Settings/LoRa.tsx index 949e2c6f7..a1b57205a 100644 --- a/apps/web/src/components/PageComponents/Settings/LoRa.tsx +++ b/apps/web/src/components/PageComponents/Settings/LoRa.tsx @@ -2,6 +2,7 @@ import { useWaitForConfig } from "@app/core/hooks/useWaitForConfig"; import { type LoRaValidation, LoRaValidationSchema, + withLoRaDefaults, } from "@app/validation/config/lora.ts"; import { DynamicForm, @@ -29,11 +30,15 @@ export const LoRa = ({ onFormInit }: LoRaConfigProps) => { const editor = useConfigEditor(); const radio = useSignal(editor?.radio ?? EMPTY_RADIO_SIGNAL); - const effectiveLora = + const effectiveLoraConfig = radio.lora ?? (getEffectiveConfig("lora") as | Protobuf.Config.Config_LoRaConfig | undefined); + const effectiveLora = effectiveLoraConfig + ? withLoRaDefaults(effectiveLoraConfig) + : undefined; + const defaultLora = config.lora ? withLoRaDefaults(config.lora) : undefined; const { t } = useTranslation("config"); @@ -50,7 +55,7 @@ export const LoRa = ({ onFormInit }: LoRaConfigProps) => { onSubmit={onSubmit} onFormInit={onFormInit} validationSchema={LoRaValidationSchema} - defaultValues={config.lora} + defaultValues={defaultLora} values={effectiveLora} fieldGroups={[ { diff --git a/apps/web/src/core/hooks/useWaitForConfig.test.tsx b/apps/web/src/core/hooks/useWaitForConfig.test.tsx new file mode 100644 index 000000000..b56c6a6ff --- /dev/null +++ b/apps/web/src/core/hooks/useWaitForConfig.test.tsx @@ -0,0 +1,59 @@ +import { create } from "@bufbuild/protobuf"; +import { CurrentDeviceContext } from "@core/hooks/useDeviceContext.ts"; +import { useDeviceStore } from "@core/stores/deviceStore/index.ts"; +import { Protobuf } from "@meshtastic/sdk"; +import { act, renderHook } from "@testing-library/react"; +import type { PropsWithChildren } from "react"; +import { beforeEach, describe, expect, it } from "vitest"; +import { useWaitForConfig } from "./useWaitForConfig.ts"; + +const DEVICE_ID = 4242; + +const wrapper = ({ children }: PropsWithChildren) => ( + + {children} + +); + +describe("useWaitForConfig", () => { + beforeEach(() => { + useDeviceStore.getState().removeDevice(DEVICE_ID); + }); + + it("resolves its suspense promise when the requested config arrives", async () => { + const device = useDeviceStore.getState().addDevice(DEVICE_ID); + const { result } = renderHook( + () => { + try { + useWaitForConfig({ configCase: "lora" }); + return "ready" as const; + } catch (pending) { + return pending as Promise; + } + }, + { wrapper }, + ); + + expect(result.current).toBeInstanceOf(Promise); + const pending = result.current as Promise; + + act(() => { + device.setConfig( + create(Protobuf.Config.ConfigSchema, { + payloadVariant: { + case: "lora", + value: create(Protobuf.Config.Config_LoRaConfigSchema, {}), + }, + }), + ); + }); + + const outcome = await Promise.race([ + pending.then(() => "resolved"), + new Promise((resolve) => + setTimeout(() => resolve("timed-out"), 50), + ), + ]); + expect(outcome).toBe("resolved"); + }); +}); diff --git a/apps/web/src/core/hooks/useWaitForConfig.ts b/apps/web/src/core/hooks/useWaitForConfig.ts index 16108feca..688bbe0d6 100644 --- a/apps/web/src/core/hooks/useWaitForConfig.ts +++ b/apps/web/src/core/hooks/useWaitForConfig.ts @@ -1,5 +1,6 @@ import { useDevice, + useDeviceStore, type ValidConfigType, type ValidModuleConfigType, } from "@core/stores"; @@ -12,13 +13,33 @@ export function useWaitForConfig({ configCase, moduleConfigCase, }: UseWaitForConfigProps): void { - const { config, moduleConfig } = useDevice(); + const device = useDevice(); + const { config, moduleConfig } = device; const isDataDefined = configCase ? config[configCase] !== undefined : moduleConfig[moduleConfigCase as ValidModuleConfigType] !== undefined; if (!isDataDefined) { - throw new Promise(() => {}); + throw new Promise((resolve) => { + const hasRequestedConfig = (): boolean => { + const current = useDeviceStore.getState().getDevice(device.id); + if (!current) return false; + return configCase + ? current.config[configCase] !== undefined + : current.moduleConfig[moduleConfigCase as ValidModuleConfigType] !== + undefined; + }; + + let unsubscribe = (): void => {}; + const check = (): void => { + if (hasRequestedConfig()) { + unsubscribe(); + resolve(); + } + }; + unsubscribe = useDeviceStore.subscribe(check); + check(); + }); } } diff --git a/apps/web/src/pages/Settings/index.tsx b/apps/web/src/pages/Settings/index.tsx index 9a2081e09..767042b4f 100644 --- a/apps/web/src/pages/Settings/index.tsx +++ b/apps/web/src/pages/Settings/index.tsx @@ -53,6 +53,7 @@ const ConfigPage = () => { ); const [isSaving, setIsSaving] = useState(false); + const [formResetKey, setFormResetKey] = useState(0); const [rhfState, setRhfState] = useState({ isDirty: false, isValid: true }); const unsubRef = useRef<(() => void) | null>(null); const [formMethods, setFormMethods] = useState(null); @@ -163,10 +164,15 @@ const ConfigPage = () => { }, [toast, t, formMethods, editor]); const handleReset = useCallback(() => { - if (formMethods) { + if (editor) { + // The editor is the source of the controlled `values` passed to each + // form. Remount the active form after discarding drafts so a controlled + // value update cannot emit a late change and recreate the draft. + editor.reset(); + setFormResetKey((key) => key + 1); + } else if (formMethods) { formMethods.reset(); } - editor?.reset(); }, [formMethods, editor]); const leftSidebar = useMemo( @@ -258,7 +264,12 @@ const ConfigPage = () => { label={activeSection?.label ?? ""} actions={actions} > - {ActiveComponent && } + {ActiveComponent && ( + + )} ); }; diff --git a/apps/web/src/validation/config/lora.test.ts b/apps/web/src/validation/config/lora.test.ts index 153e42b20..102f56842 100644 --- a/apps/web/src/validation/config/lora.test.ts +++ b/apps/web/src/validation/config/lora.test.ts @@ -1,5 +1,7 @@ +import { create } from "@bufbuild/protobuf"; import { Protobuf } from "@meshtastic/sdk"; import { describe, expect, it } from "vitest"; +import { withLoRaDefaults } from "./lora.ts"; describe("LoRa modem presets", () => { it("includes LONG_TURBO modem preset", () => { @@ -11,3 +13,13 @@ describe("LoRa modem presets", () => { ).toBe(true); }); }); + +describe("LoRa validation", () => { + it("defaults an omitted serialHalOnly form value to false", () => { + const { serialHalOnly: _, ...legacyConfig } = create( + Protobuf.Config.Config_LoRaConfigSchema, + ); + + expect(withLoRaDefaults(legacyConfig).serialHalOnly).toBe(false); + }); +}); diff --git a/apps/web/src/validation/config/lora.ts b/apps/web/src/validation/config/lora.ts index 075aec385..3dce8ddd2 100644 --- a/apps/web/src/validation/config/lora.ts +++ b/apps/web/src/validation/config/lora.ts @@ -32,3 +32,12 @@ export const LoRaValidationSchema = z.object({ }); export type LoRaValidation = z.infer; + +export function withLoRaDefaults( + config: T, +): T & { serialHalOnly: boolean } { + return { + ...config, + serialHalOnly: config.serialHalOnly ?? false, + }; +} diff --git a/packages/sdk/src/core/client/MeshClient.progress.test.ts b/packages/sdk/src/core/client/MeshClient.progress.test.ts index f2fada6a1..8a1f5e3ed 100644 --- a/packages/sdk/src/core/client/MeshClient.progress.test.ts +++ b/packages/sdk/src/core/client/MeshClient.progress.test.ts @@ -1,10 +1,13 @@ -import { create } from "@bufbuild/protobuf"; +import { create, fromBinary } from "@bufbuild/protobuf"; import * as Protobuf from "@meshtastic/protobufs"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createFakeTransport } from "../testing/createFakeTransport.ts"; import { MeshClient } from "./MeshClient.ts"; describe("MeshClient.progress", () => { + const CONFIG_ONLY_NONCE = 69420; + const NODES_ONLY_NONCE = 69421; + it("starts in the idle phase before configure() is called", () => { const { transport } = createFakeTransport(); const client = new MeshClient({ transport }); @@ -60,14 +63,40 @@ describe("MeshClient.progress", () => { expect(cur.received.modules).toBe(0); }); - it("flips to configured when onConfigComplete fires", () => { - const { transport } = createFakeTransport(); + it("requests config and nodes in separate firmware-compatible stages", async () => { + const { transport, respond, sent } = createFakeTransport(); const client = new MeshClient({ transport }); - void client.configure(); + + await client.configure(); + + const initialPackets = sent.map( + (packet) => + fromBinary(Protobuf.Mesh.ToRadioSchema, packet).payloadVariant, + ); + expect(initialPackets[0]?.case).toBe("heartbeat"); + expect(initialPackets[1]).toEqual({ + case: "wantConfigId", + value: CONFIG_ONLY_NONCE, + }); + client.events.onConfigPacket.dispatch( create(Protobuf.Config.ConfigSchema, {}), ); - client.events.onConfigComplete.dispatch(0); + respond.withConfigCompleteId(CONFIG_ONLY_NONCE); + + await vi.waitFor(() => { + expect(sent).toHaveLength(3); + }); + expect( + fromBinary(Protobuf.Mesh.ToRadioSchema, sent[2]!).payloadVariant, + ).toEqual({ case: "wantConfigId", value: NODES_ONLY_NONCE }); + expect(client.progress.value.phase).toBe("configuring"); + + respond.withConfigCompleteId(NODES_ONLY_NONCE); + + await vi.waitFor(() => { + expect(client.progress.value.phase).toBe("configured"); + }); const cur = client.progress.value; expect(cur.phase).toBe("configured"); @@ -75,6 +104,22 @@ describe("MeshClient.progress", () => { expect(cur.received.config).toBe(1); }); + it("ignores stale and out-of-order config completion nonces", async () => { + const { transport, respond, sent } = createFakeTransport(); + const client = new MeshClient({ transport }); + const completed: number[] = []; + client.events.onConfigComplete.subscribe((id) => completed.push(id)); + + await client.configure(); + respond.withConfigCompleteId(NODES_ONLY_NONCE); + respond.withConfigCompleteId(12345); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(sent).toHaveLength(2); + expect(client.progress.value.phase).toBe("configuring"); + expect(completed).toEqual([]); + }); + it("ignores packets that arrive while idle (post-completion)", () => { const { transport } = createFakeTransport(); const client = new MeshClient({ transport }); @@ -85,14 +130,13 @@ describe("MeshClient.progress", () => { expect(client.progress.value.phase).toBe("idle"); }); - it("resets counters when configure() runs again", () => { + it("resets counters when configure() runs again", async () => { const { transport } = createFakeTransport(); const client = new MeshClient({ transport }); - void client.configure(); + await client.configure(); client.events.onConfigPacket.dispatch( create(Protobuf.Config.ConfigSchema, {}), ); - client.events.onConfigComplete.dispatch(0); void client.configure(); expect(client.progress.value).toEqual({ phase: "configuring", diff --git a/packages/sdk/src/core/client/MeshClient.ts b/packages/sdk/src/core/client/MeshClient.ts index fb687a86e..b49de7288 100644 --- a/packages/sdk/src/core/client/MeshClient.ts +++ b/packages/sdk/src/core/client/MeshClient.ts @@ -74,6 +74,10 @@ const EMPTY_COUNTERS: ConnectionProgressCounters = { metadata: false, }; +const CONFIG_ONLY_NONCE = 69420; +const NODES_ONLY_NONCE = 69421; +type HandshakeStage = "idle" | "config" | "nodes" | "configured"; + /** * Orchestrator for a single connected Meshtastic device. * @@ -111,6 +115,7 @@ export class MeshClient { }); private _heartbeatIntervalId: ReturnType | undefined; + private handshakeStage: HandshakeStage = "idle"; constructor(options: MeshClientOptions) { this.log = options.logger ?? createLogger("MeshClient"); @@ -150,16 +155,15 @@ export class MeshClient { } /** - * Begin the wantConfigId → config-complete handshake. Resolves when the - * device has ack'd the wantConfigId packet (status changes to - * DeviceConfigured when the device finishes sending its configuration). + * Begin the two-stage config-only → nodes-only handshake. Resolves once + * both requests have been written; streamed completion is asynchronous. */ public async connect(): Promise { this.updateDeviceStatus(DeviceStatusEnum.DeviceConnecting); await this.configure(); } - public configure(): Promise { + public async configure(): Promise { this.log.debug( Emitter[Emitter.Configure], "⚙️ Requesting device configuration", @@ -170,17 +174,54 @@ export class MeshClient { received: { ...EMPTY_COUNTERS }, }; + this.handshakeStage = "config"; + + // Match native clients: wake the radio before beginning the two-stage + // stream. Neither packet receives a routing acknowledgement. + await this.heartbeat(); + + return this.requestConfigStage(CONFIG_ONLY_NONCE).catch((e) => { + if (this.device.status.value === DeviceStatusEnum.DeviceDisconnected) { + throw new Error("Device connection lost"); + } + throw e; + }); + } + + private requestConfigStage(nonce: number): Promise { const toRadio = create(Protobuf.Mesh.ToRadioSchema, { - payloadVariant: { case: "wantConfigId", value: this.configId }, + payloadVariant: { case: "wantConfigId", value: nonce }, }); + return this.sendRawUnacknowledged( + toBinary(Protobuf.Mesh.ToRadioSchema, toRadio), + nonce, + ); + } - return this.sendRaw(toBinary(Protobuf.Mesh.ToRadioSchema, toRadio)).catch( - (e) => { - if (this.device.status.value === DeviceStatusEnum.DeviceDisconnected) { - throw new Error("Device connection lost"); - } - throw e; - }, + /** Advance only when the completion nonce matches the active stage. */ + public handleConfigComplete(nonce: number): void { + if (this.handshakeStage === "config" && nonce === CONFIG_ONLY_NONCE) { + this.handshakeStage = "nodes"; + void this.requestConfigStage(NODES_ONLY_NONCE).catch((error) => { + this.log.error( + Emitter[Emitter.Configure], + "⚠️ Unable to request node database", + error, + ); + }); + return; + } + + if (this.handshakeStage === "nodes" && nonce === NODES_ONLY_NONCE) { + this.handshakeStage = "configured"; + this.events.onConfigComplete.dispatch(nonce); + this.updateDeviceStatus(DeviceStatusEnum.DeviceConfigured); + return; + } + + this.log.debug( + Emitter[Emitter.Configure], + `Ignoring config completion nonce ${nonce} during ${this.handshakeStage} stage`, ); } @@ -226,7 +267,9 @@ export class MeshClient { const toRadio = create(Protobuf.Mesh.ToRadioSchema, { payloadVariant: { case: "heartbeat", value: {} }, }); - return this.sendRaw(toBinary(Protobuf.Mesh.ToRadioSchema, toRadio)); + return this.sendRawUnacknowledged( + toBinary(Protobuf.Mesh.ToRadioSchema, toRadio), + ); } public setHeartbeatInterval(interval: number): void { @@ -323,6 +366,19 @@ export class MeshClient { return this.queue.wait(id); } + private async sendRawUnacknowledged( + toRadio: Uint8Array, + id: number = generatePacketId(), + ): Promise { + if (toRadio.length > 512) { + throw new PacketTooLargeError(toRadio.length); + } + return this.queue.sendUnacknowledged( + { id, data: toRadio }, + this.transport.toDevice, + ); + } + /** * Dispatch a `PacketMetadata` echo for locally-composed messages. */ @@ -340,6 +396,7 @@ export class MeshClient { public complete(): void { this.queue.clear(); + this.handshakeStage = "idle"; } public async disconnect(): Promise { diff --git a/packages/sdk/src/core/packet-codec/decodePacket.ts b/packages/sdk/src/core/packet-codec/decodePacket.ts index 363e3f4c2..f8e241c1f 100644 --- a/packages/sdk/src/core/packet-codec/decodePacket.ts +++ b/packages/sdk/src/core/packet-codec/decodePacket.ts @@ -22,6 +22,7 @@ export interface PacketSink { readonly myNodeNum: number; updateDeviceStatus(status: DeviceStatusEnum): void; configure(): Promise; + handleConfigComplete(nonce: number): void; } /** @@ -151,16 +152,7 @@ export const decodePacket = (sink: PacketSink): WritableStream => Emitter[Emitter.HandleFromRadio], `⚙️ Received config complete id: ${decodedMessage.payloadVariant.value}`, ); - sink.events.onConfigComplete.dispatch( - decodedMessage.payloadVariant.value, - ); - if (decodedMessage.payloadVariant.value === sink.configId) { - sink.log.info( - Emitter[Emitter.HandleFromRadio], - `⚙️ Config id matches client.configId: ${sink.configId}`, - ); - sink.updateDeviceStatus(DeviceStatusEnum.DeviceConfigured); - } + sink.handleConfigComplete(decodedMessage.payloadVariant.value); break; } case "rebooted": { diff --git a/packages/sdk/src/core/queue/Queue.ts b/packages/sdk/src/core/queue/Queue.ts index 2cec6e02f..6abb16587 100644 --- a/packages/sdk/src/core/queue/Queue.ts +++ b/packages/sdk/src/core/queue/Queue.ts @@ -95,6 +95,31 @@ export class Queue { return queueItem.promise; } + /** + * Flush a control packet without waiting for a routing acknowledgement. + * Firmware does not acknowledge local heartbeat or configuration requests. + */ + public async sendUnacknowledged( + item: Omit, + outputStream: WritableStream, + ): Promise { + const queuedItem: QueueItem = { + ...item, + sent: false, + added: new Date(), + promise: Promise.resolve(item.id), + }; + this.queue.push(queuedItem); + while (!queuedItem.sent) { + await this.processQueue(outputStream); + if (!queuedItem.sent) { + await new Promise((resolve) => setTimeout(resolve, 10)); + } + } + this.remove(item.id); + return item.id; + } + public async processQueue( outputStream: WritableStream, ): Promise { diff --git a/packages/sdk/src/features/config/ConfigEditor.test.ts b/packages/sdk/src/features/config/ConfigEditor.test.ts index fbf7d7cef..94e37880b 100644 --- a/packages/sdk/src/features/config/ConfigEditor.test.ts +++ b/packages/sdk/src/features/config/ConfigEditor.test.ts @@ -91,6 +91,22 @@ describe("ConfigEditor", () => { expect(editor.radio.value.lora?.region).toBe(4); }); + it("ignores protobuf metadata when comparing form values", () => { + const { transport } = createFakeTransport(); + const client = new MeshClient({ transport }); + const editor = client.config.editor; + + client.events.onConfigPacket.dispatch(loraPacket(4)); + const { $typeName: _, ...formValue } = editor.radio.value.lora!; + editor.setRadioSection( + "lora", + formValue as Protobuf.Config.Config_LoRaConfig, + ); + + expect(editor.isDirty.value).toBe(false); + expect(editor.dirtyRadioSections.value).toEqual([]); + }); + it("inbound baseline updates do not stomp pending working edits", () => { const { transport } = createFakeTransport(); const client = new MeshClient({ transport }); @@ -128,4 +144,24 @@ describe("ConfigEditor", () => { expect(editor.radio.value).toEqual({}); expect(editor.isDirty.value).toBe(false); }); + + it("reset() discards queued admin messages", () => { + const { transport } = createFakeTransport(); + const client = new MeshClient({ transport }); + const editor = client.config.editor; + + editor.queueAdminMessage( + create(Protobuf.Admin.AdminMessageSchema, { + payloadVariant: { + case: "rebootSeconds", + value: 5, + }, + }), + ); + expect(editor.isDirty.value).toBe(true); + + editor.reset(); + + expect(editor.isDirty.value).toBe(false); + }); }); diff --git a/packages/sdk/src/features/config/domain/ConfigEditor.ts b/packages/sdk/src/features/config/domain/ConfigEditor.ts index e41f7bea1..2dbb3a9a1 100644 --- a/packages/sdk/src/features/config/domain/ConfigEditor.ts +++ b/packages/sdk/src/features/config/domain/ConfigEditor.ts @@ -228,7 +228,12 @@ export class ConfigEditor { this.workingModules.value = this.baselineModules.peek(); this.workingChannels.value = new Map(this.baselineChannels.peek()); this.workingOwner.value = this.baselineOwner.peek(); - this.recomputeDirty(); + this.queuedAdminMessages.value = []; + this._dirtyRadioSections.value = []; + this._dirtyModuleSections.value = []; + this._dirtyChannels.value = []; + this._isOwnerDirty.value = false; + this._isDirty.value = false; } /** @@ -403,8 +408,8 @@ function shallowEqual(a: unknown, b: unknown): boolean { if (typeof a !== "object" || typeof b !== "object") return false; const ao = a as Record; const bo = b as Record; - const aKeys = Object.keys(ao); - const bKeys = Object.keys(bo); + const aKeys = Object.keys(ao).filter((key) => key !== "$typeName"); + const bKeys = Object.keys(bo).filter((key) => key !== "$typeName"); if (aKeys.length !== bKeys.length) return false; for (const k of aKeys) { const av = ao[k];