[codex] Fix DEXP protocol mapping#3568
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the PHONE applet’s DEXP controls by mapping them to the FlexRadio protocol keys SmartSDR actually uses (compander / compander_level), removing stale local persistence/restore, and adding regression coverage to prevent the mapping from regressing again.
Changes:
- Wire DEXP toggle/level command emission to
transmit set compander=0/1andtransmit set compander_level=<0..100>. - Parse
compander/compander_levelstatus into the existing DEXP model state so the UI follows radio-authoritative state. - Remove local
DexpEnabled/DexpLevelpersistence + reconnect restore logic; fix two Speech Processor state reads; addTransmitModeltests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/transmit_model_test.cpp | Adds regression coverage for compander-backed DEXP command emission and status parsing. |
| src/models/TransmitModel.cpp | Implements compander-backed DEXP status/command mapping and updates related state change signaling. |
| src/gui/PhoneApplet.h | Updates DEXP documentation/comments to reflect working mapping. |
| src/gui/PhoneApplet.cpp | Removes client-side DEXP persistence writes; relies on model/radio state. |
| src/gui/MainWindow.cpp | Removes now-obsolete comment about avoiding DEXP persistence on close. |
| src/gui/MainWindow_Wiring.cpp | Removes reconnect-time restore of locally persisted DEXP settings. |
| src/gui/MainWindow_Shortcuts.cpp | Fixes PROC shortcut toggle to use speechProcessorEnable() rather than companderOn(). |
| src/gui/MainWindow_Controllers.cpp | Fixes MIDI PROC enable readback to use speechProcessorEnable() rather than companderOn(). |
There was a problem hiding this comment.
Thanks @rfoust — this is a clean, well-scoped fix and the protocol reasoning checks out against the codebase, not just the SmartSDR capture.
Verified the repurposing is safe. I traced every reader of companderOn()/companderLevel():
MainWindow_Controllers.cpp:1765andMainWindow_Shortcuts.cpp:704— both stale Speech-Processor reads, correctly repointed tospeechProcessorEnable()here.RadioModel.cpp:5677(mic troubleshooting snapshot) — read-only/diagnostic, fine.
compander was effectively orphaned after the PROC path was reverted to speech_processor_enable/speech_processor_level (the pcap comments at TransmitModel.cpp:534/544, which supersede the older #661/#667 CHANGELOG note). So wiring compander/compander_level to DEXP disturbs no other live control. Dropping the DexpEnabled/DexpLevel flat-key persistence + reconnect replay is the right call for radio-authoritative state.
On the two Copilot findings — valid but low severity. Both compander and the legacy dexp blocks now write m_companderOn/m_dexpOn, and the dexp block runs later, so a single status frame carrying both keys with conflicting values would let the legacy key win. In practice a frame won't carry both (the radio speaks one protocol generation), so this is defensive-only — but the guard is trivial and makes intent explicit. If you touch it, gating the legacy keys on the canonical key being absent is enough:
if (kvs.contains("dexp") && !kvs.contains("compander")) { ... }
if (kvs.contains("noise_gate_level") && !kvs.contains("compander_level")) { ... }Not a merge blocker. Nice to see focused TransmitModel test coverage for both the command emission and the compander→DEXP status path. 73
🤖 aethersdr-agent · cost: $6.2565 · model: claude-opus-4-8
Use the radio compander keys observed in the SmartSDR capture and FlexLib for the DEXP toggle and threshold. Stop persisting DEXP as client-side state so reconnects do not override the radio-authoritative value. Fixes aethersdr#3464.
bea3fda to
395555e
Compare
Fixes aethersdr#3464. The PHONE applet DEXP controls were sending transmit set dexp=/noise_gate_level= keys the radio rejects; maps them to the SmartSDR-actual compander/compander_level keys (per a v4.2.20 capture + FlexLib v4.2.18 CompanderOn/CompanderLevel). Dual-writes m_dexpOn (PHONE) and m_companderOn (mic-diag) in sync, gates legacy keys on !compander, fixes two stale PROC reads (companderOn -> speechProcessorEnable), and removes client-side DEXP persistence/reconnect-replay (radio-authoritative). Adds transmit_model_test coverage. Co-authored-by: rfoust <rfoust@users.noreply.github.com>
Summary
Fixes #3464 by wiring the PHONE applet DEXP controls to the radio protocol keys that SmartSDR actually uses:
transmit set compander=0/1for the DEXP toggletransmit set compander_level=<0..100>for the DEXP threshold sliderThe reporter provided a SmartSDR capture on #3464 showing those command/status keys during DEXP toggle and slider changes, and FlexLib v4.2.18 exposes the same
CompanderOn/CompanderLevelcommands. AetherSDR was previously sendingtransmit set dexp=...andtransmit set noise_gate_level=..., which the radio rejects, so the UI could look changed while radio-side DEXP never changed.What Changed
compander/compander_leveltransmit status into the existing DEXP model state so the PHONE applet follows radio-authoritative state.TransmitModel::setDexp()andsetDexpLevel()to emitcompander/compander_levelcommands.DexpEnabled/DexpLevelpersistence and reconnect restore logic. DEXP is radio-side state; AetherSDR should not replay stale local settings over the radio on reconnect.companderOn()instead ofspeechProcessorEnable().TransmitModelregression coverage for DEXP command emission and compander status parsing.Maintainer Notes
This intentionally leaves the visible UI label as
DEXP; the protocol field is namedcompander, but the user-facing control is still the downward expander/noise gate.PROCremains backed by the separatespeech_processor_enable/speech_processor_levelprotocol keys.Validation
cmake --build build --target transmit_model_test --parallel./build/transmit_model_testcmake --build build --target AetherSDR --parallelpython3 tools/check_a11y.py src/gui/PhoneApplet.cpp src/gui/PhoneApplet.h src/gui/MainWindow.cpp src/gui/MainWindow_Controllers.cpp src/gui/MainWindow_Shortcuts.cpp src/gui/MainWindow_Wiring.cppgit diff --check upstream/main...HEADFixes #3464.