update for 4.2.3

This patch fixes some issues with the base client class
that caused unnecessary reconnections.

ChangeLog.md

@@ -4,6 +4,27 @@ All notable changes to this project will be documented in this file. Dates are d
 
 Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 
+#### [4.2.3](https://github.com/craigerl/aprsd/compare/4.2.2...4.2.3)
+
+> 12 October 2025
+
+- Remove printf from tcpkiss [`4b9e7fe`](https://github.com/craigerl/aprsd/commit/4b9e7fee4e85dc75d8ae79b8b2bbc1c593c00026)
+- Added line numbers in trace [`9a1c096`](https://github.com/craigerl/aprsd/commit/9a1c0961e6a51d3cb50acee423b3b36a41aabde3)
+- Stop all collectors on signal [`643e19b`](https://github.com/craigerl/aprsd/commit/643e19b0ac17d4a86a8d3931c4f2ec2fb8c9bceb)
+- Added stop_all to stats collector. [`eb8104b`](https://github.com/craigerl/aprsd/commit/eb8104be2f4e21ad2055a01f1514d23d46bbd8d8)
+- Cleanup of tcpkiss [`3d353dc`](https://github.com/craigerl/aprsd/commit/3d353dcd26b93bcfd9130708f221c48084353ffc)
+- Driver protocol is_alive is property [`7c5d9ee`](https://github.com/craigerl/aprsd/commit/7c5d9ee92d138055d8388bab22c06cd4a5213103)
+- Make fake driver conform to protocol [`093ada0`](https://github.com/craigerl/aprsd/commit/093ada06b107f01e852a51f7a9446ae51cfae96a)
+- Fixed aprsis client connected tracking [`056acc3`](https://github.com/craigerl/aprsd/commit/056acc3ba55395d168ebc72a4b9a5db93e78e144)
+- Fixed client base class connection tracking [`24019ae`](https://github.com/craigerl/aprsd/commit/24019ae353fd0fb1c4667d8549ecab19ec945f79)
+- Fixed unit tests [`961d3e9`](https://github.com/craigerl/aprsd/commit/961d3e946a73cead8f5a42a338bfdbfc369e58fd)
+
+#### [4.2.2](https://github.com/craigerl/aprsd/compare/4.2.1...4.2.2)
+
+> 8 October 2025
+
+- Fixed an issue with client.reset [`4920256`](https://github.com/craigerl/aprsd/commit/49202569a81d1d8f982efe2c6d015f44f6ec4a5a)
+
 #### [4.2.1](https://github.com/craigerl/aprsd/compare/4.2.0...4.2.1)
 
 > 7 October 2025
@@ -17,6 +38,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 - Added package [`e15322e`](https://github.com/craigerl/aprsd/commit/e15322ede384d6e463c42e9496502e2c6ea0886c)
 - Some client and driver cleanup. [`328c027`](https://github.com/craigerl/aprsd/commit/328c027ad3db594f3d5622a2cab7cafb21d1cfd6)
 - Fixed some unit tests [`af0feaf`](https://github.com/craigerl/aprsd/commit/af0feaf9c81b8b9f443e58fbe7d23f1ff7ba63d2)
+- Update Changelog for 4.2.1 release [`9bf4bfd`](https://github.com/craigerl/aprsd/commit/9bf4bfd92c847e70b6cb674632a0f5a12fd5b748)
 
 #### [4.2.0](https://github.com/craigerl/aprsd/compare/4.1.2...4.2.0)
 

aprsd/client/client.py

@@ -10,14 +10,14 @@ from oslo_config import cfg
 from aprsd.client import drivers  # noqa - ensure drivers are registered
 from aprsd.client.drivers.registry import DriverRegistry
 from aprsd.packets import core
-from aprsd.utils import keepalive_collector
+from aprsd.utils import keepalive_collector, trace
 
 CONF = cfg.CONF
 LOG = logging.getLogger('APRSD')
 LOGU = logger
 
 
-class APRSDClient:
+class APRSDClient(metaclass=trace.TraceWrapperMetaclass):
     """APRSD client class.
 
     This is a singleton class that provides a single instance of the APRSD client.
@@ -41,11 +41,13 @@ class APRSDClient:
     def __init__(self, auto_connect: bool = True):
         self.auto_connect = auto_connect
         self.connected = False
+        self.running = False
         self.login_status = {
             'success': False,
             'message': None,
         }
-        self.driver = DriverRegistry().get_driver()
+        if not self.driver:
+            self.driver = DriverRegistry().get_driver()
         if self.auto_connect:
             self.connect()
 
@@ -100,14 +102,20 @@ class APRSDClient:
         return self.driver.filter
 
     def is_alive(self):
-        return self.driver.is_alive()
+        return self.driver.is_alive
 
+    @wrapt.synchronized(lock)
     def connect(self):
         if not self.driver:
             self.driver = DriverRegistry().get_driver()
-        self.driver.setup_connection()
+        if not self.connected:
+            self.driver.setup_connection()
+            self.connected = self.driver.is_alive
+        self.running = True
 
     def close(self):
+        self.running = False
+        self.connected = False
         if not self.driver:
             return
         self.driver.close()
@@ -115,7 +123,7 @@ class APRSDClient:
     @wrapt.synchronized(lock)
     def reset(self):
         """Call this to force a rebuild/reconnect."""
-        LOG.info('Resetting client connection.')
+        LOG.warning('Resetting client connection.')
         if self.driver:
             self.driver.close()
             if self.auto_connect:
@@ -126,7 +134,11 @@ class APRSDClient:
             LOG.warning('Client not initialized, nothing to reset.')
 
     def send(self, packet: core.Packet) -> bool:
-        return self.driver.send(packet)
+        if self.running:
+            return self.driver.send(packet)
+        else:
+            LOG.error('Client not running, not sending packet.')
+            return False
 
     # For the keepalive collector
     def keepalive_check(self):
@@ -145,7 +157,8 @@ class APRSDClient:
         LOGU.opt(colors=True).info(f'<green>Client keepalive {keepalive}</green>')
 
     def consumer(self, callback: Callable, raw: bool = False):
-        return self.driver.consumer(callback=callback, raw=raw)
+        if self.running:
+            return self.driver.consumer(callback=callback, raw=raw)
 
     def decode_packet(self, *args, **kwargs) -> core.Packet:
         try:

aprsd/client/drivers/aprsis.py

@@ -10,6 +10,7 @@ from oslo_config import cfg
 from aprsd import client, exception
 from aprsd.client.drivers.lib.aprslib import APRSLibClient
 from aprsd.packets import core
+from aprsd.utils import singleton
 
 CONF = cfg.CONF
 LOG = logging.getLogger('APRSD')
@@ -17,6 +18,7 @@ LOGU = logger
 
 
 # class APRSISDriver(metaclass=trace.TraceWrapperMetaclass):
+@singleton
 class APRSISDriver:
     """This is the APRS-IS driver for the APRSD client.
 
@@ -78,16 +80,18 @@ class APRSISDriver:
         if self._client:
             self._client.stop()
             self._client.close()
+        self.connected = False
 
     def send(self, packet: core.Packet) -> bool:
         return self._client.send(packet)
 
     def setup_connection(self):
+        if self.connected:
+            return
         user = CONF.aprs_network.login
         password = CONF.aprs_network.password
         host = CONF.aprs_network.host
         port = CONF.aprs_network.port
-        self.connected = False
         backoff = 1
         retries = 3
         retry_count = 0
@@ -97,7 +101,7 @@ class APRSISDriver:
                 break
             try:
                 LOG.info(
-                    f'Creating aprslib client({host}:{port}) and logging in {user}.'
+                    f'Creating aprslib client({host}:{port}) and logging in {user}. try #{retry_count}'
                 )
                 self._client = APRSLibClient(
                     user, passwd=password, host=host, port=port
@@ -152,7 +156,7 @@ class APRSISDriver:
     def _is_stale_connection(self):
         delta = datetime.datetime.now() - self._client.aprsd_keepalive
         if delta > self.max_delta:
-            LOG.error(f'Connection is stale, last heard {delta} ago.')
+            LOG.warning(f'Connection is stale, last heard {delta} ago.')
             return True
         return False
 

aprsd/client/drivers/fake.py

@@ -64,7 +64,7 @@ class APRSDFakeDriver(metaclass=trace.TraceWrapperMetaclass):
         return None
 
     @wrapt.synchronized(lock)
-    def send(self, packet: core.Packet):
+    def send(self, packet: core.Packet) -> bool:
         """Send an APRS Message object."""
         LOG.info(f'Sending packet: {packet}')
         payload = None
@@ -84,6 +84,7 @@ class APRSDFakeDriver(metaclass=trace.TraceWrapperMetaclass):
             f"FAKE::Send '{payload}' TO '{packet.to_call}' From "
             f'\'{packet.from_call}\' with PATH "{self.path}"',
         )
+        return True
 
     def consumer(self, callback: Callable, raw: bool = False):
         LOG.debug('Start non blocking FAKE consumer')

aprsd/client/drivers/registry.py

@@ -20,6 +20,7 @@ class ClientDriver(Protocol):
     def is_configured(self) -> bool:
         pass
 
+    @property
     def is_alive(self) -> bool:
         pass
 

aprsd/client/drivers/tcpkiss.py

@@ -9,7 +9,6 @@ import datetime
 import logging
 import select
 import socket
-import time
 from typing import Any, Callable, Dict
 
 import aprslib
@@ -25,6 +24,7 @@ from aprsd import (  # noqa
     exception,
 )
 from aprsd.packets import core
+from aprsd.utils import trace
 
 CONF = cfg.CONF
 LOG = logging.getLogger('APRSD')
@@ -46,10 +46,12 @@ def handle_fend(buffer: bytes, strip_df_start: bool = True) -> bytes:
     return bytes(frame)
 
 
-# class TCPKISSDriver(metaclass=trace.TraceWrapperMetaclass):
-class TCPKISSDriver:
+class TCPKISSDriver(metaclass=trace.TraceWrapperMetaclass):
+    # class TCPKISSDriver:
     """APRSD client driver for TCP KISS connections."""
 
+    _instance = None
+
     # Class level attributes required by Client protocol
     packets_received = 0
     packets_sent = 0
@@ -62,6 +64,12 @@ class TCPKISSDriver:
     select_timeout = 1
     path = None
 
+    def __new__(cls, *args, **kwargs):
+        """This magic turns this into a singleton."""
+        if cls._instance is None:
+            cls._instance = super().__new__(cls)
+        return cls._instance
+
     def __init__(self):
         """Initialize the KISS client.
 
@@ -71,7 +79,6 @@ class TCPKISSDriver:
         super().__init__()
         self._connected = False
         self.keepalive = datetime.datetime.now()
-        self._running = False
         # This is initialized in setup_connection()
         self.socket = None
 
@@ -111,7 +118,15 @@ class TCPKISSDriver:
 
     def close(self):
         """Close the connection."""
-        self.stop()
+        self._connected = False
+        if self.socket:
+            try:
+                self.socket.close()
+            except Exception as e:
+                LOG.error(f'close: error closing socket: {e}')
+                pass
+        else:
+            LOG.warning('close: socket not initialized. no reason to close.')
 
     def send(self, packet: core.Packet):
         """Send an APRS packet.
@@ -163,6 +178,10 @@ class TCPKISSDriver:
             LOG.error('KISS is not enabled in configuration')
             return
 
+        if self._connected:
+            LOG.warning('KISS interface already connected')
+            return
+
         try:
             # Configure for TCP KISS
             if self.is_enabled():
@@ -217,22 +236,18 @@ class TCPKISSDriver:
         Raises:
             Exception: If not connected to KISS TNC
         """
-        self._running = True
-        while self._running:
-            # Ensure connection
-            if not self._connected:
-                if not self.connect():
-                    time.sleep(1)
-                    continue
+        # Ensure connection
+        if not self._connected:
+            return
 
-            # Read frame
-            frame = self.read_frame()
-            if frame:
-                LOG.warning(f'GOT FRAME: {frame} calling {callback}')
-                kwargs = {
-                    'frame': frame,
-                }
-                callback(**kwargs)
+        # Read frame
+        frame = self.read_frame()
+        if frame:
+            LOG.info(f'GOT FRAME: {frame} calling {callback}')
+            kwargs = {
+                'frame': frame,
+            }
+            callback(**kwargs)
 
     def decode_packet(self, *args, **kwargs) -> core.Packet:
         """Decode a packet from an AX.25 frame.
@@ -245,7 +260,6 @@ class TCPKISSDriver:
             LOG.warning('No frame received to decode?!?!')
             return None
 
-        LOG.warning(f'FRAME: {str(frame)}')
         try:
             aprslib_frame = aprslib.parse(str(frame))
             packet = core.factory(aprslib_frame)
@@ -257,16 +271,6 @@ class TCPKISSDriver:
             LOG.error(f'Error decoding packet: {e}')
             return None
 
-    def stop(self):
-        """Stop the KISS interface."""
-        self._running = False
-        self._connected = False
-        if self.socket:
-            try:
-                self.socket.close()
-            except Exception:
-                pass
-
     def stats(self, serializable: bool = False) -> Dict[str, Any]:
         """Get client statistics.
 
@@ -353,13 +357,19 @@ class TCPKISSDriver:
         """
         Generator for complete lines, received from the server
         """
+        if not self.socket:
+            return None
+
+        if not self._connected:
+            return None
+
         try:
             self.socket.setblocking(0)
         except OSError as e:
             LOG.error(f'socket error when setblocking(0): {str(e)}')
             raise aprslib.ConnectionDrop('connection dropped') from e
 
-        while self._running:
+        while self._connected:
             short_buf = b''
 
             try:
@@ -375,14 +385,14 @@ class TCPKISSDriver:
                     else:
                         continue
             except Exception as e:
+                # No need to log if we are not running.
+                # this happens when the client is stopped/closed.
                 LOG.error(f'Error in read loop: {e}')
                 self._connected = False
                 break
 
             try:
-                print('reading from socket')
                 short_buf = self.socket.recv(1024)
-                print(f'short_buf: {short_buf}')
                 # sock.recv returns empty if the connection drops
                 if not short_buf:
                     if not blocking:

aprsd/main.py

@@ -72,6 +72,7 @@ def main():
 
 def signal_handler(sig, frame):
     click.echo('signal_handler: called')
+    collector.Collector().stop_all()
     threads.APRSDThreadList().stop_all()
     if 'subprocess' not in str(frame):
         LOG.info(

aprsd/stats/collector.py

@@ -44,3 +44,9 @@ class Collector:
         if not isinstance(producer_name, StatsProducer):
             raise TypeError(f'Producer {producer_name} is not a StatsProducer')
         self.producers.remove(producer_name)
+
+    def stop_all(self):
+        """Stop and unregister all registered stats producers."""
+        for producer in self.producers[:]:
+            LOG.info(f'Stopping Stats producer {producer}')
+            self.unregister_producer(producer)

aprsd/utils/trace.py

@@ -5,11 +5,11 @@ import logging
 import time
 import types
 
-VALID_TRACE_FLAGS = {"method", "api"}
+VALID_TRACE_FLAGS = {'method', 'api'}
 TRACE_API = False
 TRACE_METHOD = False
 TRACE_ENABLED = False
-LOG = logging.getLogger("APRSD")
+LOG = logging.getLogger('APRSD')
 
 
 def trace(*dec_args, **dec_kwargs):
@@ -27,11 +27,12 @@ def trace(*dec_args, **dec_kwargs):
 
     def _decorator(f):
         func_name = f.__qualname__
-        func_file = "/".join(f.__code__.co_filename.split("/")[-4:])
+        func_file = '/'.join(f.__code__.co_filename.split('/')[-4:])
+        func_line_number = f.__code__.co_firstlineno
 
         @functools.wraps(f)
         def trace_logging_wrapper(*args, **kwargs):
-            filter_function = dec_kwargs.get("filter_function")
+            filter_function = dec_kwargs.get('filter_function')
             logger = LOG
 
             # NOTE(ameade): Don't bother going any further if DEBUG log level
@@ -40,16 +41,16 @@ def trace(*dec_args, **dec_kwargs):
                 return f(*args, **kwargs)
 
             all_args = inspect.getcallargs(f, *args, **kwargs)
-
             pass_filter = filter_function is None or filter_function(all_args)
 
             if pass_filter:
                 logger.debug(
-                    "==> %(func)s: call %(all_args)r file: %(file)s",
+                    '==> %(func)s: call %(all_args)r file: %(file)s:%(line_number)d',
                     {
-                        "func": func_name,
-                        "all_args": str(all_args),
-                        "file": func_file,
+                        'func': func_name,
+                        'all_args': str(all_args),
+                        'file': func_file,
+                        'line_number': func_line_number,
                     },
                 )
 
@@ -59,11 +60,11 @@ def trace(*dec_args, **dec_kwargs):
             except Exception as exc:
                 total_time = int(round(time.time() * 1000)) - start_time
                 logger.debug(
-                    "<== %(func)s: exception (%(time)dms) %(exc)r",
+                    '<== %(func)s: exception (%(time)dms) %(exc)r',
                     {
-                        "func": func_name,
-                        "time": total_time,
-                        "exc": exc,
+                        'func': func_name,
+                        'time': total_time,
+                        'exc': exc,
                     },
                 )
                 raise
@@ -78,11 +79,11 @@ def trace(*dec_args, **dec_kwargs):
 
             if pass_filter:
                 logger.debug(
-                    "<== %(func)s: return (%(time)dms) %(result)r",
+                    '<== %(func)s: return (%(time)dms) %(result)r',
                     {
-                        "func": func_name,
-                        "time": total_time,
-                        "result": mask_result,
+                        'func': func_name,
+                        'time': total_time,
+                        'result': mask_result,
                     },
                 )
             return result
@@ -174,7 +175,7 @@ def setup_tracing(trace_flags):
     except TypeError:  # Handle when trace_flags is None or a test mock
         trace_flags = []
     for invalid_flag in set(trace_flags) - VALID_TRACE_FLAGS:
-        LOG.warning("Invalid trace flag: %s", invalid_flag)
-    TRACE_METHOD = "method" in trace_flags
-    TRACE_API = "api" in trace_flags
+        LOG.warning('Invalid trace flag: %s', invalid_flag)
+    TRACE_METHOD = 'method' in trace_flags
+    TRACE_API = 'api' in trace_flags
     TRACE_ENABLED = True

tests/client/drivers/test_aprsis_driver.py

@@ -33,10 +33,13 @@ class TestAPRSISDriver(unittest.TestCase):
 
         # Create an instance of the driver
         self.driver = APRSISDriver()
+        self.driver.connected = False
 
     def tearDown(self):
         self.conf_patcher.stop()
         self.aprslib_patcher.stop()
+        self.driver._client = None
+        self.driver = None
 
     def test_implements_client_driver_protocol(self):
         """Test that APRSISDriver implements the ClientDriver Protocol."""
@@ -324,7 +327,7 @@ class TestAPRSISDriver(unittest.TestCase):
         result = self.driver._is_stale_connection()
 
         self.assertTrue(result)
-        mock_log.error.assert_called_once()
+        mock_log.warning.assert_called_once()
 
     def test_is_stale_connection_false(self):
         """Test _is_stale_connection returns False when connection is not stale."""

tests/client/drivers/test_tcpkiss_driver.py

@@ -118,9 +118,10 @@ class TestTCPKISSDriver(unittest.TestCase):
 
     def test_close(self):
         """Test close method calls stop."""
-        with mock.patch.object(self.driver, 'stop') as mock_stop:
+        with mock.patch.object(self.driver, 'socket') as mock_socket:
             self.driver.close()
-            mock_stop.assert_called_once()
+            mock_socket.close.assert_called_once()
+            self.assertFalse(self.driver._connected)
 
     @mock.patch('aprsd.client.drivers.tcpkiss.LOG')
     def test_setup_connection_success(self, mock_log):
@@ -230,18 +231,6 @@ class TestTCPKISSDriver(unittest.TestCase):
             self.driver.send(mock_packet)
         self.assertIn('KISS interface not initialized', str(context.exception))
 
-    def test_stop(self):
-        """Test stop method cleans up properly."""
-        self.driver._running = True
-        self.driver._connected = True
-        self.driver.socket = self.mock_socket
-
-        self.driver.stop()
-
-        self.assertFalse(self.driver._running)
-        self.assertFalse(self.driver._connected)
-        self.mock_socket.close.assert_called_once()
-
     def test_stats(self):
         """Test stats method returns correct data."""
         # Set up test data
@@ -401,42 +390,12 @@ class TestTCPKISSDriver(unittest.TestCase):
             mock_read_frame.assert_called_once()
             mock_callback.assert_called_once_with(frame=mock_frame)
 
-    @mock.patch('aprsd.client.drivers.tcpkiss.LOG')
-    def test_consumer_with_connect_reconnect(self, mock_log):
-        """Test consumer tries to reconnect when not connected."""
-        mock_callback = mock.MagicMock()
-
-        # Configure driver for test
-        self.driver._connected = False
-
-        # Setup to run once then stop
-        call_count = 0
-
-        def connect_side_effect():
-            nonlocal call_count
-            call_count += 1
-            # On second call, connect successfully
-            if call_count == 2:
-                self.driver._running = False
-                self.driver.socket = self.mock_socket
-                return True
-            return False
-
-        with mock.patch.object(
-            self.driver, 'connect', side_effect=connect_side_effect
-        ) as mock_connect:
-            with mock.patch('aprsd.client.drivers.tcpkiss.time.sleep') as mock_sleep:
-                self.driver.consumer(mock_callback)
-
-                self.assertEqual(mock_connect.call_count, 2)
-                mock_sleep.assert_called_once_with(1)
-
     @mock.patch('aprsd.client.drivers.tcpkiss.LOG')
     def test_read_frame_success(self, mock_log):
         """Test read_frame successfully reads a frame."""
         # Set up driver
         self.driver.socket = self.mock_socket
-        self.driver._running = True
+        self.driver._connected = True
 
         # Mock socket recv to return data
         raw_data = b'\xc0\x00test_frame\xc0'
@@ -484,7 +443,7 @@ class TestTCPKISSDriver(unittest.TestCase):
         """Test read_frame handles socket error."""
         # Set up driver
         self.driver.socket = self.mock_socket
-        self.driver._running = True
+        self.driver._connected = True
 
         # Mock setblocking to raise OSError
         self.mock_socket.setblocking.side_effect = OSError('Test error')