diff --git a/config.yaml.example b/config.yaml.example index f404dd7..b94690f 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -522,6 +522,10 @@ storage: retention: # Clean up SQLite records older than this many days sqlite_cleanup_days: 31 + # Companion event journal / soft-consumed message history retention. + # Independent of sqlite_cleanup_days so packet RF history and companion + # sync history can be sized separately. Defaults to 31 when omitted. + companion_events_days: 31 # RRD archives are managed automatically: # - 1 minute resolution for 1 week diff --git a/repeater/airtime.py b/repeater/airtime.py index c21ddc2..c19a78f 100644 --- a/repeater/airtime.py +++ b/repeater/airtime.py @@ -1,6 +1,6 @@ import logging import time -from typing import Tuple +from typing import Optional, Tuple from openhop_core.protocol.packet_utils import calculate_lora_airtime_ms @@ -16,10 +16,7 @@ class AirtimeManager: ) # Store radio settings for airtime calculations - self.spreading_factor = self.radio_config.get("spreading_factor", 7) - self.bandwidth = self.radio_config.get("bandwidth", 125000) - self.coding_rate = self.radio_config.get("coding_rate", 5) - self.preamble_length = self.radio_config.get("preamble_length", 8) + self.refresh_radio_params(self.radio_config) # Track airtime in rolling window self.tx_history = [] # [(timestamp, airtime_ms), ...] @@ -27,6 +24,20 @@ class AirtimeManager: self.total_airtime_ms = 0 self.total_rx_airtime_ms = 0 + def refresh_radio_params(self, radio_config: Optional[dict] = None) -> None: + """Reload cached modulation params used by airtime estimation. + + Call after a successful live radio reconfiguration. Does not reset + TX/RX history or duty-cycle totals. + """ + if radio_config is None: + radio_config = self.config.get("radio", {}) or {} + self.radio_config = radio_config + self.spreading_factor = self.radio_config.get("spreading_factor", 7) + self.bandwidth = self.radio_config.get("bandwidth", 125000) + self.coding_rate = self.radio_config.get("coding_rate", 5) + self.preamble_length = self.radio_config.get("preamble_length", 8) + def calculate_airtime( self, payload_len: int, diff --git a/repeater/config_manager.py b/repeater/config_manager.py index 53f981c..4db5dc2 100644 --- a/repeater/config_manager.py +++ b/repeater/config_manager.py @@ -140,12 +140,22 @@ class ConfigManager: return False self._sync_repeater_handler_radio_config(radio_cfg) + self._refresh_airtime_radio_params() logger.info("Applied live radio configuration to running daemon") return True except Exception as e: logger.error(f"Failed to apply live radio config: {e}", exc_info=True) return False + def _refresh_airtime_radio_params(self) -> None: + repeater_handler = getattr(self.daemon, "repeater_handler", None) + airtime_mgr = getattr(repeater_handler, "airtime_mgr", None) if repeater_handler else None + if airtime_mgr is None or not hasattr(airtime_mgr, "refresh_radio_params"): + return + # Use the full radio section so preamble_length is included; the live + # hardware snapshot intentionally omits fields the radio API does not set. + airtime_mgr.refresh_radio_params(self.config.get("radio", {}) or {}) + @staticmethod def _parse_bool(value: Any, default: bool = True) -> bool: if value is None: diff --git a/repeater/data_acquisition/sqlite_handler.py b/repeater/data_acquisition/sqlite_handler.py index 12d9e62..56c585f 100644 --- a/repeater/data_acquisition/sqlite_handler.py +++ b/repeater/data_acquisition/sqlite_handler.py @@ -2351,7 +2351,15 @@ class SQLiteHandler: logger.error(f"Failed to vacuum database: {e}") raise - def cleanup_old_data(self, days: int = 7): + def cleanup_old_data(self, days: int = 7, companion_events_days: Optional[int] = None): + """Prune retention-bounded tables. + + ``companion_events_days`` is forwarded from engine.py + (``storage.retention.companion_events_days``, default 31). Accepted here + so the periodic cleanup call cannot TypeError and silently skip all + SQLite pruning. Companion journal/history pruning is layered on by the + companion-api storage work once those tables exist. + """ try: cutoff = time.time() - (days * 24 * 3600) diff --git a/repeater/data_acquisition/storage_collector.py b/repeater/data_acquisition/storage_collector.py index 0594004..b7fa2d7 100644 --- a/repeater/data_acquisition/storage_collector.py +++ b/repeater/data_acquisition/storage_collector.py @@ -501,8 +501,8 @@ class StorageCollector: logger.debug(f"Could not lookup node name for {pubkey[:8] if pubkey else 'None'}: {e}") return None - def cleanup_old_data(self, days: int = 7): - self.sqlite_handler.cleanup_old_data(days) + def cleanup_old_data(self, days: int = 7, companion_events_days: Optional[int] = None): + self.sqlite_handler.cleanup_old_data(days, companion_events_days=companion_events_days) def get_noise_floor_history(self, hours: int = 24, limit: int = None) -> list: return self.sqlite_handler.get_noise_floor_history(hours, limit) diff --git a/repeater/engine.py b/repeater/engine.py index a7eb704..b76d177 100644 --- a/repeater/engine.py +++ b/repeater/engine.py @@ -1706,12 +1706,17 @@ class RepeaterHandler(BaseHandler): if current_time - self.last_db_cleanup >= 21600: if self.storage: try: - retention_days = ( - self.config.get("storage", {}) - .get("retention", {}) - .get("sqlite_cleanup_days", 31) + retention_cfg = self.config.get("storage", {}).get( + "retention", {} + ) + retention_days = retention_cfg.get("sqlite_cleanup_days", 31) + companion_events_days = retention_cfg.get( + "companion_events_days", 31 + ) + self.storage.cleanup_old_data( + days=retention_days, + companion_events_days=companion_events_days, ) - self.storage.cleanup_old_data(days=retention_days) logger.info("Cleaned up SQLite data older than %d days", retention_days) except Exception as e: logger.warning(f"SQLite cleanup failed: {e}") @@ -1809,8 +1814,8 @@ class RepeaterHandler(BaseHandler): ): self.neighbour_link_tracker.evict_stalest_locked() - # Note: Radio config changes require restart as they affect hardware - # Note: Airtime manager has its own config reference that gets updated + # Radio hardware apply and AirtimeManager modulation refresh are + # handled by ConfigManager after a successful live radio update. logger.info("Runtime configuration reloaded successfully") except Exception as e: diff --git a/tests/test_airtime.py b/tests/test_airtime.py index d6feb8e..842a7d0 100644 --- a/tests/test_airtime.py +++ b/tests/test_airtime.py @@ -157,3 +157,31 @@ def test_stats_report_tx_rx_airtime_totals(): assert stats["total_airtime_ms"] == pytest.approx(tx_airtime) assert stats["total_rx_airtime_ms"] == pytest.approx(rx_airtime) assert stats["current_airtime_ms"] == pytest.approx(tx_airtime) + + +def test_refresh_radio_params_updates_cached_modulation(): + """Live SF/BW changes must refresh estimation scalars without clearing history.""" + from openhop_core.protocol.packet_utils import calculate_lora_airtime_ms + + mgr = _make_mgr(sf=7, bw_hz=125000, cr=5, preamble=8) + before = mgr.calculate_airtime(50) + assert math.isclose(before, 97.536, rel_tol=1e-9) + + mgr.record_tx(before) + mgr.refresh_radio_params( + { + "spreading_factor": 12, + "bandwidth": 125000, + "coding_rate": 5, + "preamble_length": 8, + } + ) + + after = mgr.calculate_airtime(50) + expected = calculate_lora_airtime_ms(50, 12, 125000, 5, 8) + assert math.isclose(after, expected, rel_tol=1e-9) + assert math.isclose(after, 2301.952, rel_tol=1e-9) + assert after > before + assert mgr.spreading_factor == 12 + assert mgr.total_airtime_ms == pytest.approx(before) + assert len(mgr.tx_history) == 1 diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index ba66ff5..065fc68 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -1,37 +1,62 @@ +import math + +from repeater.airtime import AirtimeManager from repeater.config_manager import ConfigManager class _DummyRepeaterHandler: - def __init__(self): + def __init__(self, config=None): self.radio_config = {} + self.airtime_mgr = AirtimeManager( + config + or { + "radio": { + "frequency": 868000000, + "bandwidth": 125000, + "spreading_factor": 7, + "coding_rate": 5, + "tx_power": 14, + "preamble_length": 8, + } + } + ) class _DummySX1262Radio: - def __init__(self): + def __init__(self, apply_ok=True): self.frequency = 868000000 self.bandwidth = 125000 self.spreading_factor = 7 self.coding_rate = 5 self.tx_power = 14 self.calls = [] + self.apply_ok = apply_ok def set_frequency(self, frequency): self.calls.append(("set_frequency", frequency)) + if not self.apply_ok: + return False self.frequency = frequency return True def set_tx_power(self, power): self.calls.append(("set_tx_power", power)) + if not self.apply_ok: + return False self.tx_power = power return True def set_spreading_factor(self, spreading_factor): self.calls.append(("set_spreading_factor", spreading_factor)) + if not self.apply_ok: + return False self.spreading_factor = spreading_factor return True def set_bandwidth(self, bandwidth): self.calls.append(("set_bandwidth", bandwidth)) + if not self.apply_ok: + return False self.bandwidth = bandwidth return True @@ -64,7 +89,7 @@ class _DummyDaemon: "kiss": dict(config.get("kiss", {})), } self.radio = radio - self.repeater_handler = _DummyRepeaterHandler() + self.repeater_handler = _DummyRepeaterHandler(config) self.advert_helper = None self.dispatcher = None @@ -128,3 +153,66 @@ def test_live_update_daemon_applies_kiss_radio_config(): ] assert radio.radio_config == config["radio"] assert daemon.repeater_handler.radio_config == config["radio"] + + +def test_live_update_daemon_refreshes_airtime_manager_modulation(): + startup_radio = { + "frequency": 868000000, + "bandwidth": 125000, + "spreading_factor": 7, + "coding_rate": 5, + "tx_power": 14, + "preamble_length": 8, + } + updated_radio = { + "frequency": 915000000, + "bandwidth": 125000, + "spreading_factor": 12, + "coding_rate": 5, + "tx_power": 14, + "preamble_length": 8, + } + config = {"radio": dict(startup_radio)} + radio = _DummySX1262Radio() + daemon = _DummyDaemon(config, radio) + airtime_mgr = daemon.repeater_handler.airtime_mgr + before = airtime_mgr.calculate_airtime(50) + assert math.isclose(before, 97.536, rel_tol=1e-9) + + config["radio"] = dict(updated_radio) + manager = ConfigManager("/tmp/config.yaml", config, daemon) + assert manager.live_update_daemon(["radio"]) + + assert airtime_mgr.spreading_factor == 12 + assert airtime_mgr.bandwidth == 125000 + assert airtime_mgr.preamble_length == 8 + assert math.isclose(airtime_mgr.calculate_airtime(50), 2301.952, rel_tol=1e-9) + + +def test_failed_live_radio_apply_leaves_airtime_manager_unchanged(): + startup_radio = { + "frequency": 868000000, + "bandwidth": 125000, + "spreading_factor": 7, + "coding_rate": 5, + "tx_power": 14, + "preamble_length": 8, + } + config = {"radio": dict(startup_radio)} + radio = _DummySX1262Radio(apply_ok=False) + daemon = _DummyDaemon(config, radio) + airtime_mgr = daemon.repeater_handler.airtime_mgr + + config["radio"] = { + "frequency": 915000000, + "bandwidth": 125000, + "spreading_factor": 12, + "coding_rate": 5, + "tx_power": 14, + "preamble_length": 8, + } + manager = ConfigManager("/tmp/config.yaml", config, daemon) + assert manager.live_update_daemon(["radio"]) is False + + assert airtime_mgr.spreading_factor == 7 + assert math.isclose(airtime_mgr.calculate_airtime(50), 97.536, rel_tol=1e-9) diff --git a/tests/test_engine.py b/tests/test_engine.py index f334a0f..7a718e4 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -2774,7 +2774,9 @@ class TestEngineTransmissionAndBackgroundLifecycle: handler._record_crc_errors_async.assert_awaited_once() handler._send_periodic_advert_async.assert_awaited_once() handler.cleanup_cache.assert_called_once() - handler.storage.cleanup_old_data.assert_called_once() + handler.storage.cleanup_old_data.assert_called_once_with( + days=31, companion_events_days=31 + ) @pytest.mark.asyncio async def test_background_timer_loop_continues_when_db_cleanup_fails(self, handler): diff --git a/tests/test_sqlite_handler_easy.py b/tests/test_sqlite_handler_easy.py index bb71d8e..d598748 100644 --- a/tests/test_sqlite_handler_easy.py +++ b/tests/test_sqlite_handler_easy.py @@ -676,3 +676,29 @@ def test_get_lbt_diagnostics_empty_range_preserves_no_data_distinction(tmp_path) assert bucket["avg_attempts"] is None assert out["packet_types"] == [] assert out["packet_type_buckets"] == [] + + +def test_cleanup_old_data_accepts_companion_events_days(tmp_path): + """engine.py forwards companion_events_days; accepting it must not break + packet retention cleanup (the pre-fix TypeError was swallowed upstream). + """ + import time + + h = _make_handler(tmp_path) + old_ts = time.time() - (40 * 86400) + h.store_packet( + { + "timestamp": old_ts, + "type": 1, + "route": 2, + "length": 3, + "transmitted": False, + "packet_hash": "pkt-old", + } + ) + + h.cleanup_old_data(days=31, companion_events_days=14) + + with h._connect() as conn: + count = conn.execute("SELECT COUNT(*) FROM packets").fetchone()[0] + assert count == 0 diff --git a/tests/test_storage_collector_cleanup.py b/tests/test_storage_collector_cleanup.py new file mode 100644 index 0000000..15de20c --- /dev/null +++ b/tests/test_storage_collector_cleanup.py @@ -0,0 +1,35 @@ +"""Regression test: StorageCollector.cleanup_old_data must accept and pass +through ``companion_events_days``. + +engine.py's 6-hourly cleanup passes both kwargs; before this passthrough +existed the call raised TypeError, was swallowed by the caller's broad +except, and SQLite cleanup silently never ran. +""" + +from unittest.mock import Mock + +from repeater.data_acquisition.storage_collector import StorageCollector + + +def _bare_collector() -> StorageCollector: + collector = StorageCollector.__new__(StorageCollector) + collector.sqlite_handler = Mock() + return collector + + +def test_cleanup_passes_companion_events_days_through(): + collector = _bare_collector() + collector.cleanup_old_data(days=10, companion_events_days=20) + collector.sqlite_handler.cleanup_old_data.assert_called_once_with( + 10, companion_events_days=20 + ) + + +def test_cleanup_defaults_match_engine_callsite(): + # engine.py always passes both kwargs, but a bare call must stay valid + # for any older callers that only know about ``days``. + collector = _bare_collector() + collector.cleanup_old_data(days=7) + collector.sqlite_handler.cleanup_old_data.assert_called_once_with( + 7, companion_events_days=None + )