Add mission and vehicle path history odometer#2676
Conversation
Automated PR Review (Claude)0. SummaryVerdict: MINOR SUGGESTIONS Minor items to consider: 1.1, 1.2, 1.3, 4.1, 6.1 This PR adds an odometer feature that tracks total vehicle traveled distance and per-mission traveled distance. The distances are displayed in three places: the Mission Control Panel widget, the Mini Mission Control Panel mini-widget (via a hover/click popup on the WP number), and a Leaflet tooltip on the vehicle history polyline in the Map widget. A new 1. Correctness & Implementation Bugs1.1 1.2 1.3 2. AGENTS.md Adherence2.1 2.2 2.3 3. Security3.1 No obfuscated or intentionally unreadable code found. 3.2 No suspicious base64/hex/encoded blobs or binary-like strings. 3.3 No hidden Unicode, zero-width characters, or homoglyph attacks detected. 3.4 No unexpected network calls or exfiltration patterns. All computation is local. 3.5 No changes to build scripts, CI workflows, Dockerfiles, or Electron main-process code. 3.6 No new secrets, 3.7 No new dependencies added. The PR reuses 3.8 No other suspicious patterns detected. No findings. 4. Performance4.1 5. UI / UX5.1 5.2 No major UI issues. 6. Code Quality & Style6.1 6.2 6.3 7. TestsNo new tests were added for the 7.1 8. Documentation8.1 No README update needed since this feature works identically in both Lite and Standalone modes. 9. Nitpicks / Optional9.1 9.2 9.3 Generated by Claude. This is advisory; a human reviewer must still approve. |
dc8b887 to
856c557
Compare
Fixed both majors |
| export const formatDistance = (meters: number, unit: DistanceDisplayUnit): string => { | ||
| if (unit === DistanceDisplayUnit.Feet) { | ||
| const feet = meters * 3.28084 | ||
| if (feet >= 5280) return `${(feet / 5280).toFixed(2)} mi` | ||
| return `${feet.toFixed(0)} ${unitAbbreviation[DistanceDisplayUnit.Feet]}` | ||
| } | ||
| if (meters >= 1000) return `${(meters / 1000).toFixed(2)} km` | ||
| return `${meters.toFixed(1)} ${unitAbbreviation[DistanceDisplayUnit.Meters]}` | ||
| } |
There was a problem hiding this comment.
The unit module from mathjs provides this conversion already.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
856c557 to
1a6e53b
Compare
|
/review |
Automated PR Re-review 1 (Claude)Since Previous findings status
Discussion since last review
New findings0. SummaryVerdict: MINOR SUGGESTIONS Minor items to consider: 5.3, 5.4 This PR adds odometer tracking for total vehicle distance and per-mission distance. Distances are displayed in the Mission Control Panel widget, the Mini Mission Control Panel mini-widget (hover/click popup on WP number), and a Leaflet tooltip on the vehicle history polyline in the Map widget. A 1. Correctness & Implementation Bugs — ✅2. AGENTS.md Adherence — ✅3. Security — ✅4. Performance — ✅5. UI / UX5.3
All three should log via 5.4 6. Code Quality & Style6.4
7. Commit Hygiene — ✅8. Tests — ✅9. Documentation — ✅10. Nitpicks / Optional — ✅Generated by Claude. This is advisory; a human reviewer must still approve. |
1a6e53b to
f2fb244
Compare
|
/review |
Automated PR Re-review 2 (Claude)Comparing Previous findings status
New findings0. SummaryVerdict: MINOR SUGGESTIONS Minor items to consider: 1.4, 5.5 This PR adds odometer tracking for total vehicle distance and per-mission distance, displayed across the Mission Control Panel widget, Mini Mission Control Panel popup, and the map's history-line tooltip. Since the last review, the duplicated formatting logic was extracted into a 1. Correctness & Implementation Bugs1.4 Consider either: (a) moving the 2. AGENTS.md Adherence — ✅3. Security — ✅4. Performance — ✅5. UI / UX5.5 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. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Extracts the mission-store + interface-store wiring that formats the total and mission traveled distances so widgets, mini-widgets and map overlays can share a single formatting funnel instead of duplicating the pair of computeds at each call site.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ver on current wp Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
f2fb244 to
e3746ce
Compare
|
/review |
Automated PR Re-review 3 (Claude)Comparing Previous findings status
New findings0. SummaryVerdict: MINOR SUGGESTIONS Minor items to consider: 1.5, 6.5 This PR adds odometer tracking for total vehicle distance and per-mission distance, displayed across the Mission Control Panel widget, Mini Mission Control Panel popup, and the map's history-line tooltip. Since the last review, the 1. Correctness & Implementation Bugs1.5 2. AGENTS.md Adherence — ✅3. Security — ✅4. Performance — ✅5. UI / UX — ✅6. Code Quality & Style6.5 7. Commit Hygiene — ✅8. Tests — ✅9. Documentation — ✅10. Nitpicks / Optional — ✅Generated by Claude. This is advisory; a human reviewer must still approve. |
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
I had a strange problem when testing this: while the mission odometer seemed to look fine, the total one jumped several times. It initiated already on something like 1.1km, then it reset to 0 when I started a mission, and then it when back to 1.1km when I restarted Cockpit.
I suspect it's not the root cause, but there's one catch with how it is implemented right now: the odometer is never actually going to be written to the vehicle, unless the position stops being updated. That happens because the useBlueOsStorage composable has a debouncer of 3 seconds, while the position updates several times a second.
I've opened #2818 to fix that problem in the sync for this kind of setting.
Close #2675