Repository navigation
feat(kxpa100): add amplifier and optional tuner applet - #6475
Conversation
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed at head 88e9edb against the stacked base 4ca5cd34 (so this review covers the KXPA100 additions only, as the body asks). Findings below are reasoned from the code in the PR head checkout — I have no build, no test run and no way to drive the GUI, so nothing here was reproduced at runtime.
1. Issue fit
There is no linked issue and no RFC for the KXPA100. search_issues for "KXPA100" returns only PRs (#6475, #5991) — #4953, which #6452 closes, covers the KAT500/KPA500 pair, not this device. So there is no requirements list to map the diff against; I reviewed against the PR's own stated intent, which the diff does deliver: protocol, model, applet, Advanced maintenance, and TX Controls integration are all present and coherent.
On GOVERNANCE.md: the applet half is explicitly carved out — "New applets or dialogs that don't change existing UX" does not require an RFC. The TX Controls half is a different matter, because it changes what the existing ATU and MEM buttons do once a device is associated (ATU→AUTO, MEM→SAVE, % suffixes on RF Power / Tune Pwr, and the shared meter source), and GOVERNANCE.md lists "Any change to default UX behavior — what a click, shortcut, or gesture does out of the box" as RFC-requiring. You already flag this yourself ("Default-on TX Controls integration after association, the AUTO/ATU/SAVE overlay, meter selection and visual/UX choices still need maintainer direction"), and #6452 set the precedent of tracking through an issue instead. Noting it as a maintainer call, not asserting a violation.
CI: only 4 checks ran (Static checks, Sanitizer configures, docs, PDF-skipped) because the PR targets codex/kat500-applet. There is no compile lane and no test lane in that set, so there is currently zero CI compile or test evidence for this diff — as the body states. The build/link checks I could do statically are in §5.
2. Scope
| Files | What it changes | Claimed? | Verdict |
|---|---|---|---|
src/core/Kxpa100Protocol.*, Kxpa100Maintenance.* (480 lines) |
New frame parser, status decode, EEPROM/backup/fault primitives | Yes | In scope |
src/models/Kxpa100Model*.{h,cpp}, Kxpa100Types.h (887 lines) |
Transport, transaction queue, maintenance state machine | Yes | In scope |
src/models/Kxpa100TxController.* (451 lines) |
TX Controls external-tuner sequence | Yes | In scope |
src/gui/Kxpa100Applet.*, Kxpa100AdvancedWidget.* (1,825 lines) |
New applet + Advanced page | Yes | In scope |
src/gui/AppletPanel.*, MainWindow.{h,cpp}, RadioSetupDialog*.{h,cpp} |
Register applet, pass the model, build the Peripherals page | Yes | In scope |
src/gui/TxApplet.*, SharedTxMeterSelection.h, MainWindow_Wiring.cpp |
ATU/MEM overlay, meter routing, autoconnect | Yes | In scope — but see Blockers 1 and 2; this is the only shared behaviour touched |
tests/kxpa100_*.cpp (6), tests/fixtures/kxpa100/* (5), tests/tests.cmake |
New coverage; Kxpa100AdvancedWidget.cpp added to 12 existing peripherals targets |
Yes | In scope |
CMakeLists.txt, docs/architecture/aetherd-touchpoint* |
7 new sources; manifest 262→265 / 44→47 models | Yes | In scope (generated; --check passes in Static checks) |
docs/user/docs/kxpa100-amplifier-control.md, peripherals.md, sidebars.js, 6 PNGs |
User guide | Yes | In scope |
Everything in the 49-file diff is explained by the stated feature. No CHANGELOG.md edit (correct — please keep it that way). One thing a reader shouldn't have to check: commit 6ae7906 is titled "Add KPA500 radio drive and label KXPA100 DRV", but no Kpa500* file appears in this PR's file list — the KPA500 half rode in via the base merge and belongs to #5991, matching what the body says.
3. Blockers
1. src/gui/MainWindow_Wiring.cpp:6449-6453 — a KXPA100 association short-circuits the entire shared meter policy, including the KPA500/PGXL arms.
if (m_appletPanel && m_appletPanel->txApplet()->kxpa100Associated()) {
return m_kxpa100Model.amplifierMeterEligible()
? SharedTxMeterSelection{SharedTxMeterSource::Kxpa100, 100, false}
: SharedTxMeterSelection{SharedTxMeterSource::Radio, maxWatts, false};
}
const bool amplifierActive = m_radioModel.amplifier().present() ...
const bool kpa500Active = m_kpa500Conn.isConnected() && ...
return selectSharedTxMeters(maxWatts, amplifierActive, kpa500Active,
m_kat500Model.ampKeyBlockConfirmed());This return sits above every other source. Note the contrast with how the KAT500 was integrated in the same function: it is passed into selectSharedTxMeters(...) as a parameter, so one policy function still decides. The KXPA100 instead bypasses that function entirely.
Failure scenario: a KX3 or HL2 → KXPA100 → KPA500 chain (a normal Elecraft stack). The operator associates the KXPA100 and the KPA500 is in Operate. kpa500Active and amplifierActive are never evaluated, so the meters are pinned either to the KXPA100's hard-coded 100 W or to the radio's maxWatts — the 500/1500 W amplifier scale is unreachable, and m_kat500Model.ampKeyBlockConfirmed() is skipped too. The final amplifier in the chain is the one the meters should be showing.
The right precedence is a design call, so I'm not proposing the patch: either fold the KXPA100 in as another parameter to selectSharedTxMeters (consistent with the KAT500), or keep the override and document it. Also worth a second look: 100 is hard-coded here where the other arms derive their scale.
2. src/gui/TxApplet.cpp:155-156, 176 — association (not selection) permanently tears down KAT500 TX routing, even for a KXPA100 with no tuner fitted.
if (kxpa100Associated() && m_katTx) { m_katTx.reset(); }
else if (!kxpa100Associated() && !m_katTx && m_kat500) { rebuildKat500Routing(); } if (m_radioModel && m_kat500 && !kxpa100Associated()) {The gate is kxpa100Associated() → Kxpa100TxController::associated(), which at src/models/Kxpa100TxController.cpp:71-77 checks only the settings scope and the serial — it does not require tunerPresent. Meanwhile atuRoute() claims the buttons only when selected() is true, and selected() does require tunerEligible() (hence tunerPresent == true).
Failure scenario: an operator with a bare KXPA100 (no KXAT100 option) plus a real KAT500 associates the amplifier so the shared power meters read amplifier watts. m_katTx is destroyed and never rebuilt while the association stands; the KXPA100 never claims the route because it has no tuner; so atuRoute() falls back to Radio and TX Controls silently reverts to the radio's internal ATU/MEM. The KAT500 integration is gone with no indication, and neither the KXPA100 guide nor kat500-tuner-control.md mentions that owning a KXPA100 overrides it.
Blockers 1 and 2 are the same underlying question — association is currently a hard override of every sibling device rather than a participant in the existing precedence — so they probably want one ruling. That ruling is also the second half of the default-on question you already raised.
3. src/models/Kxpa100TxController.cpp:130 — the TX Controls preference write result is discarded, with no log.
doc["useTxControls"] = enabled;
scope.setFeature("kxpa100", 1, doc);docs/agents/settings.md is explicit: "Check the write result. setFeature() can refuse (read-only store, reset in progress); a mutation that silently doesn't persist while the UI repaints from the store is the worst failure shape. Log loudly at minimum." That is exactly the shape here — Kxpa100Model::m_useTxControls has already been set in memory, so the checkbox repaints as enabled while the store rejected it, and the preference reverts next session.
The body discloses the UI half ("a failed persistence write of the integration preference has no dedicated UI warning"), but canon's minimum bar is the log, and there is no log either. The two sibling writers in the same file get this right — associate() at :89 and dissociate() at :117 both return false on refusal — so this is the one outlier. No test covers it (tests/kxpa100_tx_controls_test.cpp exercises setUseTxControls and associate but never a refusing store), which is why nothing caught it.
doc["useTxControls"] = enabled;
if (!scope.setFeature("kxpa100", 1, doc)) {
qCWarning(lcKxpa100Tx) << "kxpa100: TX Controls preference did not persist";
}lcKxpa100 is defined with internal linkage in Kxpa100Model.cpp:13, so it isn't visible here; the drop-in needs a category in this TU — add #include <QLoggingCategory> and, inside the AetherSDR namespace, namespace { Q_LOGGING_CATEGORY(lcKxpa100Tx, "aether.kxpa100") }.
4. Security audit
Required and run (security-audit subagent, read-only, over the harness file list and this PR's new parser/maintenance/model/GUI code). No confirmed defects. Two parity gaps I verified myself and credit to security-audit (CodeGuard):
src/models/Kxpa100Model.cpp:22—m_socket(std::make_unique<QTcpSocket>())never sets a read-buffer cap, where the siblingsrc/core/Kpa500Connection.cpp:28does:m_socket.setReadBufferSize(4096);. A hostile raw-TCP bridge peer flooding bytes while the GUI thread is stalled grows Qt's internal read buffer unbounded. Heavily mitigated —readyReaddrains withreadAll()straight intoFrameParser::feed, whose own buffer is capped at 512 (src/core/Kxpa100Protocol.cpp:32) — so this needs a blocked event loop to matter.Kat500Model.cpp:24has the same omission. One line to match the KPA500.src/models/Kxpa100Model.cpp:48—if (m_connected || m_connecting || host.trimmed().isEmpty() || !port) { return; }does no host syntax validation, whereKpa500Connection.cpp:161-165rejects over-long hosts,/, and non-printables. Not exploitable here: the host is operator-typed in Radio Setup and goes toconnectToHostwith no URL or SSRF surface. Defence-in-depth parity only.
The agent refuted, with the discriminating reason in each case, the whole list of things worth worrying about in wire-facing code: out-of-bounds frame[2]/frame[3]/(*bytes)[0]/fields[0..3]/captured(N) indexing (every size test short-circuits ahead of its index), off-by-one in rangeValid/buildEepromWrite/parseEepromReply, memory[address + offset] escaping the 1024/16384 buffers in decodeBackup, unbounded growth in FrameParser::feed (512), the request queue (64), debug ring (1000) and history deques (4096), division-by-zero or overflow in the scaling math (all divisors compile-time constants; relayMaskForValue casts to qint64), CSV injection in faultHistoryCsv (no wire-derived field can begin =/+/-/@), path traversal in saveBackup/QSaveFile and the device-derived SN%1.KXPAConfig default name (^SN is validated as exactly five digits), and credentials (none — the only persisted values are a 5-digit serial, a bool, and applet UI prefs; nothing belongs in QtKeychain). The .KXPAConfig restore path is gated by CRC32 + full-coverage + identityValid + exact serial/firmware match + firmware pinned to 01.39 + explicit confirmed + per-block write-then-readback. No new listening socket or bind: both models are outbound connectToHost clients only.
On the harness's 888 CodeGuard findings: none of them land on a line this PR adds. Every cited location is in .github/workflows/static-checks.yml, docs/user/screens.json, docs/user/src/components/LogAnalyzer/rules.json or src/core/AutomationServer.cpp — four files this PR does not modify, none of which contains a case-insensitive kxpa reference — plus docs/architecture/aetherd-touchpoint-tags.json:10 and :1157, nowhere near this PR's +1630 hunk. That is a whole-file scan of base content, and I have not folded any of it in.
5. Nits (non-blocking)
src/gui/MainWindow_Wiring.cpp:7611-7615— the applet's association handler discards the result:Kxpa100TxController::dissociate(...)/associate(...)both returnbooland both are ignored. The Peripherals button gets this right atsrc/gui/RadioSetupDialog_Peripherals.cpp:104-105(const bool saved = ...then an explanatory tooltip either way). Since the guide offers both entry points equally ("click Use with current radio in the peripheral page or applet Settings"), one silently does nothing on failure and the other explains. Same root cause as Blocker 3.src/gui/RadioSetupDialog.h:214— two declarations collapsed onto one line:...& pageReseeds); void buildKxpa100Device(PeripheralDeviceUi& ui, QWidget* stackParent,. Compiles, just unreadable; split it at the);.tests/tests.cmake:4623-4628—kxpa100_model_testlacks theif(TARGET Qt6::SerialPort)block its siblingkat500_model_testhas at :4650-4653. Harmless for the build (everyQSerialPortuse inKxpa100Model.cppis correctly inside#ifdef HAVE_SERIALPORT, and the target doesn't linkaethercore, which is what defines itPUBLIC), but it means the serial transport arm is compiled out of the only model test, soconnectSerialhas no coverage on any platform where the KAT500 equivalent does.src/gui/TxApplet.cpp:745—m_kxpa100->status().modeis unguarded inside a block entered onselected() || busy(), while the same function guardsm_kxpa100 && m_kxpa100->status().mode16 lines later at :757. As far as I can trace this is not a reachable null (the ternary short-circuits whenbusyis true, andselected()implies the model is alive), so this is consistency only.src/models/Kxpa100TxController.cpp:198—m_tuner->tunerEligible()is dereferenced before the&& m_tunerlater in the same expression; safe only becauseassociated()short-circuits first. The null checks onm_radio/m_tunerat the end of that chain are dead.src/models/Kxpa100TxController.cpp:17—effectiveTxFrequencyKhzreadsslice->frequency(), so on a transverter band the displayed RF frequency failsmhz < 100.0and the function returns 0, making the integration unavailable with "KXPA100 requires a TX frequency from 1800 to 54000 kHz". Refusing is the correct outcome (a VHF transverter's output doesn't pass through a 1.8–54 MHz amplifier), and it's the sametxSlice()->frequency()seam #6472 tracks — flagging only so it isn't mistaken for a bug later.
6. Socket-test disclosure
Per #5254 reporting: no new socket-owning test. I grepped all six added test sources for QTcpServer|QTcpSocket|QUdpSocket|QLocalServer|listen(|bind(|connectToHost|Fake|QProcess — zero hits. kxpa100_model_test and kxpa100_applet_test link Qt6::Network only because Kxpa100Model owns a QTcpSocket member; the tests inject the transport via Kxpa100ModelTest friend access (model.m_write = ...), so nothing opens, binds, listens or connects, and no fake amplifier/tuner peer is introduced. Nothing to act on — recording it as the notification.
7. What I tried to break
- Principle VI, the carrier sequence. Walked every path out of
Phase::Carrier. The carrier only starts inoperationFinished()afterstatus().tuning == trueandoriginalIntentValid()andinternalBypassed()(Kxpa100TxController.cpp:288-295); it is dropped by the 30 s deadline, by^FE, by the 100 ms watchdog the momentisTuning()orinternalBypassed()goes false, byoperationFailed, by tuner disconnect, by radio disconnect, and by~Kxpa100TxController(m_carrier.stop()at :137). Nothing in the status path can manufacture authority — a spurious^FEor^TP0from the wire can only stop the carrier.tunePoweris never written. Held. - Antenna and freshness. Tried to start a tune with no confirmed antenna:
tunerEligible()(Kxpa100Model.cpp:17-22) requires fresh^TU/^MD/^ANandantenna.has_value(), so the "ANT1 after power-off" case in your own troubleshooting section can't be tuned into. Tried to publish stale watts to the shared meters:amplifierMeterEligible()requiresfresh("^PF")plusoperate == trueandtuning == false, so thevalue_or(0)in thestatusUpdatedlambda can't surface a stale forward reading. Both held. - The tuner-loss window. Found one state where
busy()survives a destroyed tuner —Phase::Cancelling, whererefresh()only callspollCancellation(), which early-returns on a nullm_tuner, leaving the phase set until the 5 s deadline. Every other phase self-heals to Idle throughoriginalIntentValid()/saveIntentValid()/theResultguard. I could not turn that window into a reachable crash (it needs an event loop running after the model is destroyed, and the model is a directMainWindowmember), hence the nit rather than a finding. - "Restores normal radio controls." Verified the claim rather than taking it:
updateAtuAvailabilityrestoressetCheckable(true), theMEMtext,kRadioTxButtonStyleand them_radioHasTuner/m_radioHasTunerMemoriesenable rules on the radio fallback path, and the KXPA100 arm mirrors the KAT500 arm exactly. Held. - The build CI never ran. Swept
tests.cmakeprogrammatically: all 12 targets that compileRadioSetupDialog_Peripherals.cppgainedsrc/gui/Kxpa100AdvancedWidget.cpp— none missed. All 3 targets that compileTxApplet.cpplinkaethercore, which carriesKxpa100TxController.cppviaCORE_SOURCES, so the new symbol resolves. All 7 new sources are inCMakeLists.txt; all 6 new tests haveadd_test;kxpa100_applet_testandkxpa100_advanced_widget_testjoinedAETHER_SETTINGS_CONSUMERS(the two*_tx_controls_testtargets correctly don't — they linkaethercorerather than compilingAppSettings.cpp, matching the KAT500 precedent). I found no static build break, but this is not a substitute for the compile matrix. - The tests, attacked rather than counted.
kxpa100_tx_controls_test.cppis not self-confirming: it assertsb->wire.isEmpty()across association, identity change and mode change (so a stray radio command fails it),b->wire.count("tune:on") == 1only after^TP1,tune:off == 1after^FE,tunePower() == 1unchanged, and the full fallback (^MDB→ATU/MEM/disabled, foreign identity →MEM). Those would fail with the guards broken. It usesTestSettingsProfilefor isolation. The gap is the one behind Blocker 3 — no refusing-store case — andsharedTxMeterSelection()is unreachable from any test since nothing constructsMainWindow, which is why Blocker 1 has no coverage either. - Settings canon.
associate/savePreference/dissociate/loadAssociationall usefeatureExact(never the family-widefeature()), all refuse aschema_versionnewer than 1, andassociateadditionally refusesCorrupt/Unavailableand an emptyradioId— so no accidental family-wide row. Per-radio state is in aradio_settingsfeature document viasettingsScope(), and only the client-side connection prefs use the flatPeripheralSettingshelper, which is correct. The only break is the unchecked write at :130.
8. Recommendation
Needs maintainer decision. The engineering here is careful and the hard part — the TX sequence — is right: the carrier is strictly subordinate to a confirmed tuner state, every abort path drops it, and the parser survived a dedicated adversarial pass with no confirmed defect. Blocker 3 is a two-line fix. What actually needs a ruling is Blockers 1 and 2: as written, associating a KXPA100 is a hard override of every sibling device — it short-circuits the shared meter policy above the KPA500/PGXL arms, and it permanently destroys KAT500 TX routing even when the KXPA100 has no tuner fitted. That is the same question as the default-on TX Controls behaviour you already flagged for CODEOWNERS, and it's worth answering once for all three devices in the stack before any of them merge.
Concrete next step: fix Blocker 3, get a ruling on association precedence, then retarget to main once #5991 and #6452 land so the compile matrix and test lanes actually run — right now four checks have passed and none of them built this code. Thanks for the unusually honest limits section; "a zero sampled peak is not an event-wide power measurement" and the explicit "Linux native KXPA100 validation is not yet recorded" made this much faster to review.
🤖 aethersdr-agent · cost: $26.0596 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (893 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: $28.1619 · model: claude-opus-5 |
|
Published shared RFC #6477 for the KAT500 and KXPA100 TX Controls changes: default-on behavior, AUTO/ATU/SAVE and route indication, optional-tuner coexistence, and shared amplifier-meter precedence. It records that implementation preceded the RFC and requests maintainer approval rather than treating existing code or operator approval as that decision. The KXPA100 precedence findings are explicit open items; this link does not mark them resolved. |
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
1 similar comment
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
|
Addressed the unchecked preference-write finding in signed commit A regression holds a competing transaction on the isolated test database. It failed before the fix and passes afterward, checking the categorized warning, retained saved preference, session-only change, reload behavior, successful save after releasing the lock, and no preference-triggered radio/device commands. Native Mac app build, all three focused KXPA100 suites and docs production build pass. No hardware or RF was used. The two multi-device routing/meter-precedence findings and RFC #6477 remain open. New-head stacked CI is running; this does not claim main-target platform CI. |
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
1 similar comment
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
|
Addressed the outstanding review items at
The old TxApplet/controller null-check nits became obsolete with removal of those paths. The transverter-frequency refusal noted by the reviewer remains intentional. The nine RadioSetupDialog accessibility warnings reproduce on the KAT500 base; none is new in this PR. The earlier CodeGuard report's out-of-diff findings remain outside this change. Native Mac app build, all 16 combined focused offline suites and production docs pass. The retained screenshots identify replay telemetry explicitly; no RF or hardware writes were performed. RFC #6477 is closed as withdrawn, recording Jeremy's direction and Ozy's confirmation of the radio-side TX Controls. Full main-target platform CI remains required after eventual retargeting, along with Jeremy's code review. 73, Ozy K6OZY · prepared with Codex |
rfoust
left a comment
rfoust
left a comment
There was a problem hiding this comment.
The PR delivers the combined KXPA100/KXAT100 applet, Advanced maintenance tools and exact-radio metering association described in its current body. There is no linked KXPA100 feature issue; the removal of the external TX Controls overlay is consistent with the direction recorded in #6477. The earlier meter-precedence and association-write findings are addressed. One documented-protocol compatibility blocker remains, reproduced through the production model with injected transport.
Scope
| Files/group | Change | Claimed? | Verdict |
|---|---|---|---|
Kxpa100Protocol.*, Kxpa100Maintenance.*, Kxpa100Types.h, Kxpa100Model* |
Wire parsing, confirmed state, bounded device/maintenance operations | Yes | In scope; SWR compatibility blocker below |
Kxpa100Applet.*, Kxpa100AdvancedWidget.* |
Dedicated combined applet and maintenance UI | Yes | In scope |
AppletPanel.*, MainWindow*, RadioSetupDialog* |
Registration, lifecycle and peripheral setup | Yes | In scope |
Kxpa100RadioAssociation.*, SharedTxMeterSelection.h |
Metering-only radio association and amplifier priority | Yes | In scope |
CMakeLists.txt, six KXPA100 tests, fixtures, tests/tests.cmake |
Build and offline coverage | Yes | In scope |
| User guide, sidebar, screenshots and touchpoint manifests | User documentation and generated architecture inventory | Yes | In scope; unused overlay assets noted below |
No unrelated code changes or CHANGELOG.md edits found. The current diff matches the revised feature scope, apart from obsolete title/assets.
Blocker
- [P2] Accept documented decimal SWR replies before treating the poll as timed out. See the inline comment on
src/core/Kxpa100Protocol.cpp:93-95. Elecraft's published decimal replies are rejected, leaving the routine SWR request pending until the model closes the transport. The existing protocol test explicitly requires rejection of decimal SWR and therefore preserves this incompatibility. This is an injected-transport reproduction, not a claim that firmware 01.39 was observed sending decimal replies on hardware. Please support the documented format alongside any independently captured variant and add a model regression proving that a valid decimal reply completes the poll without disconnecting.
Nits — non-blocking
- Update the title: it still advertises the removed TX Controls integration.
- Remove the three unused overlay screenshots:
kxpa100-tx-auto.png,kxpa100-tx-disconnected.png, andkxpa100-tx-manual.png.
Verification and attacks
- Reviewed head
982ce6019680cc28d58b2f16f02b06392fa7c4d0, stacked based0f2dfe8b5edea6fe7baaa8d1592137413b86826, and merge1bfef7e53ae4a9cfeba96d11bc542d3c6a5d5d79. The merge tree equals the tested head, which includes main86f592a9f5303e3581d514d9ceb8a7dead9a3235. - Full native macOS ARM64 app build passed with RADE enabled; host/system processors and final executable verified ARM64, with no RNNoise x86 sources. macOS icon generation needed normal host access; explicit Qt 6.12 runtime paths avoided the host's mixed-framework launch issue. These are excluded from PR findings.
- All six focused KXPA100 tests passed: protocol, maintenance, model, applet, Advanced widget, and TX-controls/association. Preflight found no socket/serial-opening test: invalid-endpoint refusals and transport/state injection only.
- The model tests survived timeout, identity-change, RF-guard and interrupted-restore scenarios. Removing the TCP receive-buffer cap made the supplied test fail. Association refusal, optional-tuner absence and shared-meter precedence tests passed.
- All four architecture checks passed on both base and head; engine-boundary findings were unchanged after normalizing shifted line numbers. No separate automated review skill was available; prior bot feedback was checked against this revision, followed by a second manual pass.
- Drove the built app offscreen with isolated settings, an explicit socket and TX pinned off. Connected only to
DEMO-0001; the bridge reportedmodel: "AetherSDR Demo", one slice/pan, andtransmitting: false. ATU/MEM retained their radio-side labels and were disabled for the simulator. Adding the disconnected KXPA100 entry and opening Advanced left read/restore/erase/reset disabled withConnect the KXPA100 first.The review instance was closed and its exit verified. - Current static/docs/sanitizer-configuration checks passed; release PDF attachment was skipped. All 20 commit signatures verified valid. Full platform build/test CI has not run for this stacked target and remains required after dependency landing/retargeting. Actual KXPA100 communication, EEPROM effects and Linux/Windows execution remain unverified; the demo cannot establish external-device convergence.
Recommendation: Request changes for the SWR compatibility issue, then rerun focused coverage and obtain the full platform CI before merge. No live radio connection, RF transmission or device maintenance write was performed during this review.
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
Ozy311
changed the title
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
1 similar comment
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
0113445 to
e83398b
Compare
Rebase the final KXPA100-only delta from 969a586 onto the reviewed KAT500 integration, retaining documented decimal SWR handling and main KPA500 telemetry freshness admission.
969a586 to
220f872
Compare
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
Documented decimal SWR finding fixed in final main-target head 220f872, carrying the 9a6f154 parser/scheduler fix. kxpa100_protocol_test and Kxpa100ModelTest::swrPolling pass on this head; disabling decimal handling in identical sources failed both regressions before rebase. All 10 selected headless tests and native ARM64 app build pass. The resolved thread has the detailed evidence. Title and unused overlay PNG nits are addressed. This dismissal retires the fixed finding; merge still awaits current platform/Docs CI.
jensenpat
left a comment
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed final main-target head 220f872, based on merged KAT500 main 450f7b3. No outstanding code blocker.
The prior documented-decimal SWR defect is fixed: protocol and production-scheduler tests cover decimal/zero/compact replies, fragmented delivery, clearing the active request and continued polling beyond its original timeout. Both tests failed when decimal handling was disabled in the identical pre-rebase parser source, passed after restoration, and pass on this final head. The title, obsolete overlay assets and feature-issue linkage are corrected. The old changes-requested review was dismissed with this evidence and its thread remains answered/resolved.
The rebase preserves main’s KPA500 hasLiveTxMeters admission and radio-integrated/KPA500/KEY BLOCK/KXPA100 meter precedence. No external TX Controls takeover remains. Audited all 14 test targets that compile the Peripherals page; each includes both Advanced widget sources. The previously missing operations-test dependency is repaired in 220f872.
Native macOS ARM64 app build passed. All 10 selected offscreen CTests pass: six KXPA100 tests, KAT500 meter selection, Radio Setup pair grid, TX power reconciliation and Radio Setup operations. Four architecture gates pass on main and head with unchanged normalized findings; generated references, Log Analyzer, registration and frozen gate checks pass. The actual 13-file GUI diff has the same nine legacy accessibility warnings as its base, with no new finding. The optional automated code-review skill was unavailable; manual parser/lifecycle/maintenance and failure-path review was performed.
Final-head bridge proof used fresh isolated settings and TX pinned off, connected only to DEMO-0001: one slice/pan, transmitting=false, native ATU/MEM; disconnected Advanced read/restore/erase/reset controls carry the accessible reason “Connect the KXPA100 first.” The owned app exited. No live radio, RF or EEPROM write was performed. Real-device convergence, physical persistence and cross-platform runtime are unverified by this pass. Hosted main-target platform/Docs checks must pass before merge.
3 participants
Closes #6512.
Adds the compact Elecraft KX100 applet for the KXPA100 amplifier and optional KXAT100 tuner, built on the merged KPA500/KAT500 foundations (#5991, #6452). Supports local serial and raw TCP, confirmed amplifier telemetry, manual tuner controls, Full/Mini presentation, history, diagnostics and Advanced maintenance.
.KXPAConfigbackup/restore, stored faults/CSV, memory inspection/confirmed erase and separately confirmed reset. Firmware-dependent maintenance is restricted to the verified 01.39 layout. Connecting and opening Advanced perform no maintenance write.Transport/parser queues and receive buffers are bounded, endpoints are validated, and late/uncertain transactions do not replay writes. Basic SWR polling accepts documented
nn.ndecimal replies, including zero, alongside captured compact replies. No Telnet/RFC2217 support, remote power command, firmware updater or raw Command Tester is added. Protocol knowledge comes from public references and isolated Utility I/O observations, without copying proprietary implementation.Validation at rebased head
220f8726e6330ee3b7287cb8a51e76008c2c8a83:The Full meter screenshot uses explicitly labeled replay telemetry and is not a live RF measurement:
Generated with OpenAI Codex (GPT-6 Astra)