diff --git a/lib/aprs/packet.ex b/lib/aprs/packet.ex index d15c4c9..9936fd4 100644 --- a/lib/aprs/packet.ex +++ b/lib/aprs/packet.ex @@ -134,7 +134,7 @@ defmodule Aprs.Packet do data_extended = get_change(changeset, :data_extended) if data_extended do - {data_extended.latitude, data_extended.longitude} + {data_extended[:latitude], data_extended[:longitude]} else {nil, nil} end diff --git a/lib/aprs/packets.ex b/lib/aprs/packets.ex index a0d652b..1122c16 100644 --- a/lib/aprs/packets.ex +++ b/lib/aprs/packets.ex @@ -94,7 +94,7 @@ defmodule Aprs.Packets do cond do # Standard position format is_map(data_extended) and not is_nil(data_extended[:latitude]) and not is_nil(data_extended[:longitude]) -> - {to_float(data_extended.latitude), to_float(data_extended.longitude)} + {to_float(data_extended[:latitude]), to_float(data_extended[:longitude])} # MicE packet format with components is_map(data_extended) and data_extended.__struct__ == Parser.Types.MicE -> diff --git a/lib/types/mic_e.ex b/lib/types/mic_e.ex index 59112c5..643c3dd 100644 --- a/lib/types/mic_e.ex +++ b/lib/types/mic_e.ex @@ -31,16 +31,24 @@ defmodule Parser.Types.MicE do """ def fetch(mic_e, :latitude) do # Calculate decimal latitude from components - lat = mic_e.lat_degrees + mic_e.lat_minutes / 60.0 - lat = if mic_e.lat_direction == :south, do: -lat, else: lat - {:ok, lat} + if is_number(mic_e.lat_degrees) and is_number(mic_e.lat_minutes) do + lat = mic_e.lat_degrees + mic_e.lat_minutes / 60.0 + lat = if mic_e.lat_direction == :south, do: -lat, else: lat + {:ok, lat} + else + :error + end end def fetch(mic_e, :longitude) do # Calculate decimal longitude from components - lon = mic_e.lon_degrees + mic_e.lon_minutes / 60.0 - lon = if mic_e.lon_direction == :west, do: -lon, else: lon - {:ok, lon} + if is_number(mic_e.lon_degrees) and is_number(mic_e.lon_minutes) do + lon = mic_e.lon_degrees + mic_e.lon_minutes / 60.0 + lon = if mic_e.lon_direction == :west, do: -lon, else: lon + {:ok, lon} + else + :error + end end def fetch(mic_e, key) when is_atom(key) do @@ -65,12 +73,16 @@ defmodule Parser.Types.MicE do value = case key do :latitude -> - lat = mic_e.lat_degrees + mic_e.lat_minutes / 60.0 - if mic_e.lat_direction == :south, do: -lat, else: lat + if is_number(mic_e.lat_degrees) and is_number(mic_e.lat_minutes) do + lat = mic_e.lat_degrees + mic_e.lat_minutes / 60.0 + if mic_e.lat_direction == :south, do: -lat, else: lat + end :longitude -> - lon = mic_e.lon_degrees + mic_e.lon_minutes / 60.0 - if mic_e.lon_direction == :west, do: -lon, else: lon + if is_number(mic_e.lon_degrees) and is_number(mic_e.lon_minutes) do + lon = mic_e.lon_degrees + mic_e.lon_minutes / 60.0 + if mic_e.lon_direction == :west, do: -lon, else: lon + end key when is_binary(key) -> # Handle string keys by converting to atom if it exists diff --git a/test/aprs/packets_mic_e_test.exs b/test/aprs/packets_mic_e_test.exs new file mode 100644 index 0000000..1d018c1 --- /dev/null +++ b/test/aprs/packets_mic_e_test.exs @@ -0,0 +1,164 @@ +defmodule Aprs.PacketsMicETest do + use Aprs.DataCase + + alias Aprs.Packets + alias Parser.Types.MicE + + describe "store_packet/1 with MicE data" do + test "stores packet with MicE data_extended successfully" do + # This test reproduces the exact error scenario from the bug report + mic_e_data = %MicE{ + lat_degrees: 49, + lat_minutes: 14, + lat_fractional: 72, + lat_direction: :north, + lon_direction: :east, + longitude_offset: 100, + message_code: "M02", + message_description: "In Service", + dti: "`", + heading: 0, + lon_degrees: 12, + lon_minutes: 5, + lon_fractional: 75, + speed: 0, + manufacturer: "Kenwood TH-D74A", + message: ">Harald QRV R1298,625" + } + + packet_data = %{ + base_callsign: "DG1ID", + ssid: "9", + sender: "DG1ID-9", + destination: "APRS", + data_type: "mic_e", + path: "TCPIP*", + information_field: "MicE packet data", + data_extended: mic_e_data + } + + # This should not raise a KeyError + assert {:ok, stored_packet} = Packets.store_packet(packet_data) + + # Verify the packet was stored with correct position data + assert stored_packet.sender == "DG1ID-9" + assert stored_packet.has_position == true + + # Verify coordinates were calculated correctly from MicE components + expected_lat = 49.0 + 14.0 / 60.0 + expected_lon = 12.0 + 5.0 / 60.0 + + assert_in_delta stored_packet.lat, expected_lat, 0.001 + assert_in_delta stored_packet.lon, expected_lon, 0.001 + end + + test "handles MicE data with south/west coordinates" do + mic_e_data = %MicE{ + lat_degrees: 34, + lat_minutes: 30, + lat_fractional: 0, + lat_direction: :south, + lon_direction: :west, + longitude_offset: 100, + message_code: "M01", + message_description: "En Route", + dti: "`", + heading: 90, + lon_degrees: 118, + lon_minutes: 15, + lon_fractional: 30, + speed: 25, + manufacturer: "Kenwood TH-D74A", + message: "Test message" + } + + packet_data = %{ + base_callsign: "TEST", + ssid: "1", + sender: "TEST-1", + destination: "APRS", + data_type: "mic_e", + path: "WIDE1-1,WIDE2-2", + information_field: "MicE test packet", + data_extended: mic_e_data + } + + assert {:ok, stored_packet} = Packets.store_packet(packet_data) + + # Verify south latitude is negative + expected_lat = -(34.0 + 30.0 / 60.0) + # Verify west longitude is negative + expected_lon = -(118.0 + 15.0 / 60.0) + + assert_in_delta stored_packet.lat, expected_lat, 0.001 + assert_in_delta stored_packet.lon, expected_lon, 0.001 + assert stored_packet.lat < 0 + assert stored_packet.lon < 0 + end + + test "handles MicE data with missing position components gracefully" do + # Test with incomplete MicE data (missing some coordinate components) + incomplete_mic_e = %MicE{ + lat_degrees: nil, + lat_minutes: 14, + lat_fractional: 72, + lat_direction: :north, + lon_direction: :east, + longitude_offset: 100, + message_code: "M02", + message_description: "In Service", + dti: "`", + heading: 0, + lon_degrees: 12, + lon_minutes: 5, + lon_fractional: 75, + speed: 0, + manufacturer: "Kenwood TH-D74A", + message: "Test incomplete" + } + + packet_data = %{ + base_callsign: "TEST", + ssid: "2", + sender: "TEST-2", + destination: "APRS", + data_type: "mic_e", + path: "TCPIP*", + information_field: "Incomplete MicE", + data_extended: incomplete_mic_e + } + + # Should still store the packet, but without position data + assert {:ok, stored_packet} = Packets.store_packet(packet_data) + assert stored_packet.sender == "TEST-2" + # Should not have position data when components are missing + assert stored_packet.has_position != true || stored_packet.has_position == nil + end + + test "MicE Access behavior works correctly" do + # Test the Access behavior implementation directly + mic_e = %MicE{ + lat_degrees: 40, + lat_minutes: 30, + lat_fractional: 0, + lat_direction: :north, + lon_degrees: 74, + lon_minutes: 15, + lon_fractional: 0, + lon_direction: :west + } + + # Test bracket notation (which triggers Access behavior) + assert mic_e[:latitude] == 40.5 + assert mic_e[:longitude] == -74.25 + + # Test that we can access both calculated and direct fields + assert mic_e[:lat_degrees] == 40 + assert mic_e[:message_code] == nil + + # Test get_in works + assert get_in(mic_e, [:latitude]) == 40.5 + assert get_in(mic_e, [:longitude]) == -74.25 + end + end +end diff --git a/test/parser/parser_test.exs b/test/parser/parser_test.exs index 0957cbc..cf48829 100644 --- a/test/parser/parser_test.exs +++ b/test/parser/parser_test.exs @@ -52,6 +52,46 @@ defmodule Parser.ParserTest do assert %MicE{} = sut end + test "mic_e latitude/longitude access" do + # Test the Access behavior for MicE structs + mic_e = %MicE{ + lat_degrees: 49, + lat_minutes: 14, + lat_fractional: 72, + lat_direction: :north, + lon_degrees: 12, + lon_minutes: 5, + lon_fractional: 75, + lon_direction: :east + } + + # Test bracket notation access (should work) + assert mic_e[:latitude] == 49 + 14 / 60.0 + assert mic_e[:longitude] == 12 + 5 / 60.0 + + # Test that latitude/longitude are calculated correctly + expected_lat = 49.0 + 14.0 / 60.0 + expected_lon = 12.0 + 5.0 / 60.0 + + assert_in_delta mic_e[:latitude], expected_lat, 0.001 + assert_in_delta mic_e[:longitude], expected_lon, 0.001 + + # Test south/west directions + mic_e_sw = %MicE{ + lat_degrees: 49, + lat_minutes: 14, + lat_fractional: 72, + lat_direction: :south, + lon_degrees: 12, + lon_minutes: 5, + lon_fractional: 75, + lon_direction: :west + } + + assert mic_e_sw[:latitude] < 0 + assert mic_e_sw[:longitude] < 0 + end + test "weird format" do aprs_message = ~s(ON4AVM-11>APDI23,WIDE1-1,WIDE2-2,qAR,ON0LB-10:=S4`k!OZ,C# sT/A=000049DIXPRS 2.3.0b\n)