Review: changes requested

Nice, well-targeted fix for the #3314 manual-IP parity gap — probing info get and re-evaluating Port B on deviceInfoChanged() is the right shape, and the m_pending.contains(seq) guard correctly prevents a double-init. CI is green. One real bug to fix and one design decision to make before merge; the rest are non-blocking nits.

Blocker (one-liner): m_seqInfo is set once and never reset, but sequence numbers wrap at 255 in sendCommand, so after a long session a command that reuses the info-get seq has its response hijacked as info get and re-runs the whole init sequence mid-session. Inline suggestions add the reset.

Decision needed: runInitSequence() is now fully gated on the info get response. A device that accepts the TCP connection but silently ignores info get (no body, no error terminator) never initializes — there's no init watchdog (the existing QTimers are discovery/reconnect). Error responses are handled; silent non-response isn't. Either add a short init-timeout fallback to runInitSequence(), or confirm every supported AG/ShackSwitch firmware answers info get (even with an error code). Flagged inline.

Non-blocking: the duplicated parse block and the redundant double Port-B update.

Thanks @TnxQSO-Admin — the pcap-backed framing and the #3314 parity write-up made this easy to follow.

Thanks for this, @TnxQSO-Admin — the framing is excellent. Replacing a static version heuristic with the device's own authoritative info get response is the right call (Evidence Over Assertion), the scope is tight (3 files, all in-scope), and the writeup with the pcap + the candid "Open questions" section made this easy to review. The dual parse paths (inline single-line body vs. terminator/error fallback) are a correct read of the protocol's two response shapes.

A few things before merge.

1. [blocker] m_seqInfo is never reset — confirmed. I verified ten9876's finding against the source. sendCommand cycles m_nextSeq through 1..255 and wraps (AntennaGeniusModel.cpp:388-389), and processResponse only gates on m_pending.contains(seq) (:432). m_seqInfo keeps its value for the whole session, so once the counter wraps (~255 commands — keep-alive pings alone get there on a long session) and an unrelated command reuses that seq, both the inline path and the terminator path match seq == m_seqInfo. That re-parses the unrelated body into m_device and fires a spurious full runInitSequence() (re-sending antenna list / band list / port get) mid-session. Clear it once handled, on both paths:

m_seqInfo = -1;  // clear so a wrapped-seq reuse can't re-enter the info-get path
runInitSequence();

2. [recommend] No watchdog for a silent info get non-response. This is the more consequential of your two open questions. Init is now fully gated on the info get reply. The code != 0 terminator path covers an error response, but a device that accepts TCP, sends the prologue, then silently drops info get (no body, no error terminator) never reaches runInitSequence(), and the connection sits connected-but-empty until onTcpDisconnected eventually fires. This isn't purely hypothetical: previously runInitSequence() ran unconditionally right after the prologue, so this changes behavior for every device — including the ShackSwitch hardware the existing version heuristic was written for, where we don't actually know that info get is implemented. I'd take the 5-second single-shot fallback you offered; it's cheap insurance on exactly the manual-IP path this PR targets. (A timer that's cancelled in the response handler and falls back to runInitSequence() on timeout.)

3. [nit] Factor out the duplicated key/value apply. The 6-line kvs.contains(...) block is verbatim in both the inline and terminator paths, so a new field has to be added in two places. A small applyDeviceInfo(const QString& line) helper removes the drift risk. Non-blocking.

On the other Copilot notes: the dangling-pr reference (a reference invalidated by m_pending.remove(seq) but not read afterward) is harmless as written — fine to leave, just don't add code reading pr below it. The double Port B update from both connected() and deviceInfoChanged is also harmless; collapsing it is optional cleanup.

#1 is the one I'd treat as required; #2 is a strong recommendation given it now gates init for all device types.

_{🤖 aethersdr-agent · cost: $3.0897 · model: claude-opus-4-8}

Re-reviewed at 4325f4f5 — every point from the prior review is addressed, cleanly. This supersedes my earlier changes-requested.

• m_seqInfo wrap-hijack (blocker) — now reset to -1 in both processResponse paths, so a wrapped sequence number can't re-enter the info-get handler. ✓
• Init gated on info get with no fallback (the decision) — resolved the right way: a 5 s single-shot m_initWatchdog, started when info get is sent and stopped the moment it answers (body or error), falls back to runInitSequence() if the device never responds. Traced the paths — exactly one runInitSequence() fires in every case (response-first stops the timer; watchdog-first resets m_seqInfo so a late response is ignored), and the timer is parented for cleanup. ✓
• Dedup — the parse block is now one applyDeviceInfo() helper used by both paths. ✓
• Redundant Port-B update — the connected()-handler setVisible is gone; deviceInfoChanged is the single authority. ✓

Nicely done, and the watchdog is a better answer than my suggestion since it also recovers a genuinely-stalled device. CI green on all six checks. Approving — good to merge.

Thanks @TnxQSO-Admin.