Add support for serial-based (USB) GPSs

[rafaellehmkuhl]

Summary

Adds support for reading external USB/serial NMEA GNSS receivers (Electron-only) and feeding their data into the data lake, primarily to drive dynamic points of interest.

• Electron serial IPC: new serial-list-ports handler to enumerate serial ports.
• GNSS engine: framework-agnostic NMEA 0183 parser/aggregator (GGA/RMC/GSA/GSV/VTG/GLL, checksum validation, coordinate/fix-quality decoding, coordinates withheld until a valid fix) plus an orchestrator that reads multiple devices and publishes per-device external/gnss/<id>/* variables (lat, lon, altitude, satellites, fix quality incl. RTK float/fixed, DOPs, speed, course). Includes baud auto-detection and raw serial-line capture for debugging.
• Generic console viewer: ConsoleViewer is now source-agnostic so it can render any stream (used for the per-device serial console).
• Sources settings page: new "Sources" menu with a GNSS panel listing devices (name/port/status), a per-unit config dialog (built on InteractionDialog), and an "Add device" button.

Browsers can't access serial devices outside a secure context, so this is standalone-only; documented in the README feature table.

Test plan

• Plug in a USB GNSS, add a device under Settings → Sources → GNSS, pick the port.
• Confirm auto-detect finds the baud (or 9600 works) and external/gnss/<id>/* variables populate.
• Verify the status column reflects the fix type and the serial console shows raw NMEA.
• Bind a live POI to external/gnss/<id>/latitude/longitude and confirm it tracks.
• Confirm the Sources page shows the Lite notice and disabled controls in the browser build.

cockpit-serial-gnss-support.mp4

Close #650
Contributes to #300
Contributes to #1842

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to address: 1.1, 1.2, 4.1, 5.1, 5.2, 6.1, 6.2, 6.3

This PR adds Electron-only support for reading one or more USB/serial NMEA 0183 GNSS receivers and publishing their data into the data lake as external/gnss/<id>/* variables. It introduces a framework-agnostic NMEA parser/aggregator (src/libs/sensors/nmea.ts), a serial orchestrator (src/libs/sensors/gnss.ts), a shared composable (useGnss), a new "Sources" settings page, and a per-device configuration dialog built on InteractionDialog. The ConsoleViewer is generalized to render any event stream. Architecture is well-separated: pure parsing in libs/, reactive orchestration in a composable, and minimal component logic.

---

1. Correctness & Implementation Bugs

1.1 (minor) — useGnss.ts:79–86: On startup, the for (const device of devices.value) loop runs synchronously during createState(). Because devices comes from useBlueOsStorage, its .value may still be the default [] at that point if the persisted value hasn't hydrated yet. Devices with enabled: true could be missed on first load if the storage read is async. Verify that useBlueOsStorage resolves synchronously from localStorage; if not, a watch on devices would be safer.

1.2 (minor) — src/libs/sensors/gnss.ts: The module-level const decoder = new TextDecoder() is shared across all devices. TextDecoder.decode({ stream: true }) maintains internal state (for multi-byte sequences split across chunks). Because handleDeviceBytes and autodetectBaud both use this single decoder, interleaved calls from different devices or an autodetect probe running concurrently could corrupt the decoder's internal multi-byte state. Each device runtime (and each autodetect probe) should use its own TextDecoder instance.

---

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance

4.1 (minor) — src/libs/sensors/gnss.ts (ensureLinkListener): The global onLinkData callback is invoked for every incoming serial byte event on every open link (not just GNSS ports). The pathHandlers.get(data.path) lookup is cheap, but this callback now runs on the hot serial-data path for existing serial links (MAVLink connections) as well. Since onLinkData calls ipcRenderer.on('link-data', ...) and there is no way to unsubscribe, this is permanent once a GNSS device is initialized. If the number of concurrent serial links grows, this could add overhead. Consider documenting this trade-off, or at minimum guard ensureLinkListener with a comment explaining the shared-listener pattern.

---

5. UI / UX

5.1 (minor) — src/views/ConfigurationSourcesView.vue and src/components/sources/GnssDeviceDialog.vue: The statusColor and statusIcon functions are duplicated verbatim between the two files. Extract them into a shared helper (e.g. in the composable or a utility under src/libs/sensors/gnss.ts) to avoid the maintenance risk of them diverging.

5.2 (nit) — src/components/sources/GnssDeviceDialog.vue:354: The "Delete device" button has no confirmation. Deleting removes data-lake variables and disconnects the device immediately. Consider using useInteractionDialog for a confirmation prompt, consistent with destructive actions elsewhere in the app.

---

6. Code Quality & Style

6.1 (minor) — src/composables/useGnss.ts: The generateDeviceId and createState functions are not exported but are non-trivial. Per .eslintrc.cjs, FunctionDeclaration requires JSDoc (jsdoc/require-jsdoc). These are const arrow functions so they technically avoid the rule, but createState is significant enough that a JSDoc block would help future readers.

6.2 (minor) — src/composables/useGnss.ts:28–30: connectDevice calls logUserAction before the connection is actually established (it calls startGnssDevice after the log). If startGnssDevice throws (e.g. port already in use), the log message is misleading. Move the logUserAction call to after a successful startGnssDevice, or log a different message like "Connecting..." before and "Connected" after.

6.3 (nit) — src/types/gnss.ts:1: The GnssState interface imports Ref from vue, which means this types file has a Vue dependency. Per AGENTS.md separation-of-concerns, pure type files under src/types/ ideally stay framework-agnostic. Since Ref is only a type import (not a runtime import), this is acceptable, but worth noting.

---

7. Commit Hygiene — ✅

The four commits are clean, logically separated, and well-scoped:

1. electron: expose serial port listing over IPC
2. gnss: read external serial GNSS into the data lake
3. console: make the console viewer reusable for any stream
4. sources: allow setting up USB/serial GNSS sources

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional

10.1 (nit) — src/libs/sensors/gnss.ts:130–131: probeSettleMs and validSentenceThreshold are defined as module-level constants, which is good. Consider adding a brief inline comment explaining why 300 ms settle time was chosen (OS serial port release delay, etc.) to help future maintainers tune the value.

10.2 (nit) — src/components/sources/GnssDeviceDialog.vue:143,145: The device!.id non-null assertions inside the template lambdas (:get-recent-events and :subscribe) are safe because the whole block is guarded by v-if="device", but the v-if is on a parent div rather than the same element, so the assertion relies on correct nesting. If the template structure changes, these could NPE. A local const deviceId = computed(() => device.value?.id ?? '') would be more defensive.

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 1

Done

• src/libs/sensors/gnss.ts (1.2 — shared TextDecoder state corruption): each device runtime now owns its own TextDecoder, and each auto-detect probe uses a fresh local decoder, so streamed multi-byte sequences can't corrupt each other. Removed the module-level shared decoder.
• src/libs/sensors/gnss.ts (4.1 — shared onLinkData on the hot path): added a comment on ensureLinkListener documenting the single-listener + per-path Map dispatch pattern and the no-unsubscribe trade-off.
• src/libs/sensors/gnss.ts, src/views/ConfigurationSourcesView.vue, src/components/sources/GnssDeviceDialog.vue (5.1 — duplicated statusColor/statusIcon): extracted gnssStatusColor/gnssStatusIcon/gnssStatusLabel into gnss.ts and consumed them from both the view and the dialog.
• src/components/sources/GnssDeviceDialog.vue (5.2 — delete without confirmation): "Delete device" now opens a useInteractionDialog warning/confirm before disconnecting and removing the data-lake variables.
• src/composables/useGnss.ts (6.1 — missing JSDoc): added a JSDoc block to createState.
• src/composables/useGnss.ts (6.2 — premature logUserAction): connectDevice now logs only after startGnssDevice resolves.
• src/libs/sensors/gnss.ts (10.1 — magic probeSettleMs): added a comment explaining the OS serial-release settle delay.
• src/components/sources/GnssDeviceDialog.vue (10.2 — device!.id assertions): the console lambdas now use the deviceId prop directly, dropping the non-null assertions.

Won't change (with reasoning)

• 1.1 — startup loop may miss enabled devices if storage is async: the reading pipeline is no longer started from the composable — it's booted by initGnss() in main.ts, which reads settingsManager.getKeyValue(gnssDevicesKey) synchronously (in-memory, localStorage-backed). The composable loop only mirrors state into reactive maps for the UI, so hydration timing can't drop a device from the pipeline.
• 6.3 — GnssState imports Ref from vue in types/gnss.ts: it's a type-only import (erased at build), and keeping all GNSS types in the dedicated typing file is a deliberate project convention. Leaving as-is.

Note: the reviewed SHA predated a rebase — the data-lake namespace is now external/positioning/gnss/<id>/* and the pipeline boots from main.ts, so a few line references in the review point at moved code.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Re-review 2 (Claude)

Comparing 9ae36eb380dd52136f6f687225d082f408ae7cf5 → 477b31e5e227498e5eebdcd6922e045d6c153447. The branch was rebased since the first review, so the incremental diff includes the entire PR against the new base. Re-checking is based on the full pr.diff.

Previous findings status

#	Finding	Severity	Status
1.1	Startup loop may miss enabled devices if storage is async	minor	⚪ No longer applicable
1.2	Shared TextDecoder across devices could corrupt multi-byte state	minor	✅ Addressed
4.1	Shared onLinkData on hot serial path undocumented	minor	✅ Addressed
5.1	statusColor/statusIcon duplicated between view and dialog	minor	✅ Addressed
5.2	Delete device has no confirmation	nit	✅ Addressed
6.1	Missing JSDoc on createState	minor	✅ Addressed
6.2	Premature logUserAction before connection is established	minor	✅ Addressed
6.3	GnssState imports Ref from vue in types file	nit	⚪ No longer applicable
10.1	Magic probeSettleMs without explanatory comment	nit	✅ Addressed
10.2	device!.id non-null assertions in template lambdas	nit	✅ Addressed

Discussion since last review

The author posted a detailed follow-up comment addressing all 10 findings:

• 1.1 declined: the pipeline now boots from initGnss() in main.ts via settingsManager.getKeyValue (synchronous), making the composable loop irrelevant to device initialization. Verified in the current diff — the architecture change resolves the concern.
• 6.3 declined: type-only Ref import is erased at build time and grouping all GNSS types in one file is a deliberate convention. Reasonable.
• All other findings were addressed. Each fix has been verified against the current diff.

New findings

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to address: 1.3, 6.4

This PR adds Electron-only support for reading one or more USB/serial NMEA 0183 GNSS receivers and publishing their data into the data lake as external/positioning/gnss/<id>/* variables. All previous review findings have been addressed or explained. The architecture is well-separated (pure NMEA parser in libs/, orchestrator in libs/, reactive composable, minimal component logic). Two minor new observations surfaced on re-review.

---

1. Correctness & Implementation Bugs

1.3 (minor) — src/libs/cosmos.ts: The logUserAction global helper and its declare global block are added by this PR, but the same function was already merged to master in commit 9a8d738 (feat: add global logUserAction helper). This will produce a merge conflict or a duplicate definition when the PR is merged. The author should rebase onto the latest master to drop the duplicate logUserAction chunks from this PR (the cosmos.ts global declaration/implementation and the .eslintrc.cjs globals entry), keeping only the GNSS-specific changes.

---

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style

6.4 (minor) — src/composables/useGnss.ts: disconnectDevice logs logUserAction(...) before calling await stopGnssDevice(id). If the stop fails, the log message is misleading (same pattern that was fixed in connectDevice per finding 6.2). Move the logUserAction call after the await stopGnssDevice(id) for consistency.

---

7. Commit Hygiene — ✅

The four commits remain clean and logically separated after the rebase.

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 2

Done

• src/composables/useGnss.ts (6.4 — premature logUserAction in disconnectDevice): moved the log to after await stopGnssDevice(id), matching the 6.2 fix in connectDevice.

Won't change (with reasoning)

• 1.3 — duplicate logUserAction on merge: false positive from diffing against the pre-rebase SHA. This PR's own commits don't touch logUserAction — git diff 9a8d738..HEAD -- src/libs/cosmos.ts .eslintrc.cjs shows only the serialListPorts type addition, no logUserAction. The helper comes from master commit 9a8d738 (feat: add global logUserAction helper) that the branch was rebased onto, so it's already on the base and won't duplicate on merge. The bot picked it up because it compared 9ae36eb → 477b31e5e, which spans the rebase.

All round-1 findings confirmed addressed (1.2, 4.1, 5.1, 5.2, 6.1, 6.2, 10.1, 10.2) or declined with reasoning (1.1, 6.3).

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Re-review 3 (Claude)

Comparing 477b31e5e227498e5eebdcd6922e045d6c153447 → 87953dc2e295e942ff129eac299c66718c5fe536. The incremental diff covers the full PR (the compare API returned all changes since the rebased PREV_SHA). Re-check is based on the full pr.diff.

Previous findings status

#	Finding	Severity	Status
1.3	Duplicate logUserAction on merge	minor	⚪ No longer applicable
6.4	Premature logUserAction in disconnectDevice	minor	✅ Addressed

Discussion since last review

The author posted a follow-up comment addressing both open findings:

• 6.4 — fixed: logUserAction in disconnectDevice now fires after await stopGnssDevice(id). Confirmed in the current diff.
• 1.3 — correctly identified as a false positive: the PR's own commits don't touch logUserAction; the diff artifact came from the rebase spanning the base commit 9a8d738 that introduced the helper on master. The current cosmos.ts diff only adds the serialListPorts type, confirming no duplication.

New findings

0. Summary

Verdict: READY TO MERGE

This PR adds Electron-only support for reading one or more USB/serial NMEA 0183 GNSS receivers and publishing their data into the data lake as external/positioning/gnss/<id>/* variables. All prior review findings across three rounds have been addressed, explained, or confirmed as false positives. The architecture is well-separated (pure NMEA parser in libs/sensors/nmea.ts, orchestrator in libs/sensors/gnss.ts, reactive composable in composables/useGnss.ts, minimal component logic), and the code is clean.

1. Correctness & Implementation Bugs — ✅

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Ready to go.

[ArturoManzoli]

It work very well!

Besides the auto-connect question on commit 2, there are some UI adjustments to do:

(1) Add that classic X to close the window, horizontally aligned to the title;
(2) Pull up the container's content on ~20px. There is some important head space there to save;
(3) Consider adding that block to an ExpansiblePanel called 'Device Status' or 'Connection Status' or maybe use details instead of status... This way we simplify the connection modal, save vertical space, specially to mobile, and make the regular user's experience cleaner.
(4) use theme=dark on both selectors.

[ArturoManzoli]

Is useBlueOsStorage syncing, machine-specific on port + auto enabled settings?

Maybe if the user switch to another topside computer, or someone else connects to the vehicle in parallel (this is not uncommon, I suppose) and selects the same user; then enabled:true + a stale /dev/ttyUSB[...] will auto-connect to the wrong device.

[rafaellehmkuhl]

Yeah, the possibility exists...
Any idea on how to overcome that?

[ArturoManzoli]

Every USB device should have its own serial/VID/PID. I would add that to GnssDeviceMatch.
If the device doesn't supply any ID, make the auto connect a machine-local setting