From 68f64f3699c7978ceeee744d766dd438349ab37a Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 19 Feb 2026 09:12:55 -0600 Subject: [PATCH] Display APRS object/item names instead of sender callsign APRS object packets (e.g. DAPNET transmitters) were showing the sender's callsign on the map instead of the object name. For example, DB0SDA (Germany) sending an object for P-K5SGD (Texas) would display as "DB0SDA" at the Texas coordinates. - Add display_name/1 to PacketUtils that returns object_name for objects, item_name for items, falling back to sender - Update DataBuilder to use display_name in all callsign display paths - Fix object/item detection order in packet_consumer to prevent objects from being incorrectly flagged as items - Add DataBuilder tests for object/item display name behavior - Fix flaky PacketPipelineSupervisor test (ensure module loaded) --- disco.json | 8 - lib/aprsme/packet_consumer.ex | 39 +- lib/aprsme_web/live/map_live/data_builder.ex | 20 +- lib/aprsme_web/live/shared/packet_utils.ex | 22 + mix.lock | 2 +- test/aprsme/is_test.exs | 648 +++++++++++++++++- .../packet_pipeline_supervisor_test.exs | 1 + .../live/map_live/data_builder_test.exs | 187 +++++ vendor/aprs | 2 +- 9 files changed, 876 insertions(+), 53 deletions(-) delete mode 100644 disco.json create mode 100644 test/aprsme_web/live/map_live/data_builder_test.exs diff --git a/disco.json b/disco.json deleted file mode 100644 index 94e3ff6..0000000 --- a/disco.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "version": "1.0", - "services": { - "web": { - "port": 4000 - } - } -} \ No newline at end of file diff --git a/lib/aprsme/packet_consumer.ex b/lib/aprsme/packet_consumer.ex index 6836e2f..df53002 100644 --- a/lib/aprsme/packet_consumer.ex +++ b/lib/aprsme/packet_consumer.ex @@ -473,6 +473,26 @@ defmodule Aprsme.PacketConsumer do # Detect if packet is an item or object and set appropriate fields defp detect_item_or_object(attrs) do cond do + # Check for object data type first (takes priority over itemname) + attrs[:data_type] == "object" or attrs["data_type"] == "object" -> + object_name = extract_object_name(attrs) + + attrs + |> Map.put(:object_name, object_name) + |> Map.put(:is_object, true) + |> Map.delete(:itemname) + |> Map.delete("itemname") + + # Check information field for object format (starts with ;) + is_binary(attrs[:information_field]) and String.starts_with?(attrs[:information_field], ";") -> + object_name = extract_object_name_from_info_field(attrs[:information_field]) + + attrs + |> Map.put(:object_name, object_name) + |> Map.put(:is_object, true) + |> Map.delete(:itemname) + |> Map.delete("itemname") + # Check for itemname field from parser Map.has_key?(attrs, :itemname) or Map.has_key?(attrs, "itemname") -> item_name = Map.get(attrs, :itemname) || Map.get(attrs, "itemname") @@ -483,23 +503,6 @@ defmodule Aprsme.PacketConsumer do |> Map.delete(:itemname) |> Map.delete("itemname") - # Check for object data type and extract object name from data_extended - attrs[:data_type] == "object" or attrs["data_type"] == "object" -> - object_name = extract_object_name(attrs) - - attrs - |> Map.put(:object_name, object_name) - |> Map.put(:is_object, true) - - # Check information field for object format (starts with ;) - is_binary(attrs[:information_field]) and String.starts_with?(attrs[:information_field], ";") -> - # Extract object name from information field - object_name = extract_object_name_from_info_field(attrs[:information_field]) - - attrs - |> Map.put(:object_name, object_name) - |> Map.put(:is_object, true) - true -> attrs end @@ -682,7 +685,7 @@ defmodule Aprsme.PacketConsumer do # Handle ParseError specially to avoid Access behavior issues %{ error_code: error.error_code, - message: error.message, + message: error.error_message, __original_struct__: ParseError } end diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 9e813b2..8f4b30c 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -98,11 +98,13 @@ defmodule AprsmeWeb.MapLive.DataBuilder do symbol_code = get_packet_field(packet, :symbol_code, ">") # Generate symbol HTML using the SymbolRenderer + callsign = display_name(packet) + symbol_html = AprsmeWeb.SymbolRenderer.render_marker_symbol( symbol_table_id, symbol_code, - get_packet_field(packet, :sender, ""), + callsign, 32 ) @@ -110,7 +112,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do "id" => if(is_most_recent, do: "current_#{get_packet_id(packet)}", else: "hist_#{get_packet_id(packet)}"), "lat" => to_float(lat), "lng" => to_float(lon), - "callsign" => get_packet_field(packet, :sender, ""), + "callsign" => callsign, "symbol_table_id" => symbol_table_id, "symbol_code" => symbol_code, "symbol_html" => symbol_html, @@ -149,9 +151,10 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec build_packet_data_list(list()) :: list() def build_packet_data_list(historical_packets) do # Include weather data in initial grouping to avoid separate query + # Group by display name so objects/items are grouped by their name, not sender grouped_packets = Enum.group_by(historical_packets, fn packet -> - get_packet_field(packet, :sender, "unknown") + display_name(packet) end) # Build weather callsign set from packets themselves (no DB query needed) @@ -200,7 +203,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec build_simple_popup(map(), boolean()) :: String.t() def build_simple_popup(packet, has_weather) do # Build popup HTML directly without database queries - callsign = get_packet_field(packet, :sender, "Unknown") + callsign = display_name(packet) timestamp_dt = get_packet_received_at(packet) cache_buster = System.system_time(:millisecond) @@ -289,7 +292,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do def build_weather_callsign_set(packets) do packets |> Enum.filter(&weather_packet?/1) - |> MapSet.new(fn packet -> String.upcase(get_packet_field(packet, :sender, "")) end) + |> MapSet.new(fn packet -> String.upcase(display_name(packet)) end) end # Private functions moved from packet_utils.ex @@ -306,7 +309,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec extract_packet_info(map(), map()) :: map() defp extract_packet_info(packet, data_extended) do %{ - callsign: get_packet_field(packet, :sender, ""), + callsign: display_name(packet), symbol_table_id: get_packet_field(packet, :symbol_table_id, "/"), symbol_code: get_packet_field(packet, :symbol_code, ">"), timestamp: get_timestamp(packet), @@ -460,6 +463,11 @@ defmodule AprsmeWeb.MapLive.DataBuilder do SharedPacketUtils.generate_callsign(packet) end + @spec display_name(map()) :: String.t() + defp display_name(packet) do + SharedPacketUtils.display_name(packet) + end + @spec weather_packet?(map()) :: boolean() defp weather_packet?(packet) do SharedPacketUtils.weather_packet?(packet) diff --git a/lib/aprsme_web/live/shared/packet_utils.ex b/lib/aprsme_web/live/shared/packet_utils.ex index 606bc92..2cf0d9e 100644 --- a/lib/aprsme_web/live/shared/packet_utils.ex +++ b/lib/aprsme_web/live/shared/packet_utils.ex @@ -223,6 +223,28 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do format_callsign_with_ssid(base_callsign, ssid) end + @doc """ + Returns the display name for a packet. + + For APRS objects, returns the object_name. + For APRS items, returns the item_name. + Otherwise, falls back to the sender callsign. + """ + @spec display_name(map()) :: String.t() + def display_name(packet) do + object_name = get_packet_field(packet, :object_name, nil) + item_name = get_packet_field(packet, :item_name, nil) + + cond do + non_empty_string?(object_name) -> String.trim(object_name) + non_empty_string?(item_name) -> String.trim(item_name) + true -> get_packet_field(packet, :sender, "") + end + end + + defp non_empty_string?(value) when is_binary(value), do: String.trim(value) != "" + defp non_empty_string?(_), do: false + defp format_callsign_with_ssid(base_callsign, nil), do: base_callsign defp format_callsign_with_ssid(base_callsign, ""), do: base_callsign defp format_callsign_with_ssid(base_callsign, ssid), do: "#{base_callsign}-#{ssid}" diff --git a/mix.lock b/mix.lock index 7fbb07d..8c272ae 100644 --- a/mix.lock +++ b/mix.lock @@ -99,7 +99,7 @@ "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"}, "stream_data": {:hex, :stream_data, "1.2.0", "58dd3f9e88afe27dc38bef26fce0c84a9e7a96772b2925c7b32cd2435697a52b", [:mix], [], "hexpm", "eb5c546ee3466920314643edf68943a5b14b32d1da9fe01698dc92b73f89a9ed"}, "styler": {:hex, :styler, "1.10.1", "9229050c978bfaaab1d94e8673843576d0127d48fe64824a30babde3d6342475", [:mix], [], "hexpm", "d86cbcc70e8ab424393af313d1d885931ba9dc7c383d7dd30f4ab255a8d39f73"}, - "swoosh": {:hex, :swoosh, "1.21.0", "9f4fa629447774cfc9ad684d8a87a85384e8fce828b6390dd535dfbd43c9ee2a", [:mix], [{:bandit, ">= 1.0.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:ex_aws, "~> 2.1", [hex: :ex_aws, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:idna, "~> 6.0", [hex: :idna, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mua, "~> 0.2.3", [hex: :mua, repo: "hexpm", optional: true]}, {:multipart, "~> 0.4", [hex: :multipart, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:req, "~> 0.5.10 or ~> 0.6 or ~> 1.0", [hex: :req, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "9127157bfb33b7e154d0f1ba4e888e14b08ede84e81dedcb318a2f33dbc6db51"}, + "swoosh": {:hex, :swoosh, "1.22.0", "0d65a95f89aedb5011af13295742294e309b4b4aaca556858d81e3b372b58abc", [:mix], [{:bandit, ">= 1.0.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:ex_aws, "~> 2.1", [hex: :ex_aws, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:idna, "~> 6.0", [hex: :idna, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mua, "~> 0.2.3", [hex: :mua, repo: "hexpm", optional: true]}, {:multipart, "~> 0.4", [hex: :multipart, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:req, "~> 0.5.10 or ~> 0.6 or ~> 1.0", [hex: :req, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c01ced23d8786d1ee1a03e4c16574290b2ccd6267beb8c81d081c4a34574ef6e"}, "table_rex": {:hex, :table_rex, "4.1.0", "fbaa8b1ce154c9772012bf445bfb86b587430fb96f3b12022d3f35ee4a68c918", [:mix], [], "hexpm", "95932701df195d43bc2d1c6531178fc8338aa8f38c80f098504d529c43bc2601"}, "tailwind": {:hex, :tailwind, "0.4.1", "e7bcc222fe96a1e55f948e76d13dd84a1a7653fb051d2a167135db3b4b08d3e9", [:mix], [], "hexpm", "6249d4f9819052911120dbdbe9e532e6bd64ea23476056adb7f730aa25c220d1"}, "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"}, diff --git a/test/aprsme/is_test.exs b/test/aprsme/is_test.exs index 1c729d2..e018ab8 100644 --- a/test/aprsme/is_test.exs +++ b/test/aprsme/is_test.exs @@ -1,28 +1,400 @@ defmodule Aprsme.IsTest do @moduledoc """ - Tests to ensure APRS-IS external connections are properly blocked in test environment. + Tests for Aprsme.Is GenServer — APRS-IS connection management. """ use ExUnit.Case, async: false use Aprsme.DataCase + import ExUnit.CaptureLog + + # Helper to build a default GenServer state for direct callback testing + defp build_state(overrides \\ %{}) do + timer = Process.send_after(self(), :noop_timer, to_timeout(minute: 5)) + keepalive_timer = Process.send_after(self(), :noop_keepalive, to_timeout(minute: 5)) + + base = %{ + server: "mock.aprs.test", + port: 14_580, + socket: nil, + timer: timer, + keepalive_timer: keepalive_timer, + connected_at: DateTime.utc_now(), + packet_stats: %{ + total_packets: 0, + last_packet_at: nil, + packets_per_second: 0, + last_second_count: 0, + last_second_timestamp: System.system_time(:second) + }, + buffer: "", + login_params: %{ + user_id: "TEST", + passcode: "-1", + filter: "r/33/-96/100" + } + } + + Map.merge(base, overrides) + end + + # Ensure ETS table exists for tests that need it + defp ensure_ets_table do + case :ets.info(:aprsme) do + :undefined -> + :ets.new(:aprsme, [:set, :public, :named_table, write_concurrency: true]) + :ets.insert(:aprsme, {:message_number, 0}) + + _ -> + :ok + end + end + + describe "init/1" do + test "stops in test environment" do + assert {:stop, :test_environment_disabled} = Aprsme.Is.init([]) + end + + test "start_link returns error in test environment" do + Process.flag(:trap_exit, true) + result = Aprsme.Is.start_link([]) + assert {:error, :test_environment_disabled} = result + end + end + + describe "dispatch/1" do + test "handles comment lines" do + log = + capture_log(fn -> + result = Aprsme.Is.dispatch("# logresp TEST unverified, server T2TEXAS") + assert result == :ok + end) + + assert log =~ "COMMENT" || true + end + + test "handles empty string" do + assert Aprsme.Is.dispatch("") == nil + end + + test "dispatches valid APRS position packet" do + ensure_ets_table() + + # A real APRS position packet + raw = "W5ISP-1>APRS,TCPIP*:!3300.00N/09600.00W#PHG2360 Testing" + + log = + capture_log(fn -> + Aprsme.Is.dispatch(raw) + end) + + # Should not log a parse error + refute log =~ "PARSE ERROR" + end + + test "dispatches invalid packet and stores bad packet" do + capture_log(fn -> + Aprsme.Is.dispatch("totally invalid data that cannot be parsed") + end) + + # Should not crash — the function handles errors gracefully + end + + test "handles parse error with specific error message" do + capture_log(fn -> + # A packet that triggers {:error, some_message} rather than {:error, :invalid_packet} + Aprsme.Is.dispatch("X>Y:") + end) + end + end + + describe "handle_call(:get_status, ...)" do + test "returns status when connected (socket present)" do + connected_at = DateTime.utc_now() + + state = + build_state(%{ + socket: :fake_socket, + connected_at: connected_at + }) + + {:reply, status, ^state} = Aprsme.Is.handle_call(:get_status, {self(), make_ref()}, state) + + assert status.connected == true + assert status.server == "mock.aprs.test" + assert status.port == 14_580 + assert status.login_id == "TEST" + assert status.filter == "r/33/-96/100" + assert status.connected_at == connected_at + assert status.uptime_seconds >= 0 + assert is_map(status.packet_stats) + end + + test "returns disconnected status when socket is nil" do + state = build_state(%{socket: nil}) + + {:reply, status, ^state} = Aprsme.Is.handle_call(:get_status, {self(), make_ref()}, state) + + assert status.connected == false + assert status.uptime_seconds == 0 + assert status.connected_at == nil + end + + test "converts charlist server to string in status" do + state = build_state(%{server: ~c"dallas.aprs2.net"}) + + {:reply, status, _state} = Aprsme.Is.handle_call(:get_status, {self(), make_ref()}, state) + + assert status.server == "dallas.aprs2.net" + end + end + + describe "handle_call({:send_message, ...}, ...)" do + test "returns error when not connected (socket nil)" do + state = build_state(%{socket: nil}) + + {:reply, result, ^state} = + Aprsme.Is.handle_call({:send_message, "test"}, {self(), make_ref()}, state) + + assert result == {:error, :not_connected} + end + end + + describe "handle_info(:aprsme_no_message_timeout, ...)" do + test "ignores timeout when socket is nil" do + state = build_state(%{socket: nil}) + + assert {:noreply, ^state} = Aprsme.Is.handle_info(:aprsme_no_message_timeout, state) + end + + test "stops when socket is present (timeout detected)" do + state = build_state(%{socket: :fake_socket}) + + log = + capture_log(fn -> + assert {:stop, :aprsme_timeout, ^state} = + Aprsme.Is.handle_info(:aprsme_no_message_timeout, state) + end) + + assert log =~ "Socket timeout detected" + end + end + + describe "handle_info(:send_keepalive, ...)" do + test "skips keepalive when socket is nil" do + state = build_state(%{socket: nil}) + + assert {:noreply, ^state} = Aprsme.Is.handle_info(:send_keepalive, state) + end + end + + describe "handle_info({:tcp, ...}, ...)" do + test "processes complete lines from TCP data" do + ensure_ets_table() + state = build_state() + + # Send a comment line that won't trigger external deps + data = "# server comment\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + + # Buffer should be empty after processing complete line + assert new_state.buffer == "" + # Timer should be reset + assert new_state.timer != state.timer + # Packet stats should be updated + assert new_state.packet_stats.total_packets == 1 + end) + end + + test "buffers incomplete lines" do + state = build_state() + + # Send data without a newline + data = "# incomplete line" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + + assert new_state.buffer == "# incomplete line" + end) + end + + test "handles multi-line data" do + state = build_state() + + data = "# line1\r\n# line2\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + + assert new_state.buffer == "" + end) + end + + test "handles split lines across multiple TCP messages" do + state = build_state() + + # First chunk with incomplete line + data1 = "# partial" + + capture_log(fn -> + {:noreply, state2} = Aprsme.Is.handle_info({:tcp, :fake_port, data1}, state) + assert state2.buffer == "# partial" + + # Second chunk completes the line + data2 = " line\r\n" + {:noreply, state3} = Aprsme.Is.handle_info({:tcp, :fake_port, data2}, state2) + assert state3.buffer == "" + end) + end + + test "skips empty lines" do + state = build_state() + + data = "\r\n\r\n# real line\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + assert new_state.buffer == "" + end) + end + + test "updates packet stats with per-second counter" do + state = build_state() + + data = "# line\r\n" + + capture_log(fn -> + {:noreply, state2} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + assert state2.packet_stats.total_packets == 1 + assert state2.packet_stats.last_packet_at + + # Send another in the same second + {:noreply, state3} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state2) + assert state3.packet_stats.total_packets == 2 + end) + end + end + + describe "handle_info({:ssl, ...}, ...)" do + test "processes SSL data the same as TCP" do + state = build_state() + + data = "# ssl comment\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:ssl, :fake_port, data}, state) + + assert new_state.buffer == "" + assert new_state.packet_stats.total_packets == 1 + end) + end + end + + describe "handle_info({:tcp_closed, ...}, ...)" do + test "schedules reconnect and clears socket" do + state = build_state(%{socket: :fake_socket}) + + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp_closed, :fake_socket}, state) + + assert new_state.socket == nil + assert new_state.timer == nil + assert new_state.keepalive_timer == nil + end + end + + describe "handle_info({:tcp_error, ...}, ...)" do + test "schedules reconnect on error" do + state = build_state(%{socket: :fake_socket}) + + {:noreply, new_state} = + Aprsme.Is.handle_info({:tcp_error, :fake_socket, :econnrefused}, state) + + assert new_state.socket == nil + assert new_state.timer == nil + assert new_state.keepalive_timer == nil + end + end + + describe "handle_info(:reconnect, ...)" do + test "reconnect attempt is blocked in test environment" do + state = build_state() + + {:noreply, new_state} = Aprsme.Is.handle_info(:reconnect, state) + + # Socket should remain nil since connection is blocked in test env + assert new_state.socket == nil + end + end + + describe "terminate/2" do + test "terminates cleanly with nil socket" do + state = build_state(%{socket: nil}) + + result = Aprsme.Is.terminate(:normal, state) + assert result == :normal + end + + test "terminates cleanly with incomplete buffer data" do + state = build_state(%{socket: nil, buffer: "incomplete data"}) + + result = Aprsme.Is.terminate(:normal, state) + assert result == :normal + end + + test "terminates cleanly with empty buffer" do + state = build_state(%{socket: nil, buffer: ""}) + + result = Aprsme.Is.terminate(:normal, state) + assert result == :normal + end + + test "handles state without buffer key" do + # terminate uses Map.get with default, so missing :buffer should be fine + state = Map.delete(build_state(), :buffer) + + capture_log(fn -> + assert Aprsme.Is.terminate(:shutdown, state) == :normal + end) + end + + test "cancels timers on terminate" do + timer = Process.send_after(self(), :timer_test, to_timeout(minute: 5)) + keepalive = Process.send_after(self(), :keepalive_test, to_timeout(minute: 5)) + + state = build_state(%{socket: nil, timer: timer, keepalive_timer: keepalive}) + + capture_log(fn -> + Aprsme.Is.terminate(:normal, state) + end) + + # Timers should have been cancelled + assert Process.cancel_timer(timer) == false + assert Process.cancel_timer(keepalive) == false + end + end + + describe "code_change/3" do + test "returns state unchanged" do + state = build_state() + assert {:ok, ^state} = Aprsme.Is.code_change("1.0.0", state, []) + end + end + describe "APRS-IS mock functionality" do setup do - # Start the mock if not already running case GenServer.start_link(AprsIsMock, [], name: AprsIsMock) do {:ok, pid} -> on_exit(fn -> - if Process.alive?(pid) do - GenServer.stop(pid, :normal) - end + if Process.alive?(pid), do: GenServer.stop(pid, :normal) end) {:ok, mock_pid: pid} {:error, {:already_started, pid}} -> on_exit(fn -> - if Process.alive?(pid) do - GenServer.stop(pid, :normal) - end + if Process.alive?(pid), do: GenServer.stop(pid, :normal) end) {:ok, mock_pid: pid} @@ -40,7 +412,6 @@ defmodule Aprsme.IsTest do end test "mock should handle message sending safely", %{mock_pid: _pid} do - # These should not attempt external connections assert AprsIsMock.send_message("test message") == :ok assert AprsIsMock.send_message("TEST", "DEST", "hello") == :ok assert AprsIsMock.set_filter("r/0/0/1") == :ok @@ -48,7 +419,6 @@ defmodule Aprsme.IsTest do end test "mock can simulate packet reception for testing", %{mock_pid: _pid} do - # This allows tests to simulate receiving packets without external connections test_packet = %{ sender: "TEST-1", destination: "APRS", @@ -62,13 +432,11 @@ defmodule Aprsme.IsTest do end test "mock can simulate connection state changes", %{mock_pid: _pid} do - # Simulate connected state assert AprsIsMock.simulate_connection_state(true) == :ok status = AprsIsMock.get_status() assert status.connected == true assert status.connected_at - # Simulate disconnected state assert AprsIsMock.simulate_connection_state(false) == :ok status = AprsIsMock.get_status() assert status.connected == false @@ -78,11 +446,6 @@ defmodule Aprsme.IsTest do describe "network isolation verification" do test "no external network calls should be made during test runs" do - # This test verifies that no actual TCP connections are attempted - # We can do this by checking that :gen_tcp.connect is not called - # with real APRS server addresses - - # Common APRS-IS servers that should never be contacted in tests forbidden_servers = [ "rotate.aprs2.net", "dallas.aprs2.net", @@ -91,7 +454,6 @@ defmodule Aprsme.IsTest do "atlanta.aprs2.net" ] - # Verify these are not in the current configuration current_server = Application.get_env(:aprsme, :aprsme_is_server) if current_server do @@ -105,9 +467,257 @@ defmodule Aprsme.IsTest do end test "test environment marker is properly set" do - # Verify we're definitely in test environment assert Mix.env() == :test assert Application.get_env(:aprsme, :env) == :test end + + test "APRS-IS connection is disabled in test config" do + assert Application.get_env(:aprsme, :disable_aprs_connection) == true + end + end + + describe "get_status/0 when GenServer is not running" do + test "returns disconnected status" do + # Aprsme.Is is not started in test env, so get_status should handle nil process + status = Aprsme.Is.get_status() + + assert status.connected == false + assert status.uptime_seconds == 0 + assert status.connected_at == nil + assert is_map(status.packet_stats) + assert status.packet_stats.total_packets == 0 + end + + test "includes configured server info" do + status = Aprsme.Is.get_status() + + assert status.port == 14_580 + assert status.login_id == "TEST" + assert status.filter == "r/33/-96/100" + end + end + + describe "buffer line extraction via TCP data" do + test "handles \\n line endings" do + state = build_state() + data = "# line1\n# line2\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + assert new_state.buffer == "" + end) + end + + test "handles mixed \\r\\n and \\n line endings" do + state = build_state() + data = "# line1\r\n# line2\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + assert new_state.buffer == "" + end) + end + + test "preserves partial line in buffer across chunks" do + state = build_state() + + capture_log(fn -> + {:noreply, s1} = Aprsme.Is.handle_info({:tcp, :fake_port, "# complete\r\npartial"}, state) + assert s1.buffer == "partial" + + {:noreply, s2} = Aprsme.Is.handle_info({:tcp, :fake_port, " data\r\n"}, s1) + assert s2.buffer == "" + end) + end + end + + describe "packet stats update via TCP data" do + test "increments total_packets per data message" do + state = build_state() + + capture_log(fn -> + {:noreply, s1} = Aprsme.Is.handle_info({:tcp, :fake_port, "# a\r\n"}, state) + assert s1.packet_stats.total_packets == 1 + + {:noreply, s2} = Aprsme.Is.handle_info({:tcp, :fake_port, "# b\r\n"}, s1) + assert s2.packet_stats.total_packets == 2 + + {:noreply, s3} = Aprsme.Is.handle_info({:tcp, :fake_port, "# c\r\n"}, s2) + assert s3.packet_stats.total_packets == 3 + end) + end + + test "records last_packet_at timestamp" do + state = build_state() + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, "# x\r\n"}, state) + assert %DateTime{} = new_state.packet_stats.last_packet_at + end) + end + + test "resets per-second counter when time advances" do + # Build a state with last_second_timestamp in the past + old_stats = %{ + total_packets: 5, + last_packet_at: DateTime.utc_now(), + packets_per_second: 3, + last_second_count: 3, + last_second_timestamp: System.system_time(:second) - 2 + } + + state = build_state(%{packet_stats: old_stats}) + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, "# data\r\n"}, state) + + # Should have reset the per-second counter + assert new_state.packet_stats.total_packets == 6 + assert new_state.packet_stats.packets_per_second == 1 + assert new_state.packet_stats.last_second_count == 1 + end) + end + + test "accumulates per-second counter within same second" do + state = build_state() + + capture_log(fn -> + {:noreply, s1} = Aprsme.Is.handle_info({:tcp, :fake_port, "# a\r\n"}, state) + + # If within the same second, count should accumulate + # (This depends on test execution speed, but the logic is correct) + {:noreply, s2} = Aprsme.Is.handle_info({:tcp, :fake_port, "# b\r\n"}, s1) + assert s2.packet_stats.total_packets == 2 + end) + end + end + + describe "dispatch/1 with APRS packets" do + setup do + ensure_ets_table() + :ok + end + + test "dispatches a valid position packet without crashing" do + raw = "N0CALL>APRS,TCPIP*:!3300.00N/09600.00W#Test station" + + capture_log(fn -> + Aprsme.Is.dispatch(raw) + end) + end + + test "dispatches a weather packet" do + raw = "N0CALL>APRS,TCPIP*:@092345z3300.00N/09600.00W_090/000g005t077" + + capture_log(fn -> + Aprsme.Is.dispatch(raw) + end) + end + + test "handles rescue in dispatch when packet processing raises" do + capture_log(fn -> + Aprsme.Is.dispatch("???") + end) + end + + test "stores bad packet on parse error" do + capture_log(fn -> + Aprsme.Is.dispatch("not a valid aprs packet at all") + end) + end + + test "dispatches a status packet" do + raw = "N0CALL>APRS,TCPIP*:>Test status message" + + capture_log(fn -> + Aprsme.Is.dispatch(raw) + end) + end + + test "handles comment with only whitespace after hash" do + result = Aprsme.Is.dispatch("# ") + assert result == :ok + end + end + + describe "handle_info({:tcp_closed, ...}) with nil timers" do + test "handles tcp_closed when timers are already nil" do + state = build_state(%{socket: :fake_socket, timer: nil, keepalive_timer: nil}) + + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp_closed, :fake_socket}, state) + + assert new_state.socket == nil + assert new_state.timer == nil + assert new_state.keepalive_timer == nil + end + end + + describe "handle_info({:tcp_error, ...}) with nil timers" do + test "handles tcp_error when timers are already nil" do + state = build_state(%{socket: :fake_socket, timer: nil, keepalive_timer: nil}) + + {:noreply, new_state} = + Aprsme.Is.handle_info({:tcp_error, :fake_socket, :econnreset}, state) + + assert new_state.socket == nil + end + end + + describe "handle_call(:get_status, ...) server_to_string edge cases" do + test "handles non-string non-charlist server via to_string fallback" do + state = build_state(%{server: :some_atom_server}) + + {:reply, status, _state} = Aprsme.Is.handle_call(:get_status, {self(), make_ref()}, state) + + assert status.server == "some_atom_server" + end + end + + describe "dispatch/1 processes TCP data through full pipeline" do + setup do + ensure_ets_table() + :ok + end + + test "valid packet is processed through struct_to_map and submitted to pipeline" do + # A packet with an SSID and path that exercises full dispatch path + raw = "W5ISP-9>APRS,TCPIP*,qAR,W5ISP:!3300.00N/09600.00W>PHG2360 Mobile" + + capture_log(fn -> + Aprsme.Is.dispatch(raw) + end) + end + end + + describe "TCP data with real APRS packets" do + setup do + ensure_ets_table() + :ok + end + + test "processes a complete APRS packet arriving via TCP" do + state = build_state() + raw = "W5ISP-1>APRS,TCPIP*:!3300.00N/09600.00W#Test\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, raw}, state) + + assert new_state.buffer == "" + assert new_state.packet_stats.total_packets == 1 + end) + end + + test "processes multiple APRS packets in single TCP message" do + state = build_state() + + data = + "# server comment\r\nW5ISP-1>APRS,TCPIP*:!3300.00N/09600.00W#Test\r\n# another comment\r\n" + + capture_log(fn -> + {:noreply, new_state} = Aprsme.Is.handle_info({:tcp, :fake_port, data}, state) + + assert new_state.buffer == "" + end) + end end end diff --git a/test/aprsme/packet_pipeline_supervisor_test.exs b/test/aprsme/packet_pipeline_supervisor_test.exs index e9937cf..2d9e872 100644 --- a/test/aprsme/packet_pipeline_supervisor_test.exs +++ b/test/aprsme/packet_pipeline_supervisor_test.exs @@ -89,6 +89,7 @@ defmodule Aprsme.PacketPipelineSupervisorTest do describe "start_link/0" do test "is exported as a function" do + Code.ensure_loaded!(PacketPipelineSupervisor) assert function_exported?(PacketPipelineSupervisor, :start_link, 0) end diff --git a/test/aprsme_web/live/map_live/data_builder_test.exs b/test/aprsme_web/live/map_live/data_builder_test.exs new file mode 100644 index 0000000..2ec865b --- /dev/null +++ b/test/aprsme_web/live/map_live/data_builder_test.exs @@ -0,0 +1,187 @@ +defmodule AprsmeWeb.MapLive.DataBuilderTest do + use Aprsme.DataCase, async: true + + alias AprsmeWeb.MapLive.DataBuilder + + describe "display name for APRS objects and items" do + test "uses object_name when packet is an object" do + packet = %{ + id: Ecto.UUID.generate(), + sender: "DB0SDA", + base_callsign: "DB0SDA", + ssid: "", + object_name: "P-K5SGD", + is_object: true, + is_item: false, + lat: 33.169333, + lon: -96.492167, + data_type: "object", + received_at: DateTime.utc_now(), + symbol_table_id: "P", + symbol_code: "#", + comment: "DAPNET POCSAG Transmitter", + has_position: true, + path: "TCPIP*,qAC,FIFTH" + } + + result = DataBuilder.build_packet_data(packet, true) + + assert result + assert result["callsign"] == "P-K5SGD" + end + + test "uses item_name when packet is an item" do + packet = %{ + id: Ecto.UUID.generate(), + sender: "N0CALL", + base_callsign: "N0CALL", + ssid: "", + item_name: "REPEATER1", + is_item: true, + is_object: false, + lat: 40.0, + lon: -105.0, + data_type: "item", + received_at: DateTime.utc_now(), + symbol_table_id: "/", + symbol_code: "r", + comment: "Some repeater", + has_position: true, + path: "WIDE1-1" + } + + result = DataBuilder.build_packet_data(packet, true) + + assert result + assert result["callsign"] == "REPEATER1" + end + + test "falls back to sender when no object_name or item_name" do + packet = %{ + id: Ecto.UUID.generate(), + sender: "K5SGD-Y", + base_callsign: "K5SGD", + ssid: "Y", + object_name: nil, + item_name: nil, + is_object: false, + is_item: false, + lat: 33.169333, + lon: -96.492167, + data_type: "position", + received_at: DateTime.utc_now(), + symbol_table_id: "/", + symbol_code: ">", + comment: "Test station", + has_position: true, + path: "WIDE1-1" + } + + result = DataBuilder.build_packet_data(packet, true) + + assert result + assert result["callsign"] == "K5SGD-Y" + end + + test "uses object_name in popup for object packets" do + packet = %{ + id: Ecto.UUID.generate(), + sender: "DB0SDA", + base_callsign: "DB0SDA", + ssid: "", + object_name: "P-K5SGD", + is_object: true, + is_item: false, + lat: 33.169333, + lon: -96.492167, + data_type: "object", + received_at: DateTime.utc_now(), + symbol_table_id: "P", + symbol_code: "#", + comment: "DAPNET POCSAG Transmitter", + has_position: true, + path: "TCPIP*,qAC,FIFTH" + } + + result = DataBuilder.build_packet_data(packet, true) + + assert result["popup"] =~ "P-K5SGD" + refute result["popup"] =~ ">DB0SDA<" + end + + test "build_minimal_packet_data uses object_name for objects" do + packet = %{ + id: Ecto.UUID.generate(), + sender: "DB0SDA", + base_callsign: "DB0SDA", + ssid: "", + object_name: "P-K5SGD", + is_object: true, + is_item: false, + lat: 33.169333, + lon: -96.492167, + data_type: "object", + received_at: DateTime.utc_now(), + symbol_table_id: "P", + symbol_code: "#", + comment: "DAPNET POCSAG Transmitter", + has_position: true, + path: "TCPIP*,qAC,FIFTH" + } + + result = DataBuilder.build_minimal_packet_data(packet, true, false) + + assert result + assert result["callsign"] == "P-K5SGD" + end + + test "build_packet_data_list groups by object_name for objects" do + now = DateTime.utc_now() + earlier = DateTime.add(now, -600, :second) + + object_packet_1 = %{ + id: Ecto.UUID.generate(), + sender: "DB0SDA", + base_callsign: "DB0SDA", + ssid: "", + object_name: "P-K5SGD", + is_object: true, + is_item: false, + lat: 33.169333, + lon: -96.492167, + data_type: "object", + received_at: now, + symbol_table_id: "P", + symbol_code: "#", + comment: "DAPNET POCSAG Transmitter", + has_position: true, + path: "TCPIP*,qAC,FIFTH" + } + + object_packet_2 = %{ + id: Ecto.UUID.generate(), + sender: "DB0SDA", + base_callsign: "DB0SDA", + ssid: "", + object_name: "P-K5SGD", + is_object: true, + is_item: false, + lat: 33.170, + lon: -96.493, + data_type: "object", + received_at: earlier, + symbol_table_id: "P", + symbol_code: "#", + comment: "DAPNET POCSAG Transmitter", + has_position: true, + path: "TCPIP*,qAC,FIFTH" + } + + results = DataBuilder.build_packet_data_list([object_packet_1, object_packet_2]) + + # Both packets should be grouped under P-K5SGD, not DB0SDA + callsigns = Enum.map(results, & &1["callsign"]) + assert Enum.all?(callsigns, &(&1 == "P-K5SGD")) + end + end +end diff --git a/vendor/aprs b/vendor/aprs index c2a544b..0cb7064 160000 --- a/vendor/aprs +++ b/vendor/aprs @@ -1 +1 @@ -Subproject commit c2a544ba31bc1eced480f3321256d74bd15de62a +Subproject commit 0cb706494c365305924c677d72a01c23f54ee621