From 6ea9889369d73af2ed7032ce4088272b6e55eee7 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Wed, 18 Feb 2026 14:11:25 -0500 Subject: [PATCH] make consumer call signature consistent. --- aprsd/client/drivers/kiss_common.py | 20 +++++++++++++++-- aprsd/client/drivers/serialkiss.py | 24 ++++++++++++++++++--- aprsd/client/drivers/tcpkiss.py | 19 +++++++++++++++- aprsd/threads/rx.py | 18 ++++++++++++++-- tests/client/drivers/test_kiss_common.py | 10 ++++++--- tests/client/drivers/test_tcpkiss_driver.py | 10 ++++++--- 6 files changed, 87 insertions(+), 14 deletions(-) diff --git a/aprsd/client/drivers/kiss_common.py b/aprsd/client/drivers/kiss_common.py index ed63d72..fe9c26e 100644 --- a/aprsd/client/drivers/kiss_common.py +++ b/aprsd/client/drivers/kiss_common.py @@ -131,10 +131,19 @@ class KISSDriver(metaclass=trace.TraceWrapperMetaclass): """Start consuming frames with the given callback. Args: - callback: Function to call with received packets + callback: Function to call with received packets. + Called with frame= keyword argument to match + the signature used by other drivers (packet=). + raw: If True, callback receives raw frame data. + If False, callback receives decoded packet. Raises: Exception: If not connected to KISS TNC + + Note: + The callback signature should accept keyword arguments: + - For raw frames: callback(frame=frame_obj) + - For decoded packets: callback(packet=packet_obj) """ # Ensure connection if not self._connected: @@ -144,7 +153,14 @@ class KISSDriver(metaclass=trace.TraceWrapperMetaclass): frame = self.read_frame() if frame: LOG.info(f'GOT FRAME: {frame} calling {callback}') - callback(frame) + if raw: + # Pass raw frame with keyword argument for consistency + callback(frame=frame) + else: + # Decode frame to packet and pass with keyword argument + packet = self.decode_packet(frame) + if packet: + callback(packet=packet) def read_frame(self): """Read a frame from the KISS interface. diff --git a/aprsd/client/drivers/serialkiss.py b/aprsd/client/drivers/serialkiss.py index 97467be..c332f7e 100644 --- a/aprsd/client/drivers/serialkiss.py +++ b/aprsd/client/drivers/serialkiss.py @@ -89,7 +89,16 @@ class SerialKISSDriver(KISSDriver): pass def setup_connection(self): - """Set up the KISS interface.""" + """Set up the KISS interface. + + This is the Protocol-defined method that initializes the connection. + It internally calls connect() to establish the actual serial connection. + + Note: + This method follows the ClientDriver Protocol. Use this method + for standard connection setup. The connect() method is an internal + KISS-specific helper for establishing the serial port connection. + """ if not self.is_enabled(): LOG.error('KISS is not enabled in configuration') return @@ -99,7 +108,7 @@ class SerialKISSDriver(KISSDriver): return try: - # Configure for TCP KISS + # Configure for Serial KISS if self.is_enabled(): LOG.info( f'Serial KISS Connection to {CONF.kiss_serial.device}:{CONF.kiss_serial.baudrate}' @@ -116,7 +125,16 @@ class SerialKISSDriver(KISSDriver): self._connected = False def connect(self): - """Connect to the KISS interface.""" + """Establish serial connection to the KISS device. + + This is a KISS-specific internal method that handles the low-level + serial port connection. It is called by setup_connection(). + + Note: + This method is NOT part of the ClientDriver Protocol. It is specific + to KISS drivers and handles serial port establishment and configuration. + External code should use setup_connection() instead. + """ if not self.is_enabled(): LOG.error('KISS is not enabled in configuration') return diff --git a/aprsd/client/drivers/tcpkiss.py b/aprsd/client/drivers/tcpkiss.py index 976ccbe..044bd1d 100644 --- a/aprsd/client/drivers/tcpkiss.py +++ b/aprsd/client/drivers/tcpkiss.py @@ -145,7 +145,16 @@ class TCPKISSDriver(KISSDriver): return True def setup_connection(self): - """Set up the KISS interface.""" + """Set up the KISS interface. + + This is the Protocol-defined method that initializes the connection. + It internally calls connect() to establish the actual TCP connection. + + Note: + This method follows the ClientDriver Protocol. Use this method + for standard connection setup. The connect() method is an internal + KISS-specific helper for establishing the TCP socket connection. + """ if not self.is_enabled(): LOG.error('KISS is not enabled in configuration') return @@ -186,6 +195,14 @@ class TCPKISSDriver(KISSDriver): def connect(self) -> bool: """Establish TCP connection to the KISS host. + This is a KISS-specific internal method that handles the low-level + socket connection. It is called by setup_connection(). + + Note: + This method is NOT part of the ClientDriver Protocol. It is specific + to KISS drivers and handles TCP socket establishment and configuration. + External code should use setup_connection() instead. + Returns: bool: True if connection successful, False otherwise """ diff --git a/aprsd/threads/rx.py b/aprsd/threads/rx.py index 0878502..21a7614 100644 --- a/aprsd/threads/rx.py +++ b/aprsd/threads/rx.py @@ -94,12 +94,26 @@ class APRSDRXThread(APRSDThread): """Put the raw packet on the queue. The processing of the packet will happen in a separate thread. + + Accepts packets/frames as either: + - Positional arg: process_packet(frame) + - Keyword args: process_packet(raw=frame), process_packet(frame=frame), or process_packet(packet=pkt) """ - if not args: + # Extract the packet/frame from either args or kwargs + if args: + data = args[0] + elif 'raw' in kwargs: + data = kwargs['raw'] + elif 'frame' in kwargs: + data = kwargs['frame'] + elif 'packet' in kwargs: + data = kwargs['packet'] + else: LOG.warning('No frame received to process?!?!') return + self.pkt_count += 1 - self.packet_queue.put(args[0]) + self.packet_queue.put(data) class APRSDFilterThread(APRSDThread): diff --git a/tests/client/drivers/test_kiss_common.py b/tests/client/drivers/test_kiss_common.py index d7cd65b..5224e9a 100644 --- a/tests/client/drivers/test_kiss_common.py +++ b/tests/client/drivers/test_kiss_common.py @@ -175,11 +175,15 @@ class TestKISSDriver(unittest.TestCase): self.driver._connected = True callback = mock.MagicMock() mock_frame = b'test_frame' + mock_packet = mock.MagicMock() with mock.patch.object(self.driver, 'read_frame', return_value=mock_frame): - with mock.patch('aprsd.client.drivers.kiss_common.LOG'): - self.driver.consumer(callback) - callback.assert_called() + with mock.patch.object( + self.driver, 'decode_packet', return_value=mock_packet + ): + with mock.patch('aprsd.client.drivers.kiss_common.LOG'): + self.driver.consumer(callback) + callback.assert_called_once_with(packet=mock_packet) def test_read_frame_not_implemented(self): """Test read_frame() raises NotImplementedError.""" diff --git a/tests/client/drivers/test_tcpkiss_driver.py b/tests/client/drivers/test_tcpkiss_driver.py index f2f8f80..019bc5b 100644 --- a/tests/client/drivers/test_tcpkiss_driver.py +++ b/tests/client/drivers/test_tcpkiss_driver.py @@ -373,6 +373,7 @@ class TestTCPKISSDriver(unittest.TestCase): """Test consumer processes frames and calls callback.""" mock_callback = mock.MagicMock() mock_frame = mock.MagicMock() + mock_packet = mock.MagicMock() # Configure driver for test self.driver._connected = True @@ -386,10 +387,13 @@ class TestTCPKISSDriver(unittest.TestCase): with mock.patch.object( self.driver, 'read_frame', side_effect=side_effect ) as mock_read_frame: - self.driver.consumer(mock_callback) + with mock.patch.object( + self.driver, 'decode_packet', return_value=mock_packet + ): + self.driver.consumer(mock_callback) - mock_read_frame.assert_called_once() - mock_callback.assert_called_once_with(mock_frame) + mock_read_frame.assert_called_once() + mock_callback.assert_called_once_with(packet=mock_packet) @mock.patch('aprsd.client.drivers.tcpkiss.LOG') def test_read_frame_success(self, mock_log):