From 7da91f5ccbe050bcd53b46db1aed2f450f5767d6 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Thu, 10 Jul 2025 16:30:48 -0500
Subject: [PATCH] fix some accidental weather decoded packets

---
 lib/aprsme/packet.ex        |  36 +++---------
 test/aprsme/packet_test.exs | 107 ++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 28 deletions(-)
 create mode 100644 test/aprsme/packet_test.exs

diff --git a/lib/aprsme/packet.ex b/lib/aprsme/packet.ex
index 45b7a47..c80f029 100644
--- a/lib/aprsme/packet.ex
+++ b/lib/aprsme/packet.ex
@@ -311,31 +311,12 @@ defmodule Aprsme.Packet do
       |> put_equipment_fields(combined_data)
       |> put_message_fields(combined_data)
 
-    # If any weather fields are present, set data_type to "weather"
-    weather_fields = [
-      :temperature,
-      :humidity,
-      :wind_speed,
-      :wind_direction,
-      :wind_gust,
-      :pressure,
-      :rain_1h,
-      :rain_24h,
-      :rain_since_midnight,
-      :snow
-    ]
-
-    has_weather =
-      Enum.any?(weather_fields, fn field ->
-        v = result[field] || result[to_string(field)]
-        not is_nil(v)
-      end)
-
-    if has_weather and (result[:data_type] != "weather" and result["data_type"] != "weather") do
-      Map.put(result, :data_type, "weather")
-    else
-      result
-    end
+    # Don't override data_type - trust the APRS parser's determination
+    # The parser already correctly identifies weather packets by:
+    # 1. Data type indicator (e.g., "_" for weather)
+    # 2. Symbol (e.g., "/_" for weather station)
+    # 3. Actual weather data patterns in the content
+    result
   end
 
   defp put_symbol_fields(map, data_extended) do
@@ -443,9 +424,8 @@ defmodule Aprsme.Packet do
   defp process_map_weather_data(attrs, weather) do
     weather = Map.drop(weather, [:raw_weather_data, "raw_weather_data"])
 
-    attrs
-    |> Map.merge(weather)
-    |> Map.put(:data_type, "weather")
+    # Only merge weather data, don't override data_type
+    Map.merge(attrs, weather)
   end
 
   # Helper to put a value only if it's not nil
diff --git a/test/aprsme/packet_test.exs b/test/aprsme/packet_test.exs
new file mode 100644
index 0000000..f89d4f2
--- /dev/null
+++ b/test/aprsme/packet_test.exs
@@ -0,0 +1,107 @@
+defmodule Aprsme.PacketTest do
+  use Aprsme.DataCase, async: true
+
+  alias Aprsme.Packet
+
+  describe "extract_additional_data/2" do
+    test "position packet with course/speed should not be classified as weather" do
+      # This is the problematic packet from KG5GKC-12
+      raw_packet = "KG5GKC-12>APAT51,WIDE1-1,WIDE2-2,qAO,NI2C-10:!3310.04N/09640.40Wk038/023/A=000623AT-MOBILE KG5GKC@YAHOO.COM"
+      
+      # Simulate the parsed data from APRS parser
+      attrs = %{
+        sender: "KG5GKC-12",
+        destination: "APAT51",
+        path: "WIDE1-1,WIDE2-2,qAO,NI2C-10",
+        information_field: "!3310.04N/09640.40Wk038/023/A=000623AT-MOBILE KG5GKC@YAHOO.COM",
+        data_type: :position,
+        base_callsign: "KG5GKC",
+        ssid: "12",
+        data_extended: %{
+          latitude: Decimal.new("33.167333"),
+          longitude: Decimal.new("-96.673333"),
+          symbol_table_id: "/",
+          symbol_code: "k",
+          comment: "038/023/A=000623AT-MOBILE KG5GKC@YAHOO.COM",
+          course: 38,
+          speed: 23.0,
+          data_type: :position,
+          has_position: true
+        }
+      }
+
+      result = Packet.extract_additional_data(attrs, raw_packet)
+      
+      # The data_type should be "position" (normalized to string)
+      assert result[:data_type] == "position" or result[:data_type] == :position
+      assert result[:symbol_code] == "k"
+      assert result[:symbol_table_id] == "/"
+      assert result[:course] == 38
+      assert result[:speed] == 23.0
+      assert result[:raw_packet] == raw_packet
+    end
+
+    test "actual weather packet should be classified as weather" do
+      raw_packet = "KC0ABC>APRS:_10090556c220s004g005t077P000h50b09900"
+      
+      attrs = %{
+        sender: "KC0ABC",
+        destination: "APRS",
+        path: "",
+        information_field: "_10090556c220s004g005t077P000h50b09900",
+        data_type: :weather,
+        base_callsign: "KC0ABC",
+        ssid: nil,
+        data_extended: %{
+          timestamp: "10090556",
+          wind_direction: 220,
+          wind_speed: 4.0,
+          wind_gust: 5.0,
+          temperature: 77.0,
+          rain_since_midnight: 0.0,
+          humidity: 50.0,
+          pressure: 990.0,
+          data_type: :weather
+        }
+      }
+
+      result = Packet.extract_additional_data(attrs, raw_packet)
+      
+      # Weather packet should remain as weather (normalized to string)
+      assert result[:data_type] == "weather" or result[:data_type] == :weather
+      assert result[:temperature] == 77.0
+      assert result[:humidity] == 50.0
+      assert result[:pressure] == 990.0
+    end
+
+    test "position packet with weather symbol should use parser's determination" do
+      raw_packet = "KC0ABC>APRS:=3310.04N/09640.40W_PHG5130"
+      
+      attrs = %{
+        sender: "KC0ABC",
+        destination: "APRS",
+        path: "",
+        information_field: "=3310.04N/09640.40W_PHG5130",
+        data_type: :position,  # Parser determined this is position despite weather symbol
+        base_callsign: "KC0ABC",
+        ssid: nil,
+        data_extended: %{
+          latitude: Decimal.new("33.167333"),
+          longitude: Decimal.new("-96.673333"),
+          symbol_table_id: "/",
+          symbol_code: "_",
+          comment: "PHG5130",
+          data_type: :position,
+          has_position: true
+        }
+      }
+
+      result = Packet.extract_additional_data(attrs, raw_packet)
+      
+      # Should trust parser's determination (normalized to string)
+      assert result[:data_type] == "position" or result[:data_type] == :position
+      assert result[:symbol_code] == "_"
+    end
+  end
+
+end
\ No newline at end of file