more tests

.cursor/rules/elixir.mdc

@@ -3,4 +3,4 @@ description:
 globs: 
 alwaysApply: true
 ---
-This is an elixir & phoenix project that uses esbuild and as much liveview as possible. Do not directly try to interact with npm. Ensure liveview is used as much as possible to minimize javascript. When writing or changing code, ensure the tests still pass and you did not break other parts of the code by running "mix test".
+This is an elixir & phoenix project that uses esbuild and as much liveview as possible. Do not directly try to interact with npm. Ensure liveview is used as much as possible to minimize javascript. When writing or changing code, ensure the tests still pass and you did not break other parts of the code by running "mix test". Ensure you are writing idiomatic elixir and using as much function and pattern matching as possible instead of if/case conditionals. Keep your responses breif and do not say "you're right" or echo back what I've said. Keep the output short and sweet only telling me issues that come up or what you've done. Fix any compile warnings you create or encounter as well. Do not be emotional, be straight to the point and do not use fluff words.

lib/aprs/packets.ex

@@ -275,6 +275,7 @@ defmodule Aprs.Packets do
       * `:limit` - Maximum number of packets to return
       * `:page` - Page number for pagination
   """
+  @impl true
   def get_packets_for_replay(opts \\ %{}) do
     base_query = from(p in Packet, order_by: [asc: p.received_at], where: p.has_position == true)
 
@@ -293,6 +294,7 @@ defmodule Aprs.Packets do
   @doc """
   Gets historical packet count for a map area.
   """
+  @impl true
   @spec get_historical_packet_count(map()) :: non_neg_integer()
   def get_historical_packet_count(opts \\ %{}) do
     base_query = from(p in Packet, select: count(p.id), where: p.has_position == true)
@@ -343,6 +345,7 @@ defmodule Aprs.Packets do
   Gets recent packets for the map view.
   This is used for initial map loading to show only recent packets.
   """
+  @impl true
   @spec get_recent_packets(map()) :: [struct()]
   def get_recent_packets(opts \\ %{}) do
     # Always limit to the last hour
@@ -367,6 +370,7 @@ defmodule Aprs.Packets do
   ## Returns
     * Stream of packets with timing information
   """
+  @impl true
   def stream_packets_for_replay(opts \\ %{}) do
     packets = get_packets_for_replay(opts)
     playback_speed = Map.get(opts, :playback_speed, 1.0)
@@ -440,6 +444,7 @@ defmodule Aprs.Packets do
   - Default retention is 365 days (1 year) (configurable via :packet_retention_days)
   - Returns the number of packets deleted
   """
+  @impl true
   def clean_old_packets do
     retention_days = Application.get_env(:aprs, :packet_retention_days, 365)
     cutoff_time = DateTime.add(DateTime.utc_now(), -retention_days * 86_400, :second)
@@ -463,6 +468,7 @@ defmodule Aprs.Packets do
   ## Returns
   - Number of packets deleted
   """
+  @impl true
   @spec clean_packets_older_than(pos_integer()) :: non_neg_integer()
   def clean_packets_older_than(days) when is_integer(days) and days > 0 do
     cutoff_time = DateTime.add(DateTime.utc_now(), -days * 86_400, :second)

lib/aprs/packets_behaviour.ex

@@ -8,4 +8,6 @@ defmodule Aprs.PacketsBehaviour do
   @callback get_recent_packets(map()) :: [Aprs.Packet.t()]
   @callback get_historical_packet_count(map()) :: non_neg_integer()
   @callback stream_packets_for_replay(map()) :: Enumerable.t()
+  @callback clean_old_packets() :: non_neg_integer()
+  @callback clean_packets_older_than(pos_integer()) :: non_neg_integer()
 end

lib/aprs/workers/packet_cleanup_worker.ex

@@ -19,7 +19,6 @@ defmodule Aprs.Workers.PacketCleanupWorker do
   # Import modules needed for database operations
 
   alias Aprs.Packet
-  alias Aprs.Packets
   alias Aprs.Repo
 
   require Logger
@@ -33,7 +32,7 @@ defmodule Aprs.Workers.PacketCleanupWorker do
     _total_before = count_total_packets()
 
     # Perform the cleanup of old packets with custom age
-    deleted_count = Packets.clean_packets_older_than(days)
+    deleted_count = packets_module().clean_packets_older_than(days)
 
     # Log results
     Logger.info("APRS packet cleanup complete: removed #{deleted_count} packets older than #{days} days")
@@ -54,6 +53,7 @@ defmodule Aprs.Workers.PacketCleanupWorker do
 
     # Log results
     retention_days = Application.get_env(:aprs, :packet_retention_days, 365)
+
     Logger.info("APRS packet cleanup complete: removed #{deleted_count} packets older than #{retention_days} days")
 
     # Return success
@@ -66,7 +66,7 @@ defmodule Aprs.Workers.PacketCleanupWorker do
 
   defp cleanup_old_packets do
     # Use the existing function from Packets context
-    Packets.clean_old_packets()
+    packets_module().clean_old_packets()
   end
 
   @doc """
@@ -84,10 +84,14 @@ defmodule Aprs.Workers.PacketCleanupWorker do
   def cleanup_packets_older_than(days) when is_integer(days) and days > 0 do
     Logger.info("Starting APRS packet cleanup for packets older than #{days} days")
 
-    deleted_count = Packets.clean_packets_older_than(days)
+    deleted_count = packets_module().clean_packets_older_than(days)
 
     Logger.info("APRS packet cleanup complete: removed #{deleted_count} packets older than #{days} days")
 
     deleted_count
   end
+
+  defp packets_module do
+    Application.get_env(:aprs, :packets_module, Aprs.Packets)
+  end
 end

lib/parser/helpers.ex

@@ -108,18 +108,22 @@ defmodule Parser.Helpers do
   def parse_df_strength(s), do: {nil, "Unknown strength: #{<<s>>}"}
 
   # Compressed position helpers
-  @spec convert_compressed_lat(binary()) :: float()
-  def convert_compressed_lat(lat) do
+  @spec convert_compressed_lat(binary()) :: {:ok, float()} | {:error, String.t()}
+  def convert_compressed_lat(lat) when is_binary(lat) and byte_size(lat) == 4 do
     [l1, l2, l3, l4] = to_charlist(lat)
-    90 - ((l1 - 33) * 91 ** 3 + (l2 - 33) * 91 ** 2 + (l3 - 33) * 91 + l4 - 33) / 380_926
+    {:ok, 90 - ((l1 - 33) * 91 ** 3 + (l2 - 33) * 91 ** 2 + (l3 - 33) * 91 + l4 - 33) / 380_926}
   end
 
-  @spec convert_compressed_lon(binary()) :: float()
-  def convert_compressed_lon(lon) do
+  def convert_compressed_lat(_), do: {:error, "Invalid compressed latitude"}
+
+  @spec convert_compressed_lon(binary()) :: {:ok, float()} | {:error, String.t()}
+  def convert_compressed_lon(lon) when is_binary(lon) and byte_size(lon) == 4 do
     [l1, l2, l3, l4] = to_charlist(lon)
-    -180 + ((l1 - 33) * 91 ** 3 + (l2 - 33) * 91 ** 2 + (l3 - 33) * 91 + l4 - 33) / 190_463
+    {:ok, -180 + ((l1 - 33) * 91 ** 3 + (l2 - 33) * 91 ** 2 + (l3 - 33) * 91 + l4 - 33) / 190_463}
   end
 
+  def convert_compressed_lon(_), do: {:error, "Invalid compressed longitude"}
+
   # PEET Logging parsing
   @spec parse_peet_logging(binary()) :: map()
   def parse_peet_logging(<<"*", peet_data::binary>>) do

test/aprs/workers/packet_cleanup_worker_test.exs

@@ -0,0 +1,49 @@
+defmodule Aprs.Workers.PacketCleanupWorkerTest do
+  use Aprs.DataCase, async: true
+
+  import Mox
+
+  alias Aprs.PacketsMock
+  alias Aprs.Workers.PacketCleanupWorker
+
+  # Make sure mocks are verified when the test exits
+  setup :verify_on_exit!
+
+  describe "perform/1 with cleanup_days" do
+    test "cleans up packets older than the specified number of days" do
+      days = 30
+      job = %Oban.Job{args: %{"cleanup_days" => days}}
+
+      expect(PacketsMock, :clean_packets_older_than, fn ^days ->
+        5
+      end)
+
+      assert :ok = PacketCleanupWorker.perform(job)
+    end
+  end
+
+  describe "perform/1 without cleanup_days" do
+    test "cleans up packets using the default retention period" do
+      job = %Oban.Job{args: %{}}
+
+      expect(PacketsMock, :clean_old_packets, fn ->
+        10
+      end)
+
+      assert :ok = PacketCleanupWorker.perform(job)
+    end
+  end
+
+  describe "cleanup_packets_older_than/1" do
+    test "cleans up packets older than the given number of days" do
+      days = 60
+      deleted_count = 15
+
+      expect(PacketsMock, :clean_packets_older_than, fn ^days ->
+        deleted_count
+      end)
+
+      assert ^deleted_count = PacketCleanupWorker.cleanup_packets_older_than(days)
+    end
+  end
+end

test/parser/helpers_test.exs

@@ -1,6 +1,5 @@
 defmodule Parser.HelpersTest do
   use ExUnit.Case, async: true
-  use ExUnitProperties
 
   alias Parser.Helpers
 
@@ -12,21 +11,13 @@ defmodule Parser.HelpersTest do
     end
 
     test "parse_nmea_coordinate errors on invalid input" do
-      assert Helpers.parse_nmea_coordinate("bad", "N") ==
-               {:error, "Invalid coordinate value"}
-
-      assert Helpers.parse_nmea_coordinate("4916.45", "Q") ==
-               {:error, "Invalid coordinate direction"}
-
-      assert Helpers.parse_nmea_coordinate(nil, nil) ==
-               {:error, "Invalid coordinate format"}
+      assert Helpers.parse_nmea_coordinate("bad", "N") == {:error, "Invalid coordinate value"}
+      assert Helpers.parse_nmea_coordinate("4916.45", "Q") == {:error, "Invalid coordinate direction"}
+      assert Helpers.parse_nmea_coordinate(nil, nil) == {:error, "Invalid coordinate format"}
     end
 
-    property "parse_nmea_coordinate returns ok or error for any string" do
-      check all v <- StreamData.string(:printable), d <- StreamData.string(:printable) do
-        result = Helpers.parse_nmea_coordinate(v, d)
-        assert match?({:ok, _}, result) or match?({:error, _}, result)
-      end
+    test "parse_nmea_sentence returns not implemented" do
+      assert Helpers.parse_nmea_sentence("anything") == {:error, "NMEA parsing not implemented"}
     end
   end
 
@@ -63,22 +54,34 @@ defmodule Parser.HelpersTest do
   end
 
   describe "compressed position helpers" do
-    test "convert_compressed_lat and lon return floats" do
-      lat = Helpers.convert_compressed_lat("abcd")
-      lon = Helpers.convert_compressed_lon("abcd")
-      assert is_struct(lat, Decimal) or is_nil(lat) or is_number(lat)
-      assert is_struct(lon, Decimal) or is_nil(lon) or is_number(lon)
+    test "convert_compressed_lat and lon handle valid input" do
+      {:ok, lat} = Helpers.convert_compressed_lat("9RZV")
+      {:ok, lon} = Helpers.convert_compressed_lon("9RZV")
+      assert is_float(lat)
+      assert is_float(lon)
+      assert lat >= -90 and lat <= 90
+      assert lon >= -180 and lon <= 180
+    end
+
+    test "convert_compressed_lat and lon return error for invalid input" do
+      assert Helpers.convert_compressed_lat("abcde") == {:error, "Invalid compressed latitude"}
+      assert Helpers.convert_compressed_lon("abcde") == {:error, "Invalid compressed longitude"}
+      assert Helpers.convert_compressed_lat(123) == {:error, "Invalid compressed latitude"}
+      assert Helpers.convert_compressed_lon(123) == {:error, "Invalid compressed longitude"}
     end
   end
 
   describe "KISS/TNC2 utilities" do
     test "kiss_to_tnc2 decodes KISS frames" do
       frame = <<0xC0, 0x00, 65, 66, 67, 0xC0>>
-      assert Parser.Helpers.kiss_to_tnc2(frame) == "ABC"
+      assert Helpers.kiss_to_tnc2(frame) == "ABC"
+
+      frame = <<0xC0, 0x00, 0x41, 0xDB, 0xDC, 0x42, 0xDB, 0xDD, 0x43, 0xC0>>
+      assert Helpers.kiss_to_tnc2(frame) == "A\xC0B\xDB" <> "C"
     end
 
     test "kiss_to_tnc2 returns error for invalid frame" do
-      assert Parser.Helpers.kiss_to_tnc2("notkiss") == %{
+      assert Helpers.kiss_to_tnc2("notkiss") == %{
                error_code: :packet_invalid,
                error_message: "Unknown error"
              }
@@ -86,9 +89,180 @@ defmodule Parser.HelpersTest do
 
     test "tnc2_to_kiss encodes TNC2 to KISS" do
       tnc2 = "A\xDBB\xC0C"
-      kiss = Parser.Helpers.tnc2_to_kiss(tnc2)
+      kiss = Helpers.tnc2_to_kiss(tnc2)
       assert :binary.part(kiss, 0, 1) == <<0xC0>>
       assert :binary.part(kiss, byte_size(kiss) - 1, 1) == <<0xC0>>
     end
   end
+
+  describe "telemetry helpers" do
+    test "parse_telemetry_sequence parses valid values" do
+      assert Helpers.parse_telemetry_sequence("123") == 123
+      assert Helpers.parse_telemetry_sequence("abc") == nil
+    end
+
+    test "parse_analog_values parses valid values" do
+      assert Helpers.parse_analog_values(["1.23", "4.56", ""]) == [1.23, 4.56, nil]
+      assert Helpers.parse_analog_values(["abc", "1.23"]) == [nil, 1.23]
+    end
+
+    test "parse_digital_values parses valid values" do
+      assert Helpers.parse_digital_values(["101"]) == [true, false, true]
+      assert Helpers.parse_digital_values(["1", "0"]) == [true, false]
+      assert Helpers.parse_digital_values(["abc"]) == [nil, nil, nil]
+      assert Helpers.parse_digital_values([123]) == [nil]
+    end
+
+    test "parse_coefficient parses valid values" do
+      assert Helpers.parse_coefficient("1.23") == 1.23
+      assert Helpers.parse_coefficient("123") == 123
+      assert Helpers.parse_coefficient("abc") == "abc"
+    end
+  end
+
+  describe "weather helpers" do
+    test "extract_timestamp handles valid formats" do
+      assert Helpers.extract_timestamp("092345z123") == "092345z"
+      assert Helpers.extract_timestamp("092345h123") == "092345h"
+      assert Helpers.extract_timestamp("092345/123") == "092345/"
+      assert Helpers.extract_timestamp("12345") == nil
+    end
+
+    test "remove_timestamp removes timestamp when present" do
+      assert Helpers.remove_timestamp("092345z123") == "123"
+      assert Helpers.remove_timestamp("092345h456") == "456"
+      assert Helpers.remove_timestamp("092345/789") == "789"
+      assert Helpers.remove_timestamp("12345") == "12345"
+    end
+
+    test "parse_wind_direction extracts direction" do
+      assert Helpers.parse_wind_direction("123/045") == 123
+      assert Helpers.parse_wind_direction("invalid") == nil
+    end
+
+    test "parse_wind_speed extracts speed" do
+      assert Helpers.parse_wind_speed("123/045") == 45
+      assert Helpers.parse_wind_speed("invalid") == nil
+    end
+
+    test "parse_wind_gust extracts gust speed" do
+      assert Helpers.parse_wind_gust("g045") == 45
+      assert Helpers.parse_wind_gust("invalid") == nil
+    end
+
+    test "parse_temperature extracts temperature" do
+      assert Helpers.parse_temperature("t072") == 72
+      assert Helpers.parse_temperature("invalid") == nil
+    end
+
+    test "parse_rainfall_1h extracts rainfall" do
+      assert Helpers.parse_rainfall_1h("r010") == 10
+      assert Helpers.parse_rainfall_1h("invalid") == nil
+    end
+
+    test "parse_rainfall_24h extracts rainfall" do
+      assert Helpers.parse_rainfall_24h("p024") == 24
+      assert Helpers.parse_rainfall_24h("invalid") == nil
+    end
+
+    test "parse_rainfall_since_midnight extracts rainfall" do
+      assert Helpers.parse_rainfall_since_midnight("P036") == 36
+      assert Helpers.parse_rainfall_since_midnight("invalid") == nil
+    end
+
+    test "parse_humidity extracts humidity" do
+      assert Helpers.parse_humidity("h00") == 100
+      assert Helpers.parse_humidity("h75") == 75
+      assert Helpers.parse_humidity("invalid") == nil
+    end
+
+    test "parse_pressure extracts pressure" do
+      assert Helpers.parse_pressure("b10150") == 1015.0
+      assert Helpers.parse_pressure("invalid") == nil
+    end
+
+    test "parse_luminosity extracts luminosity" do
+      assert Helpers.parse_luminosity("l123") == 123
+      assert Helpers.parse_luminosity("L456") == 456
+      assert Helpers.parse_luminosity("invalid") == nil
+    end
+
+    test "parse_snow extracts snow depth" do
+      assert Helpers.parse_snow("s012") == 12
+      assert Helpers.parse_snow("invalid") == nil
+    end
+  end
+
+  describe "PEET logging helpers" do
+    test "parse_peet_logging handles data with and without prefix" do
+      assert Helpers.parse_peet_logging("*1234") == %{peet_data: "1234", data_type: :peet_logging}
+      assert Helpers.parse_peet_logging("1234") == %{peet_data: "1234", data_type: :peet_logging}
+    end
+  end
+
+  describe "invalid/test data helpers" do
+    test "parse_invalid_test_data handles data with and without comma" do
+      assert Helpers.parse_invalid_test_data(",1234") == %{
+               test_data: "1234",
+               data_type: :invalid_test_data
+             }
+
+      assert Helpers.parse_invalid_test_data("1234") == %{
+               test_data: "1234",
+               data_type: :invalid_test_data
+             }
+    end
+  end
+
+  describe "utility helpers" do
+    test "count_spaces counts spaces correctly" do
+      assert Helpers.count_spaces("a b c") == 2
+      assert Helpers.count_spaces("abc") == 0
+      assert Helpers.count_spaces(" ") == 1
+    end
+
+    test "count_leading_braces counts braces correctly" do
+      assert Helpers.count_leading_braces("}}abc") == 2
+      assert Helpers.count_leading_braces("abc") == 0
+      assert Helpers.count_leading_braces("}a}bc") == 1
+    end
+
+    test "calculate_position_ambiguity determines ambiguity level" do
+      assert Helpers.calculate_position_ambiguity("12 34.56N", "123 45.67W") == 1
+      assert Helpers.calculate_position_ambiguity("12  34.56N", "123  45.67W") == 2
+      assert Helpers.calculate_position_ambiguity("12   34.56N", "123   45.67W") == 3
+      assert Helpers.calculate_position_ambiguity("12    34.56N", "123    45.67W") == 4
+      assert Helpers.calculate_position_ambiguity("1234.56N", "12345.67W") == 0
+    end
+
+    test "calculate_compressed_ambiguity determines compressed ambiguity" do
+      assert Helpers.calculate_compressed_ambiguity(" ") == 0
+      assert Helpers.calculate_compressed_ambiguity("!") == 1
+      assert Helpers.calculate_compressed_ambiguity("\"") == 2
+      assert Helpers.calculate_compressed_ambiguity("#") == 3
+      assert Helpers.calculate_compressed_ambiguity("$") == 4
+      assert Helpers.calculate_compressed_ambiguity("X") == 0
+    end
+
+    test "validate_position_data validates positions" do
+      assert {:ok, {lat, lon}} = Helpers.validate_position_data("4916.45N", "12311.12W")
+      assert_in_delta Decimal.to_float(lat), 49.2742, 0.0001
+      assert_in_delta Decimal.to_float(lon), -123.1853, 0.0001
+
+      assert Helpers.validate_position_data("invalid", "12311.12W") == {:error, :invalid_position}
+      assert Helpers.validate_position_data("4916.45N", "invalid") == {:error, :invalid_position}
+    end
+
+    test "validate_timestamp always returns nil" do
+      assert Helpers.validate_timestamp("any") == nil
+    end
+
+    test "find_matches handles named and unnamed captures" do
+      regex = ~r/(?<val>\d+)/
+      assert Helpers.find_matches(regex, "abc123def") == %{"val" => "123"}
+
+      regex = ~r/(\d+)/
+      assert Helpers.find_matches(regex, "abc123def") == %{0 => "123", 1 => "123"}
+    end
+  end
 end