Skip to content

Radio tab actions, saved ordering, and connection handoff - #6889

Open
jensenpat wants to merge 4 commits into
aethersdr:mainfrom
jensenpat:aether/radio-tab-connect-flow
Open

jensenpat wants to merge 4 commits into
aethersdr:mainfrom
jensenpat:aether/radio-tab-connect-flow

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Depends on #6885.

  • Add radio-tab Connect/Switch, Disconnect, Rename, Connection settings, and Remove from tabs.
  • Confirm replacement of the current connection and retain the selected Location, radio, and path when moving between tabs and the dialog.
  • Persist dragged tab order across restarts.
  • Let explicit connection choices supersede startup auto-connect; return failed IP connections to their saved Location.
  • Use named theme colors for readable highlighted menu text.
Screenshots

Dark menu
Light menu
Connection switching
IP connection recovery

Share saved connection identities between workspace tabs and Locations; fence superseded startup attempts; confirm and sequence single-session replacements; retain precise dialog recovery context. Persist browser-style tab ordering and use named canon colors for every menu selection state. Add bridge, rendered-menu, lifecycle and startup regression coverage plus user documentation and screenshots.
Preserve the selected-radio hierarchy from aethersdr#6885 while retaining active-path disabling and confirmed single-session switching.

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #6889 (stacked on #6885)

What I ran: this PR's own delta (136dba0b1..d878d7e3c), built on Linux with Qt 6.12.0 both on its head and merged with current main (155850f50).

  • I drove it through the automation bridge against the built-in demo radio (DEMO-0001): offscreen, isolated settings store, TX disabled.
  • I added a second, unreachable saved connection (TEST-NET 192.0.2.10) so there was a second tab to work with.
  • Findings that belong to #6885 are in my review there.

Issue fit. There is no issue of its own. The body's claims are radio-tab Connect, Switch, Disconnect, Rename, Settings and Remove; switch confirmation; persisted drag order; explicit choices superseding auto-connect; and failed IP connects returning to their Location. Most of these held up when driven (see the end). Two did not survive: the old radio being adopted as the new tab, and Connect on "Discovered nearby".

CI. Green, but the runs were created at 06:28Z, before main moved (tip 14:09Z), and the PR is now CONFLICTING. The only conflict is the generated touchpoint manifest, which regenerates cleanly.

Scope

File / group Claimed in body? Verdict
RadioTabBar.{h,cpp}, tab menu, drag order (RadioSwitcher.tabOrder) Yes In scope
MainWindow_Session.cpp switch controller and auto-connect supersession Yes In scope
ConnectionPanel target and handoff API Yes In scope
RadioModel::prepareWanConnection → rebuildBackendForFamily("flex") (RadioModel.cpp:3895) No (only in window-chrome.md) Family-string branch (#5554). Maintainer accepts it as SmartLink-is-Flex-only, provided the body says so
The title-bar "+" now opens Locations instead of the discovery popover; tabs take right-click for their own menu No Behavior change. Maintainer accepts it, provided the body says so
New bridge verb titlebar showRadioMenu; selectRadio also matches by serial No New public surface. Maintainer accepts it, provided the body says so
menu-reference.md, window-chrome.md, screenshots Partly Fine, but menu-reference.md silently drops "…or remove the tab" (see nit 3)

Blockers

1. Connect on a "Discovered nearby" radio silently does nothing when that radio is saved at another Location. Reproduced on the demo.

  • Steps: the demo is saved under Home. Select the empty POTA Location; the demo is listed as "AetherSDR Demo · Discovered nearby", and its card has an enabled Connect button. Click it.
  • get radio reported connected: false at 2, 4, 6, 8 and 10 s. No status message appeared; the status line still showed the unrelated earlier "No radio responded at 192.0.2.10."
  • Cause: connectWorkspaceSelection() (src/gui/ConnectionPanel.cpp:4756-4760) returns on an empty connectionTarget(). connectionTargets() (:3799) skips radios saved at another Location, but refreshWorkspace still lists them as nearby:<family>:<serial> entries.
  • #6885's version of this function found the radio in discovery and connected, so this is a regression this PR introduces.
  • Fix: resolve nearby: entries to a local target, or show and disable the entry with a "Saved at Home" hint. Don't return silently.
  • Note that the first nearby row selected by default in that list was a real FLEX on my LAN, so this view is the main way users reach discovered radios.

2. The old radio's automatic reconnect can be recorded as the newly chosen tab. Reasoned from the code. The demo can't reach this path: SimBackend leaves m_lastInfo.address null, so RadioModel's reconnect timer never arms. I confirmed that with sim disconnect; the demo stayed "Link lost" with no retry.

  • Radio A drops unexpectedly, and RadioModel's 5 s reconnect timer (RadioModel.cpp:2707) is armed with m_connectAttemptActive == false.
  • The operator picks tab B. beginExplicitRadioChoice() (MainWindow_Session.cpp:323-345) only calls m_radioModel.disconnectFromRadio() when isConnectAttemptInFlight() is true, so the timer stays armed.
  • If B parks before connectToRadio() runs, for example in the SmartLink "attention" phase waiting for sign-in, the timer reconnects A.
  • connectionStateChanged(true) (:748-751) then adopts m_pendingRadioTarget (B) as m_activeRadioTarget without comparing serials. Tab B shows Connected while radio A is live, and rememberSuccessfulTarget saves B's path as lastConnection.
  • An operator who keys up thinking they are on B is transmitting on A, which puts it in Principle VI territory.
  • Fix:
    • stop the pending reconnect in beginExplicitRadioChoice() whenever the radio isn't connected, not only when an attempt is in flight;
    • and in the connected branch, adopt the pending target only when its serial matches m_radioModel.serial(). A routed path with no serial can be matched by its probed serial. Otherwise fall through to the else branch.

Nits (non-blocking)

  1. m_radioDisconnectPending is cleared only on a connectionStateChanged(false) edge (MainWindow_Session.cpp:340, :545, :770).
    • An aborted attempt that never reached Connected may never emit that edge. SimBackend::disconnectRadio returns early when !m_connected, and a Flex socket still in ConnectingState is suspect too.
    • If it sticks, every later connect waits in m_afterRadioDisconnect forever.
    • I could not make it stick on the demo: choosing the demo during the TEST-NET probe connected cleanly within 0.5 s. A timeout, or clearing it in failRadioSwitch, would make it robust.
  2. Cancelling the client-slot picker leaves the switch pending. connectSelectedRadio (~:555) returns "Connection canceled" without failRadioSwitch(). The tab stays "Connecting…" and auto-connect stays suppressed.
  3. Removed behavior in "Remove from tabs." The old remove path cleared m_lastSessionTab, stopped m_heartbeatMissTimer and cleared the link alarm (from #6198, 5f09371e2). handleRadioTabAction's remove does none of these. m_lastSessionTab is now never assigned (MainWindow.h:2033), so it is dead state. Please carry the cleanup across, or say why it's no longer needed.
  4. Tab reordering is mouse-only. docs/a11y.md says "every interactive widget reachable by keyboard, with a non-mouse activation path … new code shouldn't add to the list." Move left / Move right items in the tab context menu would close it.
  5. LastRoutedRadioIp is now written on every connect, including SmartLink (MainWindow_Session.cpp:763-765). It reads lastRadioInfo(), which a WAN connect never sets. So after a routed session followed by a SmartLink one, the WAN serial is paired with the old routed IP. Write these keys only for non-WAN sessions, as before.
  6. One failure produces two state transitions. setManualMessage now emits connectionNeedsAttention on errors during a target attempt (ConnectionPanel.cpp:~1824), and the startup-probe failure then also emits connectionAttemptFailed.
    • In my run, a failed TEST-NET switch left the connection panel and a separate "Connection failed" window (radioSwitchReconnectPrevious) both open.
    • That window is also untracked: no QPointer; see docs/style/dialog-patterns.md.
  7. Editing the Icom user name or IP clears the typed password on every keystroke (ConnectionPanel.cpp:4319-4328). Fixing a typo in the user name wipes the password. Clear it only when the identity actually changes, for example on editingFinished with a changed value.
  8. Browsing the radio list writes the whole workspace on every row change and rebuilds every list (ConnectionPanel.cpp:4330). It is correct (40 rapid row changes ran without a crash), but it is a settings write per arrow key. Keep browsedRadios in memory and persist it later.
  9. refreshRadioTabs cost. It calls validateConnectionTarget for every target. Each call enumerates QMediaDevices and rebuilds connectionTargets(), which is O(n²). It runs on every discovery tick and every TransmitModel::stateChanged, so on each MOX edge. Enumerate once per refresh.
  10. RadioSwitcher.tabOrder keeps ids of deleted connections forever (MainWindow_Session.cpp:918-930), and the settings.save() results there are unchecked.
  11. A cached SmartLink token is never dropped. A failed refresh never removes the account's m_sessionCredentials entry (SmartLinkClient.cpp:179 vs :300), so auto-login keeps re-posting a revoked token until restart. Manual sign-in does recover. Remove the entry in the refresh-failure branch.
  12. Misleading tooltip. On the demo tab, Rename is disabled with "Save this radio to a Location before renaming it…", but the demo was already saved in Home. Only the simulator-label half of the message applies.
  13. Test coverage. The tab render, drag and Escape tests and the stale-probe test are real: they fail without the change. But nothing covers the MainWindow switch controller (prompt, disconnect, continue, fail), the TX refusal, m_startupConnectSuperseded, or the tabOrder write and reload sort. "Persists across restarts" is backed only by a widget signal; I verified it on the demo instead (below).

What I tried to break that held

  • Persisted tab order. I dragged "Testnet rig" ahead of the Simulator (drag … -200 0). RadioSwitcher stored {"tabOrder":["…e690720d…","…67794e0a…"],"version":1}, and after killing and relaunching the process the tabs came back in the dragged order.
  • Switch confirmation. With the demo connected, "Switch to this radio…" on the TEST-NET tab opened radioSwitchConfirmation: "Switch from Home · Simulator (not on the air) to Home · Testnet rig?" Cancel kept connected: true, serial: DEMO-0001. Switch disconnected the demo.
  • Failed IP connect returns to its Location. The panel reopened on "Radios at Home" with tabs Testnet rig, Connection failed and Simulator, Available.
  • Choosing again mid-connect. Choosing the demo while the TEST-NET probe was in flight connected the demo within 0.5 s, and the tab states were correct for 36 s.
  • Tab actions. Disconnect from the tab kept the tab as "Available". Connect, Rename and Remove are disabled appropriately while connected.
  • TX safety, by reading the code. requestRadioTarget and performRadioSwitch both re-check isTransmitting, isMox and isTuning. Nothing restores TX on the new session. The confirm-dialog generation counter guards a stale accept.
  • Static gates. The engine-boundary and command-plane gates, compared per file between merge base and head, are flat. Every theme token used already exists on main.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants