refactor: device connection logic, added nonce to get config only (#946)

* refactor: device connection logic, added nonce to get config only on connect. * Update packages/web/src/core/services/MeshService.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update packages/web/src/pages/Connections/useConnections.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * code review fixes * fixes from code review * ui fixes * refactored meshService, moved code into deviceStore. Fixed some connnection issues * formatting fixes --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-11-11 20:56:22 -05:00
parent 7f21b3b531
commit 648a9c3640
17 changed files with 662 additions and 213 deletions
@@ -1,4 +1,5 @@
 import AddConnectionDialog from "@app/components/Dialog/AddConnectionDialog/AddConnectionDialog";
+import { TimeAgo } from "@app/components/generic/TimeAgo";
 import { ConnectionStatusBadge } from "@app/components/PageComponents/Connections/ConnectionStatusBadge";
 import type { Connection } from "@app/core/stores/deviceStore/types";
 import { useConnections } from "@app/pages/Connections/useConnections";
@@ -39,8 +40,8 @@ import {
  ArrowLeft,
  LinkIcon,
  MoreHorizontal,
-  PlugZap,
  RotateCw,
+  RouterIcon,
  Star,
  StarOff,
  Trash2,
@@ -71,7 +72,6 @@ export const Connections = () => {
    syncConnectionStatuses();
    refreshStatuses();
  }, []);
-
  const sorted = useMemo(() => {
    const copy = [...connections];
    return copy.sort((a, b) => {
@@ -81,7 +81,9 @@ export const Connections = () => {
      if (!a.isDefault && b.isDefault) {
        return 1;
      }
-      if (a.status === "connected" && b.status !== "connected") {
+      const aConnected = a.status === "connected" || a.status === "configured";
+      const bConnected = b.status === "connected" || b.status === "configured";
+      if (aConnected && !bConnected) {
        return -1;
      }
      return a.name.localeCompare(b.name);
@@ -111,7 +113,7 @@ export const Connections = () => {
        </div>
        <div className="flex items-center ml-2 gap-2">
          <Button onClick={() => setAddOpen(true)} className="gap-2">
-            <PlugZap className="size-4" />
+            <RouterIcon className="size-5" />
            {t("button.addConnection")}
          </Button>
        </div>
@@ -131,13 +133,13 @@ export const Connections = () => {
          </CardContent>
          <CardFooter>
            <Button onClick={() => setAddOpen(true)} className="gap-2">
-              <PlugZap className="size-4" />
+              <RouterIcon className="size-5" />
              {t("button.addConnection")}
            </Button>
          </CardFooter>
        </Card>
      ) : (
-        <div className="grid gap-4 sm:grid-cols-2 lg:grid-cols-3">
+        <div className="grid gap-4 grid-cols-1 lg:grid-cols-2 xl:grid-cols-3">
          {sorted.map((c) => (
            <ConnectionCard
              key={c.id}
@@ -223,7 +225,10 @@ export const Connections = () => {
                interpolation: { escapeValue: false },
              }),
            });
-            if (created.status === "connected") {
+            if (
+              created.status === "connected" ||
+              created.status === "configured"
+            ) {
              navigate({ to: "/" });
            }
          } else {
@@ -268,8 +273,10 @@ function ConnectionCard({
  const { t } = useTranslation("connections");

  const Icon = connectionTypeIcon(connection.type);
-  const isBusy = connection.status === "connecting";
-  const isConnected = connection.status === "connected";
+  const isBusy =
+    connection.status === "connecting" || connection.status === "configuring";
+  const isConnected =
+    connection.status === "connected" || connection.status === "configured";
  const isError = connection.status === "error";

  return (
@@ -367,10 +374,11 @@ function ConnectionCard({
          </p>
        ) : connection.lastConnectedAt ? (
          <p className="text-sm text-slate-500 dark:text-slate-400">
-            {t("lastConnectedAt", {
-              date: new Date(connection.lastConnectedAt),
-            })}
-            :{new Date(connection.lastConnectedAt).toLocaleString()}
+            {t("lastConnectedAt", { date: "" })}{" "}
+            <TimeAgo
+              timestamp={connection.lastConnectedAt}
+              className="text-sm text-slate-500 dark:text-slate-400"
+            />
          </p>
        ) : (
          <p className="text-sm text-slate-500 dark:text-slate-400">
@@ -20,13 +20,15 @@ import { MeshDevice } from "@meshtastic/core";
 import { TransportHTTP } from "@meshtastic/transport-http";
 import { TransportWebBluetooth } from "@meshtastic/transport-web-bluetooth";
 import { TransportWebSerial } from "@meshtastic/transport-web-serial";
-import { useCallback, useRef } from "react";
+import { useCallback } from "react";

-type LiveRefs = {
-  bt: Map<ConnectionId, BluetoothDevice>;
-  serial: Map<ConnectionId, SerialPort>;
-  meshDevices: Map<ConnectionId, MeshDevice>;
-};
+// Local storage for cleanup only (not in Zustand)
+const transports = new Map<ConnectionId, BluetoothDevice | SerialPort>();
+const heartbeats = new Map<ConnectionId, ReturnType<typeof setInterval>>();
+const configSubscriptions = new Map<ConnectionId, () => void>();
+
+const HEARTBEAT_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes
+const CONFIG_HEARTBEAT_INTERVAL_MS = 5000; // 5s during configuration

 export function useConnections() {
  const connections = useDeviceStore((s) => s.savedConnections);
@@ -37,11 +39,9 @@ export function useConnections() {
    (s) => s.removeSavedConnection,
  );

-  const live = useRef<LiveRefs>({
-    bt: new Map(),
-    serial: new Map(),
-    meshDevices: new Map(),
-  });
+  // DeviceStore methods
+  const setActiveConnectionId = useDeviceStore((s) => s.setActiveConnectionId);
+
  const { addDevice } = useDeviceStore();
  const { addNodeDB } = useNodeDBStore();
  const { addMessageStore } = useMessageStore();
@@ -62,37 +62,67 @@ export function useConnections() {

  const removeConnection = useCallback(
    (id: ConnectionId) => {
-      // Disconnect MeshDevice first
-      const meshDevice = live.current.meshDevices.get(id);
-      if (meshDevice) {
-        try {
-          meshDevice.disconnect();
-        } catch {}
-        live.current.meshDevices.delete(id);
+      const conn = connections.find((c) => c.id === id);
+
+      // Stop heartbeat
+      const heartbeatId = heartbeats.get(id);
+      if (heartbeatId) {
+        clearInterval(heartbeatId);
+        heartbeats.delete(id);
+        console.log(`[useConnections] Heartbeat stopped for connection ${id}`);
      }

-      // Close live refs if open
-      const bt = live.current.bt.get(id);
-      if (bt?.gatt?.connected) {
-        try {
-          bt.gatt.disconnect();
-        } catch {
-          // Ignore errors
-        }
+      // Unsubscribe from config complete event
+      const unsubConfigComplete = configSubscriptions.get(id);
+      if (unsubConfigComplete) {
+        unsubConfigComplete();
+        configSubscriptions.delete(id);
+        console.log(
+          `[useConnections] Config subscription cleaned up for connection ${id}`,
+        );
      }
-      const sp = live.current.serial.get(id);
-      if (sp && "close" in sp) {
-        try {
-          (sp as SerialPort & { close: () => Promise<void> }).close();
-        } catch {
-          // Ignore errors
+
+      // Get device and MeshDevice from Device.connection
+      if (conn?.meshDeviceId) {
+        const { getDevice, removeDevice } = useDeviceStore.getState();
+        const device = getDevice(conn.meshDeviceId);
+
+        if (device?.connection) {
+          // Disconnect MeshDevice
+          try {
+            device.connection.disconnect();
+          } catch {}
        }
+
+        // Close transport if it's BT or Serial
+        const transport = transports.get(id);
+        if (transport) {
+          const bt = transport as BluetoothDevice;
+          if (bt.gatt?.connected) {
+            try {
+              bt.gatt.disconnect();
+            } catch {}
+          }
+
+          const sp = transport as SerialPort & { close?: () => Promise<void> };
+          if (sp.close) {
+            try {
+              sp.close();
+            } catch {}
+          }
+
+          transports.delete(id);
+        }
+
+        // Clean up orphaned Device
+        try {
+          removeDevice(conn.meshDeviceId);
+        } catch {}
      }
-      live.current.bt.delete(id);
-      live.current.serial.delete(id);
+
      removeSavedConnectionFromStore(id);
    },
-    [removeSavedConnectionFromStore],
+    [connections, removeSavedConnectionFromStore],
  );

  const setDefaultConnection = useCallback(
@@ -115,36 +145,114 @@ export function useConnections() {
        | Awaited<ReturnType<typeof TransportHTTP.create>>
        | Awaited<ReturnType<typeof TransportWebBluetooth.createFromDevice>>
        | Awaited<ReturnType<typeof TransportWebSerial.createFromPort>>,
-      options?: {
-        setHeartbeat?: boolean;
-        onDisconnect?: () => void;
-      },
+      btDevice?: BluetoothDevice,
+      serialPort?: SerialPort,
    ): number => {
-      const deviceId = randId();
+      // Reuse existing meshDeviceId if available to prevent duplicate nodeDBs,
+      // but only if the corresponding nodeDB still exists. Otherwise, generate a new ID.
+      const conn = connections.find((c) => c.id === id);
+      let deviceId = conn?.meshDeviceId;
+      if (deviceId && !useNodeDBStore.getState().getNodeDB(deviceId)) {
+        deviceId = undefined;
+      }
+      deviceId = deviceId ?? randId();
+
      const device = addDevice(deviceId);
      const nodeDB = addNodeDB(deviceId);
      const messageStore = addMessageStore(deviceId);
      const meshDevice = new MeshDevice(transport, deviceId);
-      meshDevice.configure();
-      setSelectedDevice(deviceId);
-      device.addConnection(meshDevice);
-      subscribeAll(device, meshDevice, messageStore, nodeDB);
-      live.current.meshDevices.set(id, meshDevice);

-      if (options?.setHeartbeat) {
-        const HEARTBEAT_INTERVAL = 5 * 60 * 1000;
-        meshDevice.setHeartbeatInterval(HEARTBEAT_INTERVAL);
+      setSelectedDevice(deviceId);
+      device.addConnection(meshDevice); // This stores meshDevice in Device.connection
+      subscribeAll(device, meshDevice, messageStore, nodeDB);
+
+      // Store transport locally for cleanup (BT/Serial only)
+      if (btDevice || serialPort) {
+        transports.set(id, btDevice || serialPort);
      }

+      // Set active connection and link device bidirectionally
+      setActiveConnectionId(id);
+      device.setConnectionId(id);
+
+      // Listen for config complete event (with nonce/ID)
+      const unsubConfigComplete = meshDevice.events.onConfigComplete.subscribe(
+        (configCompleteId) => {
+          console.log(
+            `[useConnections] Configuration complete with ID: ${configCompleteId}`,
+          );
+          device.setConnectionPhase("configured");
+          updateStatus(id, "configured");
+
+          // Switch from fast config heartbeat to slow maintenance heartbeat
+          const oldHeartbeat = heartbeats.get(id);
+          if (oldHeartbeat) {
+            clearInterval(oldHeartbeat);
+            console.log(
+              `[useConnections] Switching to maintenance heartbeat (5 min interval)`,
+            );
+          }
+
+          const maintenanceHeartbeat = setInterval(() => {
+            meshDevice.heartbeat().catch((error) => {
+              console.warn("[useConnections] Heartbeat failed:", error);
+            });
+          }, HEARTBEAT_INTERVAL_MS);
+          heartbeats.set(id, maintenanceHeartbeat);
+        },
+      );
+      configSubscriptions.set(id, unsubConfigComplete);
+
+      // Start configuration
+      device.setConnectionPhase("configuring");
+      updateStatus(id, "configuring");
+      console.log("[useConnections] Starting configuration");
+
+      meshDevice
+        .configure()
+        .then(() => {
+          console.log(
+            "[useConnections] Configuration complete, starting heartbeat",
+          );
+          // Send initial heartbeat after configure completes
+          meshDevice
+            .heartbeat()
+            .then(() => {
+              // Start fast heartbeat after first successful heartbeat
+              const configHeartbeatId = setInterval(() => {
+                meshDevice.heartbeat().catch((error) => {
+                  console.warn(
+                    "[useConnections] Config heartbeat failed:",
+                    error,
+                  );
+                });
+              }, CONFIG_HEARTBEAT_INTERVAL_MS);
+              heartbeats.set(id, configHeartbeatId);
+              console.log(
+                `[useConnections] Heartbeat started for connection ${id} (5s interval during config)`,
+              );
+            })
+            .catch((error) => {
+              console.warn("[useConnections] Initial heartbeat failed:", error);
+            });
+        })
+        .catch((error) => {
+          console.error(`[useConnections] Failed to configure:`, error);
+          updateStatus(id, "error", error.message);
+        });
+
      updateSavedConnection(id, { meshDeviceId: deviceId });
      return deviceId;
    },
    [
+      connections,
      addDevice,
      addNodeDB,
      addMessageStore,
      setSelectedDevice,
+      setActiveConnectionId,
      updateSavedConnection,
+      updateStatus,
    ],
  );

@@ -154,7 +262,7 @@ export function useConnections() {
      if (!conn) {
        return false;
      }
-      if (conn.status === "connected") {
+      if (conn.status === "configured" || conn.status === "connected") {
        return true;
      }

@@ -175,7 +283,7 @@ export function useConnections() {
          const isTLS = url.protocol === "https:";
          const transport = await TransportHTTP.create(url.host, isTLS);
          setupMeshDevice(id, transport);
-          updateStatus(id, "connected");
+          // Status will be set to "configured" by onConfigComplete event
          return true;
        }

@@ -183,7 +291,7 @@ export function useConnections() {
          if (!("bluetooth" in navigator)) {
            throw new Error("Web Bluetooth not supported");
          }
-          let bleDevice = live.current.bt.get(id);
+          let bleDevice = transports.get(id) as BluetoothDevice | undefined;
          if (!bleDevice) {
            // Try to recover permitted devices
            const getDevices = (
@@ -198,10 +306,6 @@ export function useConnections() {
                bleDevice = known.find(
                  (d: BluetoothDevice) => d.id === conn.deviceId,
                );
-                // If found, store it for future use
-                if (bleDevice) {
-                  live.current.bt.set(id, bleDevice);
-                }
              }
            }
          }
@@ -222,17 +326,16 @@ export function useConnections() {
              "Bluetooth device not available. Re-select the device.",
            );
          }
-          live.current.bt.set(id, bleDevice);

          const transport =
            await TransportWebBluetooth.createFromDevice(bleDevice);
-          setupMeshDevice(id, transport, { setHeartbeat: true });
+          setupMeshDevice(id, transport, bleDevice);

          bleDevice.addEventListener("gattserverdisconnected", () => {
            updateStatus(id, "disconnected");
          });

-          updateStatus(id, "connected");
+          // Status will be set to "configured" by onConfigComplete event
          return true;
        }

@@ -240,7 +343,7 @@ export function useConnections() {
          if (!("serial" in navigator)) {
            throw new Error("Web Serial not supported");
          }
-          let port = live.current.serial.get(id);
+          let port = transports.get(id) as SerialPort | undefined;
          if (!port) {
            // Find a previously granted port by vendor/product
            const ports: SerialPort[] = await (
@@ -296,11 +399,9 @@ export function useConnections() {
            }
          }

-          live.current.serial.set(id, port);
-
          const transport = await TransportWebSerial.createFromPort(port);
-          setupMeshDevice(id, transport, { setHeartbeat: true });
-          updateStatus(id, "connected");
+          setupMeshDevice(id, transport, undefined, port);
+          // Status will be set to "configured" by onConfigComplete event
          return true;
        }
      } catch (err: unknown) {
@@ -320,39 +421,68 @@ export function useConnections() {
        return;
      }
      try {
-        // Disconnect MeshDevice first
-        const meshDevice = live.current.meshDevices.get(id);
-        if (meshDevice) {
-          try {
-            meshDevice.disconnect();
-          } catch {
-            // Ignore errors
-          }
-          live.current.meshDevices.delete(id);
+        // Stop heartbeat
+        const heartbeatId = heartbeats.get(id);
+        if (heartbeatId) {
+          clearInterval(heartbeatId);
+          heartbeats.delete(id);
+          console.log(
+            `[useConnections] Heartbeat stopped for connection ${id}`,
+          );
        }

-        if (conn.type === "bluetooth") {
-          const dev = live.current.bt.get(id);
-          if (dev?.gatt?.connected) {
-            dev.gatt.disconnect();
-          }
+        // Unsubscribe from config complete event
+        const unsubConfigComplete = configSubscriptions.get(id);
+        if (unsubConfigComplete) {
+          unsubConfigComplete();
+          configSubscriptions.delete(id);
+          console.log(
+            `[useConnections] Config subscription cleaned up for connection ${id}`,
+          );
        }
-        if (conn.type === "serial") {
-          const port = live.current.serial.get(id);
-          if (port) {
+
+        // Get device and meshDevice from Device.connection
+        if (conn.meshDeviceId) {
+          const { getDevice } = useDeviceStore.getState();
+          const device = getDevice(conn.meshDeviceId);
+
+          if (device?.connection) {
+            // Disconnect MeshDevice
            try {
-              const portWithClose = port as SerialPort & {
-                close: () => Promise<void>;
-                readable: ReadableStream | null;
-              };
-              // Only close if the port is open (has readable stream)
-              if (portWithClose.readable) {
-                await portWithClose.close();
-              }
-            } catch (err) {
-              console.warn("Error closing serial port:", err);
+              device.connection.disconnect();
+            } catch {
+              // Ignore errors
            }
-            live.current.serial.delete(id);
+          }
+
+          // Close transport connections
+          const transport = transports.get(id);
+          if (transport) {
+            if (conn.type === "bluetooth") {
+              const dev = transport as BluetoothDevice;
+              if (dev.gatt?.connected) {
+                dev.gatt.disconnect();
+              }
+            }
+            if (conn.type === "serial") {
+              const port = transport as SerialPort & {
+                close?: () => Promise<void>;
+                readable?: ReadableStream | null;
+              };
+              if (port.close && port.readable) {
+                try {
+                  await port.close();
+                } catch (err) {
+                  console.warn("Error closing serial port:", err);
+                }
+              }
+            }
+          }
+
+          // Clear the device's connectionId link
+          if (device) {
+            device.setConnectionId(null);
+            device.setConnectionPhase("disconnected");
          }
        }
      } finally {
@@ -379,7 +509,7 @@ export function useConnections() {
      const conn = addConnection(input);
      // If a Bluetooth device was provided, store it to avoid re-prompting
      if (btDevice && conn.type === "bluetooth") {
-        live.current.bt.set(conn.id, btDevice);
+        transports.set(conn.id, btDevice);
      }
      await connect(conn.id, { allowPrompt: true });
      // Get updated connection from store after connect
@@ -395,11 +525,14 @@ export function useConnections() {
    // HTTP: test endpoint reachability
    // Bluetooth/Serial: check permission grants

-    // HTTP connections: test reachability if not already connected
+    // HTTP connections: test reachability if not already connected/configured
    const httpChecks = connections
      .filter(
        (c): c is Connection & { type: "http"; url: string } =>
-          c.type === "http" && c.status !== "connected",
+          c.type === "http" &&
+          c.status !== "connected" &&
+          c.status !== "configured" &&
+          c.status !== "configuring",
      )
      .map(async (c) => {
        const ok = await testHttpReachable(c.url);
@@ -412,7 +545,10 @@ export function useConnections() {
    const btChecks = connections
      .filter(
        (c): c is Connection & { type: "bluetooth"; deviceId?: string } =>
-          c.type === "bluetooth" && c.status !== "connected",
+          c.type === "bluetooth" &&
+          c.status !== "connected" &&
+          c.status !== "configured" &&
+          c.status !== "configuring",
      )
      .map(async (c) => {
        if (!("bluetooth" in navigator)) {
@@ -445,7 +581,11 @@ export function useConnections() {
          type: "serial";
          usbVendorId?: number;
          usbProductId?: number;
-        } => c.type === "serial" && c.status !== "connected",
+        } =>
+          c.type === "serial" &&
+          c.status !== "connected" &&
+          c.status !== "configured" &&
+          c.status !== "configuring",
      )
      .map(async (c) => {
        if (!("serial" in navigator)) {
@@ -493,13 +633,16 @@ export function useConnections() {
    // Update all connection statuses
    connections.forEach((conn) => {
      const shouldBeConnected = activeConnection?.id === conn.id;
+      const isConnectedState =
+        conn.status === "connected" ||
+        conn.status === "configured" ||
+        conn.status === "configuring";

      // Update status if it doesn't match reality
-      if (shouldBeConnected && conn.status !== "connected") {
-        updateSavedConnection(conn.id, { status: "connected" });
-      } else if (!shouldBeConnected && conn.status === "connected") {
+      if (!shouldBeConnected && isConnectedState) {
        updateSavedConnection(conn.id, { status: "disconnected" });
      }
+      // Don't force status to "connected" if shouldBeConnected - let the connection flow set the proper status
    });
  }, [connections, selectedDeviceId, updateSavedConnection]);