refactor(packet_processor): split cond/if into multi-clause helpers

- render_for_zoom/3: zoom ≤ 8 uses heatmap; high zoom with marker_data
  pushes to the client; high zoom + nil marker_data is a no-op. Each
  branch is now a separate function head.
- push_new_packet/3: a boolean-dispatched pair replaces the popup-open
  conditional on the push_event payload.
- prepare_packet_state: the two rebinding `if`s turn into a pipeline
  through prune_if_over_cap/1 (pattern-matched on map_size/1) and
  build_marker_data/5 with a zoom ≤ 8 short-circuit clause, plus
  maybe_tag_convert_from/3 for the replacement-marker tag.
- moved_significantly?/3: the duplicated guard check between
  dispatch_visibility and batch_dispatch collapses into one helper that
  pattern-matches on the coordinate tuple returned by CoordinateUtils.

Adds extra tests covering the new significance / stale-coords branches
and the connection card LiveComponent render paths. Coverage 65.89 → 66.41%.

lib/aprsme_web/live/map_live/packet_processor.ex

@@ -74,11 +74,9 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
   end
 
   defp dispatch_visibility({true, true}, packet, lat, lon, callsign_key, socket) do
-    existing_packet = socket.assigns.visible_packets[callsign_key]
-    {existing_lat, existing_lon, _} = CoordinateUtils.get_coordinates(existing_packet)
+    existing = socket.assigns.visible_packets[callsign_key]
 
-    if is_number(existing_lat) and is_number(existing_lon) and
-         GeoUtils.significant_movement?(existing_lat, existing_lon, lat, lon, 50) do
+    if moved_significantly?(existing, lat, lon) do
       handle_valid_postgres_packet(packet, lat, lon, socket)
     else
       new_visible_packets = Map.put(socket.assigns.visible_packets, callsign_key, packet)
@@ -88,23 +86,37 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
 
   defp dispatch_visibility(_, _packet, _lat, _lon, _callsign_key, socket), do: socket
 
+  # A station's marker is moved only if we can compare the two coordinates AND
+  # the delta exceeds the 50-meter significance threshold. Any missing/invalid
+  # existing coordinate means "not a significant move" (keep existing marker).
+  defp moved_significantly?(existing_packet, new_lat, new_lon) do
+    case CoordinateUtils.get_coordinates(existing_packet) do
+      {elat, elon, _} when is_number(elat) and is_number(elon) ->
+        GeoUtils.significant_movement?(elat, elon, new_lat, new_lon, 50)
+
+      _ ->
+        false
+    end
+  end
+
   @doc """
   Handle valid postgres packet by adding it to visible packets and displaying it.
   """
   @spec handle_valid_postgres_packet(map(), number() | nil, number() | nil, Socket.t()) :: Socket.t()
   def handle_valid_postgres_packet(packet, _lat, _lon, socket) do
     {socket, marker_data} = prepare_packet_state(packet, socket)
+    render_for_zoom(socket.assigns.map_zoom, marker_data, socket)
+  end
 
-    cond do
-      socket.assigns.map_zoom <= 8 ->
-        send_heat_map_for_current_bounds(socket)
+  # Zoom 8 and below use the heatmap instead of individual markers.
+  defp render_for_zoom(zoom, _marker_data, socket) when zoom <= 8 do
+    send_heat_map_for_current_bounds(socket)
+  end
 
-      marker_data ->
-        send_marker_with_popup_check(socket, marker_data)
+  defp render_for_zoom(_zoom, nil, socket), do: socket
 
-      true ->
-        socket
-    end
+  defp render_for_zoom(_zoom, marker_data, socket) do
+    send_marker_with_popup_check(socket, marker_data)
   end
 
   @doc """
@@ -143,13 +155,11 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
     {socket, marker_data, nil}
   end
 
-  # In bounds, existing marker — update if significant movement
+  # In bounds, existing marker — update if significant movement.
   defp batch_dispatch({true, true}, packet, lat, lon, callsign_key, socket) do
-    existing_packet = socket.assigns.visible_packets[callsign_key]
-    {existing_lat, existing_lon, _} = CoordinateUtils.get_coordinates(existing_packet)
+    existing = socket.assigns.visible_packets[callsign_key]
 
-    if is_number(existing_lat) and is_number(existing_lon) and
-         GeoUtils.significant_movement?(existing_lat, existing_lon, lat, lon, 50) do
+    if moved_significantly?(existing, lat, lon) do
       {socket, marker_data} = prepare_packet_state(packet, socket)
       {socket, marker_data, nil}
     else
@@ -169,35 +179,41 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
   # so the JS can convert the old marker to a historical dot via O(1) lookup.
   defp prepare_packet_state(packet, socket) do
     callsign_key = SharedPacketUtils.get_callsign_key(packet)
-
-    # Check if we're replacing an existing visible packet for this callsign
     had_existing = Map.has_key?(socket.assigns.visible_packets, callsign_key)
 
-    new_visible_packets = Map.put(socket.assigns.visible_packets, callsign_key, packet)
-
     new_visible_packets =
-      if map_size(new_visible_packets) > @max_visible_packets do
-        SharedPacketUtils.prune_oldest_packets(new_visible_packets, @max_visible_packets)
-      else
-        new_visible_packets
-      end
+      socket.assigns.visible_packets
+      |> Map.put(callsign_key, packet)
+      |> prune_if_over_cap()
 
     socket = assign(socket, :visible_packets, new_visible_packets)
-
-    marker_data =
-      if socket.assigns.map_zoom > 8 do
-        data = DataBuilder.build_packet_data(packet, true, get_locale(socket))
-
-        if data && had_existing do
-          Map.put(data, "convert_from", callsign_key)
-        else
-          data
-        end
-      end
+    marker_data = build_marker_data(socket.assigns.map_zoom, packet, callsign_key, had_existing, socket)
 
     {socket, marker_data}
   end
 
+  # Cap the visible-packets map at @max_visible_packets; pattern-match on
+  # map_size so we only call the pruner when we're actually over the cap.
+  defp prune_if_over_cap(packets) when map_size(packets) > @max_visible_packets do
+    SharedPacketUtils.prune_oldest_packets(packets, @max_visible_packets)
+  end
+
+  defp prune_if_over_cap(packets), do: packets
+
+  # At low zoom, we use the heatmap instead of individual markers, so no
+  # marker_data is built at all.
+  defp build_marker_data(zoom, _packet, _key, _had_existing, _socket) when zoom <= 8, do: nil
+
+  defp build_marker_data(_zoom, packet, callsign_key, had_existing, socket) do
+    case DataBuilder.build_packet_data(packet, true, get_locale(socket)) do
+      nil -> nil
+      data -> maybe_tag_convert_from(data, had_existing, callsign_key)
+    end
+  end
+
+  defp maybe_tag_convert_from(data, true, callsign_key), do: Map.put(data, "convert_from", callsign_key)
+  defp maybe_tag_convert_from(data, false, _callsign_key), do: data
+
   # Private helper functions
 
   defp within_bounds?(coords, bounds) do
@@ -213,13 +229,18 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
   end
 
   defp send_marker_with_popup_check(socket, marker_data) do
-    # Strip convert_from — the single-packet JS handler uses its own scan logic
+    # Strip convert_from — the single-packet JS handler uses its own scan logic.
     marker_data = Map.delete(marker_data, "convert_from")
+    push_new_packet(socket.assigns.station_popup_open, socket, marker_data)
+  end
 
-    if socket.assigns.station_popup_open do
-      LiveView.push_event(socket, "new_packet", Map.put(marker_data, :openPopup, false))
-    else
-      LiveView.push_event(socket, "new_packet", marker_data)
-    end
+  # When a popup is already open, keep it open by pinning openPopup=false on
+  # the fresh marker so the client doesn't re-open or steal focus.
+  defp push_new_packet(true, socket, marker_data) do
+    LiveView.push_event(socket, "new_packet", Map.put(marker_data, :openPopup, false))
+  end
+
+  defp push_new_packet(false, socket, marker_data) do
+    LiveView.push_event(socket, "new_packet", marker_data)
   end
 end

test/aprsme_web/live/map_live/packet_processor_test.exs

@@ -157,5 +157,73 @@ defmodule AprsmeWeb.MapLive.PacketProcessorTest do
       assert Map.keys(updated_socket.assigns.visible_packets) == ["K5GVL-10"]
       assert updated_socket.assigns.visible_packets["K5GVL-10"].id == "packet-2"
     end
+
+    test "skips marker update when movement is below significance threshold" do
+      # ~1m move — well below the 50m threshold used by GeoUtils.significant_movement?/5.
+      existing = build_packet(%{id: "packet-1", lat: 33.10000, lon: -96.50000})
+      socket = build_socket(%{visible_packets: %{"K5GVL-10" => existing}})
+      tiny_move = build_packet(%{id: "packet-2", lat: 33.10001, lon: -96.50000})
+
+      {updated_socket, marker_data, removed_id} =
+        PacketProcessor.process_packet_for_marker_data(tiny_move, socket)
+
+      assert marker_data == nil
+      assert removed_id == nil
+      # State is still updated silently so the newest packet is stored.
+      assert updated_socket.assigns.visible_packets["K5GVL-10"].id == "packet-2"
+    end
+
+    test "emits a marker when movement exceeds significance threshold" do
+      existing = build_packet(%{id: "packet-1", lat: 33.10, lon: -96.50})
+      socket = build_socket(%{visible_packets: %{"K5GVL-10" => existing}})
+      # ~0.03° ≈ 3 km — far above 50m.
+      big_move = build_packet(%{id: "packet-2", lat: 33.13, lon: -96.53})
+
+      {_, marker_data, _} = PacketProcessor.process_packet_for_marker_data(big_move, socket)
+      assert is_map(marker_data)
+    end
+
+    test "treats invalid existing coordinates as 'not moved' — no marker emitted" do
+      existing_bad = build_packet(%{id: "packet-1", lat: nil, lon: nil})
+      socket = build_socket(%{visible_packets: %{"K5GVL-10" => existing_bad}})
+      incoming = build_packet(%{id: "packet-2", lat: 33.11, lon: -96.51})
+
+      {updated_socket, marker_data, _} =
+        PacketProcessor.process_packet_for_marker_data(incoming, socket)
+
+      # No significant-move decision possible → replace silently (no marker emitted).
+      assert marker_data == nil
+      assert updated_socket.assigns.visible_packets["K5GVL-10"].id == "packet-2"
+    end
+  end
+
+  describe "handle_packet_visibility/5" do
+    test "returns socket unchanged when coordinates are nil" do
+      socket = build_socket()
+      packet = build_packet()
+      result = PacketProcessor.handle_packet_visibility(packet, nil, nil, "K5GVL-10", socket)
+      assert result == socket
+    end
+
+    test "removes an existing marker when packet goes out of bounds" do
+      existing = build_packet()
+      socket = build_socket(%{visible_packets: %{"K5GVL-10" => existing}})
+      # Out-of-bounds coords.
+      result = PacketProcessor.handle_packet_visibility(existing, 40.0, -80.0, "K5GVL-10", socket)
+      refute Map.has_key?(result.assigns.visible_packets, "K5GVL-10")
+    end
+  end
+
+  describe "remove_marker_from_map/2" do
+    test "removes the callsign key from visible_packets" do
+      existing = build_packet()
+
+      socket =
+        build_socket(%{visible_packets: %{"K5GVL-10" => existing, "OTHER-1" => existing}})
+
+      result = PacketProcessor.remove_marker_from_map("K5GVL-10", socket)
+      refute Map.has_key?(result.assigns.visible_packets, "K5GVL-10")
+      assert Map.has_key?(result.assigns.visible_packets, "OTHER-1")
+    end
   end
 end

test/aprsme_web/live/status_live/connection_card_component_test.exs

@@ -0,0 +1,82 @@
+defmodule AprsmeWeb.StatusLive.ConnectionCardComponentTest do
+  use ExUnit.Case, async: true
+
+  import Phoenix.LiveViewTest
+
+  alias AprsmeWeb.StatusLive.ConnectionCardComponent
+
+  defp assigns(overrides \\ %{}) do
+    defaults = %{
+      aprs_status: %{
+        connected: true,
+        server: "rotate.aprs.net",
+        port: 14_580,
+        login_id: "K5ABC",
+        connected_at: ~U[2024-01-15 12:00:00Z],
+        uptime_seconds: 3600,
+        filter: "m/100",
+        packet_stats: %{
+          total_packets: 12_345,
+          packets_per_second: 4,
+          last_packet_at: DateTime.utc_now()
+        },
+        stored_packet_count: 99_999
+      },
+      health_score: 4
+    }
+
+    Map.merge(defaults, overrides)
+  end
+
+  describe "render/1" do
+    test "renders Connected badge when aprs_status.connected is true" do
+      html = render_component(&ConnectionCardComponent.render/1, assigns())
+      assert html =~ "Connected"
+      refute html =~ "Disconnected"
+    end
+
+    test "renders Disconnected badge when aprs_status.connected is false" do
+      a = assigns()
+      a = %{a | aprs_status: %{a.aprs_status | connected: false}}
+      html = render_component(&ConnectionCardComponent.render/1, a)
+      assert html =~ "Disconnected"
+    end
+
+    test "shows 'Not connected' when connected_at is nil" do
+      a = assigns()
+      a = %{a | aprs_status: %{a.aprs_status | connected_at: nil}}
+      html = render_component(&ConnectionCardComponent.render/1, a)
+      assert html =~ "Not connected"
+    end
+
+    test "shows 'None' for last_packet_at when nil" do
+      a = assigns()
+      stats = %{a.aprs_status.packet_stats | last_packet_at: nil}
+      a = %{a | aprs_status: %{a.aprs_status | packet_stats: stats}}
+      html = render_component(&ConnectionCardComponent.render/1, a)
+      assert html =~ "None"
+    end
+
+    test "includes server and port" do
+      html = render_component(&ConnectionCardComponent.render/1, assigns())
+      assert html =~ "rotate.aprs.net"
+      assert html =~ "14580"
+    end
+
+    test "includes login_id" do
+      html = render_component(&ConnectionCardComponent.render/1, assigns())
+      assert html =~ "K5ABC"
+    end
+
+    test "renders packets_per_minute as packets_per_second * 60" do
+      html = render_component(&ConnectionCardComponent.render/1, assigns())
+      # 4 * 60 = 240
+      assert html =~ "240"
+    end
+
+    test "renders health score numerator" do
+      html = render_component(&ConnectionCardComponent.render/1, assigns(%{health_score: 2}))
+      assert html =~ "2/5"
+    end
+  end
+end