Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 30 additions & 24 deletions src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<ConnectionPanel
state={{
status: 'configured',
myNodeNum: 1,
connectionType: 'tcp',
firmwareVersion: '2.5.3',
}}
onConnect={vi.fn().mockResolvedValue(undefined)}
onAutoConnect={vi.fn().mockResolvedValue(undefined)}
onDisconnect={vi.fn().mockResolvedValue(undefined)}
mqttStatus="disconnected"
protocol="meshtastic"
suppressMountAutoConnect
/>,
);
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(
<ConnectionPanel
state={{
status: 'configured',
myNodeNum: 1,
connectionType: 'tcp',
firmwareVersion: '2.5.3',
}}
onConnect={vi.fn().mockResolvedValue(undefined)}
onAutoConnect={vi.fn().mockResolvedValue(undefined)}
onDisconnect={vi.fn().mockResolvedValue(undefined)}
mqttStatus="disconnected"
protocol="meshtastic"
suppressMountAutoConnect
/>,
);
expect(screen.getByText('Link quality')).toBeInTheDocument();
expect(screen.getByText('—')).toBeInTheDocument();
expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled();
},
);
});
78 changes: 65 additions & 13 deletions src/renderer/hooks/useHostLinkMeter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
40 changes: 34 additions & 6 deletions src/renderer/hooks/useHostLinkMeter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -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;
}
Expand All @@ -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');
Expand All @@ -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;

Expand All @@ -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;
Expand Down
13 changes: 12 additions & 1 deletion src/renderer/lib/hostLinkQuality.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -88,7 +94,12 @@ export async function probeHttpLinkRttMs(httpAddress: string): Promise<number |
}
}

/** Probe TCP connect RTT via main process (connect then destroy). */
/**
* Probe TCP connect RTT via main process (connect then destroy).
*
* Never call this with `protocol: 'meshtastic'` (or omit `protocol`, whose
* default is `'meshtastic'`) — see `parseTcpProbeTarget`'s docstring for why.
*/
export async function probeTcpLinkRttMs(
address: string,
protocol: 'meshtastic' | 'meshcore' = 'meshtastic',
Expand Down
Loading