From d093ec33a8df7a0da58c2f4aab6ac35059de5161 Mon Sep 17 00:00:00 2001 From: Joe WB3IHY Date: Sun, 2 Aug 2026 14:06:53 -0400 Subject: [PATCH 1/3] fix(chat): budget real wire bytes and pace meshtastic multi-part sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reserve the reply_id field (5 fixed32 bytes) and a byte-accurate reply prefix estimate so near-limit Meshtastic and MeshCore replies split into multiple parts instead of overflowing into a TOO_LARGE NAK from the radio. Count real UTF-8 wire bytes instead of Unicode codepoints when chunking, since non-ASCII text (Cyrillic, CJK, emoji) could previously pass the composer's codepoint-based check while exceeding the true wire byte limit. Pace successive Meshtastic text sends — both the live composer loop and the outbox drain loop — by 2.5s to stay clear of firmware's RATE_LIMIT_EXCEEDED threshold on TEXT_MESSAGE_APP, which rejects a second locally-originated text packet sent within 2s of the last. --- src/renderer/components/ChatComposer.test.tsx | 67 ++++++++++++++ src/renderer/components/ChatComposer.tsx | 8 ++ src/renderer/hooks/useChatOutbox.test.ts | 39 ++++++++ src/renderer/hooks/useChatOutbox.ts | 14 +++ src/renderer/lib/chatComposerLimits.test.ts | 88 ++++++++++++++++++- src/renderer/lib/chatComposerLimits.ts | 76 +++++++++++++--- src/renderer/lib/timeConstants.ts | 10 +++ 7 files changed, 289 insertions(+), 13 deletions(-) diff --git a/src/renderer/components/ChatComposer.test.tsx b/src/renderer/components/ChatComposer.test.tsx index 291150ead..22f83458a 100644 --- a/src/renderer/components/ChatComposer.test.tsx +++ b/src/renderer/components/ChatComposer.test.tsx @@ -6,6 +6,7 @@ import { axe } from 'vitest-axe'; import { hydrateAxeThemeColors } from '@/renderer/lib/a11yTestHelpers'; import { MESHTASTIC_PAYLOAD_LIMIT } from '@/renderer/lib/chatComposerLimits'; import { draftsStorageKey } from '@/renderer/lib/chatPanelProtocolStorage'; +import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '@/renderer/lib/timeConstants'; import { ChatComposer } from './ChatComposer'; @@ -337,6 +338,72 @@ describe('ChatComposer', () => { expect(screen.getByRole('button', { name: 'Send 2 parts' })).toBeInTheDocument(); }); + it('paces meshtastic multi-chunk sends to avoid firmware RATE_LIMIT_EXCEEDED', async () => { + // Regression: firmware rejects a second TEXT_MESSAGE_APP within 2s of the first + // (Routing_Error.RATE_LIMIT_EXCEEDED). Chunks must not fire back-to-back. + vi.useFakeTimers(); + try { + const onSendChunk = vi.fn().mockResolvedValue(undefined); + render( + , + ); + const textarea = screen.getByRole('textbox'); + fireEvent.change(textarea, { target: { value: 'a'.repeat(250) } }); + fireEvent.click(screen.getByRole('button', { name: 'Send 2 parts' })); + + await vi.advanceTimersByTimeAsync(0); + expect(onSendChunk).toHaveBeenCalledTimes(1); + expect(onSendChunk).toHaveBeenNthCalledWith( + 1, + expect.stringContaining('[1/2]'), + expect.objectContaining({ chunkIndex: 0 }), + ); + + await vi.advanceTimersByTimeAsync(MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 100); + expect(onSendChunk).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(200); + expect(onSendChunk).toHaveBeenCalledTimes(2); + expect(onSendChunk).toHaveBeenNthCalledWith( + 2, + expect.stringContaining('[2/2]'), + expect.objectContaining({ chunkIndex: 1 }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it('does not delay single-chunk meshtastic sends', async () => { + vi.useFakeTimers(); + try { + const onSendChunk = vi.fn().mockResolvedValue(undefined); + render( + , + ); + const textarea = screen.getByRole('textbox'); + fireEvent.change(textarea, { target: { value: 'hello' } }); + fireEvent.click(screen.getByRole('button', { name: 'Send' })); + + await vi.advanceTimersByTimeAsync(0); + expect(onSendChunk).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('hides GIF button when MeshCore Open wire compat is disabled', () => { render( 0 && protocol === 'meshtastic') { + // Firmware rate-limits locally-originated TEXT_MESSAGE_APP packets to one per 2s + // (RATE_LIMIT_EXCEEDED) — chunks fired back-to-back would trip this on chunk 2+. + await new Promise((resolve) => { + setTimeout(resolve, MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS); + }); + } await onSendChunk(textsToSend[i], { replyId: i === 0 && typeof replyKey === 'number' ? replyKey : undefined, replyHash: i === 0 ? reticulumReplyHash : undefined, diff --git a/src/renderer/hooks/useChatOutbox.test.ts b/src/renderer/hooks/useChatOutbox.test.ts index 5944ba4f3..e63e9b4b2 100644 --- a/src/renderer/hooks/useChatOutbox.test.ts +++ b/src/renderer/hooks/useChatOutbox.test.ts @@ -1,6 +1,7 @@ import { renderHook, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '@/renderer/lib/timeConstants'; import type { OutboxEntry } from '@/shared/electron-api.types'; import { useChatOutbox } from './useChatOutbox'; @@ -177,6 +178,44 @@ describe('useChatOutbox', () => { await waitFor(() => { expect(mockOutbox.updateStatus).toHaveBeenCalledWith(4, 'queued', undefined, undefined); }); + // This is the second meshtastic send in the hook's lifetime, so it is paced behind the + // first (see 'paces successive meshtastic sends' below) — wait for it to fully settle so + // no timer is left dangling into later tests. + await waitFor( + () => { + expect(sendFn).toHaveBeenCalledTimes(2); + }, + { timeout: MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS + 2_000 }, + ); + }); + + it('paces successive meshtastic sends within one drain to avoid RATE_LIMIT_EXCEEDED', async () => { + // Regression: firmware rejects a second TEXT_MESSAGE_APP within 2s of the first + // (Routing_Error.RATE_LIMIT_EXCEEDED). Rows draining back-to-back must be paced. + const rowA = makeEntry({ id: 30, payload: 'first' }); + const rowB = makeEntry({ id: 31, payload: 'second' }); + vi.mocked(mockOutbox.list).mockResolvedValue([rowA, rowB]); + const sendFn = vi.fn().mockResolvedValue(undefined); + renderHook(() => useChatOutbox({ protocol: 'meshtastic', isSendAvailable: true, sendFn })); + + await waitFor(() => { + expect(sendFn).toHaveBeenCalledTimes(1); + }); + expect(sendFn).toHaveBeenNthCalledWith(1, 'first', 0, undefined, undefined); + + // The second row must not send immediately after the first. + await new Promise((resolve) => { + setTimeout(resolve, MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 500); + }); + expect(sendFn).toHaveBeenCalledTimes(1); + + await waitFor( + () => { + expect(sendFn).toHaveBeenCalledTimes(2); + }, + { timeout: 2_000 }, + ); + expect(sendFn).toHaveBeenNthCalledWith(2, 'second', 0, undefined, undefined); }); it('does not drain when isSendAvailable is false', async () => { diff --git a/src/renderer/hooks/useChatOutbox.ts b/src/renderer/hooks/useChatOutbox.ts index 437344deb..879d096ff 100644 --- a/src/renderer/hooks/useChatOutbox.ts +++ b/src/renderer/hooks/useChatOutbox.ts @@ -4,6 +4,7 @@ import type { MeshProtocol } from '@/renderer/lib/types'; import type { OutboxEntry, OutboxEntryInput, OutboxStatus } from '@/shared/electron-api.types'; import { registerChatOutboxDrainListener } from '../lib/chatOutboxDrain'; +import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '../lib/timeConstants'; export type { OutboxEntry }; @@ -147,6 +148,8 @@ export function useChatOutbox({ const drainingRef = useRef(false); const isSendAvailableRef = useRef(isSendAvailable); const sendFnRef = useRef(sendFn); + /** Persists across drainOnce() calls so pacing holds even when drain is re-triggered mid-backoff. */ + const lastMeshtasticSendAtRef = useRef(0); useEffect(() => { isSendAvailableRef.current = isSendAvailable; sendFnRef.current = sendFn; @@ -192,6 +195,17 @@ export function useChatOutbox({ const now = Date.now(); for (const row of freshRows.filter((r) => isEligibleForDrain(r, now))) { if (!isSendAvailableRef.current) break; + if (protocol === 'meshtastic') { + // Firmware rejects a second TEXT_MESSAGE_APP within 2s of the last one + // (Routing_Error.RATE_LIMIT_EXCEEDED) — pace queued Meshtastic rows the same + // way ChatComposer paces multi-chunk sends, across drainOnce() calls too. + const wait = + MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - (Date.now() - lastMeshtasticSendAtRef.current); + if (wait > 0) { + await new Promise((resolve) => setTimeout(resolve, wait)); + } + lastMeshtasticSendAtRef.current = Date.now(); + } await sendOneOutboxRow(row, sendFnRef.current, updateRow, removeRow); } } catch (err: unknown) { diff --git a/src/renderer/lib/chatComposerLimits.test.ts b/src/renderer/lib/chatComposerLimits.test.ts index 1f7ac66ee..0a8ff3463 100644 --- a/src/renderer/lib/chatComposerLimits.test.ts +++ b/src/renderer/lib/chatComposerLimits.test.ts @@ -4,6 +4,7 @@ import { computeComposerLimitStatus, computeComposerTotalMaxChars, countMessageChars, + countMessageWireBytes, getChatPayloadLimit, getComposerWireOverhead, getMeshcoreChannelPayloadLimit, @@ -11,6 +12,7 @@ import { MAX_CHUNKS, MESHCORE_PAYLOAD_LIMIT, MESHTASTIC_PAYLOAD_LIMIT, + MESHTASTIC_REPLY_ID_WIRE_BYTES, RETICULUM_LXMF_PAYLOAD_LIMIT, splitChatMessage, } from './chatComposerLimits'; @@ -50,10 +52,26 @@ describe('getMeshcoreRoomPayloadLimit', () => { }); describe('getComposerWireOverhead', () => { - it('returns 0 for meshtastic replies', () => { + it('returns 0 for meshtastic when no reply is pending', () => { expect(getComposerWireOverhead({ protocol: 'meshtastic', replyToSenderName: 'Bob' })).toBe(0); }); + it('reserves 5 wire bytes for meshtastic replies (fixed32 reply_id field)', () => { + expect( + getComposerWireOverhead({ + protocol: 'meshtastic', + replyToSenderName: 'Bob', + replyKey: 2_113_407_456, + }), + ).toBe(MESHTASTIC_REPLY_ID_WIRE_BYTES); + }); + + it('returns 0 for meshtastic when replyKey is 0', () => { + expect( + getComposerWireOverhead({ protocol: 'meshtastic', replyToSenderName: 'Bob', replyKey: 0 }), + ).toBe(0); + }); + it('counts MeshCore reply prefix on first chunk', () => { expect(getComposerWireOverhead({ protocol: 'meshcore', replyToSenderName: 'Bob' })).toBe(7); }); @@ -78,6 +96,44 @@ describe('getComposerWireOverhead', () => { }), ).toBe(countMessageChars('@[Bob#1780235760847] ')); }); + + it('reserves the "Unknown" fallback length for an all-emoji sender name (keyless)', () => { + // Regression: sanitizeMeshcoreWireName strips an all-pictographic name to '', and the real + // wire builder falls back to the literal "Unknown" — a naive estimate from the raw name + // ("@[😀] ", 5 chars) under-reserves by 6 bytes versus the true "@[Unknown] " (11 bytes). + expect(getComposerWireOverhead({ protocol: 'meshcore', replyToSenderName: '😀' })).toBe( + countMessageWireBytes('@[Unknown] '), + ); + }); + + it('reserves the "Unknown" fallback length for an all-emoji sender name (keyed)', () => { + expect( + getComposerWireOverhead({ + protocol: 'meshcore', + replyToSenderName: '🔥🔥', + replyKey: 1_780_235_760_847, + useKeyedReplies: true, + }), + ).toBe(countMessageWireBytes('@[Unknown#1780235760847] ')); + }); +}); + +describe('countMessageWireBytes', () => { + it('matches countMessageChars for ASCII text', () => { + expect(countMessageWireBytes('hello')).toBe(countMessageChars('hello')); + }); + + it('counts multi-byte UTF-8 characters by their real byte cost, not codepoint count', () => { + // Cyrillic characters are 2 bytes each in UTF-8, but 1 codepoint each. + const text = 'привет'; + expect(countMessageChars(text)).toBe(6); + expect(countMessageWireBytes(text)).toBe(12); + }); + + it('counts an emoji as 4 bytes despite being 1 codepoint', () => { + expect(countMessageChars('🦊')).toBe(1); + expect(countMessageWireBytes('🦊')).toBe(4); + }); }); describe('countMessageChars', () => { @@ -197,6 +253,36 @@ describe('splitChatMessage', () => { expect(splitChatMessage(fitsWithout, 'meshcore', limit, overhead)).not.toEqual([]); }); + it('splits a max-length meshtastic reply instead of overflowing the radio payload', () => { + // Regression: a 228-char reply previously fit in one chunk (overhead was ignored), + // silently overflowing the true wire payload once the 5-byte fixed32 reply_id field + // was added by the SDK/radio, which the firmware NAKed as TOO_LARGE. + const text = 'a'.repeat(MESHTASTIC_PAYLOAD_LIMIT); + const overhead = getComposerWireOverhead({ protocol: 'meshtastic', replyKey: 2_113_407_456 }); + expect(overhead).toBe(MESHTASTIC_REPLY_ID_WIRE_BYTES); + expect(splitChatMessage(text, 'meshtastic', MESHTASTIC_PAYLOAD_LIMIT, 0)).toEqual([]); + const chunks = splitChatMessage(text, 'meshtastic', MESHTASTIC_PAYLOAD_LIMIT, overhead); + expect(chunks).not.toEqual([]); + expect(chunks).not.toBeNull(); + }); + + it('splits multi-byte text that fits the codepoint limit but not the real byte limit', () => { + // Regression: Cyrillic 'п' is 1 codepoint but 2 UTF-8 bytes. 200 codepoints is under the + // 228-codepoint limit (previously judged "fits in one message"), but 400 real wire bytes — + // nearly double the true 228-byte Meshtastic payload — which the radio would NAK as TOO_LARGE. + const text = 'п'.repeat(200); + expect(countMessageChars(text)).toBeLessThanOrEqual(MESHTASTIC_PAYLOAD_LIMIT); + expect(countMessageWireBytes(text)).toBeGreaterThan(MESHTASTIC_PAYLOAD_LIMIT); + const chunks = splitChatMessage(text, 'meshtastic'); + expect(chunks).not.toBeNull(); + expect(chunks!.length).toBeGreaterThan(1); + for (const chunk of chunks!) { + expect(countMessageWireBytes(chunk)).toBeLessThanOrEqual(MESHTASTIC_PAYLOAD_LIMIT); + } + const bodies = chunks!.map((c) => c.replace(/^\[\d+\/\d+\] /, '')); + expect(bodies.join('')).toBe(text); + }); + it('returns null when text requires more than MAX_CHUNKS chunks', () => { const text = 'x'.repeat(9 * 127 + 1); expect(splitChatMessage(text, 'meshcore')).toBeNull(); diff --git a/src/renderer/lib/chatComposerLimits.ts b/src/renderer/lib/chatComposerLimits.ts index 60ae21d88..ef8940704 100644 --- a/src/renderer/lib/chatComposerLimits.ts +++ b/src/renderer/lib/chatComposerLimits.ts @@ -1,3 +1,7 @@ +import { + formatMeshcoreWireReplyPrefix, + formatMeshcoreWireTapbackPrefix, +} from './meshcoreChannelText'; import type { MeshProtocol } from './types'; export const MESHTASTIC_PAYLOAD_LIMIT = 228; @@ -12,6 +16,15 @@ export const MESHCORE_MAX_NAME_LEN = 32; export const MESHCORE_NAME_SUFFIX_LEN = 2; // ": " export const MESHCORE_ROOM_PUBKEY_PREFIX_LEN = 4; +/** + * Meshtastic `Data.reply_id` (field 7) is `fixed32`: 1 tag byte + 4 fixed value bytes = 5 bytes, + * always, whenever a reply is sent — regardless of the referenced packet id's value. Unlike + * MeshCore's reply prefix, this overhead is invisible wire bytes, not visible text, but it still + * has to come out of the same payload budget or a near-limit reply overflows the radio's true + * packet size and gets NAKed as TOO_LARGE. + */ +export const MESHTASTIC_REPLY_ID_WIRE_BYTES = 5; + export type ComposerWireContext = 'channel' | 'dm' | 'room'; export type ComposerLimitPhase = 'ok' | 'warn' | 'split' | 'overMax'; @@ -59,7 +72,7 @@ export function getComposerPayloadLimit(opts: { return getMeshcoreChannelPayloadLimit(opts.senderDisplayName ?? ''); } -/** MeshCore reply wire prefix on the first chunk only (keyless companion or keyed Open). */ +/** Reply wire overhead on the first chunk only (MeshCore visible prefix; Meshtastic reply_id field). */ export function getComposerWireOverhead(opts: { protocol: MeshProtocol; replyToSenderName?: string; @@ -67,19 +80,55 @@ export function getComposerWireOverhead(opts: { /** When true, count keyed `@[Name#key] ` overhead (MeshCore Open compat). */ useKeyedReplies?: boolean; }): number { + if (opts.protocol === 'meshtastic') { + return opts.replyKey != null && Number.isFinite(opts.replyKey) && opts.replyKey !== 0 + ? MESHTASTIC_REPLY_ID_WIRE_BYTES + : 0; + } if (opts.protocol !== 'meshcore' || !opts.replyToSenderName?.trim()) return 0; - const cleanName = opts.replyToSenderName.trim(); const key = opts.replyKey; - if (opts.useKeyedReplies && key != null && Number.isFinite(key) && key > 0) { - return countMessageChars(`@[${cleanName}#${Math.trunc(key)}] `); - } - return countMessageChars(`@[${cleanName}] `); + // Reuse the exact wire-format builders (incl. sanitize + "Unknown" fallback for all-emoji + // names) so this estimate can never drift from what actually goes out on the wire. + const prefix = + opts.useKeyedReplies && key != null && Number.isFinite(key) && key > 0 + ? formatMeshcoreWireReplyPrefix(opts.replyToSenderName, key) + : formatMeshcoreWireTapbackPrefix(opts.replyToSenderName); + return countMessageWireBytes(`${prefix} `); } export function countMessageChars(text: string): number { return Array.from(text).length; } +const wireTextEncoder = new TextEncoder(); + +/** + * Real transport wire byte length (UTF-8), as opposed to `countMessageChars`' codepoint count. + * Meshtastic and MeshCore both encode outbound text with `TextEncoder` before transmission, so + * a codepoint count alone understates cost for any non-ASCII text (non-Latin scripts, emoji) — + * this is what must be checked against real byte-based wire limits like `MESHTASTIC_PAYLOAD_LIMIT`. + */ +export function countMessageWireBytes(text: string): number { + return wireTextEncoder.encode(text).length; +} + +/** + * Number of leading codepoints from `chars` whose combined UTF-8 byte length fits `byteBudget`. + * Always takes at least one codepoint to guarantee forward progress, even when a single + * multi-byte codepoint alone exceeds the budget (only possible with a pathologically tiny limit). + */ +function takeCharsWithinByteBudget(chars: readonly string[], byteBudget: number): number { + let bytes = 0; + let count = 0; + for (const ch of chars) { + const chBytes = countMessageWireBytes(ch); + if (count > 0 && bytes + chBytes > byteBudget) break; + bytes += chBytes; + count++; + } + return count; +} + /** Max user-typed characters across MAX_CHUNKS split messages. */ export function computeComposerTotalMaxChars( singleMessageLimit: number, @@ -180,13 +229,16 @@ export function splitChatMessage( if (bodyLimit <= 0) return bodies; const remaining = chars.slice(pos); - if (remaining.length <= bodyLimit) { + if (countMessageWireBytes(remaining.join('')) <= bodyLimit) { bodies.push(remaining.join('')); break; } - const window = remaining.slice(0, bodyLimit); - let breakAt = bodyLimit; - for (let i = bodyLimit - 1; i > 0; i--) { + // bodyLimit is a byte budget (real wire limits are byte limits), so the window must be + // sized by accumulated UTF-8 byte length, not codepoint count. + const windowLen = takeCharsWithinByteBudget(remaining, bodyLimit); + const window = remaining.slice(0, windowLen); + let breakAt = windowLen; + for (let i = windowLen - 1; i > 0; i--) { if (window[i] === ' ' || window[i] === '\n') { breakAt = i; break; @@ -194,13 +246,13 @@ export function splitChatMessage( } const body = window.slice(0, breakAt).join('').trimEnd(); bodies.push(body); - pos += breakAt === bodyLimit ? bodyLimit : breakAt + 1; + pos += breakAt === windowLen ? windowLen : breakAt + 1; isFirst = false; } return bodies; } - if (countMessageChars(trimmed) + overhead <= limit) return []; + if (countMessageWireBytes(trimmed) + overhead <= limit) return []; const estimatedPrefixLen = `[${MAX_CHUNKS}/${MAX_CHUNKS}] `.length; const bodies = chunkBodies(estimatedPrefixLen); diff --git a/src/renderer/lib/timeConstants.ts b/src/renderer/lib/timeConstants.ts index 4aea54f59..0f7389d46 100644 --- a/src/renderer/lib/timeConstants.ts +++ b/src/renderer/lib/timeConstants.ts @@ -284,3 +284,13 @@ export const NOMAD_PAGE_FETCH_RETRY_SETTLE_MS = 750; * the latest selection after this debounce. */ export const NOMAD_PAGE_FETCH_DEBOUNCE_MS = 300; + +/** + * Minimum gap between successive TEXT_MESSAGE_APP sends to the connected Meshtastic radio. + * Firmware's PhoneAPI rate-limits locally-originated text packets to one per 2s + * (`Throttle::isWithinTimespanMs(lastPortNumToRadio[TEXT_MESSAGE_APP], TWO_SECONDS_MS)` in + * `PhoneAPI.cpp`) and rejects a closer one with `Routing_Error.RATE_LIMIT_EXCEEDED` — a + * multi-chunk split message sent back-to-back trips this on chunk 2+. Padded above the + * firmware's exact 2000ms boundary for serial/BLE/TCP write and timer-granularity slack. + */ +export const MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS = 2_500; From 85a8b596a3b254e5ef709df3ac2460b6d7a4aba6 Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Sun, 2 Aug 2026 12:32:21 -0600 Subject: [PATCH 2/3] fix(chat): share meshtastic text pacing and budget meshcore name bytes Stamp the TEXT_MESSAGE_APP send slot after each attempt settles and share one clock between the composer and outbox so slow writes or overlapping drains cannot shrink under firmware's RATE_LIMIT_EXCEEDED window. Charge UTF-8 wire bytes for MeshCore channel display names against the 160-byte payload max. --- src/renderer/components/ChatComposer.test.tsx | 4 +- src/renderer/components/ChatComposer.tsx | 35 +++++----- src/renderer/hooks/useChatOutbox.test.ts | 2 + src/renderer/hooks/useChatOutbox.ts | 20 ++---- src/renderer/lib/chatComposerLimits.test.ts | 5 ++ src/renderer/lib/chatComposerLimits.ts | 5 +- .../lib/meshtasticTextSendPacing.test.ts | 69 +++++++++++++++++++ src/renderer/lib/meshtasticTextSendPacing.ts | 35 ++++++++++ 8 files changed, 143 insertions(+), 32 deletions(-) create mode 100644 src/renderer/lib/meshtasticTextSendPacing.test.ts create mode 100644 src/renderer/lib/meshtasticTextSendPacing.ts diff --git a/src/renderer/components/ChatComposer.test.tsx b/src/renderer/components/ChatComposer.test.tsx index 22f83458a..6ab4698ab 100644 --- a/src/renderer/components/ChatComposer.test.tsx +++ b/src/renderer/components/ChatComposer.test.tsx @@ -1,11 +1,12 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { axe } from 'vitest-axe'; import { hydrateAxeThemeColors } from '@/renderer/lib/a11yTestHelpers'; import { MESHTASTIC_PAYLOAD_LIMIT } from '@/renderer/lib/chatComposerLimits'; import { draftsStorageKey } from '@/renderer/lib/chatPanelProtocolStorage'; +import { resetMeshtasticTextSendPacingForTests } from '@/renderer/lib/meshtasticTextSendPacing'; import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '@/renderer/lib/timeConstants'; import { ChatComposer } from './ChatComposer'; @@ -60,6 +61,7 @@ vi.mock('react-i18next', () => ({ describe('ChatComposer', () => { beforeEach(() => { localStorage.clear(); + resetMeshtasticTextSendPacingForTests(); }); it('has no axe violations when connected', async () => { diff --git a/src/renderer/components/ChatComposer.tsx b/src/renderer/components/ChatComposer.tsx index b89d14d53..47a948905 100644 --- a/src/renderer/components/ChatComposer.tsx +++ b/src/renderer/components/ChatComposer.tsx @@ -36,7 +36,7 @@ import { normalizeMeshcoreGifOutboundWire, parseMeshcoreGifId, } from '../lib/meshcoreGifWire'; -import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '../lib/timeConstants'; +import { withMeshtasticTextSendPacing } from '../lib/meshtasticTextSendPacing'; import { HelpTooltip } from './HelpTooltip'; import MentionAutocomplete, { buildMentionCandidates } from './MentionAutocomplete'; import { useToast } from './Toast'; @@ -461,24 +461,25 @@ export function ChatComposer({ setChatActionError(null); try { for (let i = 0; i < textsToSend.length; i++) { - if (i > 0 && protocol === 'meshtastic') { - // Firmware rate-limits locally-originated TEXT_MESSAGE_APP packets to one per 2s - // (RATE_LIMIT_EXCEEDED) — chunks fired back-to-back would trip this on chunk 2+. - await new Promise((resolve) => { - setTimeout(resolve, MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS); + const sendChunk = () => + onSendChunk(textsToSend[i], { + replyId: i === 0 && typeof replyKey === 'number' ? replyKey : undefined, + replyHash: i === 0 ? reticulumReplyHash : undefined, + chunkIndex: i, + floodScopeOverride: + floodScopeOverride === '__unscoped__' + ? '' + : floodScopeOverride + ? floodScopeOverride + : undefined, }); + // Shared Meshtastic TEXT_MESSAGE_APP pacing (with outbox drain) — firmware rejects + // a second locally-originated text within ~2s (RATE_LIMIT_EXCEEDED). + if (protocol === 'meshtastic') { + await withMeshtasticTextSendPacing(sendChunk); + } else { + await sendChunk(); } - await onSendChunk(textsToSend[i], { - replyId: i === 0 && typeof replyKey === 'number' ? replyKey : undefined, - replyHash: i === 0 ? reticulumReplyHash : undefined, - chunkIndex: i, - floodScopeOverride: - floodScopeOverride === '__unscoped__' - ? '' - : floodScopeOverride - ? floodScopeOverride - : undefined, - }); } rememberFloodScopeIfNeeded(floodScopeOverride); clearSentDraft(draftSnapshot); diff --git a/src/renderer/hooks/useChatOutbox.test.ts b/src/renderer/hooks/useChatOutbox.test.ts index e63e9b4b2..65aa380a4 100644 --- a/src/renderer/hooks/useChatOutbox.test.ts +++ b/src/renderer/hooks/useChatOutbox.test.ts @@ -1,6 +1,7 @@ import { renderHook, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { resetMeshtasticTextSendPacingForTests } from '@/renderer/lib/meshtasticTextSendPacing'; import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '@/renderer/lib/timeConstants'; import type { OutboxEntry } from '@/shared/electron-api.types'; @@ -32,6 +33,7 @@ describe('useChatOutbox', () => { const mockOutbox = window.electronAPI.chat.outbox; beforeEach(() => { + resetMeshtasticTextSendPacingForTests(); vi.mocked(mockOutbox.list).mockClear(); vi.mocked(mockOutbox.add).mockClear(); vi.mocked(mockOutbox.updateStatus).mockClear(); diff --git a/src/renderer/hooks/useChatOutbox.ts b/src/renderer/hooks/useChatOutbox.ts index 879d096ff..f37ae1984 100644 --- a/src/renderer/hooks/useChatOutbox.ts +++ b/src/renderer/hooks/useChatOutbox.ts @@ -4,7 +4,7 @@ import type { MeshProtocol } from '@/renderer/lib/types'; import type { OutboxEntry, OutboxEntryInput, OutboxStatus } from '@/shared/electron-api.types'; import { registerChatOutboxDrainListener } from '../lib/chatOutboxDrain'; -import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from '../lib/timeConstants'; +import { withMeshtasticTextSendPacing } from '../lib/meshtasticTextSendPacing'; export type { OutboxEntry }; @@ -148,8 +148,6 @@ export function useChatOutbox({ const drainingRef = useRef(false); const isSendAvailableRef = useRef(isSendAvailable); const sendFnRef = useRef(sendFn); - /** Persists across drainOnce() calls so pacing holds even when drain is re-triggered mid-backoff. */ - const lastMeshtasticSendAtRef = useRef(0); useEffect(() => { isSendAvailableRef.current = isSendAvailable; sendFnRef.current = sendFn; @@ -195,18 +193,14 @@ export function useChatOutbox({ const now = Date.now(); for (const row of freshRows.filter((r) => isEligibleForDrain(r, now))) { if (!isSendAvailableRef.current) break; + const sendRow = () => sendOneOutboxRow(row, sendFnRef.current, updateRow, removeRow); + // Shared with ChatComposer so live multi-chunk sends and outbox drain cannot race + // firmware's TEXT_MESSAGE_APP RATE_LIMIT_EXCEEDED window. if (protocol === 'meshtastic') { - // Firmware rejects a second TEXT_MESSAGE_APP within 2s of the last one - // (Routing_Error.RATE_LIMIT_EXCEEDED) — pace queued Meshtastic rows the same - // way ChatComposer paces multi-chunk sends, across drainOnce() calls too. - const wait = - MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - (Date.now() - lastMeshtasticSendAtRef.current); - if (wait > 0) { - await new Promise((resolve) => setTimeout(resolve, wait)); - } - lastMeshtasticSendAtRef.current = Date.now(); + await withMeshtasticTextSendPacing(sendRow); + } else { + await sendRow(); } - await sendOneOutboxRow(row, sendFnRef.current, updateRow, removeRow); } } catch (err: unknown) { console.warn('[useChatOutbox] drainOnce failed', err); diff --git a/src/renderer/lib/chatComposerLimits.test.ts b/src/renderer/lib/chatComposerLimits.test.ts index 0a8ff3463..14a7dc4cc 100644 --- a/src/renderer/lib/chatComposerLimits.test.ts +++ b/src/renderer/lib/chatComposerLimits.test.ts @@ -43,6 +43,11 @@ describe('getMeshcoreChannelPayloadLimit', () => { it('caps name length at 32 characters', () => { expect(getMeshcoreChannelPayloadLimit('x'.repeat(40))).toBe(126); }); + + it('reserves UTF-8 wire bytes for multi-byte display names', () => { + // Cyrillic 'п' is 2 UTF-8 bytes; 10 codepoints → 20 wire bytes + ": " (2) → body 138. + expect(getMeshcoreChannelPayloadLimit('п'.repeat(10))).toBe(160 - 20 - 2); + }); }); describe('getMeshcoreRoomPayloadLimit', () => { diff --git a/src/renderer/lib/chatComposerLimits.ts b/src/renderer/lib/chatComposerLimits.ts index ef8940704..efe8a6d50 100644 --- a/src/renderer/lib/chatComposerLimits.ts +++ b/src/renderer/lib/chatComposerLimits.ts @@ -45,7 +45,10 @@ export function getChatPayloadLimit(protocol: MeshProtocol, override?: number): } export function getMeshcoreChannelPayloadLimit(displayName: string): number { - const nameLen = Math.min(countMessageChars(displayName.trim()), MESHCORE_MAX_NAME_LEN); + // Cap by codepoints (companion name length), then charge real UTF-8 bytes for `Name: ` + // against the 160-byte wire max — multi-byte names previously under-reserved the body. + const cappedName = Array.from(displayName.trim()).slice(0, MESHCORE_MAX_NAME_LEN).join(''); + const nameLen = countMessageWireBytes(cappedName); return Math.max(1, MESHCORE_WIRE_MAX - nameLen - MESHCORE_NAME_SUFFIX_LEN); } diff --git a/src/renderer/lib/meshtasticTextSendPacing.test.ts b/src/renderer/lib/meshtasticTextSendPacing.test.ts new file mode 100644 index 000000000..c2ee37db2 --- /dev/null +++ b/src/renderer/lib/meshtasticTextSendPacing.test.ts @@ -0,0 +1,69 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + resetMeshtasticTextSendPacingForTests, + withMeshtasticTextSendPacing, +} from './meshtasticTextSendPacing'; +import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from './timeConstants'; + +describe('withMeshtasticTextSendPacing', () => { + beforeEach(() => { + resetMeshtasticTextSendPacingForTests(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + resetMeshtasticTextSendPacingForTests(); + }); + + it('does not delay the first send', async () => { + const send = vi.fn().mockResolvedValue('ok'); + const pending = withMeshtasticTextSendPacing(send); + await vi.advanceTimersByTimeAsync(0); + await expect(pending).resolves.toBe('ok'); + expect(send).toHaveBeenCalledTimes(1); + }); + + it('paces a second send from completion of the first, not from start', async () => { + // Regression: stamping before await send() let a slow first write shrink the gap + // under firmware's 2s RATE_LIMIT_EXCEEDED window. + const slowSend = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + setTimeout(resolve, 800); + }), + ); + const second = vi.fn().mockResolvedValue(undefined); + + const firstPending = withMeshtasticTextSendPacing(slowSend); + await vi.advanceTimersByTimeAsync(800); + await firstPending; + + const secondPending = withMeshtasticTextSendPacing(second); + await vi.advanceTimersByTimeAsync(MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 100); + expect(second).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(200); + await secondPending; + expect(second).toHaveBeenCalledTimes(1); + }); + + it('stamps even when send rejects so the next attempt still waits', async () => { + const failing = vi.fn().mockRejectedValue(new Error('radio busy')); + const next = vi.fn().mockResolvedValue(undefined); + + const first = withMeshtasticTextSendPacing(failing); + const firstExpectation = expect(first).rejects.toThrow('radio busy'); + await vi.advanceTimersByTimeAsync(0); + await firstExpectation; + + const secondPending = withMeshtasticTextSendPacing(next); + await vi.advanceTimersByTimeAsync(MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 100); + expect(next).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(200); + await secondPending; + expect(next).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/renderer/lib/meshtasticTextSendPacing.ts b/src/renderer/lib/meshtasticTextSendPacing.ts new file mode 100644 index 000000000..4f716990b --- /dev/null +++ b/src/renderer/lib/meshtasticTextSendPacing.ts @@ -0,0 +1,35 @@ +import { MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS } from './timeConstants'; + +/** + * Shared completion timestamp for Meshtastic TEXT_MESSAGE_APP sends. + * Module-level so ChatComposer multi-chunk sends and useChatOutbox drain share one clock + * and cannot race the firmware's ~2s PhoneAPI rate limit. + */ +let lastMeshtasticTextSendAtMs = 0; + +/** Test-only: clear the shared pacing clock between cases. */ +export function resetMeshtasticTextSendPacingForTests(): void { + lastMeshtasticTextSendAtMs = 0; +} + +/** + * Wait until the Meshtastic text-send slot is free, run `send`, then stamp completion. + * Stamping after `send` settles (not before) keeps the next gap measured from when the + * prior attempt finished — including IPC / SDK work — so a slow write cannot shrink the + * radio-visible interval under firmware's 2s RATE_LIMIT_EXCEEDED window. + */ +export async function withMeshtasticTextSendPacing(send: () => Promise | T): Promise { + if (lastMeshtasticTextSendAtMs > 0) { + const wait = MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - (Date.now() - lastMeshtasticTextSendAtMs); + if (wait > 0) { + await new Promise((resolve) => { + setTimeout(resolve, wait); + }); + } + } + try { + return await send(); + } finally { + lastMeshtasticTextSendAtMs = Date.now(); + } +} From e58a682bee5023b728a5a81f6ea97974e81cbfdf Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Sun, 2 Aug 2026 12:35:26 -0600 Subject: [PATCH 3/3] fix(chat): derive meshtastic send interval from MS_PER_SECOND Also switch outbox pacing tests to Vitest fake timers so they no longer wait on real wall-clock delays. --- src/renderer/hooks/useChatOutbox.test.ts | 82 +++++++++++------------- src/renderer/lib/timeConstants.ts | 2 +- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/renderer/hooks/useChatOutbox.test.ts b/src/renderer/hooks/useChatOutbox.test.ts index 65aa380a4..551712b4b 100644 --- a/src/renderer/hooks/useChatOutbox.test.ts +++ b/src/renderer/hooks/useChatOutbox.test.ts @@ -164,60 +164,56 @@ describe('useChatOutbox', () => { }); it('retry resets status to queued and triggers drain', async () => { - const entry = makeEntry({ id: 4, status: 'failed' }); - vi.mocked(mockOutbox.list).mockResolvedValue([entry]); - const sendFn = vi.fn().mockResolvedValue(undefined); - const { result } = renderHook(() => - useChatOutbox({ protocol: 'meshtastic', isSendAvailable: true, sendFn }), - ); - await waitFor(() => { + vi.useFakeTimers(); + try { + const entry = makeEntry({ id: 4, status: 'failed' }); + vi.mocked(mockOutbox.list).mockResolvedValue([entry]); + const sendFn = vi.fn().mockResolvedValue(undefined); + const { result } = renderHook(() => + useChatOutbox({ protocol: 'meshtastic', isSendAvailable: true, sendFn }), + ); + await vi.advanceTimersByTimeAsync(0); expect(result.current.rows).toHaveLength(0); - }); // initially drains the failed row and fails again - // Now manually call retry - vi.mocked(mockOutbox.updateStatus).mockResolvedValue(undefined); - vi.mocked(mockOutbox.list).mockResolvedValue([{ ...entry, status: 'queued' }]); - result.current.retry(4); - await waitFor(() => { + expect(sendFn).toHaveBeenCalledTimes(1); + + vi.mocked(mockOutbox.updateStatus).mockResolvedValue(undefined); + vi.mocked(mockOutbox.list).mockResolvedValue([{ ...entry, status: 'queued' }]); + result.current.retry(4); + await vi.advanceTimersByTimeAsync(0); expect(mockOutbox.updateStatus).toHaveBeenCalledWith(4, 'queued', undefined, undefined); - }); - // This is the second meshtastic send in the hook's lifetime, so it is paced behind the - // first (see 'paces successive meshtastic sends' below) — wait for it to fully settle so - // no timer is left dangling into later tests. - await waitFor( - () => { - expect(sendFn).toHaveBeenCalledTimes(2); - }, - { timeout: MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS + 2_000 }, - ); + // Second send is paced behind the first — advance past the interval so no timer dangles. + expect(sendFn).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS); + expect(sendFn).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } }); it('paces successive meshtastic sends within one drain to avoid RATE_LIMIT_EXCEEDED', async () => { // Regression: firmware rejects a second TEXT_MESSAGE_APP within 2s of the first // (Routing_Error.RATE_LIMIT_EXCEEDED). Rows draining back-to-back must be paced. - const rowA = makeEntry({ id: 30, payload: 'first' }); - const rowB = makeEntry({ id: 31, payload: 'second' }); - vi.mocked(mockOutbox.list).mockResolvedValue([rowA, rowB]); - const sendFn = vi.fn().mockResolvedValue(undefined); - renderHook(() => useChatOutbox({ protocol: 'meshtastic', isSendAvailable: true, sendFn })); + vi.useFakeTimers(); + try { + const rowA = makeEntry({ id: 30, payload: 'first' }); + const rowB = makeEntry({ id: 31, payload: 'second' }); + vi.mocked(mockOutbox.list).mockResolvedValue([rowA, rowB]); + const sendFn = vi.fn().mockResolvedValue(undefined); + renderHook(() => useChatOutbox({ protocol: 'meshtastic', isSendAvailable: true, sendFn })); - await waitFor(() => { + await vi.advanceTimersByTimeAsync(0); expect(sendFn).toHaveBeenCalledTimes(1); - }); - expect(sendFn).toHaveBeenNthCalledWith(1, 'first', 0, undefined, undefined); + expect(sendFn).toHaveBeenNthCalledWith(1, 'first', 0, undefined, undefined); - // The second row must not send immediately after the first. - await new Promise((resolve) => { - setTimeout(resolve, MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 500); - }); - expect(sendFn).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - 100); + expect(sendFn).toHaveBeenCalledTimes(1); - await waitFor( - () => { - expect(sendFn).toHaveBeenCalledTimes(2); - }, - { timeout: 2_000 }, - ); - expect(sendFn).toHaveBeenNthCalledWith(2, 'second', 0, undefined, undefined); + await vi.advanceTimersByTimeAsync(200); + expect(sendFn).toHaveBeenCalledTimes(2); + expect(sendFn).toHaveBeenNthCalledWith(2, 'second', 0, undefined, undefined); + } finally { + vi.useRealTimers(); + } }); it('does not drain when isSendAvailable is false', async () => { diff --git a/src/renderer/lib/timeConstants.ts b/src/renderer/lib/timeConstants.ts index 0f7389d46..4a0c99dfb 100644 --- a/src/renderer/lib/timeConstants.ts +++ b/src/renderer/lib/timeConstants.ts @@ -293,4 +293,4 @@ export const NOMAD_PAGE_FETCH_DEBOUNCE_MS = 300; * multi-chunk split message sent back-to-back trips this on chunk 2+. Padded above the * firmware's exact 2000ms boundary for serial/BLE/TCP write and timer-granularity slack. */ -export const MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS = 2_500; +export const MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS = 2.5 * MS_PER_SECOND;