mirror of
https://github.com/pyMC-dev/pyMC_Repeater.git
synced 2026-08-07 09:23:06 +02:00
refactor(airtime): introduce refresh_radio_params method for dynamic modulation updates
Added a new method to the AirtimeManager class to refresh modulation parameters without clearing transmission history. Updated ConfigManager to call this method after applying live radio configurations. Enhanced tests to verify that modulation updates occur correctly during live updates.
This commit is contained in:
@@ -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
|
||||
|
||||
+16
-5
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
+12
-7
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
)
|
||||
Reference in New Issue
Block a user