From 62080424bb2379ae76962909519e1f36e81e00fb Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Thu, 19 Mar 2026 17:30:34 -0700 Subject: [PATCH] Multi-ack. Closes #81. --- AGENTS.md | 4 +- app/AGENTS.md | 4 +- app/routers/debug.py | 4 ++ app/routers/radio.py | 9 ++++ app/services/radio_commands.py | 7 +++ frontend/AGENTS.md | 1 + .../settings/SettingsRadioSection.tsx | 24 ++++++++++ frontend/src/test/settingsModal.test.tsx | 13 ++++++ frontend/src/types.ts | 2 + tests/test_radio_commands_service.py | 34 ++++++++++++++ tests/test_radio_router.py | 44 +++++++++++++++++++ 11 files changed, 142 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cf01398..dfb3d08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -307,8 +307,8 @@ All endpoints are prefixed with `/api` (e.g., `/api/health`). |--------|----------|-------------| | GET | `/api/health` | Connection status, fanout statuses, bots_disabled flag | | GET | `/api/debug` | Support snapshot: recent logs, live radio probe, contact/channel drift audit, and running version/git info | -| GET | `/api/radio/config` | Radio configuration, including `path_hash_mode`, `path_hash_mode_supported`, and whether adverts include current node location | -| PATCH | `/api/radio/config` | Update name, location, advert-location on/off, radio params, and `path_hash_mode` when supported | +| GET | `/api/radio/config` | Radio configuration, including `path_hash_mode`, `path_hash_mode_supported`, advert-location on/off, and `multi_acks_enabled` | +| PATCH | `/api/radio/config` | Update name, location, advert-location on/off, `multi_acks_enabled`, radio params, and `path_hash_mode` when supported | | PUT | `/api/radio/private-key` | Import private key to radio | | POST | `/api/radio/advertise` | Send advertisement (`mode`: `flood` or `zero_hop`, default `flood`) | | POST | `/api/radio/discover` | Run a short mesh discovery sweep for nearby repeaters/sensors | diff --git a/app/AGENTS.md b/app/AGENTS.md index 186b1da..be2d2d0 100644 --- a/app/AGENTS.md +++ b/app/AGENTS.md @@ -169,8 +169,8 @@ app/ - `GET /debug` — support snapshot with recent logs, live radio probe, slot/contact audits, and version/git info ### Radio -- `GET /radio/config` — includes `path_hash_mode`, `path_hash_mode_supported`, and advert-location on/off -- `PATCH /radio/config` — may update `path_hash_mode` (`0..2`) when firmware supports it +- `GET /radio/config` — includes `path_hash_mode`, `path_hash_mode_supported`, advert-location on/off, and `multi_acks_enabled` +- `PATCH /radio/config` — may update `path_hash_mode` (`0..2`) when firmware supports it, and `multi_acks_enabled` - `PUT /radio/private-key` - `POST /radio/advertise` — manual advert send; request body may set `mode` to `flood` or `zero_hop` (defaults to `flood`) - `POST /radio/discover` — short mesh discovery sweep for nearby repeaters/sensors diff --git a/app/routers/debug.py b/app/routers/debug.py index a290a94..c98f2f9 100644 --- a/app/routers/debug.py +++ b/app/routers/debug.py @@ -78,6 +78,7 @@ class DebugChannelAudit(BaseModel): class DebugRadioProbe(BaseModel): performed: bool errors: list[str] = Field(default_factory=list) + multi_acks_enabled: bool | None = None self_info: dict[str, Any] | None = None device_info: dict[str, Any] | None = None stats_core: dict[str, Any] | None = None @@ -234,6 +235,9 @@ async def _probe_radio() -> DebugRadioProbe: return DebugRadioProbe( performed=True, errors=errors, + multi_acks_enabled=bool(mc.self_info.get("multi_acks", 0)) + if mc.self_info is not None + else None, self_info=dict(mc.self_info or {}), device_info=device_info, stats_core=stats_core, diff --git a/app/routers/radio.py b/app/routers/radio.py index 7c7b594..795c106 100644 --- a/app/routers/radio.py +++ b/app/routers/radio.py @@ -81,6 +81,10 @@ class RadioConfigResponse(BaseModel): default="current", description="Whether adverts include the node's current location state", ) + multi_acks_enabled: bool = Field( + default=False, + description="Whether the radio sends an extra direct ACK transmission", + ) class RadioConfigUpdate(BaseModel): @@ -99,6 +103,10 @@ class RadioConfigUpdate(BaseModel): default=None, description="Whether adverts include the node's current location state", ) + multi_acks_enabled: bool | None = Field( + default=None, + description="Whether the radio sends an extra direct ACK transmission", + ) class PrivateKeyUpdate(BaseModel): @@ -222,6 +230,7 @@ async def get_radio_config() -> RadioConfigResponse: path_hash_mode=radio_manager.path_hash_mode, path_hash_mode_supported=radio_manager.path_hash_mode_supported, advert_location_source=advert_location_source, + multi_acks_enabled=bool(info.get("multi_acks", 0)), ) diff --git a/app/services/radio_commands.py b/app/services/radio_commands.py index c1e6e18..9718a37 100644 --- a/app/services/radio_commands.py +++ b/app/services/radio_commands.py @@ -44,6 +44,13 @@ async def apply_radio_config_update( f"Failed to set advert location policy: {result.payload}" ) + if update.multi_acks_enabled is not None: + multi_acks = 1 if update.multi_acks_enabled else 0 + logger.info("Setting multi ACKs to %d", multi_acks) + result = await mc.commands.set_multi_acks(multi_acks) + if result is not None and result.type == EventType.ERROR: + raise RadioCommandRejectedError(f"Failed to set multi ACKs: {result.payload}") + if update.name is not None: logger.info("Setting radio name to %s", update.name) await mc.commands.set_name(update.name) diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 4a92efe..d92a280 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -250,6 +250,7 @@ High-level state is delegated to hooks: ### Radio settings behavior - `SettingsRadioSection.tsx` surfaces `path_hash_mode` only when `config.path_hash_mode_supported` is true. +- `SettingsRadioSection.tsx` also exposes `multi_acks_enabled` as a checkbox for the radio's extra direct-ACK transmission behavior. - Advert-location control is intentionally only `off` vs `include node location`. Companion-radio firmware does not reliably distinguish saved coordinates from live GPS in this path. - The advert action is mode-aware: the radio settings section exposes both flood and zero-hop manual advert buttons, both routed through the same `onAdvertise(mode)` seam. - Mesh discovery in the radio section is limited to node classes that currently answer discovery control-data requests in firmware: repeaters and sensors. diff --git a/frontend/src/components/settings/SettingsRadioSection.tsx b/frontend/src/components/settings/SettingsRadioSection.tsx index d562dad..4b3f68e 100644 --- a/frontend/src/components/settings/SettingsRadioSection.tsx +++ b/frontend/src/components/settings/SettingsRadioSection.tsx @@ -5,6 +5,7 @@ import { Label } from '../ui/label'; import { Button } from '../ui/button'; import { Separator } from '../ui/separator'; import { toast } from '../ui/sonner'; +import { Checkbox } from '../ui/checkbox'; import { RADIO_PRESETS } from '../../utils/radioPresets'; import { stripRegionScopePrefix } from '../../utils/regionScope'; import type { @@ -64,6 +65,7 @@ export function SettingsRadioSection({ const [cr, setCr] = useState(''); const [pathHashMode, setPathHashMode] = useState('0'); const [advertLocationSource, setAdvertLocationSource] = useState<'off' | 'current'>('current'); + const [multiAcksEnabled, setMultiAcksEnabled] = useState(false); const [gettingLocation, setGettingLocation] = useState(false); const [busy, setBusy] = useState(false); const [rebooting, setRebooting] = useState(false); @@ -98,6 +100,7 @@ export function SettingsRadioSection({ setCr(String(config.radio.cr)); setPathHashMode(String(config.path_hash_mode)); setAdvertLocationSource(config.advert_location_source ?? 'current'); + setMultiAcksEnabled(config.multi_acks_enabled ?? false); }, [config]); useEffect(() => { @@ -190,6 +193,9 @@ export function SettingsRadioSection({ ...(advertLocationSource !== (config.advert_location_source ?? 'current') ? { advert_location_source: advertLocationSource } : {}), + ...(multiAcksEnabled !== (config.multi_acks_enabled ?? false) + ? { multi_acks_enabled: multiAcksEnabled } + : {}), radio: { freq: parsedFreq, bw: parsedBw, @@ -579,6 +585,24 @@ export function SettingsRadioSection({ library.

+
+
+ setMultiAcksEnabled(checked === true)} + className="mt-0.5" + /> +
+ +

+ When enabled, the radio sends one extra direct ACK transmission before the normal + ACK for received direct messages. This is a firmware-level receive behavior, not a + RemoteTerm retry setting. +

+
+
+
{config.path_hash_mode_supported && ( diff --git a/frontend/src/test/settingsModal.test.tsx b/frontend/src/test/settingsModal.test.tsx index e45f560..68b5a35 100644 --- a/frontend/src/test/settingsModal.test.tsx +++ b/frontend/src/test/settingsModal.test.tsx @@ -37,6 +37,7 @@ const baseConfig: RadioConfig = { path_hash_mode: 0, path_hash_mode_supported: false, advert_location_source: 'current', + multi_acks_enabled: false, }; const baseHealth: HealthStatus = { @@ -332,6 +333,18 @@ describe('SettingsModal', () => { }); }); + it('saves multi-acks through radio config save', async () => { + const { onSave } = renderModal(); + openRadioSection(); + + fireEvent.click(screen.getByLabelText('Extra Direct ACK Transmission')); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ multi_acks_enabled: true })); + }); + }); + it('saves changed max contacts value through onSaveAppSettings', async () => { const { onSaveAppSettings } = renderModal(); openRadioSection(); diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 4e549a5..dceed39 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -16,6 +16,7 @@ export interface RadioConfig { path_hash_mode: number; path_hash_mode_supported: boolean; advert_location_source?: 'off' | 'current'; + multi_acks_enabled?: boolean; } export interface RadioConfigUpdate { @@ -26,6 +27,7 @@ export interface RadioConfigUpdate { radio?: RadioSettings; path_hash_mode?: number; advert_location_source?: 'off' | 'current'; + multi_acks_enabled?: boolean; } export type RadioDiscoveryTarget = 'repeaters' | 'sensors' | 'all'; diff --git a/tests/test_radio_commands_service.py b/tests/test_radio_commands_service.py index 84ba427..fdcd10e 100644 --- a/tests/test_radio_commands_service.py +++ b/tests/test_radio_commands_service.py @@ -33,6 +33,7 @@ def _mock_meshcore_with_info(): mc.commands.set_radio = AsyncMock() mc.commands.set_path_hash_mode = AsyncMock(return_value=_radio_result()) mc.commands.set_advert_loc_policy = AsyncMock(return_value=_radio_result()) + mc.commands.set_multi_acks = AsyncMock(return_value=_radio_result()) mc.commands.send_appstart = AsyncMock() mc.commands.import_private_key = AsyncMock(return_value=_radio_result()) return mc @@ -84,6 +85,39 @@ class TestApplyRadioConfigUpdate: mc.commands.set_advert_loc_policy.assert_awaited_once_with(1) mc.commands.send_appstart.assert_awaited_once() + @pytest.mark.asyncio + async def test_updates_multi_acks_enabled(self): + mc = _mock_meshcore_with_info() + + await apply_radio_config_update( + mc, + RadioConfigUpdate(multi_acks_enabled=True), + path_hash_mode_supported=True, + set_path_hash_mode=MagicMock(), + sync_radio_time_fn=AsyncMock(), + ) + + mc.commands.set_multi_acks.assert_awaited_once_with(1) + mc.commands.send_appstart.assert_awaited_once() + + @pytest.mark.asyncio + async def test_raises_when_radio_rejects_multi_acks(self): + mc = _mock_meshcore_with_info() + mc.commands.set_multi_acks = AsyncMock( + return_value=_radio_result(EventType.ERROR, {"error": "nope"}) + ) + + with pytest.raises(RadioCommandRejectedError): + await apply_radio_config_update( + mc, + RadioConfigUpdate(multi_acks_enabled=False), + path_hash_mode_supported=True, + set_path_hash_mode=MagicMock(), + sync_radio_time_fn=AsyncMock(), + ) + + mc.commands.send_appstart.assert_not_awaited() + @pytest.mark.asyncio async def test_raises_when_radio_rejects_advert_location_source(self): mc = _mock_meshcore_with_info() diff --git a/tests/test_radio_router.py b/tests/test_radio_router.py index d05171f..c4acde8 100644 --- a/tests/test_radio_router.py +++ b/tests/test_radio_router.py @@ -75,6 +75,7 @@ def _mock_meshcore_with_info(): "radio_sf": 7, "radio_cr": 5, "adv_loc_policy": 2, + "multi_acks": 0, } mc.commands = MagicMock() mc.commands.set_name = AsyncMock() @@ -82,6 +83,7 @@ def _mock_meshcore_with_info(): mc.commands.set_tx_power = AsyncMock() mc.commands.set_radio = AsyncMock() mc.commands.set_advert_loc_policy = AsyncMock(return_value=_radio_result()) + mc.commands.set_multi_acks = AsyncMock(return_value=_radio_result()) mc.commands.send_appstart = AsyncMock() mc.commands.import_private_key = AsyncMock(return_value=_radio_result()) mc.commands.send_node_discover_req = AsyncMock(return_value=_radio_result()) @@ -104,6 +106,17 @@ class TestGetRadioConfig: assert response.radio.freq == 910.525 assert response.radio.cr == 5 assert response.advert_location_source == "current" + assert response.multi_acks_enabled is False + + @pytest.mark.asyncio + async def test_maps_multi_acks_to_response(self): + mc = _mock_meshcore_with_info() + mc.self_info["multi_acks"] = 1 + + with patch("app.routers.radio.require_connected", return_value=mc): + response = await get_radio_config() + + assert response.multi_acks_enabled is True @pytest.mark.asyncio async def test_maps_any_nonzero_advert_location_policy_to_current(self): @@ -172,6 +185,7 @@ class TestUpdateRadioConfig: path_hash_mode=0, path_hash_mode_supported=False, advert_location_source="current", + multi_acks_enabled=False, ) with ( @@ -187,6 +201,36 @@ class TestUpdateRadioConfig: mc.commands.set_advert_loc_policy.assert_awaited_once_with(1) assert result == expected + @pytest.mark.asyncio + async def test_updates_multi_acks_enabled(self): + mc = _mock_meshcore_with_info() + expected = RadioConfigResponse( + public_key="aa" * 32, + name="NodeA", + lat=10.0, + lon=20.0, + tx_power=17, + max_tx_power=22, + radio=RadioSettings(freq=910.525, bw=62.5, sf=7, cr=5), + path_hash_mode=0, + path_hash_mode_supported=False, + advert_location_source="current", + multi_acks_enabled=True, + ) + + with ( + patch("app.routers.radio.require_connected", return_value=mc), + patch.object(radio_manager, "_meshcore", mc), + patch("app.routers.radio.sync_radio_time", new_callable=AsyncMock), + patch( + "app.routers.radio.get_radio_config", new_callable=AsyncMock, return_value=expected + ), + ): + result = await update_radio_config(RadioConfigUpdate(multi_acks_enabled=True)) + + mc.commands.set_multi_acks.assert_awaited_once_with(1) + assert result == expected + def test_model_rejects_negative_path_hash_mode(self): with pytest.raises(ValidationError): RadioConfigUpdate(path_hash_mode=-1)