From b45993a8bedcce351f5aa29486e8d0e43ef27a5d Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Fri, 20 Feb 2026 18:06:20 -0600 Subject: [PATCH] Fix APRS object display and comment encoding issues - Strip Mic-E telemetry from comments before database storage - Show sender callsign on map labels instead of object names for clarity - Object names still used for grouping to maintain trail organization - Add comprehensive tests for new encoding utilities Fixes display issues where weather balloon "X3234025" was shown instead of station "WA0YMH", and Mic-E telemetry bytes like "!w>`!" were appearing in comments. --- lib/aprsme/encoding.ex | 15 ++++++++ lib/aprsme/encoding_utils.ex | 27 ++++++++++++-- lib/aprsme_web/live/info_live/show.html.heex | 2 +- lib/aprsme_web/live/map_live/data_builder.ex | 30 ++++++++++++---- lib/aprsme_web/live/shared/packet_utils.ex | 17 +++++++++ test/aprsme/encoding_test.exs | 32 +++++++++++++++++ test/aprsme/encoding_utils_test.exs | 31 ++++++++++++++++ .../live/map_live/data_builder_test.exs | 36 +++++++++++++------ 8 files changed, 170 insertions(+), 20 deletions(-) diff --git a/lib/aprsme/encoding.ex b/lib/aprsme/encoding.ex index 8b342ae..3cb7055 100644 --- a/lib/aprsme/encoding.ex +++ b/lib/aprsme/encoding.ex @@ -19,6 +19,21 @@ defmodule Aprsme.Encoding do @spec sanitize_string(any()) :: binary() def sanitize_string(_), do: "" + @doc """ + Removes Mic-E telemetry data from APRS comments. + Mic-E telemetry appears as sequences like !w>`! at the start of comments. + The pattern is: ! followed by 3 characters followed by ! + """ + @spec strip_mice_telemetry(binary()) :: binary() + def strip_mice_telemetry(comment) when is_binary(comment) do + # Pattern: one or more sequences of !...! (5 chars each) at the start + # Examples: !w>`!, !w_'!, !w_P!, !w;i!, !w;D! + String.replace(comment, ~r/^(?:![^!]{3}!)+/, "") + end + + def strip_mice_telemetry(nil), do: nil + def strip_mice_telemetry(other), do: other + @doc """ Type-safe float conversion with validation """ diff --git a/lib/aprsme/encoding_utils.ex b/lib/aprsme/encoding_utils.ex index 36d1018..ae033a6 100644 --- a/lib/aprsme/encoding_utils.ex +++ b/lib/aprsme/encoding_utils.ex @@ -31,6 +31,27 @@ defmodule Aprsme.EncodingUtils do def sanitize_string(nil), do: nil def sanitize_string(other), do: other + @doc """ + Sanitizes a comment string by removing Mic-E telemetry and control characters. + + ## Examples + + iex> Aprsme.EncodingUtils.sanitize_comment("!w>`!Clb=6.4m/s t=-66.7C") + "Clb=6.4m/s t=-66.7C" + + iex> Aprsme.EncodingUtils.sanitize_comment("Normal comment") + "Normal comment" + """ + @spec sanitize_comment(binary() | nil | any()) :: binary() | nil | any() + def sanitize_comment(comment) when is_binary(comment) do + comment + |> Encoding.strip_mice_telemetry() + |> Encoding.sanitize_string() + end + + def sanitize_comment(nil), do: nil + def sanitize_comment(other), do: other + @doc """ Converts various types to float with validation for safety. @@ -261,13 +282,15 @@ defmodule Aprsme.EncodingUtils do defp sanitize_string_fields(packet) do Enum.reduce(@string_fields, packet, fn field, acc -> string_key = to_string(field) + # Use sanitize_comment for comment field to strip Mic-E telemetry + sanitizer = if field == :comment, do: &sanitize_comment/1, else: &sanitize_string/1 cond do Map.has_key?(acc, field) -> - Map.update(acc, field, nil, &sanitize_string/1) + Map.update(acc, field, nil, sanitizer) Map.has_key?(acc, string_key) -> - Map.update(acc, string_key, nil, &sanitize_string/1) + Map.update(acc, string_key, nil, sanitizer) true -> acc diff --git a/lib/aprsme_web/live/info_live/show.html.heex b/lib/aprsme_web/live/info_live/show.html.heex index ef1825b..865ae74 100644 --- a/lib/aprsme_web/live/info_live/show.html.heex +++ b/lib/aprsme_web/live/info_live/show.html.heex @@ -136,7 +136,7 @@
{gettext("Comment")}
<%= if is_binary(@packet.comment) do %> - {Aprsme.EncodingUtils.sanitize_string(@packet.comment)} + {Aprsme.EncodingUtils.sanitize_comment(@packet.comment)} <% else %> {@packet.comment} <% end %> diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 6a52d73..c48dc45 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -127,16 +127,22 @@ defmodule AprsmeWeb.MapLive.DataBuilder do historical_dot_html(callsign) end + raw_comment = get_packet_field(packet, :comment, "") + clean_comment = Aprsme.EncodingUtils.sanitize_comment(raw_comment) + + # Use map_label for display, but keep display_name for grouping + label = map_label(packet) + %{ "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" => callsign, + "callsign" => label, "callsign_group" => callsign, "symbol_table_id" => symbol_table_id, "symbol_code" => symbol_code, "symbol_html" => symbol_html, - "comment" => get_packet_field(packet, :comment, ""), + "comment" => clean_comment, "timestamp" => get_packet_timestamp_unix(packet), "historical" => !is_most_recent, "is_most_recent_for_callsign" => is_most_recent, @@ -263,9 +269,12 @@ defmodule AprsmeWeb.MapLive.DataBuilder do |> IO.iodata_to_binary() else # Build standard popup + raw_comment = get_packet_field(packet, :comment, "") + clean_comment = Aprsme.EncodingUtils.sanitize_comment(raw_comment) + %{ callsign: callsign, - comment: get_packet_field(packet, :comment, ""), + comment: clean_comment, timestamp_dt: timestamp_dt, cache_buster: cache_buster, weather: false, @@ -331,12 +340,16 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec extract_packet_info(map(), map()) :: map() defp extract_packet_info(packet, data_extended) do + raw_comment = get_packet_field(packet, :comment, "") + clean_comment = Aprsme.EncodingUtils.sanitize_comment(raw_comment) + %{ - callsign: display_name(packet), + callsign: map_label(packet), + callsign_group: 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), - comment: get_packet_field(packet, :comment, ""), + comment: clean_comment, safe_data_extended: convert_tuples_to_strings(data_extended), is_weather_packet: weather_packet?(packet) } @@ -447,7 +460,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do %{ "id" => packet_id, "callsign" => packet_info.callsign, - "callsign_group" => packet_info.callsign, + "callsign_group" => packet_info.callsign_group, "base_callsign" => get_packet_field(packet, :base_callsign, ""), "ssid" => get_packet_field(packet, :ssid, nil), "lat" => to_float(lat), @@ -492,6 +505,11 @@ defmodule AprsmeWeb.MapLive.DataBuilder do SharedPacketUtils.display_name(packet) end + @spec map_label(map()) :: String.t() + defp map_label(packet) do + SharedPacketUtils.map_label(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 a5f0031..768708a 100644 --- a/lib/aprsme_web/live/shared/packet_utils.ex +++ b/lib/aprsme_web/live/shared/packet_utils.ex @@ -232,6 +232,23 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do end end + @doc """ + Get the map label for a packet. + + For map display, always show the sender callsign regardless of whether + it's an object or item packet. This avoids confusion where the map label + shows something different from what the user expects (like a balloon ID + instead of the station callsign). + + If the packet is an object or item, you can append the object/item name + in the popup or detail view. + """ + @spec map_label(map()) :: String.t() + def map_label(packet) do + # Always return the sender for map labels + get_packet_field(packet, :sender, "") + end + defp non_empty_string?(value) when is_binary(value), do: String.trim(value) != "" defp non_empty_string?(_), do: false diff --git a/test/aprsme/encoding_test.exs b/test/aprsme/encoding_test.exs index d14271b..68aff7c 100644 --- a/test/aprsme/encoding_test.exs +++ b/test/aprsme/encoding_test.exs @@ -169,4 +169,36 @@ defmodule Aprsme.EncodingTest do assert Encoding.to_hex(<<255>>) == "FF" end end + + describe "strip_mice_telemetry/1" do + test "removes Mic-E telemetry prefix from comment" do + assert Encoding.strip_mice_telemetry("!w>`!Clb=6.4m/s t=-66.7C") == "Clb=6.4m/s t=-66.7C" + assert Encoding.strip_mice_telemetry("!w_'!Clb=4.6m/s t=-3.8C") == "Clb=4.6m/s t=-3.8C" + assert Encoding.strip_mice_telemetry("!w_P!Clb=4.8m/s t=-65.7C") == "Clb=4.8m/s t=-65.7C" + assert Encoding.strip_mice_telemetry("!w;i!Clb=2.5m/s t=-66.4C") == "Clb=2.5m/s t=-66.4C" + assert Encoding.strip_mice_telemetry("!w;D!Clb=2.8m/s t=-65.3C") == "Clb=2.8m/s t=-65.3C" + end + + test "removes multiple consecutive Mic-E telemetry sequences" do + assert Encoding.strip_mice_telemetry("!w>`!!abc!Normal comment") == "Normal comment" + end + + test "does not remove exclamation marks from middle of comment" do + assert Encoding.strip_mice_telemetry("Normal! comment!") == "Normal! comment!" + end + + test "leaves normal comments unchanged" do + assert Encoding.strip_mice_telemetry("Normal comment") == "Normal comment" + assert Encoding.strip_mice_telemetry("Hello World") == "Hello World" + assert Encoding.strip_mice_telemetry("") == "" + end + + test "handles nil input" do + assert Encoding.strip_mice_telemetry(nil) == nil + end + + test "handles non-binary input" do + assert Encoding.strip_mice_telemetry(123) == 123 + end + end end diff --git a/test/aprsme/encoding_utils_test.exs b/test/aprsme/encoding_utils_test.exs index ecb47f5..ca93d68 100644 --- a/test/aprsme/encoding_utils_test.exs +++ b/test/aprsme/encoding_utils_test.exs @@ -371,4 +371,35 @@ defmodule Aprsme.EncodingUtilsTest do assert info.char_count == 4 end end + + describe "sanitize_comment/1" do + test "removes Mic-E telemetry from comment" do + assert EncodingUtils.sanitize_comment("!w>`!Clb=6.4m/s t=-66.7C") == "Clb=6.4m/s t=-66.7C" + assert EncodingUtils.sanitize_comment("!w_'!Clb=4.6m/s t=-3.8C") == "Clb=4.6m/s t=-3.8C" + end + + test "sanitizes both Mic-E telemetry and control characters" do + # Comment with both Mic-E prefix and invalid UTF-8 + invalid_binary = "!w>`!" <> <<72, 101, 211, 108, 111>> + result = EncodingUtils.sanitize_comment(invalid_binary) + + assert String.valid?(result) + refute String.starts_with?(result, "!") + assert String.contains?(result, "He") + assert String.contains?(result, "lo") + end + + test "handles normal comments without Mic-E telemetry" do + assert EncodingUtils.sanitize_comment("Normal comment") == "Normal comment" + assert EncodingUtils.sanitize_comment("Hello World") == "Hello World" + end + + test "handles nil input" do + assert EncodingUtils.sanitize_comment(nil) == nil + end + + test "handles non-binary input" do + assert EncodingUtils.sanitize_comment(123) == 123 + end + end 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 index cd8f978..c79d81b 100644 --- a/test/aprsme_web/live/map_live/data_builder_test.exs +++ b/test/aprsme_web/live/map_live/data_builder_test.exs @@ -4,7 +4,7 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do alias AprsmeWeb.MapLive.DataBuilder describe "display name for APRS objects and items" do - test "uses object_name when packet is an object" do + test "uses sender for map label but object_name for grouping when packet is an object" do packet = %{ id: Ecto.UUID.generate(), sender: "DB0SDA", @@ -27,10 +27,13 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do result = DataBuilder.build_packet_data(packet, true) assert result - assert result["callsign"] == "P-K5SGD" + # Map label shows sender for clarity + assert result["callsign"] == "DB0SDA" + # But grouping uses object name + assert result["callsign_group"] == "P-K5SGD" end - test "uses item_name when packet is an item" do + test "uses sender for map label but item_name for grouping when packet is an item" do packet = %{ id: Ecto.UUID.generate(), sender: "N0CALL", @@ -53,7 +56,10 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do result = DataBuilder.build_packet_data(packet, true) assert result - assert result["callsign"] == "REPEATER1" + # Map label shows sender for clarity + assert result["callsign"] == "N0CALL" + # But grouping uses item name + assert result["callsign_group"] == "REPEATER1" end test "falls back to sender when no object_name or item_name" do @@ -83,7 +89,7 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do assert result["callsign"] == "K5SGD-Y" end - test "uses object_name in popup for object packets" do + test "uses sender in popup for object packets" do packet = %{ id: Ecto.UUID.generate(), sender: "DB0SDA", @@ -105,11 +111,12 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do result = DataBuilder.build_packet_data(packet, true) - assert result["popup"] =~ "P-K5SGD" - refute result["popup"] =~ ">DB0SDA<" + # Popup now shows sender for consistency with map label + assert result["popup"] =~ "DB0SDA" + refute result["popup"] =~ "P-K5SGD" end - test "build_minimal_packet_data uses object_name for objects" do + test "build_minimal_packet_data uses sender for map label but object_name for grouping" do packet = %{ id: Ecto.UUID.generate(), sender: "DB0SDA", @@ -132,7 +139,10 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do result = DataBuilder.build_minimal_packet_data(packet, true, false) assert result - assert result["callsign"] == "P-K5SGD" + # Map label shows sender for clarity + assert result["callsign"] == "DB0SDA" + # But grouping uses object name + assert result["callsign_group"] == "P-K5SGD" end test "build_minimal_packet_data uses red dot HTML for historical (non-most-recent) packets" do @@ -325,9 +335,13 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do results = DataBuilder.build_packet_data_list([object_packet_1, object_packet_2]) - # Both packets should be grouped under P-K5SGD, not DB0SDA + # Map labels should show sender callsigns = Enum.map(results, & &1["callsign"]) - assert Enum.all?(callsigns, &(&1 == "P-K5SGD")) + assert Enum.all?(callsigns, &(&1 == "DB0SDA")) + + # But grouping should be by object name + groups = Enum.map(results, & &1["callsign_group"]) + assert Enum.all?(groups, &(&1 == "P-K5SGD")) end end end