From 48943cdc52b439336c3d0e7f7f5322a92a3873a4 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 23 Apr 2026 14:57:30 -0500 Subject: [PATCH] refactor: Is.get_status pattern-matches on process presence The original get_status/0 had a nested case/try that duplicated the "disconnected" status map across two branches. Split into: - status_from/1 with nil + is_pid heads for the "no GenServer" and "GenServer alive" paths. The alive path isolates the catch :exit, _ fallback for a non-responding process. - disconnected_status/1 builds the shared status map once, parameterised on the default server string to preserve the original branch-specific fallback behavior. Removes ~20 duplicated lines; all 61 Is tests still pass. --- lib/aprsme/is/is.ex | 72 +++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/lib/aprsme/is/is.ex b/lib/aprsme/is/is.ex index a29cc22..b84f4d0 100644 --- a/lib/aprsme/is/is.ex +++ b/lib/aprsme/is/is.ex @@ -125,50 +125,40 @@ defmodule Aprsme.Is do end def get_status do - case Process.whereis(__MODULE__) do - nil -> - # GenServer is not running (disconnected) - server = Application.get_env(:aprsme, :aprs_is_server, nil) - port = Application.get_env(:aprsme, :aprs_is_port, 14_580) - {stored_packet_count, oldest_packet_timestamp} = safe_packet_storage_stats() + status_from(Process.whereis(__MODULE__)) + end - %{ - connected: false, - server: server_to_string(server), - port: port, - connected_at: nil, - uptime_seconds: 0, - login_id: Application.get_env(:aprsme, :aprs_is_login_id, "W5ISP"), - filter: Application.get_env(:aprsme, :aprs_is_default_filter, "r/33/-96/100"), - packet_stats: default_packet_stats(), - stored_packet_count: stored_packet_count, - oldest_packet_timestamp: oldest_packet_timestamp - } + # GenServer isn't running — return a disconnected snapshot. + defp status_from(nil), do: disconnected_status(nil) - _pid -> - try do - GenServer.call(__MODULE__, :get_status, 5000) - catch - :exit, _ -> - # GenServer exists but not responding - server = Application.get_env(:aprsme, :aprs_is_server, ~c"rotate.aprs2.net") - port = Application.get_env(:aprsme, :aprs_is_port, 14_580) - {stored_packet_count, oldest_packet_timestamp} = safe_packet_storage_stats() + defp status_from(pid) when is_pid(pid) do + GenServer.call(__MODULE__, :get_status, 5000) + catch + # GenServer exists but isn't responding — treat as disconnected but + # fall back to the rotate.aprs2.net default server string. + :exit, _ -> disconnected_status(~c"rotate.aprs2.net") + end - %{ - connected: false, - server: server_to_string(server), - port: port, - connected_at: nil, - uptime_seconds: 0, - login_id: Application.get_env(:aprsme, :aprs_is_login_id, "W5ISP"), - filter: Application.get_env(:aprsme, :aprs_is_default_filter, "r/33/-96/100"), - packet_stats: default_packet_stats(), - stored_packet_count: stored_packet_count, - oldest_packet_timestamp: oldest_packet_timestamp - } - end - end + # Build the "I'm not connected" status map. `default_server` is only used + # when the application config has no server set; pass nil to mean "use + # whatever is in config, or nil". + defp disconnected_status(default_server) do + server = Application.get_env(:aprsme, :aprs_is_server, default_server) + port = Application.get_env(:aprsme, :aprs_is_port, 14_580) + {stored_packet_count, oldest_packet_timestamp} = safe_packet_storage_stats() + + %{ + connected: false, + server: server_to_string(server), + port: port, + connected_at: nil, + uptime_seconds: 0, + login_id: Application.get_env(:aprsme, :aprs_is_login_id, "W5ISP"), + filter: Application.get_env(:aprsme, :aprs_is_default_filter, "r/33/-96/100"), + packet_stats: default_packet_stats(), + stored_packet_count: stored_packet_count, + oldest_packet_timestamp: oldest_packet_timestamp + } end defp safe_packet_storage_stats do