Skip to content

Commit adcc36c

Browse files
CopilotMikefly123
andauthored
Fix radio manager to handle oversized packets gracefully (#317)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mikefly123 <61564344+Mikefly123@users.noreply.github.com>
1 parent 9af3cbe commit adcc36c

File tree

3 files changed

+165
-0
lines changed

3 files changed

+165
-0
lines changed

circuitpython-workspaces/flight-software/src/pysquared/hardware/radio/manager/base.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ def send(self, data: bytes) -> bool:
7373
self._log.warning("Radio send attempt failed: Not licensed.")
7474
return False
7575

76+
# Check if data exceeds max packet size
77+
max_size = self.get_max_packet_size()
78+
if len(data) > max_size:
79+
self._log.warning(
80+
"Data exceeds max packet size, truncating",
81+
data_length=len(data),
82+
max_packet_size=max_size,
83+
)
84+
data = data[:max_size]
85+
7686
sent = self._send_internal(data)
7787

7888
if not sent:

cpython-workspaces/flight-software-unit-tests/src/unit-tests/hardware/radio/manager/test_radio_base.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
the default `get_max_packet_size` returns the correct value.
66
"""
77

8+
from unittest.mock import MagicMock
9+
810
import pytest
911
from pysquared.hardware.radio.manager.base import BaseRadioManager
1012
from pysquared.hardware.radio.modulation import LoRa
@@ -82,3 +84,94 @@ def test_get_max_packet_size():
8284

8385
# Check that get_max_packet_size returns the default packet size
8486
assert mock_manager.get_max_packet_size() == 128 # Default value
87+
88+
89+
def test_send_oversized_packet_truncates():
90+
"""Tests that oversized packets are truncated and a warning is logged.
91+
92+
This test verifies that when a packet larger than max_packet_size is sent,
93+
the BaseRadioManager logs a warning and truncates the data to max_packet_size
94+
before sending.
95+
"""
96+
# Create a mock instance of the BaseRadioManager
97+
mock_manager = BaseRadioManager.__new__(BaseRadioManager)
98+
mock_manager._log = MagicMock()
99+
mock_manager._radio_config = MagicMock()
100+
mock_manager._radio_config.license = "test_license"
101+
102+
# Mock get_max_packet_size to return a small value for testing
103+
mock_manager.get_max_packet_size = MagicMock(return_value=10)
104+
105+
# Mock _send_internal to capture what data is actually sent
106+
sent_data = []
107+
108+
def capture_send(data: bytes) -> bool:
109+
"""Captures sent data for verification."""
110+
sent_data.append(data)
111+
return True
112+
113+
mock_manager._send_internal = capture_send
114+
115+
# Create data larger than max packet size
116+
oversized_data = b"This is a message that is longer than 10 bytes"
117+
118+
# Send the oversized data
119+
result = mock_manager.send(oversized_data)
120+
121+
# Verify that send was successful
122+
assert result is True
123+
124+
# Verify that a warning was logged
125+
mock_manager._log.warning.assert_called_once()
126+
warning_call = mock_manager._log.warning.call_args
127+
assert warning_call[0][0] == "Data exceeds max packet size, truncating"
128+
assert warning_call[1]["data_length"] == len(oversized_data)
129+
assert warning_call[1]["max_packet_size"] == 10
130+
131+
# Verify that the data was truncated to max_packet_size
132+
assert len(sent_data) == 1
133+
assert sent_data[0] == oversized_data[:10]
134+
assert len(sent_data[0]) == 10
135+
136+
137+
def test_send_exact_size_packet_no_warning():
138+
"""Tests that packets at exactly max_packet_size do not trigger a warning.
139+
140+
This test verifies that when a packet of exactly max_packet_size is sent,
141+
no warning is logged and the data is sent as-is.
142+
"""
143+
# Create a mock instance of the BaseRadioManager
144+
mock_manager = BaseRadioManager.__new__(BaseRadioManager)
145+
mock_manager._log = MagicMock()
146+
mock_manager._radio_config = MagicMock()
147+
mock_manager._radio_config.license = "test_license"
148+
149+
# Mock get_max_packet_size to return a small value for testing
150+
mock_manager.get_max_packet_size = MagicMock(return_value=10)
151+
152+
# Mock _send_internal to capture what data is actually sent
153+
sent_data = []
154+
155+
def capture_send(data: bytes) -> bool:
156+
"""Captures sent data for verification."""
157+
sent_data.append(data)
158+
return True
159+
160+
mock_manager._send_internal = capture_send
161+
162+
# Create data exactly at max packet size
163+
exact_size_data = b"0123456789" # Exactly 10 bytes
164+
165+
# Send the data
166+
result = mock_manager.send(exact_size_data)
167+
168+
# Verify that send was successful
169+
assert result is True
170+
171+
# Verify that no warning was logged about truncation
172+
mock_manager._log.warning.assert_not_called()
173+
174+
# Verify that the data was sent as-is
175+
assert len(sent_data) == 1
176+
assert sent_data[0] == exact_size_data
177+
assert len(sent_data[0]) == 10

cpython-workspaces/flight-software-unit-tests/src/unit-tests/hardware/radio/manager/test_rfm9x_manager.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ def test_send_success_bytes(
361361
mock_radio_config.modulation = "LoRa"
362362
mock_radio_instance = MagicMock()
363363
mock_radio_instance.send = MagicMock()
364+
mock_radio_instance.max_packet_length = 252 # RFM9x max packet length
364365
mock_rfm9x.return_value = mock_radio_instance
365366

366367
manager = RFM9xManager(
@@ -440,6 +441,7 @@ def test_send_exception(
440441
mock_radio_config.modulation = "LoRa"
441442
mock_radio_instance = MagicMock()
442443
mock_radio_instance.send = MagicMock()
444+
mock_radio_instance.max_packet_length = 252 # RFM9x max packet length
443445
send_error = RuntimeError("SPI Error")
444446
mock_radio_instance.send.side_effect = send_error
445447
mock_rfm9x.return_value = mock_radio_instance
@@ -933,3 +935,63 @@ def test_get_rssi(
933935
rssi_value = manager.get_rssi()
934936

935937
assert rssi_value == expected_rssi
938+
939+
940+
def test_send_oversized_packet_truncates(
941+
mock_rfm9x: MagicMock,
942+
mock_logger: MagicMock,
943+
mock_spi: MagicMock,
944+
mock_chip_select: MagicMock,
945+
mock_reset: MagicMock,
946+
mock_radio_config: RadioConfig,
947+
):
948+
"""Tests that oversized packets are truncated and a warning is logged.
949+
950+
Args:
951+
mock_rfm9x: Mocked RFM9x class.
952+
mock_logger: Mocked Logger instance.
953+
mock_spi: Mocked SPI bus.
954+
mock_chip_select: Mocked chip select pin.
955+
mock_reset: Mocked reset pin.
956+
mock_radio_config: Mocked RadioConfig instance.
957+
"""
958+
mock_radio_config.modulation = "LoRa"
959+
mock_radio_instance = MagicMock()
960+
mock_radio_instance.max_packet_length = 252 # RFM9x max packet length
961+
mock_radio_instance.send = MagicMock(return_value=True)
962+
mock_rfm9x.return_value = mock_radio_instance
963+
964+
manager = RFM9xManager(
965+
mock_logger,
966+
mock_radio_config,
967+
mock_spi,
968+
mock_chip_select,
969+
mock_reset,
970+
)
971+
972+
# Create oversized data (253 bytes, which is 1 byte over the limit)
973+
oversized_data = b"x" * 253
974+
975+
# Send the oversized data
976+
result = manager.send(oversized_data)
977+
978+
# Verify that send was successful
979+
assert result is True
980+
981+
# Verify that a warning was logged about truncation
982+
mock_logger.warning.assert_called()
983+
warning_calls = [call for call in mock_logger.warning.call_args_list]
984+
truncation_warning = None
985+
for call in warning_calls:
986+
if call[0][0] == "Data exceeds max packet size, truncating":
987+
truncation_warning = call
988+
break
989+
990+
assert truncation_warning is not None
991+
assert truncation_warning[1]["data_length"] == 253
992+
assert truncation_warning[1]["max_packet_size"] == 252
993+
994+
# Verify that the data sent to the radio was truncated to 252 bytes
995+
mock_radio_instance.send.assert_called_once()
996+
sent_data = mock_radio_instance.send.call_args[0][0]
997+
assert len(sent_data) == 252

0 commit comments

Comments
 (0)