feat(streamdeck): slice targeting, connection status, and radio-sourced key values - #4514
#4514feat(streamdeck): slice targeting, connection status, and radio-sourced key values#4514milenjb wants to merge 2 commits intoaethersdr:mainaethersdr/AetherSDR:mainfrom milenjb:fix/streamdeck-slice-targetingmilenjb/AetherSDR:fix/streamdeck-slice-targetingCopy head branch name to clipboard
Conversation
…ed key values Fixes aethersdr#4513. The bundled Elgato plugin addressed trx0 on every command, so it only ever drove slice A, and every key carried a static manifest label — an operator could neither point the deck at the slice they were using nor read any radio value without looking at the AetherSDR window. Slice targeting. A Slice Target action cycles TRX0 -> TX -> ACTIVE and applies to mode, DSP, squelch, lock, RIT/XIT, split, slice volume, slice mute and the VFO dial. TRX0 stays the default so an existing deck is unchanged. TX follows tx_enable; ACTIVE follows the active_slice broadcast from aethersdr#4160. PTT, MOX and TUNE deliberately ignore the selection and address the slice that already holds transmit. TciProtocol::cmdTrx reassigns the TX slice before keying, so the previous hardcoded trx:0 could move transmit to another band and antenna on a keypress (Principle VI). There is no per-slice-index mode by design: trx_count is sent only in the init burst with no add/remove events, and TciProtocol::resolveSliceForTrx falls back to the first slice for an out-of-range trx, so a numbered picker built on a stale count would silently act on the wrong receiver. TX and ACTIVE are broadcast on every change and cannot go stale. Feedback on the controller. A TCI Connection Status action — first in the action list — makes the most common "the plugin does nothing" cause visible. Keys carrying a value show it, sourced from the radio and displaying "—" until the radio has actually reported, rather than a plausible default that reads as authoritative and is wrong (Principle II). State is re-requested with one-arg GETs on connect, when a key appears, and when the target changes. Also adds a Tune Power encoder, groups the actions into categories instead of one flat list, and exposes the CWR, SAM, NFM and RTTY modes the server supports but the plugin never offered. Fixed while testing on a FLEX-6600: - TUNE could not be switched off. cmdTune produces no notification and TciServer emits no tune: broadcast, so the plugin's flag never changed and every press re-sent "start". Now optimistic, and reconciled from trx: going false so stopping TUNE from the AetherSDR window is picked up (Principle II). - The FT8 action keyed SSB, and the action is removed rather than repaired. TciProtocol::tciToSmartSDR maps unrecognised names to a USB default instead of rejecting them, and "ft8" has never been in that map — so the button took the slice out of a digital mode and put it on SSB. Measured on a FLEX-6600: from DIGU, pressing FT8 produced modulation:0,ft8 from the notification path and modulation:0,usb from the model in the same millisecond, with rx_filter_band moving 0,3000 -> 100,2800. FT8 is not a modulation but an operating convention on DIGU, so repairing it would only have duplicated the existing DIGU action; the action was dropped instead of shipping one function twice under two names. - Per-slice state was cross-contaminated: the trx argument was discarded when parsing, so rx_nb_enable:1,true overwrote receiver 0's flag. - The VFO dial stalled mid-spin as inbound echoes overwrote ticks accumulated since the last send — the command and truth paths forming the feedback loop Principle II prohibits. Guarded, same echo-bounce class as aethersdr#4350. - Command records are split on the TCI terminator rather than newlines, which AetherSDR never sends. Adds the first tests for any bundled plugin: test.js, 13 tests run with `npm test`, using only node:test and node:assert so the plugin keeps its "no build step, no npm dependencies" property. Each behavioural test spawns the real plugin.js against fake Stream Deck and TCI sockets. streamdeck-plugins.yml now also runs on pull requests touching plugins/ — previously it ran only on release tags, so a syntax error would have shipped. test.js is excluded from the packaged bundle. plugin.js takes one testability change: the TCI port may be overridden with AETHERSDR_TCI_PORT, defaulting to 50001, so a test run cannot collide with a live AetherSDR or an already-running plugin instance. No AetherSDR or TCI changes; plugin only. The committed .streamDeckPlugin bundle is regenerated from source. Verified on Linux + OpenDeck + Mirabox N3EN against a FLEX-6600 (firmware 4.2.20.41343) on AetherSDR 26.7.4.1, across thirteen test groups with TCI traffic captured. Genuine Elgato hardware, Windows and macOS are unverified.
|
Not sure if relevant but here's the hardware test plan I followed while working on this. While it is very extensive I am certain it does not cover every possible scenario. |
There was a problem hiding this comment.
Genuinely strong contribution — the slice-targeting design, the decision to pin PTT/MOX/TUNE to radio.txTrx because cmdTrx reassigns the TX slice, and the reasoning for not building a trx_count-based slice picker are all correct and well argued in the comments where they'll survive. The first tests for any bundled plugin, with no new dependencies and mutation-checked, is exactly the right shape. I verified locally: all 13 tests pass, the committed .streamDeckPlugin bundle is byte-identical to source for plugin.js/manifest.json/package.json and correctly excludes test.js, and ws really is vendored so CI needs no install step.
The one thing I'd like addressed is that the "radio is authoritative" guard was applied to the display path but not the command path — keys correctly show —, but pressing them still acts on the invented defaults underneath. I reproduced this against a silent fake TCI server: Tune Down emits vfo:0,0,30000; and Tune Power emits tune_drive:0,50;. The window is narrow (sub-second after connect, or after a target switch), but it's the exact invariant the PR sets out to establish.
Would like fixed before merge
- Frequency/power/volume actions command the radio from fabricated defaults while their labels correctly show
—(plugin.js:723) radio.volumeBeforeMute || 50— un-muting from a genuinely-zero master volume jumps it to 50% (plugin.js:692)
Polish
nudgeRedrawAll()per learned key makes profile load O(N²) — I measured 14,524 host messages for a 50-key layout (plugin.js:466)- The new PR-triggered job inherits workflow-level
contents: write(.github/workflows/streamdeck-plugins.yml:9)
Non-blocking notes
- Removing
mode-ft8breaks existing decks that use it. Your reasoning is sound and you disclosed it clearly — flagging only so it lands in the release notes, since the failure mode for an operator is a silent "unknown action" rather than an error. - The TCI gaps you documented (
split_enablenot engaging split,volume:empty-arg parsed as SET-to-100%, master mute and RADE having no TCI verb) are worth their own issues — thevolume:one in particular is the same class as #4345 and will bite the next client author too. - CodeGuard's two CG-PATH-001 path-traversal hits on the workflow are false positives: both are literal relative paths in a
zipinvocation with no externally-controlled input, and line 42 isn't even touched by this PR. Dismissed, no action needed.
🤖 aethersdr-agent · cost: $6.3984 · model: claude-opus-5
| "com.aethersdr.radio.tune-up": { keyDown: () => { const s = targetSlice(); s.frequency += TUNE_STEP_HZ; tciSend(`vfo:${targetTrx()},0,${s.frequency};`); } }, | ||
| "com.aethersdr.radio.tune-down": { keyDown: () => { const s = targetSlice(); s.frequency = Math.max(VFO_FREQ_MIN, s.frequency - TUNE_STEP_HZ); tciSend(`vfo:${targetTrx()},0,${s.frequency};`); } }, |
There was a problem hiding this comment.
The known/shown() guard covers what the key displays, but not what it commands. These two handlers compute an absolute frequency from targetSlice().frequency, which is 0 until the radio has reported — so before the first vfo: arrives, Tune Down sends the radio to 30 kHz and Tune Up to 1 kHz.
Reproduced against a fake TCI server that accepts the connection but answers nothing:
vfo:0,0,30000; ← Tune Down
vfo:0,0,31000; ← Tune Up
vfo:0,0,3800000; ← Band Up landed on 80m (cursor derived from freq 0 → 160m)
tune_drive:0,50; ← Tune Power stepped up from the fabricated 25 W baseline
rx_volume:0,60; ← Slice Volume Up stepped from the fabricated 50
The window is narrow — requestState() fires 150 ms after connect and the init burst normally beats a keypress — but it's guaranteed to exist at every startup and again briefly after a target switch to a slice whose vfo hasn't been read. And unlike a wrong label, this one moves the radio.
The VFO dial has the same shape (getValue: () => targetSlice().frequency → clamp(0 + ticks*1000, 30000, …) → 30 kHz on the first detent), as do rf-power/tune-power/slice-volume-* and the !s.split/!s.locked toggles, which all invert a default the radio never confirmed.
Simplest fix for the frequency pair, using the helpers already here:
| "com.aethersdr.radio.tune-up": { keyDown: () => { const s = targetSlice(); s.frequency += TUNE_STEP_HZ; tciSend(`vfo:${targetTrx()},0,${s.frequency};`); } }, | |
| "com.aethersdr.radio.tune-down": { keyDown: () => { const s = targetSlice(); s.frequency = Math.max(VFO_FREQ_MIN, s.frequency - TUNE_STEP_HZ); tciSend(`vfo:${targetTrx()},0,${s.frequency};`); } }, | |
| "com.aethersdr.radio.tune-up": { keyDown: () => { const trx = targetTrx(); if (!known.has(`vfo:${trx}`)) return; const s = slice(trx); s.frequency += TUNE_STEP_HZ; tciSend(`vfo:${trx},0,${s.frequency};`); } }, | |
| "com.aethersdr.radio.tune-down": { keyDown: () => { const trx = targetTrx(); if (!known.has(`vfo:${trx}`)) return; const s = slice(trx); s.frequency = Math.max(VFO_FREQ_MIN, s.frequency - TUNE_STEP_HZ); tciSend(`vfo:${trx},0,${s.frequency};`); } }, |
For the dial and the relative power/volume steppers the same idea applies — ignore the input until the corresponding known key is set, rather than stepping from an invented baseline. Worth a test alongside the existing "radio-sourced values" one, asserting that a press before any radio report emits nothing.
| radio.volumeBeforeMute = radio.volume; | ||
| tciSend(volumeSetCommand(0)); | ||
| } else { | ||
| tciSend(volumeSetCommand(radio.volumeBeforeMute || 50)); |
There was a problem hiding this comment.
|| 50 is a falsy-coalesce on a value where 0 is legitimate. If the operator already had master volume at 0 and presses Mute All twice, volumeBeforeMute is 0, so un-muting sends volume:50; and the master fader jumps to 50% — audibly, from silence, on a key labelled "UNMUTE".
volumeBeforeMute initialises to 50 and is only ever assigned from radio.volume, so the fallback isn't protecting against undefined — dropping it is sufficient:
| tciSend(volumeSetCommand(radio.volumeBeforeMute || 50)); | |
| tciSend(volumeSetCommand(radio.volumeBeforeMute)); |
| function nudgeRedrawAll() { | ||
| const push = () => { | ||
| refreshKeypads(true); | ||
| for (const context of keypads.keys()) { | ||
| sdSend({ event: "setImage", context, payload: { target: 0 } }); | ||
| } | ||
| }; | ||
| setTimeout(push, 120); | ||
| setTimeout(push, 600); | ||
| } |
There was a problem hiding this comment.
This is quadratic in key count on a profile load, and it's invoked twice per key — once by the generic learner above and again by the willAppear branch below, for the same message.
Each push() walks every registered keypad, and nudgeRedrawAll() is called once per key that appears, so N keys → N × 2 × N setImage messages plus the forced refreshKeypads inside each. Measured against the real plugin with a full manifest-derived layout:
Profile load of 50 keys -> setTitle=4524 setImage=10000 total=14524
~14.5k WebSocket messages inside ~3 s. A 32-key deck still lands around 6k. That's the same repaint-storm class the rx_smeter test pins — the meter path is now bounded, but the layout path isn't.
The per-key fan-out isn't needed: since push() already repaints every key, one coalesced round per layout event does the same job. Guarding with a single pending-timer handle (the way requestState() already coalesces its GET round) collapses this to a constant two pushes regardless of key count, and would let you drop the duplicate call in the willAppear branch too.
| test-elgato: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
The new test-elgato job inherits the workflow-level permissions: contents: write (line 9), which exists for the release-upload job. That now grants write scope to a pull_request-triggered job whose whole purpose is to execute repo-controlled JavaScript — test.js spawns plugin.js as a child process.
For fork PRs GITHUB_TOKEN is read-only regardless, so the exposure is limited to same-repo branches; but ci.yml already documents the least-privilege convention here ("no writes, no token-bearing API calls"), and this job needs nothing:
| test-elgato: | |
| runs-on: ubuntu-latest | |
| test-elgato: | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read |
Separately, minor: actions/checkout is SHA-pinned on line 18 but actions/setup-node@v4 on line 19 is a mutable tag — worth pinning to match, since they sit two lines apart.
|
Cisco CodeGuard — static analysis of this PR (2 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $7.3435 · model: claude-opus-5 |
|
Antares Detector — candidate vulnerable file(s)
Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting. 🤖 aethersdr-agent · cost: $7.3435 · model: claude-opus-5 |
Addresses review on aethersdr#4514. The "radio is authoritative" guard was applied to the display path but not the command path: keys correctly showed "—" before the radio had reported, while pressing them still stepped from the struct's initial value and sent the result. Against a TCI server that accepts the socket and answers nothing, a cold start produced vfo:0,0,30000 from Tune Down, tune_drive:0,50 from Tune Power, and six more commands built on invented baselines. Relative gestures — those computing an absolute value from a baseline the radio supplies — now refuse until it has been reported: tune up/down, band up/down, the power cycles, master and slice volume, the flag toggles, and all four dials via a `ready()` predicate on createDial. Absolute actions (band buttons, modes, PTT/MOX/TUNE) need no baseline and stay live. The frequency half was largely unreachable in practice — TciProtocol::resolveSliceForTrx returns nullptr on a disconnected radio so handleVfoRequest drops the command, and generateInitBurst sends vfo: for every slice on connect. The power half was not: cmdTuneDrive's SET path is guarded by `if (m_model)` rather than isConnected(), so it writes TransmitModel whatever the radio state, and the plugin was the only thing in front of a fabricated transmit power. Also from review: - Drop the `|| 50` fallback when restoring volume after Mute All. Zero is a legitimate level and the field can never be undefined, so the coalesce could only turn a genuine silence into a jump to half volume. - Coalesce nudgeRedrawAll() and refreshKeypadsSoon() behind single pending handles, and drop the duplicate nudge for the same message. Both walked every key and were invoked per appearing key, making a profile load quadratic: 50 keys went from 14,524 host messages to 664. - Give the PR-triggered test job `permissions: contents: read` instead of inheriting the workflow's `contents: write`, and SHA-pin actions/setup-node to match the pinned checkout. Two tests added: a press before any radio report commands nothing, and the same presses do command once state is known, so the guard cannot regress into disabling the actions. The fake radio now seeds the per-slice flags its real counterpart sends. 15 tests pass. Refs aethersdr#4513.
|
All four addressed. Thanks — the command-path finding is a real one and I'd Command path now gated on known state. Split into two rules rather than
Verified against the same silent server: zero writes now, only the One correction worth recording, since it changes which half matters. I couldn't
For context on why the nudge exists at all, since it isn't in the diff: a key Workflow — FT8 in the release notes — agreed, and I'd rather the team write that 15 tests pass. Bundle rebuilt and still excludes |
Summary
Fixes #4513.
Makes the bundled Elgato Stream Deck plugin slice-aware and gives it feedback
on the controller itself. Every command previously addressed
trx0, so theplugin only ever drove slice A, and every key showed a static label from the
manifest — so an operator could neither point the deck at the slice they were
using nor see any radio value without looking at the AetherSDR window.
Headline changes:
TRX0 → TX → ACTIVE, applied to mode, DSP, squelch, lock, RIT/XIT, split,slice volume, slice mute and the VFO dial. Default stays TRX0, so an existing
deck behaves exactly as before.
most common "the plugin does nothing" cause was previously invisible.
target slice, mute-all — sourced from the radio, showing
—until the radiohas reported rather than a fabricated default.
encoders.
Audio, DSP, DVK, Dials) instead of one flat list, plus the CWR, SAM, NFM and
RTTY modes the server supports but the plugin never exposed.
Fixed in passing, each found during hardware testing: TUNE could not be
switched off (no
tune:broadcast exists, so the plugin's flag never changed);the FT8 action keyed SSB (
tciToSmartSDRmaps unknown names to aUSBdefaultrather than rejecting them); per-slice state was cross-contaminated because the
trx argument was discarded when parsing; the VFO dial stalled mid-spin fighting
inbound echoes; keys showed stale values after being added to a deck.
No AetherSDR/TCI changes — this is plugin-only.
Behaviour changes worth review
PTT/MOX/TUNE now address the TX slice rather than
trx0.TciProtocol::cmdTrxreassigns the TX slice before keying, so the previoushardcoded
trx:0could move transmit to another band and antenna on akeypress. Verified on air: with transmit on slice B and the target set to
TRX0, PTT keys slice B and transmit does not move.
The FT8 action is removed, because it was broken rather than redundant.
TciProtocol::tciToSmartSDRmaps unrecognised names to aUSBdefaultinstead of rejecting them, and
"ft8"has never appeared in that map in anycommit — so the button took the slice out of a digital mode and put it on
SSB. Measured on a FLEX-6600, starting from DIGU:
FT8 is not a modulation, it is an operating convention on DIGU — so
repairing the action would only have duplicated the existing DIGU button.
It was dropped rather than shipping one function twice under two names.
Existing decks with an FT8 key will show an unknown action. Trivial to
reinstate as a DIGU-mapped preset if you would rather keep the name.
Worth noting independently of this action: any unrecognised modulation
name silently keys USB rather than being rejected, so a future typo in any
client fails silently and on the air.
Key labels update at runtime where they were previously static.
Design note: no per-slice-index target
Deliberately omitted rather than overlooked.
trx_countis sent only in theinit burst with no slice add/remove events, and
TciProtocol::resolveSliceForTrxfalls back to the first slice for anout-of-range trx — so a numbered picker built on a stale count would silently
act on the wrong receiver, with no error. Confirmed on hardware: a session
opened with one slice reported
trx_count:1and never revised it while twofurther slices came up and broadcast as trx 1 and trx 2. TX and ACTIVE are
broadcast on every change, so they cannot go stale. The rationale is recorded
in the
TARGET_MODEScomment so it is not "fixed" later.Constitution principle honored
Principle II — The Radio Is Authoritative On Live State. Displayed values
come from the radio, never from a remembered or invented default: a key shows
—until the radio reports. Where a command has no status echo (tune:has nobroadcast at all) the plugin updates optimistically and then reconciles — a
trx:...,falsesupersedes the optimistic tune flag, so stopping TUNE from theAetherSDR window is picked up. The dial echo guard exists to satisfy the same
principle's prohibition on the command and truth paths forming a feedback loop:
inbound echoes were overwriting ticks accumulated mid-spin, so the dial fought
the radio and stalled.
Principle VI — AetherSDR Never Transmits Without Operator Intent. PTT, MOX
and TUNE are pinned to the slice that already holds transmit, never the
user-selected target, because
cmdTrxreassigns the TX slice before keying —so a keypress could otherwise put a signal out on a band and antenna the
operator did not choose.
Test plan
cmake --build build) — unaffected; no C++ changesAutomated tests — the first for any bundled plugin
The three plugins had no tests at all: the Elgato
package.jsoncarried onlythe npm stub, and
streamdeck-plugins.ymlran on release tags to zipdirectories, so a syntax error in
plugin.jswould have shipped in a release.Adds
test.js— 13 tests, run withnpm test. No new dependencies: ituses only
node:testandnode:assert, so the plugin keeps the "no buildstep, no npm dependencies" property in its own header, and
wsremains thesingle runtime dependency.
wsis already vendored, so CI needs no installstep. The workflow now also runs on PRs touching
plugins/**, andtest.jsisexcluded from the packaged bundle.
Each behavioural test spawns the real
plugin.jsagainst fake Stream Deck andTCI sockets, so the shipped file is what runs.
plugin.jsgained onetestability change: the TCI port may be overridden with
AETHERSDR_TCI_PORT,defaulting to 50001 — without it a test would have to bind the real port and
would be answered by, or collide with, a live AetherSDR.
Covered, each derived from a defect actually hit during this work:
Descriptionvolumeread shapevolume:NaNlabels, noNaNtrx reaching the radiorx_smetercauses no repaint stormtune_drive, neverdriveThe suite was mutation-checked rather than assumed: reverting the TX-slice
pinning, removing the title dedup, and dropping the trx boundary guard each
make the corresponding test fail.
Verified on Linux + OpenDeck + Mirabox N3EN, against a FLEX-6600
(firmware 4.2.20.41343) running AetherSDR 26.7.4.1, with TCI traffic
captured throughout. Thirteen groups:
Selected evidence:
every PTT/MOX emitted
trx:1,truewithtx_frequencymatching B — twentykeypresses, never
trx:0ortrx:2. The previous hardcodedtrx:0wouldhave moved transmit to A on every press.
rx_nb_enable:1,rx_nr_enable:2,rx_nr_enable:0— DSP addressing three different slices as the targetchanged, with
drive:andtune_drive:correctly following the TX sliceinstead. A later run exercised four slices.
and volume landing only on the dB-round-trip-safe percentages.
plugin, both clients working; a reconnect re-seeding every slice within a
millisecond; ~20 rapid toggles with no runaway or stuck state.
Squelch does not act in CW and APF appears mode-gated similarly — expected
AetherSDR behaviour, not plugin faults.
A key added to a deck mid-session can keep its manifest label in OpenDeck until
the host redraws. The title is applied and persisted correctly — OpenDeck
stores it against the key instance while the canvas renders the action
definition — so the plugin re-asserts the title and nudges a redraw on layout
changes. Behaviour on genuine Elgato hardware is unverified.
Genuine Elgato hardware, Windows and macOS are unverified. Testing was
Linux + OpenDeck only.
TCI gaps found (not addressed here — no plugin-side workaround exists)
split_enabledoes not engage split. Accepted, stored and echoed, butTciRoutingState::splitRequested()is only ever read back — init burst, GETreply, diagnostic dump, internal route sequencing. Nothing drives a
radio-side split from it. Tested with one slice and with two on the same
band. This is the substance of TCI: SPLIT_ENABLE bidirectional command has no effect when sent by client #1686. The plugin's split action is kept so it
starts working when this does.
AudioEngine::setMutedis reachablefrom shortcuts, MIDI and the GUI but has no TCI verb.
AudioEngine::setRadeModehas no TCI verband no registered controller capability.
volume:with an empty argument is parsed as SET-to-100%.TciProtocol::cmdVolumetests for an empty argument list, but"volume:"splits to one empty string, which is not empty, so a GET-shaped command
drives master volume to full. The plugin sends the bare
volumeform toavoid it. Same class as TCI: spec-compliant
DRIVE:<trx>;read is treated as a power SET (sets power to the TRX index) #4345.Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V) — n/a, no C++ changes
reverse-engineered from a proprietary binary (Principle IV)
MeterSmoother(AGENTS.md convention) — n/aDocs updated in
resources/help/configuring-aethersdr-controls.md: slicetargeting and why PTT/MOX/TUNE ignore it, the dial set, the action categories,
and why there is no per-slice-index option. The committed
com.aethersdr.radio.streamDeckPluginbundle is regenerated from source, andthe manifest
Descriptionno longer states an action count.