diff --git a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx index 233e80247..9f0bdbe7a 100644 --- a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx +++ b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx @@ -141,28 +141,34 @@ describe('ConnectionPanel host link meter', () => { }); }); - it('shows Link quality for Meshtastic TCP', async () => { - vi.mocked(window.electronAPI.getPlatform).mockReturnValue('darwin'); - vi.mocked(window.electronAPI.hostLink.probeTcpRtt).mockResolvedValue(120); - render( - , - ); - expect(screen.getByText('Link quality')).toBeInTheDocument(); - await waitFor(() => { - expect(window.electronAPI.hostLink.probeTcpRtt).toHaveBeenCalled(); - }); - }); + it.each(['linux', 'darwin', 'win32'] as const)( + 'shows Link quality with no data for Meshtastic TCP and never probes on %s (device RST hazard)', + (platform) => { + // Meshtastic raw-TCP RTT probing opens a second connection to the same host:port as + // the live session — confirmed via live packet capture to get the device to RST the + // real session. See useHostLinkMeter.ts's meshtasticTcpProbeUnsafe comment. + // Platform-independent behavior, so covered on all three platforms per convention. + vi.mocked(window.electronAPI.getPlatform).mockReturnValue(platform); + vi.mocked(window.electronAPI.hostLink.probeTcpRtt).mockResolvedValue(120); + render( + , + ); + expect(screen.getByText('Link quality')).toBeInTheDocument(); + expect(screen.getByText('—')).toBeInTheDocument(); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); + }, + ); }); diff --git a/src/renderer/hooks/useHostLinkMeter.test.ts b/src/renderer/hooks/useHostLinkMeter.test.ts index fb2d050bf..12ec2e833 100644 --- a/src/renderer/hooks/useHostLinkMeter.test.ts +++ b/src/renderer/hooks/useHostLinkMeter.test.ts @@ -77,22 +77,74 @@ describe('useHostLinkMeter', () => { }, ); - it('returns ip-rtt for Meshtastic TCP via probeTcpRtt', async () => { - const { result } = renderHook(() => - useHostLinkMeter({ - protocol: 'meshtastic', - connectionType: 'tcp', - status: 'configured', - hostAddress: '10.0.0.5:4403', - platform: 'darwin', - }), + it.each(['linux', 'darwin', 'win32'] as const)( + 'returns ip-rtt with no data for Meshtastic raw TCP and never opens a competing probe connection on %s', + async (platform) => { + // Meshtastic raw-TCP RTT probing used to open a second, separate connection to the + // exact same host:port as the live session every poll tick — confirmed via live + // packet capture to intermittently get the *real* session RST'd by the device + // (5/5 reproduced samples; unaffected by upstream PR #808's setNoDelay/setKeepAlive, + // which only touches the main session's socket). Do not re-enable this probe for + // 'meshtastic' + 'tcp' without deriving RTT from the already-open session instead. + // Platform-independent behavior (no OS-specific mechanism involved), so covered on + // all three platforms per project convention rather than a single-platform case. + // + // kind stays 'ip-rtt' (not 'unavailable') with null rttMs/level — same rendering as + // an HTTP probe failure ("—" + no-data bars). A dedicated 'unavailable' kind would + // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on this + // WiFi/TCP-only transport. + vi.useFakeTimers(); + const { result } = renderHook(() => + useHostLinkMeter({ + protocol: 'meshtastic', + connectionType: 'tcp', + status: 'configured', + hostAddress: '10.0.0.5:4403', + platform, + }), + ); + expect(result.current.kind).toBe('ip-rtt'); + expect(result.current.rttMs).toBeNull(); + expect(result.current.level).toBeNull(); + + // Advance well past several poll intervals to confirm no deferred/interval probe fires. + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); + }); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); + }, + ); + + it('clears stale HTTP RTT when Meshtastic switches from HTTP to TCP', async () => { + // Render logic derives the displayed RTT from meshtasticTcpProbeUnsafe directly + // rather than trusting rttMs state, so a prior HTTP probe result can never render + // as if it were the (disabled) TCP link quality, without depending on the probe + // effect's cleanup (setRttMs(null)) having committed first. Verifies the settled + // end state; RTL's act()-wrapped rerender flushes that cleanup synchronously in + // this environment, so this does not exercise the specific single-render flash + // the derivation also guards against in a real browser (passive effects commit + // after paint there) — the render-time guard is defense in depth regardless. + const { result, rerender } = renderHook( + (props: { connectionType: 'http' | 'tcp' }) => + useHostLinkMeter({ + protocol: 'meshtastic', + connectionType: props.connectionType, + status: 'configured', + hostAddress: 'meshtastic.local', + platform: 'darwin', + }), + { initialProps: { connectionType: 'http' } }, ); - expect(result.current.kind).toBe('ip-rtt'); + await waitFor(() => { - expect(result.current.rttMs).toBe(80); - expect(result.current.level).toBe(3); + expect(result.current.rttMs).toBe(40); }); - expect(window.electronAPI.hostLink.probeTcpRtt).toHaveBeenCalledWith('10.0.0.5', 4403); + + rerender({ connectionType: 'tcp' }); + + expect(result.current.kind).toBe('ip-rtt'); + expect(result.current.rttMs).toBeNull(); + expect(result.current.level).toBeNull(); }); it('returns ip-rtt for MeshCore TCP/IP (http transport) via probeTcpRtt', async () => { diff --git a/src/renderer/hooks/useHostLinkMeter.ts b/src/renderer/hooks/useHostLinkMeter.ts index 8f647fa06..378f5abdf 100644 --- a/src/renderer/hooks/useHostLinkMeter.ts +++ b/src/renderer/hooks/useHostLinkMeter.ts @@ -55,6 +55,23 @@ export function useHostLinkMeter(opts: { isConnectedStatus(status) && (connectionType === 'ble' || connectionType === 'http' || connectionType === 'tcp'); + // Meshtastic raw-TCP RTT probing opens a second, separate socket to the exact same + // host:port as the live protocol session every poll tick. Captured via live packet + // capture: the device intermittently RSTs the *real* session within ~15-210ms of a + // probe cycle overlapping a real outbound write (5/5 reproduced samples, both before + // and after PR #808's setNoDelay/setKeepAlive change — unaffected, since that only + // touches the main session's socket). Meshtastic WiFi/TCP firmware likely tracks very + // few concurrent API connections; a second churn-y connection to the same port + // destabilizes it. Do not reintroduce a competing connect for this transport without + // deriving RTT from the already-open session instead. + // + // Deliberate blunt/interim tradeoff: this disables the probe (and the signal-bars UI) + // for *every* Meshtastic TCP session, not only ones that hit the collision, because + // there is no cheap signal here for "is a competing probe currently unsafe" short of + // the real fix above. A previously-working, cosmetic-only feature regressing is an + // acceptable cost against dropping the live connection. + const meshtasticTcpProbeUnsafe = protocol === 'meshtastic' && connectionType === 'tcp'; + // BLE RSSI via Noble (macOS / Windows) useEffect(() => { if (!active || connectionType !== 'ble') { @@ -78,7 +95,11 @@ export function useHostLinkMeter(opts: { // HTTP / TCP RTT probe useEffect(() => { - if (!active || (connectionType !== 'http' && connectionType !== 'tcp')) { + if ( + !active || + (connectionType !== 'http' && connectionType !== 'tcp') || + meshtasticTcpProbeUnsafe + ) { setRttMs(null); return; } @@ -98,8 +119,6 @@ export function useHostLinkMeter(opts: { let next: number | null = null; if (protocol === 'meshtastic' && connectionType === 'http') { next = await probeHttpLinkRttMs(address); - } else if (protocol === 'meshtastic' && connectionType === 'tcp') { - next = await probeTcpLinkRttMs(address, 'meshtastic'); } else if (protocol === 'meshcore' && connectionType === 'http') { // MeshCore "http" transport is TCP/IP host:port next = await probeTcpLinkRttMs(address, 'meshcore'); @@ -117,7 +136,7 @@ export function useHostLinkMeter(opts: { if (timer) clearInterval(timer); setRttMs(null); }; - }, [active, connectionType, hostAddress, protocol]); + }, [active, connectionType, hostAddress, protocol, meshtasticTcpProbeUnsafe]); if (!active || !connectionType) return IDLE; @@ -129,8 +148,17 @@ export function useHostLinkMeter(opts: { } if (connectionType === 'http' || connectionType === 'tcp') { - const level = rttMs != null ? rttToSignalLevel(rttMs) : null; - return { kind: 'ip-rtt', rssi: null, rttMs, level }; + // meshtasticTcpProbeUnsafe: force the displayed RTT to null rather than trusting + // rttMs state directly — on the render right after switching from Meshtastic HTTP + // to TCP, this branch runs before the probe effect's cleanup has cleared rttMs + // (effects commit after render), so a stale HTTP RTT value could otherwise flash + // as if it were the (disabled) TCP link quality. Same "—" / no-data rendering as + // an HTTP probe failure — not a separate 'unavailable' kind, which would + // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on a + // WiFi/TCP-only transport. + const displayedRttMs = meshtasticTcpProbeUnsafe ? null : rttMs; + const level = displayedRttMs != null ? rttToSignalLevel(displayedRttMs) : null; + return { kind: 'ip-rtt', rssi: null, rttMs: displayedRttMs, level }; } return IDLE; diff --git a/src/renderer/lib/hostLinkQuality.ts b/src/renderer/lib/hostLinkQuality.ts index e15842bb8..f185c5d87 100644 --- a/src/renderer/lib/hostLinkQuality.ts +++ b/src/renderer/lib/hostLinkQuality.ts @@ -53,6 +53,12 @@ export interface ParsedTcpProbeTarget { * Parse a TCP probe target. * - `meshtastic`: default port 4403 (`parseMeshtasticTcpAddress`) * - `meshcore`: default port 5000 (`parseTcpAddress`) + * + * The `'meshtastic'` branch is currently unreachable from any production call site — + * `useHostLinkMeter.ts` deliberately never probes Meshtastic raw TCP (opening a second + * connection to the same host:port as the live session has been confirmed to get the + * device to RST the real connection). Do not wire a new call site for it without reading + * that hook's `meshtasticTcpProbeUnsafe` comment first. */ export function parseTcpProbeTarget( address: string, @@ -88,7 +94,12 @@ export async function probeHttpLinkRttMs(httpAddress: string): Promise