Clear a serial adapter's persisted line settings on unplug

lib/universal_proxy/hardware.ex

@@ -269,6 +269,32 @@ defmodule UniversalProxy.Hardware do
     |> Map.new()
   end
 
+  @doc """
+  Map of USB `serial_number => port_id` for every currently-connected USB
+  serial adapter (adapters without a serial number are omitted — the UART
+  server's hotplug tracking skips those too). `UniversalProxy.UART.Server`
+  snapshots this while devices are present so it can clear each device's
+  persisted line settings at unplug time, when the sysfs path needed to
+  derive the port id is already gone.
+  """
+  @spec port_ids_by_serial(keyword()) :: %{String.t() => String.t()}
+  def port_ids_by_serial(opts \\ []) do
+    enumerated = Keyword.get_lazy(opts, :enumerated, &Enumerate.safe/0)
+    bus_paths = Keyword.get_lazy(opts, :bus_paths, fn -> bus_paths_from_sysfs(opts) end)
+
+    enumerated
+    |> Enum.filter(fn {name, info} ->
+      usb_serial?(name) and Enumerate.present?(info[:serial_number])
+    end)
+    |> Enum.flat_map(fn {tty_name, info} ->
+      case Map.get(bus_paths, tty_name) do
+        nil -> []
+        slot_sub -> [{info[:serial_number], port_id(slot_sub)}]
+      end
+    end)
+    |> Map.new()
+  end
+
   @usb_devices_dir "/sys/bus/usb/devices"
   # A device-level USB path under /sys/bus/usb/devices (e.g. "1-1.1.3"),
   # as opposed to a root hub ("usb1") or an interface ("1-1.3:1.0").

lib/universal_proxy/uart/server.ex

@@ -13,6 +13,9 @@ defmodule UniversalProxy.UART.Server do
   Periodically polls `Circuits.UART.enumerate()` to detect USB hotplug
   events. When the set of connected serial numbers changes, the ESPHome
   supervisor is restarted so clients reconnect with the updated device list.
+  A removed device's persisted line settings are also cleared from
+  `UniversalProxy.UART.SettingsStore`, so a different adapter later
+  plugged into the same physical port never inherits a stale baud rate.
 
   Incoming UART data is broadcast to PubSub topic `"uart:<friendly_name>"`.
   """
@@ -21,7 +24,9 @@ defmodule UniversalProxy.UART.Server do
 
   require Logger
 
+  alias UniversalProxy.Hardware
   alias UniversalProxy.UART.PortConfig
+  alias UniversalProxy.UART.SettingsStore
 
   @pubsub UniversalProxy.PubSub
   @hotplug_interval 5_000
@@ -112,7 +117,7 @@ defmodule UniversalProxy.UART.Server do
   # -- Server Callbacks --
 
   @impl true
-  def init(_opts) do
+  def init(opts) do
     # Restart hygiene: a Server-only crash leaves PortSupervisor children
     # running (`:rest_for_one` restarts the Server but never touches the
     # PortSupervisor started before it), and the fresh Server has no
@@ -125,7 +130,14 @@ defmodule UniversalProxy.UART.Server do
     :timer.send_interval(@hotplug_interval, self(), :check_hotplug)
     known = current_serial_set()
     Logger.info("UART server started, #{MapSet.size(known)} serial devices detected")
-    {:ok, %{ports: %{}, known_serials: known}}
+
+    {:ok,
+     %{
+       ports: %{},
+       known_serials: known,
+       port_ids_by_serial: safe_port_ids_by_serial(),
+       settings_store: Keyword.get(opts, :settings_store, SettingsStore)
+     }}
   end
 
   @impl true
@@ -249,13 +261,21 @@ defmodule UniversalProxy.UART.Server do
 
       if MapSet.size(removed) > 0 do
         Logger.info("UART hotplug: devices removed: #{Enum.join(removed, ", ")}")
+        clear_removed_settings(removed, state.port_ids_by_serial, state.settings_store)
       end
 
       Task.Supervisor.start_child(UniversalProxy.TaskSupervisor, fn ->
         UniversalProxy.ESPHome.Supervisor.restart()
       end)
 
-      {:noreply, %{state | known_serials: current}}
+      # Refreshed only here, in the changed branch — a same-set poll
+      # can't move a device between slots without its serial
+      # appearing/disappearing in some tick, EXCEPT an unplug+replug of
+      # the same device into a DIFFERENT slot within one
+      # @hotplug_interval window. That corner keeps the old entry keyed
+      # to the old slot, which the next open in the new slot re-learns
+      # anyway, so it's left unhandled.
+      {:noreply, %{state | known_serials: current, port_ids_by_serial: safe_port_ids_by_serial()}}
     else
       {:noreply, state}
     end
@@ -382,6 +402,62 @@ defmodule UniversalProxy.UART.Server do
     MapSet.new(serials)
   end
 
+  # `Hardware.port_ids_by_serial/1` only walks sysfs and reads
+  # `Circuits.UART.enumerate/0` today — no GenServer call involved — but
+  # this runs on every hotplug poll, so keep it defensive and cheap
+  # rather than assume that stays true forever.
+  @spec safe_port_ids_by_serial() :: %{String.t() => String.t()}
+  defp safe_port_ids_by_serial do
+    Hardware.port_ids_by_serial()
+  rescue
+    e ->
+      Logger.warning(
+        "UART port_ids_by_serial failed: #{Exception.format(:error, e, __STACKTRACE__)}"
+      )
+
+      %{}
+  catch
+    :exit, _ -> %{}
+  end
+
+  # Public (`@doc false`) so tests can exercise the settings-clearing
+  # logic directly against a test-local SettingsStore, without going
+  # through `handle_info(:check_hotplug, _)` — that handler always fires
+  # a real `ESPHome.Supervisor.restart/0` on a serial-set change, which
+  # must not run against the app-global supervision tree from a test.
+  @doc false
+  def clear_removed_settings(removed, port_ids_by_serial, settings_store) do
+    Enum.each(removed, fn serial ->
+      case Map.fetch(port_ids_by_serial, serial) do
+        {:ok, port_id} -> clear_settings(port_id, serial, settings_store)
+        :error -> :ok
+      end
+    end)
+  end
+
+  # Best-effort: a wedged or absent SettingsStore must not crash the
+  # UART server over a hotplug event it can't fully act on (public-API
+  # `catch :exit` idiom, CLAUDE.md) — worst case a stale baud lingers
+  # until a later unplug succeeds in clearing it.
+  defp clear_settings(port_id, serial, settings_store) do
+    case SettingsStore.delete_opts(settings_store, port_id) do
+      :ok ->
+        Logger.info(
+          "UART hotplug: cleared persisted line settings for #{port_id} (#{serial} unplugged)"
+        )
+
+      {:error, reason} ->
+        Logger.warning(
+          "UART hotplug: failed to clear persisted line settings for #{port_id}: #{inspect(reason)}"
+        )
+    end
+  catch
+    :exit, reason ->
+      Logger.warning(
+        "UART hotplug: settings store unavailable while clearing #{port_id}: #{inspect(reason)}"
+      )
+  end
+
   defp present?(nil), do: false
   defp present?(""), do: false
   defp present?(s) when is_binary(s), do: String.trim(s) != ""

lib/universal_proxy/uart/settings_store.ex

@@ -17,6 +17,15 @@ defmodule UniversalProxy.UART.SettingsStore do
   Keyed by `UniversalProxy.Hardware`'s stable port id (`"p_" <> slot`),
   which survives replug on the same physical port.
 
+  A port id changes hands when `UniversalProxy.UART.Server` sees the
+  slot's USB serial number disappear from a hotplug poll and calls
+  `delete_opts/2` to clear it, so a different adapter plugged into the
+  same physical port won't normally inherit the previous device's baud.
+  One accepted corner (documented at the call site in `UART.Server`): a
+  swap that lands entirely between two hotplug polls can leave the stale
+  entry in place briefly, until the next set-change poll or the new
+  adapter's first explicit open re-learns the settings.
+
   The DETS file lives on the writable data partition on Nerves
   (`/data/uart_settings.dets`) and in `_build/` on the host for
   development. It is owned at the top-level application supervisor (a peer
@@ -61,6 +70,17 @@ defmodule UniversalProxy.UART.SettingsStore do
     GenServer.call(server, {:get, port_id})
   end
 
+  @doc """
+  Forget the persisted line settings for `port_id`. Called by
+  `UniversalProxy.UART.Server` when the adapter in that slot is unplugged,
+  so a different device plugged into the same port can never inherit a
+  stale baud rate. Deleting an absent id is a no-op `:ok`.
+  """
+  @spec delete_opts(GenServer.server(), String.t()) :: :ok | {:error, term()}
+  def delete_opts(server \\ __MODULE__, port_id) when is_binary(port_id) do
+    GenServer.call(server, {:delete, port_id})
+  end
+
   @doc """
   Snapshot of every persisted port's line settings, keyed by port id.
   Used by `Hardware.list_ports/0` to decorate port maps in one call
@@ -110,6 +130,17 @@ defmodule UniversalProxy.UART.SettingsStore do
     {:reply, read_opts(state.table, port_id), state}
   end
 
+  def handle_call({:delete, port_id}, _from, state) do
+    with :ok <- :dets.delete(state.table, port_id),
+         :ok <- :dets.sync(state.table) do
+      {:reply, :ok, state}
+    else
+      {:error, reason} ->
+        Logger.error("UART settings store delete failed: #{inspect(reason)}")
+        {:reply, {:error, reason}, state}
+    end
+  end
+
   def handle_call(:all, _from, state) do
     all = :dets.foldl(fn {id, opts}, acc -> Map.put(acc, id, opts) end, %{}, state.table)
     {:reply, all, state}

test/universal_proxy/hardware_test.exs

@@ -693,6 +693,58 @@ defmodule UniversalProxy.HardwareTest do
     end
   end
 
+  describe "port_ids_by_serial/1" do
+    test "maps serial_number to port_id for every connected adapter with both" do
+      result =
+        Hardware.port_ids_by_serial(
+          enumerated: %{
+            "ttyUSB0" => %{vendor_id: 0x0403, product_id: 0x6001, serial_number: "BG018NOP"},
+            "ttyACM0" => %{vendor_id: 0x303A, product_id: 0x4001, serial_number: "DH001K8R"}
+          },
+          bus_paths: %{"ttyUSB0" => "1-1.3", "ttyACM0" => "1-1.1"}
+        )
+
+      assert result == %{"BG018NOP" => "p_1_1_3", "DH001K8R" => "p_1_1_1"}
+    end
+
+    test "omits adapters without a serial number" do
+      result =
+        Hardware.port_ids_by_serial(
+          enumerated: %{
+            "ttyUSB0" => %{vendor_id: 0x1A86, product_id: 0x7523, serial_number: nil}
+          },
+          bus_paths: %{"ttyUSB0" => "1-1.4"}
+        )
+
+      assert result == %{}
+    end
+
+    test "omits a tty with no matching bus path" do
+      result =
+        Hardware.port_ids_by_serial(
+          enumerated: %{
+            "ttyUSB0" => %{vendor_id: 0x0403, product_id: 0x6001, serial_number: "BG018NOP"}
+          },
+          bus_paths: %{}
+        )
+
+      assert result == %{}
+    end
+
+    test "skips built-in SoC UARTs (ttyAMA, ttyS)" do
+      result =
+        Hardware.port_ids_by_serial(
+          enumerated: %{
+            "ttyAMA0" => %{serial_number: "should-not-appear"},
+            "ttyUSB0" => %{vendor_id: 0x0403, product_id: 0x6001, serial_number: "BG018NOP"}
+          },
+          bus_paths: %{"ttyUSB0" => "1-1.3"}
+        )
+
+      assert result == %{"BG018NOP" => "p_1_1_3"}
+    end
+  end
+
   describe "usb_hubs/1" do
     # Build a fake /sys/bus/usb/devices tree with a hub, a non-hub device,
     # and a root hub, then assert only the real hub is detected.

test/universal_proxy/uart/server_test.exs

@@ -1,7 +1,12 @@
 defmodule UniversalProxy.UART.ServerTest do
-  use ExUnit.Case, async: true
+  # async: false because the settings-clearing tests open a DETS-backed
+  # SettingsStore on a fixed table atom — DETS table names must be atoms,
+  # so serialization keeps the fixed atom safe to reuse (same rationale as
+  # SettingsStoreTest/PskStoreTest/ConfigStoreTest).
+  use ExUnit.Case, async: false
 
   alias UniversalProxy.UART.Server
+  alias UniversalProxy.UART.SettingsStore
 
   describe "zwa2_device?/1" do
     test "matches ZWA-2 by VID/PID" do
@@ -61,4 +66,74 @@ defmodule UniversalProxy.UART.ServerTest do
       refute Server.irdroid_device?(%{serial_number: "abc123"})
     end
   end
+
+  # Exercises the unplug-clear logic directly against a test-local
+  # SettingsStore instead of via `handle_info(:check_hotplug, _)` — that
+  # handler always fires a real `ESPHome.Supervisor.restart/0` on a
+  # serial-set change, which must not run against the app-global
+  # supervision tree from a test.
+  describe "clear_removed_settings/3" do
+    setup do
+      path =
+        Path.join(
+          System.tmp_dir!(),
+          "uart_server_settings_test_#{System.unique_integer([:positive])}.dets"
+        )
+
+      File.rm(path)
+
+      store =
+        start_supervised!(
+          {SettingsStore, name: nil, table: :uart_server_test_settings, dets_path: path}
+        )
+
+      on_exit(fn -> File.rm(path) end)
+
+      %{store: store}
+    end
+
+    @opts [speed: 9600, data_bits: 8, stop_bits: 1, parity: :none, flow_control: :none]
+
+    test "deletes the persisted entry for a removed device's port id", %{store: store} do
+      :ok = SettingsStore.put_opts(store, "p_1_1", @opts)
+
+      assert Server.clear_removed_settings(
+               MapSet.new(["SERIAL1"]),
+               %{"SERIAL1" => "p_1_1"},
+               store
+             ) == :ok
+
+      assert SettingsStore.get_opts(store, "p_1_1") == nil
+    end
+
+    test "a removed serial with no known port id is a no-op", %{store: store} do
+      :ok = SettingsStore.put_opts(store, "p_1_1", @opts)
+
+      Server.clear_removed_settings(MapSet.new(["UNKNOWN"]), %{}, store)
+
+      assert SettingsStore.get_opts(store, "p_1_1") == @opts
+    end
+
+    test "leaves other ports' settings untouched", %{store: store} do
+      :ok = SettingsStore.put_opts(store, "p_1_1", @opts)
+      :ok = SettingsStore.put_opts(store, "p_1_2", @opts)
+
+      Server.clear_removed_settings(MapSet.new(["SERIAL1"]), %{"SERIAL1" => "p_1_1"}, store)
+
+      assert SettingsStore.get_opts(store, "p_1_1") == nil
+      assert SettingsStore.get_opts(store, "p_1_2") == @opts
+    end
+
+    test "a dead/unavailable settings store does not crash the caller" do
+      dead = spawn(fn -> :ok end)
+      ref = Process.monitor(dead)
+      assert_receive {:DOWN, ^ref, :process, ^dead, _reason}
+
+      assert Server.clear_removed_settings(
+               MapSet.new(["SERIAL1"]),
+               %{"SERIAL1" => "p_1_1"},
+               dead
+             ) == :ok
+    end
+  end
 end

test/universal_proxy/uart/settings_store_test.exs

@@ -75,6 +75,21 @@ defmodule UniversalProxy.UART.SettingsStoreTest do
     File.rm(path)
   end
 
+  describe "delete_opts/2" do
+    test "removes a persisted entry", %{store: store} do
+      opts = [speed: 115_200, data_bits: 8, stop_bits: 1, parity: :none, flow_control: :none]
+      :ok = SettingsStore.put_opts(store, "p_1_1", opts)
+
+      assert SettingsStore.delete_opts(store, "p_1_1") == :ok
+      assert SettingsStore.get_opts(store, "p_1_1") == nil
+      assert SettingsStore.all_opts(store) == %{}
+    end
+
+    test "deleting an unknown port id is a no-op :ok", %{store: store} do
+      assert SettingsStore.delete_opts(store, "p_unknown") == :ok
+    end
+  end
+
   describe "all_opts/1" do
     test "returns an empty map when the store has no records", %{store: store} do
       assert SettingsStore.all_opts(store) == %{}