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

@@ -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
   """

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

lib/aprsme_web/live/info_live/show.html.heex

@@ -136,7 +136,7 @@
                   <dt class="text-xs font-medium text-gray-500 dark:text-gray-400">{gettext("Comment")}</dt>
                   <dd class="mt-1 text-sm">
                     <%= if is_binary(@packet.comment) do %>
-                      {Aprsme.EncodingUtils.sanitize_string(@packet.comment)}
+                      {Aprsme.EncodingUtils.sanitize_comment(@packet.comment)}
                     <% else %>
                       {@packet.comment}
                     <% end %>

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)

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
 

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

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

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