From 2300dcd69cf97486ba9d292dea873d58bb2235dc Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 26 Mar 2026 18:48:27 -0500 Subject: [PATCH] fix: stabilize moving station tracking --- lib/aprsme/packets/prepared_queries.ex | 5 +- .../live/map_live/display_manager.ex | 26 ++++++-- lib/aprsme_web/live/map_live/index.ex | 51 +++++++++++++++- lib/aprsme_web/live/shared/packet_utils.ex | 11 ++-- test/aprsme/packets/prepared_queries_test.exs | 60 +++++++++++++++++++ .../live/map_live/display_manager_test.exs | 39 ++++++++++++ .../live/map_live/packet_processor_test.exs | 27 ++++++--- 7 files changed, 200 insertions(+), 19 deletions(-) diff --git a/lib/aprsme/packets/prepared_queries.ex b/lib/aprsme/packets/prepared_queries.ex index 5c80ac0..c236f00 100644 --- a/lib/aprsme/packets/prepared_queries.ex +++ b/lib/aprsme/packets/prepared_queries.ex @@ -24,7 +24,10 @@ defmodule Aprsme.Packets.PreparedQueries do fragment("upper(?)", p.sender) == ^normalized or fragment("upper(?)", p.object_name) == ^normalized or fragment("upper(?)", p.item_name) == ^normalized, - order_by: [desc: p.received_at], + order_by: [ + desc: fragment("CASE WHEN ? IS NULL THEN 0 ELSE 1 END", p.location), + desc: p.received_at + ], limit: 1, select: %{p | lat: fragment("ST_Y(?)", p.location), lon: fragment("ST_X(?)", p.location)} ) diff --git a/lib/aprsme_web/live/map_live/display_manager.ex b/lib/aprsme_web/live/map_live/display_manager.ex index a89a8b7..638037a 100644 --- a/lib/aprsme_web/live/map_live/display_manager.ex +++ b/lib/aprsme_web/live/map_live/display_manager.ex @@ -90,7 +90,14 @@ defmodule AprsmeWeb.MapLive.DisplayManager do def send_trail_line_for_tracked_callsign(socket) do callsign = socket.assigns.tracked_callsign threshold = socket.assigns.packet_age_threshold - packets = trail_packets_for_callsign(socket.assigns.visible_packets, callsign, threshold) + + packets = + trail_packets_for_callsign( + socket.assigns.visible_packets, + socket.assigns.historical_packets, + callsign, + threshold + ) if packets == [] do socket @@ -152,9 +159,9 @@ defmodule AprsmeWeb.MapLive.DisplayManager do socket end - defp trail_packets_for_callsign(visible_packets, callsign, threshold) do - visible_packets - |> Map.values() + defp trail_packets_for_callsign(visible_packets, historical_packets, callsign, threshold) do + (Map.values(visible_packets) ++ Map.values(historical_packets)) + |> Enum.uniq_by(&trail_packet_key/1) |> Enum.filter(&tracked_packet?(&1, callsign, threshold)) |> Enum.sort_by(&packet_timestamp/1, :asc) end @@ -195,4 +202,15 @@ defmodule AprsmeWeb.MapLive.DisplayManager do |> packet_received_at() |> DateTime.to_unix(:microsecond) end + + defp trail_packet_key(packet) do + Map.get(packet, :id) || + Map.get(packet, "id") || + { + Map.get(packet, :sender) || Map.get(packet, "sender"), + Map.get(packet, :lat) || Map.get(packet, "lat"), + Map.get(packet, :lon) || Map.get(packet, "lon"), + packet_received_at(packet) + } + end end diff --git a/lib/aprsme_web/live/map_live/index.ex b/lib/aprsme_web/live/map_live/index.ex index fdb275c..148bbad 100644 --- a/lib/aprsme_web/live/map_live/index.ex +++ b/lib/aprsme_web/live/map_live/index.ex @@ -1053,7 +1053,13 @@ defmodule AprsmeWeb.MapLive.Index do if String.upcase(packet_sender) == String.upcase(socket.assigns.tracked_callsign) do # Update the tracked callsign's latest packet - socket = assign(socket, :tracked_callsign_latest_packet, packet) + socket = + assign( + socket, + :tracked_callsign_latest_packet, + preferred_tracked_packet(socket.assigns.tracked_callsign_latest_packet, packet) + ) + process_packet_for_display(packet, socket) else {:noreply, socket} @@ -1072,7 +1078,13 @@ defmodule AprsmeWeb.MapLive.Index do packet_sender = Map.get(packet, :sender, Map.get(packet, "sender", "")) if String.upcase(packet_sender) == String.upcase(socket.assigns.tracked_callsign) do - socket = assign(socket, :tracked_callsign_latest_packet, packet) + socket = + assign( + socket, + :tracked_callsign_latest_packet, + preferred_tracked_packet(socket.assigns.tracked_callsign_latest_packet, packet) + ) + PacketProcessor.process_packet_for_marker_data(packet, socket) else {socket, nil, nil} @@ -1815,6 +1827,41 @@ defmodule AprsmeWeb.MapLive.Index do push_event(socket, "add_markers", %{markers: markers}) end + defp preferred_tracked_packet(nil, packet), do: packet + + defp preferred_tracked_packet(current_packet, incoming_packet) do + cond do + CoordinateUtils.has_position_data?(incoming_packet) -> + incoming_packet + + CoordinateUtils.has_position_data?(current_packet) -> + current_packet + + newer_packet?(incoming_packet, current_packet) -> + incoming_packet + + true -> + current_packet + end + end + + defp newer_packet?(left, right) do + case {packet_received_at(left), packet_received_at(right)} do + {%DateTime{} = left_dt, %DateTime{} = right_dt} -> + DateTime.after?(left_dt, right_dt) + + {%NaiveDateTime{} = left_dt, %NaiveDateTime{} = right_dt} -> + NaiveDateTime.after?(left_dt, right_dt) + + _ -> + false + end + end + + defp packet_received_at(packet) do + Map.get(packet, :received_at) || Map.get(packet, "received_at") + end + @impl true def terminate(_reason, socket) do # Unregister from connection monitor diff --git a/lib/aprsme_web/live/shared/packet_utils.ex b/lib/aprsme_web/live/shared/packet_utils.ex index 35f2839..7843452 100644 --- a/lib/aprsme_web/live/shared/packet_utils.ex +++ b/lib/aprsme_web/live/shared/packet_utils.ex @@ -10,10 +10,13 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do Get unique callsign key from packet. """ @spec get_callsign_key(map()) :: binary() - def get_callsign_key(%{id: id}) when not is_nil(id), do: to_string(id) - def get_callsign_key(%{"id" => id}) when not is_nil(id), do: to_string(id) - def get_callsign_key(%{sender: sender}) when is_binary(sender), do: sender - def get_callsign_key(%{"sender" => sender}) when is_binary(sender), do: sender + def get_callsign_key(packet) when is_map(packet) do + case display_name(packet) do + "" -> [:positive] |> System.unique_integer() |> to_string() + callsign -> callsign + end + end + def get_callsign_key(_packet), do: [:positive] |> System.unique_integer() |> to_string() @doc """ diff --git a/test/aprsme/packets/prepared_queries_test.exs b/test/aprsme/packets/prepared_queries_test.exs index 3713e0c..fca3d7e 100644 --- a/test/aprsme/packets/prepared_queries_test.exs +++ b/test/aprsme/packets/prepared_queries_test.exs @@ -100,6 +100,66 @@ defmodule Aprsme.Packets.PreparedQueriesTest do end end + describe "get_latest_packet_for_callsign/1" do + test "prefers the latest positioned packet over a newer non-position packet" do + positioned_received_at = DateTime.utc_now() |> DateTime.add(-60, :second) |> DateTime.truncate(:second) + status_received_at = DateTime.truncate(DateTime.utc_now(), :second) + + {:ok, _} = + create_positioned_packet(%{ + sender: "K5GVL-10", + base_callsign: "K5GVL", + ssid: "10", + lat: Decimal.new("33.1000"), + lon: Decimal.new("-96.6000"), + received_at: positioned_received_at + }) + + {:ok, _} = + %Packet{} + |> Packet.changeset(%{ + sender: "K5GVL-10", + base_callsign: "K5GVL", + ssid: "10", + destination: "APRS", + data_type: "status", + received_at: status_received_at, + has_position: false, + raw_packet: "K5GVL-10>APRS:>status update" + }) + |> Repo.insert() + + packet = PreparedQueries.get_latest_packet_for_callsign("K5GVL-10") + + assert packet.has_position == true + assert_in_delta packet.lat, 33.1, 0.01 + assert_in_delta packet.lon, -96.6, 0.01 + end + + test "falls back to the latest packet when no positioned packet exists" do + received_at = DateTime.truncate(DateTime.utc_now(), :second) + + {:ok, _} = + %Packet{} + |> Packet.changeset(%{ + sender: "STATUS-ONLY", + base_callsign: "STATUS", + ssid: "0", + destination: "APRS", + data_type: "status", + received_at: received_at, + has_position: false, + raw_packet: "STATUS-ONLY>APRS:>status update" + }) + |> Repo.insert() + + packet = PreparedQueries.get_latest_packet_for_callsign("STATUS-ONLY") + + assert packet.sender == "STATUS-ONLY" + assert packet.has_position == false + end + end + describe "get_latest_positions_for_callsigns/1" do test "returns positions for multiple callsigns" do {:ok, _} = diff --git a/test/aprsme_web/live/map_live/display_manager_test.exs b/test/aprsme_web/live/map_live/display_manager_test.exs index 90ba108..1d70063 100644 --- a/test/aprsme_web/live/map_live/display_manager_test.exs +++ b/test/aprsme_web/live/map_live/display_manager_test.exs @@ -213,6 +213,45 @@ defmodule AprsmeWeb.MapLive.DisplayManagerTest do } end + test "includes historical packets in tracked trail at low zoom", %{socket: socket} do + visible_packet = %{ + id: "1", + sender: "W5ISP-9", + lat: 30.124, + lon: -97.457, + received_at: ~U[2024-01-01 10:05:00Z] + } + + historical_packet = %{ + id: "2", + sender: "W5ISP-9", + lat: 30.123, + lon: -97.456, + received_at: ~U[2024-01-01 10:00:00Z] + } + + socket = + Map.put(socket, :assigns, %{ + socket.assigns + | visible_packets: %{"W5ISP-9" => visible_packet}, + historical_packets: %{"hist-1" => historical_packet}, + tracked_callsign: "W5ISP-9", + packet_age_threshold: ~U[2024-01-01 09:00:00Z] + }) + + result_socket = DisplayManager.send_trail_line_for_tracked_callsign(socket) + + push_events = get_in(result_socket.private, [:live_temp, :push_events]) || [] + [[event_name, event_data]] = push_events + + assert event_name == "show_trail_line" + + assert event_data.points == [ + %{lat: 30.123, lng: -97.456, timestamp: "2024-01-01T10:00:00Z"}, + %{lat: 30.124, lng: -97.457, timestamp: "2024-01-01T10:05:00Z"} + ] + end + test "returns socket unchanged when no packets", %{socket: socket} do socket = Map.put(socket, :assigns, %{ diff --git a/test/aprsme_web/live/map_live/packet_processor_test.exs b/test/aprsme_web/live/map_live/packet_processor_test.exs index 89ec831..d7dc2c3 100644 --- a/test/aprsme_web/live/map_live/packet_processor_test.exs +++ b/test/aprsme_web/live/map_live/packet_processor_test.exs @@ -95,14 +95,12 @@ defmodule AprsmeWeb.MapLive.PacketProcessorTest do end test "returns convert_from when replacing an existing visible packet" do - # First packet for K5GVL-10 — get_callsign_key uses :id when present first_packet = build_packet(%{id: "packet-1", lat: 33.1, lon: -96.5}) - # Use a packet without :id so get_callsign_key falls through to :sender - first_packet_no_id = Map.delete(first_packet, :id) - socket = build_socket(%{visible_packets: %{"K5GVL-10" => first_packet_no_id}}) + socket = build_socket(%{visible_packets: %{"K5GVL-10" => first_packet}}) - # Second packet for same callsign (also no :id so key = sender) - second_packet = %{lat: 33.2, lon: -96.4} |> build_packet() |> Map.delete(:id) + # Second packet for same callsign has a different packet ID but should still + # replace the existing marker for that station. + second_packet = build_packet(%{id: "packet-2", lat: 33.2, lon: -96.4}) {_updated_socket, marker_data, _removed} = PacketProcessor.process_packet_for_marker_data(second_packet, socket) @@ -122,11 +120,11 @@ defmodule AprsmeWeb.MapLive.PacketProcessorTest do test "returns removed callsign key when out-of-bounds packet had existing marker" do # Set up socket with an existing visible packet - existing_packet = %{lat: 33.1, lon: -96.5} |> build_packet() |> Map.delete(:id) + existing_packet = build_packet(%{id: "packet-1", lat: 33.1, lon: -96.5}) socket = build_socket(%{visible_packets: %{"K5GVL-10" => existing_packet}}) # New packet from same callsign is out of bounds - out_of_bounds_packet = %{lat: 40.0, lon: -80.0} |> build_packet() |> Map.delete(:id) + out_of_bounds_packet = build_packet(%{id: "packet-2", lat: 40.0, lon: -80.0}) {updated_socket, marker_data, removed_id} = PacketProcessor.process_packet_for_marker_data(out_of_bounds_packet, socket) @@ -146,5 +144,18 @@ defmodule AprsmeWeb.MapLive.PacketProcessorTest do assert removed_id == nil end + + test "keeps one visible packet per station even when packet IDs change" do + first_packet = build_packet(%{id: "packet-1", lat: 33.1, lon: -96.5}) + socket = build_socket(%{visible_packets: %{"K5GVL-10" => first_packet}}) + second_packet = build_packet(%{id: "packet-2", lat: 33.2, lon: -96.4}) + + {updated_socket, marker_data, _removed_id} = + PacketProcessor.process_packet_for_marker_data(second_packet, socket) + + assert marker_data["convert_from"] == "K5GVL-10" + assert Map.keys(updated_socket.assigns.visible_packets) == ["K5GVL-10"] + assert updated_socket.assigns.visible_packets["K5GVL-10"].id == "packet-2" + end end end