Prefer device-reported speed on the Analysis tab, with per-device unit settings - #7
Merged
Merged
Conversation
…per device The Analysis tab's speed chart and average/max cards were computed entirely by differentiating consecutive GPS position samples, which amplifies normal GPS jitter into wild spikes (e.g. a boat under steady autopilot showing a 21mph "maximum speed"). Devices already send their own reported speed in Events.data.location.speed, but it was never read. Add two per-device settings (Admin -> Devices): "Input speed" (the unit the device's own reported speed is sent in — knots/km/h/m/s/mph, or "not reported" to keep today's behaviour) and "Display speed" (the unit shown throughout the Analysis tab). The Analysis loader now prefers the device's reported speed per point when configured, falling back to the position-derived calculation — with a visible warning — only for points that lack a reported speed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1tFbZfhQvqCNtbqopqTbM
There was a problem hiding this comment.
Pull request overview
This PR improves speed-related analytics on the Analysis tab by preferring device-reported speed readings (when configured per device) and standardizing speed unit conversion/display across charts, cards, and map coloring. This reduces GPS-jitter-induced spikes from position-derived speed while keeping backward compatibility for devices that don’t report speed.
Changes:
- Added per-device
inputSpeedUnit(nullable) anddisplaySpeedUnit(default"mph") to persist speed unit preferences. - Updated the Analysis loader/UI to use device-reported speed when available, fall back to derived speed when not, and show a warning banner when fallback was needed.
- Centralized speed unit conversion/labels and updated map route coloring + legend to respect the selected display unit.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/database/schema/Devices.ts | Adds inputSpeedUnit and displaySpeedUnit columns to the Devices schema. |
| website/database/migrations/0010_flashy_amphibian.sql | Migration to add the two new device speed unit columns. |
| website/database/migrations/meta/0010_snapshot.json | Drizzle snapshot update reflecting the schema change. |
| website/database/migrations/meta/_journal.json | Adds migration journal entry for 0010_flashy_amphibian. |
| website/app/utils/speedUnits.ts | Introduces shared speed unit constants, labels, and conversion helpers. |
| website/app/routes/date/analysis.tsx | Loads per-device speed settings, prefers device speed with derived fallback, updates chart/cards/alert behavior, and passes unit metadata to map. |
| website/app/routes/admin/devices.tsx | Adds Admin UI + server parsing/persistence for the new per-device speed unit settings. |
| website/app/components/AnalysisMap/speedColor.ts | Generalizes legend/range utilities away from mph-specific naming to support arbitrary display units. |
| website/app/components/AnalysisMap/AnalysisMap.tsx | Adds optional speedUnit prop passthrough to the client map. |
| website/app/components/AnalysisMap/AnalysisMap.client.tsx | Uses display-speed values for coloring and formats popup speed using the selected display unit. |
Suppressed comments (1)
website/app/routes/date/analysis.tsx:414
pointsWithDerivedSpeedis now populated with resolved speeds (device-reported when available, otherwise derived), so the name is misleading after this change. Renaming it (e.g.pointsWithResolvedSpeed) would make the code easier to follow and avoid confusion when reasoning about the device/derived fallback behavior.
const pointsWithDerivedSpeed = pointRows.map((point) => ({
id: Number(point.id),
timestamp: Number(point.timestamp),
latitude: Number(point.latitude),
longitude: Number(point.longitude),
speedMps: 0,
}));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
184
to
192
| const points = db.$with("points").as( | ||
| db | ||
| .select({ | ||
| id: Schema.Events.id, | ||
| timestamp: Schema.Events.timestamp, | ||
| latitude: Schema.Events.latitude, | ||
| longitude: Schema.Events.longitude, | ||
| data: Schema.Events.data, | ||
| }) |
3 tasks
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
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
The Analysis tab's "Speed over time" chart and Average/Maximum speed cards were computed entirely by differentiating consecutive GPS position samples (distance / time between fixes). This amplifies ordinary GPS position jitter into large speed spikes — e.g. a boat holding a steady course under autopilot was showing a 21.6 mph "maximum speed" that never actually happened.
Devices already send their own reported speed (stored in
Events.data.location.speed), but the Analysis loader never read it.knots/km/h/m/s/mph), or "Not reported" to keep the existing calculated-only behaviour.Test plan
npx tsc -b— no new type errors (two pre-existing, unrelated errors remain intimingPoints.tsx/timingPointsHistoricComparison.tsx)npm run db:generate/wrangler d1 migrations apply --local)inputSpeedUnit: kts,displaySpeedUnit: mphand synthetic events with steady 6 kt reported speed but GPS-jittered positions — Analysis tab showed a smooth ~7 mph line with spikes only at the points where reported speed was deliberately missing, each correctly flagged by the warning bannerGenerated by Claude Code