Conversation
- Add rsReticulum-rnode-tcp-activity-keepalive.patch (Python ACTIVITY_KEEPALIVE parity: idle detect() every 3.5s so Wi‑Fi RNodes do not FIN at ~6s). - Wire apply script into ensure-rsReticulum-patches / clone-ratspeak-stack. - Track upstream ratspeak/rsReticulum#15 in update.sh RATSPEAK_PATCH_ENTRIES. Ships the fix in mesh-client builds while the upstream PR may or may not merge.
- Surface Admin Start pairing PIN prominently (large PIN + status) and warn that the radio display may stay blank; never use Meshtastic 123456. - Extend BLE connect grace / connecting hint to 60s to match OS passkey. - Latch blePairingTimedOut from sidecar logs into Connection + Diagnostics. - Debounce pairing-transition reconnects (1s → 30s) via rsReticulum overlay. - Document wrong-PIN / Admin PIN location / empty macOS Bluetooth list. Reporter used 123456 after Admin Start pairing showed no on-display PIN; 7s hold + real PIN connected. These changes make that path clearer and less thrashy while the OS passkey dialog is open.
📝 WalkthroughWalkthroughThis change adds BLE pairing-timeout tracking and diagnostics, updates pairing UI state and PIN guidance, extends BLE connection polling to roughly 60 seconds, and adds rsReticulum overlays for TCP activity keepalives and pairing-transition retry delays. ChangesBLE pairing and RNode reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AdminPanel
participant RNodeFlasherSection
participant BluetoothConfig
participant OSBluetooth
AdminPanel->>RNodeFlasherSection: Start pairing
RNodeFlasherSection->>BluetoothConfig: Show pairing pending
RNodeFlasherSection->>OSBluetooth: Start Bluetooth pairing
OSBluetooth-->>RNodeFlasherSection: Return generated PIN
RNodeFlasherSection->>BluetoothConfig: Show PIN and clear pending state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
src/main/reticulumSidecarIssueTracker.ts (2)
196-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the retention documentation for BLE latches.
retainInterfaces()now prunesbleBondRemovedandblePairingTimedOut, but the preceding comment only describes TCP/TX issues. Mention the BLE maps so future changes do not accidentally omit them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/reticulumSidecarIssueTracker.ts` around lines 196 - 197, Update the comment immediately preceding the retainInterfaces logic to document that retainMapKeys also prunes the BLE latch maps bleBondRemoved and blePairingTimedOut, alongside the existing TCP/TX retention behavior.
21-22: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid super-linear backtracking in the BLE interface regex.
The combination of
.*?,(.+?), and the end-of-line alternative can cause excessive backtracking on long or malformed sidecar lines. Parse the known prefix/delimiter deterministically or use a bounded expression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/reticulumSidecarIssueTracker.ts` around lines 21 - 22, Update BLE_RNODE_CONNECT_FAILED_IFACE_RE used for parsing BLE RNode connection failures to avoid the unbounded lazy wildcards and end-of-line backtracking. Parse the known prefix and name delimiter deterministically, or bound the captured interface name so malformed or very long sidecar lines cannot cause super-linear matching, while preserving extraction of valid interface names.Source: Linters/SAST tools
src/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.ts (1)
16-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the shared time constant for new durations.
Express the added values with
MS_PER_SECOND(for example,55 * MS_PER_SECONDand60 * MS_PER_SECOND) instead of inline millisecond literals.As per coding guidelines, “Use shared validation and time helpers, including
clampTcpPort(),MS_PER_SECOND, andsrc/renderer/lib/timeConstants.ts, rather than inline equivalents.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.ts` around lines 16 - 23, Update the duration values in the reticulum refresh configuration, including RETICULUM_BLE_CONNECT_GRACE_MS, to derive from the shared MS_PER_SECOND constant instead of inline millisecond literals. Preserve the existing duration values and locate the shared constant through the established timeConstants import.Source: Coding guidelines
src/renderer/locales/cs/translation.json (1)
1090-1091: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTranslate the newly added pairing guidance.
The Czech, German, and Spanish locales now display English text for pairing-critical UI and diagnostics.
src/renderer/locales/cs/translation.json#L1090-L1091: translate BLE offline and connecting guidance.src/renderer/locales/cs/translation.json#L1163-L1164: translate pairing-timeout alert text.src/renderer/locales/cs/translation.json#L1517-L1517: translate the runtime timeout message.src/renderer/locales/cs/translation.json#L1683-L1686: translate flasher pairing labels and hint.src/renderer/locales/de/translation.json#L1090-L1091: translate BLE offline and connecting guidance.src/renderer/locales/de/translation.json#L1163-L1164: translate pairing-timeout alert text.src/renderer/locales/de/translation.json#L1517-L1517: translate the runtime timeout message.src/renderer/locales/de/translation.json#L1683-L1686: translate flasher pairing labels and hint.src/renderer/locales/es/translation.json#L1090-L1091: translate BLE offline and connecting guidance.src/renderer/locales/es/translation.json#L1163-L1164: translate pairing-timeout alert text.src/renderer/locales/es/translation.json#L1517-L1517: translate the runtime timeout message.src/renderer/locales/es/translation.json#L1683-L1686: translate flasher pairing labels and hint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/locales/cs/translation.json` around lines 1090 - 1091, Translate all newly added pairing-related strings into the appropriate locale languages, replacing English text while preserving the existing translation keys and JSON structure: BLE offline/connecting guidance, pairing-timeout alerts, runtime timeout messages, and flasher pairing labels/hints in src/renderer/locales/cs/translation.json at lines 1090-1091, 1163-1164, 1517, and 1683-1686; apply the same updates in src/renderer/locales/de/translation.json and src/renderer/locales/es/translation.json at the corresponding ranges.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reticulum-sidecar/README.md`:
- Line 33: Update the required manual overlay sequence in the README to include
apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh alongside the
existing overlays, ensuring full-stack manual builds apply the BLE pairing
debounce fix.
In `@scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh`:
- Around line 44-62: Make both overlay scripts fail closed when the patch cannot
be applied: in
scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh lines 44-62
and scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh lines 45-63, use a
failed git apply --check to trigger the pinned-ref fallback, then exit nonzero
if application on that ref fails unless an explicit applied or upstream marker
is verified; preserve successful application and confirmed
already-upstream/incompatible handling.
In `@src/main/reticulumSidecarIssueTracker.test.ts`:
- Line 6: Update the import of parseBlePairingTimedOutIfaceForTests in
reticulumSidecarIssueTracker.test.ts to reference its defining module,
reticulumSidecarIssueTracker.ts, instead of ../shared/reticulum-types; leave
other imports unchanged.
In `@src/renderer/components/flasher/RNodeFlasherSection.tsx`:
- Around line 492-512: Update the pairing flow around onStartPairing and its
component lifecycle to store the pending timer and attempt generation in refs.
Clear the timer and invalidate the generation when Bluetooth is disabled, before
starting a new attempt, and during unmount; ensure timeout, completion, error,
and PIN callbacks only update state for the current attempt. Add a fake-timer
test covering disable, re-enable, and starting pairing again.
In `@src/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsx`:
- Around line 116-131: Update the BLE pairing-timeout test around
ReticulumSidecarIssueAlertsBlock to capture the rendered container, call
hydrateAxeThemeColors(), run vitest-axe against that container, and assert
toHaveNoViolations() while preserving the existing text assertions.
In `@src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts`:
- Around line 252-256: Update the BLE pairing timeout diagnostic in
ReticulumDiagnosticEngine, including the cause text and runtimeCauseI18n key or
locale entries for blePairingTimedOut, to use neutral wording such as “passkey
exchange timed out” rather than asserting that a passkey was not entered. Keep
the alert contract and interpolation of the node name unchanged.
In `@src/renderer/locales/fr/translation.json`:
- Around line 1090-1091: Translate all newly added BLE pairing guidance into the
appropriate locale-specific user-facing copy, preserving the English source
meaning and existing JSON keys. In src/renderer/locales/fr/translation.json,
src/renderer/locales/id/translation.json,
src/renderer/locales/it/translation.json,
src/renderer/locales/ja/translation.json,
src/renderer/locales/ko/translation.json,
src/renderer/locales/nl/translation.json, and
src/renderer/locales/pl/translation.json, update the BLE hint keys at lines
1090-1091, sidecar keys at lines 1163-1164, the runtime key at line 1517, and
flasher keys at lines 1683-1686; do not leave these strings in English.
In `@src/renderer/locales/it/translation.json`:
- Line 1682: Update the wifiNetmaskPlaceholder translation value to remove the
trailing period, leaving the valid netmask literal 255.255.255.0.
In `@src/renderer/locales/pt-BR/translation.json`:
- Around line 1090-1091: Translate the newly added BLE pairing, timeout,
diagnostic, and flasher strings at all listed sites in
src/renderer/locales/pt-BR/translation.json,
src/renderer/locales/ru/translation.json,
src/renderer/locales/tr/translation.json,
src/renderer/locales/uk/translation.json, and
src/renderer/locales/zh/translation.json, covering ranges 1090-1091, 1163-1164,
1517, and 1683-1686 in each file. Keep the English locale as the source of
truth, preserve the existing translation keys, remove any unused English keys if
applicable, and run the i18n checks.
---
Nitpick comments:
In `@src/main/reticulumSidecarIssueTracker.ts`:
- Around line 196-197: Update the comment immediately preceding the
retainInterfaces logic to document that retainMapKeys also prunes the BLE latch
maps bleBondRemoved and blePairingTimedOut, alongside the existing TCP/TX
retention behavior.
- Around line 21-22: Update BLE_RNODE_CONNECT_FAILED_IFACE_RE used for parsing
BLE RNode connection failures to avoid the unbounded lazy wildcards and
end-of-line backtracking. Parse the known prefix and name delimiter
deterministically, or bound the captured interface name so malformed or very
long sidecar lines cannot cause super-linear matching, while preserving
extraction of valid interface names.
In `@src/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.ts`:
- Around line 16-23: Update the duration values in the reticulum refresh
configuration, including RETICULUM_BLE_CONNECT_GRACE_MS, to derive from the
shared MS_PER_SECOND constant instead of inline millisecond literals. Preserve
the existing duration values and locate the shared constant through the
established timeConstants import.
In `@src/renderer/locales/cs/translation.json`:
- Around line 1090-1091: Translate all newly added pairing-related strings into
the appropriate locale languages, replacing English text while preserving the
existing translation keys and JSON structure: BLE offline/connecting guidance,
pairing-timeout alerts, runtime timeout messages, and flasher pairing
labels/hints in src/renderer/locales/cs/translation.json at lines 1090-1091,
1163-1164, 1517, and 1683-1686; apply the same updates in
src/renderer/locales/de/translation.json and
src/renderer/locales/es/translation.json at the corresponding ranges.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: adf57a95-50b6-4c6c-bb8f-a08a5d628f3d
📒 Files selected for processing (42)
AGENTS.mddocs/diagnostics.mddocs/reticulum-sidecar-ipc.mddocs/reticulum.mddocs/troubleshooting.mdreticulum-sidecar/README.mdreticulum-sidecar/patches/README.mdreticulum-sidecar/patches/rsReticulum-ble-rnode-pairing-transition-debounce.patchreticulum-sidecar/patches/rsReticulum-rnode-tcp-activity-keepalive.patchscripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.shscripts/apply-rsReticulum-rnode-tcp-activity-keepalive.shscripts/clone-ratspeak-stack.shscripts/ensure-rsReticulum-patches.shscripts/update.shsrc/main/reticulumSidecarIssueTracker.test.tssrc/main/reticulumSidecarIssueTracker.tssrc/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsxsrc/renderer/components/ReticulumSidecarIssueAlertsBlock.tsxsrc/renderer/components/ReticulumStackPanel.test.tsxsrc/renderer/components/flasher/BluetoothConfig.tsxsrc/renderer/components/flasher/RNodeFlasherSection.tsxsrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.test.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.tssrc/renderer/lib/reticulum/useReticulumInterfaceSnapshot.test.tssrc/renderer/locales/cs/translation.jsonsrc/renderer/locales/de/translation.jsonsrc/renderer/locales/en/translation.jsonsrc/renderer/locales/es/translation.jsonsrc/renderer/locales/fr/translation.jsonsrc/renderer/locales/id/translation.jsonsrc/renderer/locales/it/translation.jsonsrc/renderer/locales/ja/translation.jsonsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/nl/translation.jsonsrc/renderer/locales/pl/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/locales/zh/translation.jsonsrc/shared/reticulum-types.ts
Comment on lines
+44
to
+62
| if git -C "${RNS_DIR}" apply --check "${PATCH_FILE}" 2>&1 | grep -q 'patch does not apply'; then | ||
| echo "ble_rnode pairing-transition debounce overlay not needed (already upstream or incompatible with current rsReticulum HEAD)" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "ble_rnode pairing-transition debounce patch did not apply on current HEAD; checking out pinned ref ${RS_RETICULUM_REF:0:12}" | ||
| current_head="$(git -C "${RNS_DIR}" rev-parse HEAD)" | ||
| if [[ "${current_head}" != "${RS_RETICULUM_REF}" ]]; then | ||
| git -C "${RNS_DIR}" fetch origin --tags | ||
| git -C "${RNS_DIR}" checkout "${RS_RETICULUM_REF}" | ||
| fi | ||
|
|
||
| if apply_patch 2> /dev/null; then | ||
| echo "applied ${PATCH_FILE} on rsReticulum @ ${RS_RETICULUM_REF:0:12}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "ble_rnode pairing-transition debounce overlay not needed (already upstream or incompatible with pinned rsReticulum ref)" | ||
| exit 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when a required overlay cannot be applied.
git apply --check normally emits “patch does not apply” for a conflict, so the early success path makes the pinned-ref fallback unreachable. A failure on the pinned ref also exits successfully, allowing builds to proceed without either reliability fix.
scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh#L44-L62: fall back to the pinned ref on a failed check, and exit nonzero if application still fails unless an explicit applied/upstream marker is verified.scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh#L45-L63: apply the same fail-closed behavior.
📍 Affects 2 files
scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh#L44-L62(this comment)scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh#L45-L63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh` around
lines 44 - 62, Make both overlay scripts fail closed when the patch cannot be
applied: in scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh
lines 44-62 and scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh lines
45-63, use a failed git apply --check to trigger the pinned-ref fallback, then
exit nonzero if application on that ref fails unless an explicit applied or
upstream marker is verified; preserve successful application and confirmed
already-upstream/incompatible handling.
| import { RETICULUM_INTERFACE_ISSUE_ALERT_STALE_MS } from '../shared/reticulum-types'; | ||
| import { | ||
| parseBleBondRemovedIfaceForTests, | ||
| parseBlePairingTimedOutIfaceForTests, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Import the test helper from its defining module.
parseBlePairingTimedOutIfaceForTests is exported by src/main/reticulumSidecarIssueTracker.ts at Lines [319-321], but the changed import targets ../shared/reticulum-types. Importing from the DTO module will fail because the helper is not defined there.
Proposed fix
-import { parseBlePairingTimedOutIfaceForTests } from '../shared/reticulum-types';
+import { parseBlePairingTimedOutIfaceForTests } from './reticulumSidecarIssueTracker';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| parseBlePairingTimedOutIfaceForTests, | |
| import { parseBlePairingTimedOutIfaceForTests } from './reticulumSidecarIssueTracker'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/reticulumSidecarIssueTracker.test.ts` at line 6, Update the import
of parseBlePairingTimedOutIfaceForTests in reticulumSidecarIssueTracker.test.ts
to reference its defining module, reticulumSidecarIssueTracker.ts, instead of
../shared/reticulum-types; leave other imports unchanged.
Comment on lines
+252
to
+256
| id: rfRowId(homeNodeId, `reticulum/ble-pairing-timed-out/${name}`), | ||
| nodeId: homeNodeId, | ||
| condition: 'reticulum/ble-pairing-timed-out', | ||
| cause: `BLE RNode "${name}" pairing timed out (passkey not entered)`, | ||
| causeI18n: runtimeCauseI18n('blePairingTimedOut', { name }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use neutral wording for the pairing timeout.
The alert contract records a timeout while waiting for the OS passkey/TX-characteristic/SMP exchange; it does not prove that the user failed to enter a PIN. Change this to wording such as “passkey exchange timed out” and update the corresponding locale strings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts` around lines 252 -
256, Update the BLE pairing timeout diagnostic in ReticulumDiagnosticEngine,
including the cause text and runtimeCauseI18n key or locale entries for
blePairingTimedOut, to use neutral wording such as “passkey exchange timed out”
rather than asserting that a passkey was not entered. Keep the alert contract
and interpolation of the node name unchanged.
Comment on lines
+1090
to
+1091
| "offlineHintBle": "Your OS may prompt for a 6-digit Bluetooth PIN (macOS, Windows, or Linux). RNode generates a new code each pairing — never use 123456 (that is Meshtastic’s default, not RNode). The RNode need not appear in System Settings → Bluetooth before mesh-client connects; the OS dialog appears when the stack pairs. Headless radios: connect USB → Reticulum Admin → Bluetooth → Start pairing → enter the PIN shown in the Admin panel (not on the radio display) into the OS dialog. Radios with a display: hold the button ~7 s on Heltec devices (timing varies; ~10 s on many boards enables Wi-Fi AP instead), read the PIN on screen, then enter it in the OS dialog. Clear failed bonds in your OS Bluetooth settings before retrying.", | ||
| "connectingHintBle": "BLE RNode links can take up to 60 seconds after the stack starts (OS passkey window). Enter the current RNode PIN if prompted — never 123456. This is normal, not an error. The device may not appear in System Settings until after a successful bond." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Translate the new BLE pairing guidance in every affected locale. The same English-only regression appears in the local-interface hints, sidecar alerts, runtime diagnostics, and flasher strings.
src/renderer/locales/fr/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/id/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/it/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/ja/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/ko/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/nl/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].src/renderer/locales/pl/translation.json#L1090-L1091: translate the BLE hints, sidecar keys at Lines [1163-1164], runtime key at Line [1517], and flasher keys at Lines [1683-1686].
As per coding guidelines: English is the source of truth, but locale files should not replace user-facing localized copy with English.
📍 Affects 7 files
src/renderer/locales/fr/translation.json#L1090-L1091(this comment)src/renderer/locales/id/translation.json#L1090-L1091src/renderer/locales/it/translation.json#L1090-L1091src/renderer/locales/ja/translation.json#L1090-L1091src/renderer/locales/ko/translation.json#L1090-L1091src/renderer/locales/nl/translation.json#L1090-L1091src/renderer/locales/pl/translation.json#L1090-L1091
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/locales/fr/translation.json` around lines 1090 - 1091, Translate
all newly added BLE pairing guidance into the appropriate locale-specific
user-facing copy, preserving the English source meaning and existing JSON keys.
In src/renderer/locales/fr/translation.json,
src/renderer/locales/id/translation.json,
src/renderer/locales/it/translation.json,
src/renderer/locales/ja/translation.json,
src/renderer/locales/ko/translation.json,
src/renderer/locales/nl/translation.json, and
src/renderer/locales/pl/translation.json, update the BLE hint keys at lines
1090-1091, sidecar keys at lines 1163-1164, the runtime key at line 1517, and
flasher keys at lines 1683-1686; do not leave these strings in English.
Source: Coding guidelines
Comment on lines
+1090
to
+1091
| "offlineHintBle": "Your OS may prompt for a 6-digit Bluetooth PIN (macOS, Windows, or Linux). RNode generates a new code each pairing — never use 123456 (that is Meshtastic’s default, not RNode). The RNode need not appear in System Settings → Bluetooth before mesh-client connects; the OS dialog appears when the stack pairs. Headless radios: connect USB → Reticulum Admin → Bluetooth → Start pairing → enter the PIN shown in the Admin panel (not on the radio display) into the OS dialog. Radios with a display: hold the button ~7 s on Heltec devices (timing varies; ~10 s on many boards enables Wi-Fi AP instead), read the PIN on screen, then enter it in the OS dialog. Clear failed bonds in your OS Bluetooth settings before retrying.", | ||
| "connectingHintBle": "BLE RNode links can take up to 60 seconds after the stack starts (OS passkey window). Enter the current RNode PIN if prompted — never 123456. This is normal, not an error. The device may not appear in System Settings until after a successful bond." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Translate the new pairing strings in every reviewed locale. The same newly added keys are English in five non-English locale files, affecting core pairing and diagnostics flows.
src/renderer/locales/pt-BR/translation.json#L1090-L1091: translate BLE connection guidance.src/renderer/locales/pt-BR/translation.json#L1163-L1164: translate timeout alert text and hint.src/renderer/locales/pt-BR/translation.json#L1517-L1517: translate the runtime diagnostic message.src/renderer/locales/pt-BR/translation.json#L1683-L1686: translate flasher pairing labels and instructions.src/renderer/locales/ru/translation.json#L1090-L1091: translate BLE connection guidance.src/renderer/locales/ru/translation.json#L1163-L1164: translate timeout alert text and hint.src/renderer/locales/ru/translation.json#L1517-L1517: translate the runtime diagnostic message.src/renderer/locales/ru/translation.json#L1683-L1686: translate flasher pairing labels and instructions.src/renderer/locales/tr/translation.json#L1090-L1091: translate BLE connection guidance.src/renderer/locales/tr/translation.json#L1163-L1164: translate timeout alert text and hint.src/renderer/locales/tr/translation.json#L1517-L1517: translate the runtime diagnostic message.src/renderer/locales/tr/translation.json#L1683-L1686: translate flasher pairing labels and instructions.src/renderer/locales/uk/translation.json#L1090-L1091: translate BLE connection guidance.src/renderer/locales/uk/translation.json#L1163-L1164: translate timeout alert text and hint.src/renderer/locales/uk/translation.json#L1517-L1517: translate the runtime diagnostic message.src/renderer/locales/uk/translation.json#L1683-L1686: translate flasher pairing labels and instructions.src/renderer/locales/zh/translation.json#L1090-L1091: translate BLE connection guidance.src/renderer/locales/zh/translation.json#L1163-L1164: translate timeout alert text and hint.src/renderer/locales/zh/translation.json#L1517-L1517: translate the runtime diagnostic message.src/renderer/locales/zh/translation.json#L1683-L1686: translate flasher pairing labels and instructions.
As per coding guidelines: English is the source of truth; add strings to English and use t('your.key'); remove unused English keys and run the i18n checks when changing translations.
📍 Affects 5 files
src/renderer/locales/pt-BR/translation.json#L1090-L1091(this comment)src/renderer/locales/pt-BR/translation.json#L1163-L1164src/renderer/locales/pt-BR/translation.json#L1517-L1517src/renderer/locales/pt-BR/translation.json#L1683-L1686src/renderer/locales/ru/translation.json#L1090-L1091src/renderer/locales/ru/translation.json#L1163-L1164src/renderer/locales/ru/translation.json#L1517-L1517src/renderer/locales/ru/translation.json#L1683-L1686src/renderer/locales/tr/translation.json#L1090-L1091src/renderer/locales/tr/translation.json#L1163-L1164src/renderer/locales/tr/translation.json#L1517-L1517src/renderer/locales/tr/translation.json#L1683-L1686src/renderer/locales/uk/translation.json#L1090-L1091src/renderer/locales/uk/translation.json#L1163-L1164src/renderer/locales/uk/translation.json#L1517-L1517src/renderer/locales/uk/translation.json#L1683-L1686src/renderer/locales/zh/translation.json#L1090-L1091src/renderer/locales/zh/translation.json#L1163-L1164src/renderer/locales/zh/translation.json#L1517-L1517src/renderer/locales/zh/translation.json#L1683-L1686
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/locales/pt-BR/translation.json` around lines 1090 - 1091,
Translate the newly added BLE pairing, timeout, diagnostic, and flasher strings
at all listed sites in src/renderer/locales/pt-BR/translation.json,
src/renderer/locales/ru/translation.json,
src/renderer/locales/tr/translation.json,
src/renderer/locales/uk/translation.json, and
src/renderer/locales/zh/translation.json, covering ranges 1090-1091, 1163-1164,
1517, and 1683-1686 in each file. Keep the English locale as the source of
truth, preserve the existing translation keys, remove any unused English keys if
applicable, and run the i18n checks.
Source: Coding guidelines
- Fail closed on rsReticulum overlay apply (match packet-tap); document BLE debounce overlay in sidecar README. - Isolate Admin pairing pending timer/generation so disable/re-pair cannot clear a newer attempt; cover with fake-timer unit tests. - Neutralize blePairingTimedOut copy (passkey exchange timed out), parse BLE iface names without backtracking regex, use <output> for pairing status, derive local-health delays from MS_PER_SECOND, and axe-test the new alert. - Translate pairing/timeout strings across locales; fix IT wifi netmask typo.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh`:
- Around line 51-54: The existing-overlay guards use substring checks that can
match unrelated text; update both
scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh (lines 51-54)
and scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh (lines 53-55) to
verify the relevant constant declaration and its expected use, so each guard
only exits when the complete semantic marker is present.
In `@src/renderer/components/flasher/RNodeFlasherSection.tsx`:
- Around line 509-526: Keep the RNode session active in the pairing flow around
startBluetoothPairing until the PIN callback delivers a value, the pairing
timeout expires, or an error occurs. Make the callback settle a promise and
await that promise before returning, while preserving attempt cancellation and
cleanup behavior. Add or update tests to verify the PIN response is received
before runWithRNode closes the connection, including timeout and error
settlement paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f3d47428-b9c9-4e60-a64e-53929fde38d4
📒 Files selected for processing (27)
reticulum-sidecar/README.mdscripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.shscripts/apply-rsReticulum-rnode-tcp-activity-keepalive.shsrc/main/reticulumSidecarIssueTracker.tssrc/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsxsrc/renderer/components/flasher/BluetoothConfig.tsxsrc/renderer/components/flasher/RNodeFlasherSection.tsxsrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/flasher/rnodeBluetoothPairingSession.test.tssrc/renderer/lib/flasher/rnodeBluetoothPairingSession.tssrc/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.tssrc/renderer/locales/cs/translation.jsonsrc/renderer/locales/de/translation.jsonsrc/renderer/locales/en/translation.jsonsrc/renderer/locales/es/translation.jsonsrc/renderer/locales/fr/translation.jsonsrc/renderer/locales/id/translation.jsonsrc/renderer/locales/it/translation.jsonsrc/renderer/locales/ja/translation.jsonsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/nl/translation.jsonsrc/renderer/locales/pl/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/locales/zh/translation.json
🚧 Files skipped from review as they are similar to previous changes (19)
- src/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsx
- src/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.ts
- reticulum-sidecar/README.md
- src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts
- src/renderer/components/flasher/BluetoothConfig.tsx
- src/renderer/locales/en/translation.json
- src/renderer/locales/nl/translation.json
- src/renderer/locales/tr/translation.json
- src/renderer/locales/ja/translation.json
- src/renderer/locales/zh/translation.json
- src/renderer/locales/it/translation.json
- src/renderer/locales/es/translation.json
- src/renderer/locales/id/translation.json
- src/renderer/locales/fr/translation.json
- src/renderer/locales/de/translation.json
- src/renderer/locales/ko/translation.json
- src/renderer/locales/pl/translation.json
- src/renderer/locales/cs/translation.json
- src/renderer/locales/uk/translation.json
- startBluetoothPairing now settles on CMD_BT_PIN or timeout (not the KISS write), so runWithRNode no longer closes USB before the PIN arrives. - Cover PIN delivery and timeout with fake-timer SerialPort mocks. - Strengthen rsReticulum overlay "already present" guards to require const declaration + use site; bound BLE iface name parse whitespace for Sonar.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/reticulumSidecarIssueTracker.ts`:
- Around line 31-43: The errorMatch expression in
parseBleRNodeConnectFailedIfaceName permits unbounded whitespace before “error”,
causing excessive backtracking on long non-matching input. Replace it with an
indexOf-based separator or a regex that bounds the preceding whitespace, while
preserving extraction of the name before the error marker.
In `@src/renderer/lib/flasher/rnode.ts`:
- Around line 452-461: Update the RNode.CMD_BT_PIN callback to catch errors
thrown by pinCallback(pin) and reject the pairing promise while ensuring the
flasher session is closed, rather than allowing the exception to escape after
finish(). Add a regression test covering a throwing PIN callback and verify the
promise rejects and cleanup completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b38753ce-c984-402f-b0fc-a83ad51789f0
📒 Files selected for processing (29)
reticulum-sidecar/README.mdscripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.shscripts/apply-rsReticulum-rnode-tcp-activity-keepalive.shsrc/main/reticulumSidecarIssueTracker.tssrc/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsxsrc/renderer/components/flasher/BluetoothConfig.tsxsrc/renderer/components/flasher/RNodeFlasherSection.tsxsrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/flasher/rnode.test.tssrc/renderer/lib/flasher/rnode.tssrc/renderer/lib/flasher/rnodeBluetoothPairingSession.test.tssrc/renderer/lib/flasher/rnodeBluetoothPairingSession.tssrc/renderer/lib/reticulum/reticulumLocalInterfaceRefresh.tssrc/renderer/locales/cs/translation.jsonsrc/renderer/locales/de/translation.jsonsrc/renderer/locales/en/translation.jsonsrc/renderer/locales/es/translation.jsonsrc/renderer/locales/fr/translation.jsonsrc/renderer/locales/id/translation.jsonsrc/renderer/locales/it/translation.jsonsrc/renderer/locales/ja/translation.jsonsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/nl/translation.jsonsrc/renderer/locales/pl/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/locales/zh/translation.json
🚧 Files skipped from review as they are similar to previous changes (24)
- src/renderer/lib/flasher/rnodeBluetoothPairingSession.ts
- reticulum-sidecar/README.md
- src/renderer/lib/flasher/rnodeBluetoothPairingSession.test.ts
- scripts/apply-rsReticulum-rnode-tcp-activity-keepalive.sh
- src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts
- src/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsx
- src/renderer/components/flasher/RNodeFlasherSection.tsx
- scripts/apply-rsReticulum-ble-rnode-pairing-transition-debounce.sh
- src/renderer/locales/it/translation.json
- src/renderer/locales/id/translation.json
- src/renderer/locales/en/translation.json
- src/renderer/locales/pl/translation.json
- src/renderer/locales/ko/translation.json
- src/renderer/locales/pt-BR/translation.json
- src/renderer/locales/de/translation.json
- src/renderer/locales/uk/translation.json
- src/renderer/locales/nl/translation.json
- src/renderer/locales/ja/translation.json
- src/renderer/locales/ru/translation.json
- src/renderer/locales/tr/translation.json
- src/renderer/locales/cs/translation.json
- src/renderer/locales/zh/translation.json
- src/renderer/locales/es/translation.json
- src/renderer/locales/fr/translation.json
Comment on lines
+31
to
+43
| /** Deterministic parse — avoids super-linear regex backtracking on long sidecar lines. */ | ||
| function parseBleRNodeConnectFailedIfaceName(plain: string): string | null { | ||
| const failedIdx = plain.toLowerCase().indexOf(BLE_RNODE_CONNECT_FAILED_PREFIX.toLowerCase()); | ||
| if (failedIdx < 0) return null; | ||
| const afterFailed = plain.slice(failedIdx + BLE_RNODE_CONNECT_FAILED_PREFIX.length); | ||
| const nameKey = /\bname[ \t]{0,16}=[ \t]{0,16}/i.exec(afterFailed); | ||
| if (nameKey?.index == null) return null; | ||
| const rest = afterFailed.slice(nameKey.index + nameKey[0].length); | ||
| const errorMatch = /[ \t]+error[ \t]{0,16}=/i.exec(rest); | ||
| const name = (errorMatch?.index != null ? rest.slice(0, errorMatch.index) : rest).trim(); | ||
| return name.length > 0 ? name : null; | ||
| } | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' src/main/reticulumSidecarIssueTracker.tsRepository: Colorado-Mesh/mesh-client
Length of output: 5041
🏁 Script executed:
node - <<'JS'
const re = /[ \t]+error[ \t]{0,16}=/i;
function timeFor(n) {
const s = ' '.repeat(n) + 'x';
const t0 = process.hrtime.bigint();
re.exec(s);
const t1 = process.hrtime.bigint();
return Number(t1 - t0) / 1e6;
}
for (const n of [1e3, 2e3, 4e3, 8e3, 16e3, 32e3, 64e3]) {
console.log(n, timeFor(n).toFixed(3), 'ms');
}
JSRepository: Colorado-Mesh/mesh-client
Length of output: 275
Bound the whitespace before error (src/main/reticulumSidecarIssueTracker.ts:39)
/[ \t]+error[ \t]{0,16}=/i backtracks badly on long non-matching whitespace runs; use a bounded separator or an indexOf-based split instead.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 39-39: Simplify this regular expression to reduce its runtime, as it has super-linear performance due to backtracking.
🪛 OpenGrep (1.25.0)
[ERROR] 36-36: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 39-39: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/reticulumSidecarIssueTracker.ts` around lines 31 - 43, The
errorMatch expression in parseBleRNodeConnectFailedIfaceName permits unbounded
whitespace before “error”, causing excessive backtracking on long non-matching
input. Replace it with an indexOf-based separator or a regex that bounds the
preceding whitespace, while preserving extraction of the name before the error
marker.
Source: Linters/SAST tools
Comment on lines
+452
to
461
| this.callbacks.set(RNode.CMD_BT_PIN, (response) => { | ||
| const pin = | ||
| ((response[0] ?? 0) << 24) | | ||
| ((response[1] ?? 0) << 16) | | ||
| ((response[2] ?? 0) << 8) | | ||
| (response[3] ?? 0); | ||
| finish(() => { | ||
| pinCallback(pin); | ||
| resolve(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files
fd -a 'rnode.ts' src || true
fd -a '*rnode*test*' test src || true
fd -a '*flasher*test*' test src || true
# Show outlines if available, otherwise fall back to targeted reads
if command -v ast-grep >/dev/null 2>&1; then
echo '--- AST outline: src/renderer/lib/flasher/rnode.ts ---'
ast-grep outline src/renderer/lib/flasher/rnode.ts --view expanded || true
fi
echo '--- target source excerpt ---'
nl -ba src/renderer/lib/flasher/rnode.ts | sed -n '380,520p'
echo '--- search for pairing tests ---'
rg -n "startBluetoothPairing|CMD_BT_PIN|pinCallback|Bluetooth Pairing|pairing" src test || trueRepository: Colorado-Mesh/mesh-client
Length of output: 6611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- rnode.ts around command dispatch ---'
sed -n '180,240p' src/renderer/lib/flasher/rnode.ts
echo '--- rnode.ts around startBluetoothPairing ---'
sed -n '425,470p' src/renderer/lib/flasher/rnode.ts
echo '--- candidate test files ---'
git ls-files | rg '(^|/)(.*rnode.*|.*flasher.*|.*pair.*test.*)\.(test|spec)\.(ts|tsx|js|jsx)$|(^|/)(.*rnode.*|.*flasher.*|.*pair.*)\.(test|spec)\.(ts|tsx|js|jsx)$|(^|/).*__tests__/.*(rnode|flasher|pair).*'
echo '--- pairing-related references ---'
rg -n "startBluetoothPairing|CMD_BT_PIN|pinCallback|Bluetooth Pairing|pairing" src test .Repository: Colorado-Mesh/mesh-client
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- rnode.test.ts pairing block ---'
sed -n '1,220p' src/renderer/lib/flasher/rnode.test.ts
echo '--- rnodeBluetoothPairingSession.test.ts ---'
sed -n '1,220p' src/renderer/lib/flasher/rnodeBluetoothPairingSession.test.tsRepository: Colorado-Mesh/mesh-client
Length of output: 6471
Reject when the PIN callback throws. finish() clears the timeout and marks the attempt settled before pinCallback(pin), so a thrown callback escapes onCommandReceived() but never reaches resolve() or reject(). That leaves the pairing promise pending and the flasher session open. Add a regression test for a throwing callback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/lib/flasher/rnode.ts` around lines 452 - 461, Update the
RNode.CMD_BT_PIN callback to catch errors thrown by pinCallback(pin) and reject
the pairing promise while ensuring the flasher session is closed, rather than
allowing the exception to escape after finish(). Add a regression test covering
a throwing PIN callback and verify the promise rejects and cleanup completes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
blePairingTimedOutinto Connection + Diagnostics, and debounce pairing-transition reconnect thrash via rsReticulum overlay.Test plan
pnpm run updateapplies both rsReticulum overlays (rnode-tcp-activity-keepalive,ble-rnode-pairing-transition-debounce)123456)blePairingTimedOutbanner + diagnostics row; Forget/re-pair recoverspnpm run test:run(or at least sidecar issue tracker / diagnostic / flasher-related tests) passesSummary by CodeRabbit