From 90c5537fea0da00ce234b1cbd44bd4263675a1c7 Mon Sep 17 00:00:00 2001 From: Jos Nienhuis <6952249+joszz@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:44:14 +0200 Subject: [PATCH 1/7] refactor: share command sending and form controls ahead of climate schedules Backend: - VehicleCommands moves to Core so the Worker can name a command's topic too. Validate now returns the English message; the Api adds the code. - VehicleCommandSender holds the topic, value check and gated publish that the command endpoint did inline, so other callers send commands the same way. Frontend: - Maintenance form classes become shared .form-stack/.form-row/.form-group/ .form-control, and .maintenance-list becomes .status-card-list. - The trip purpose buttons become a reusable ChipGroup (one-of-N or several-of-N, aria-pressed), with the purpose colours as modifiers. - useDistanceField becomes useMeasureField(units, quantity), so a temperature field round-trips the same way a distance does. - The climate popup's slider and switch rows become RangeControl and DetailListSwitch. The sliders now carry an aria-label, and the seat label follows the slider while dragging. Co-Authored-By: Claude Opus 5.5 --- documentation/ARCHITECTURE.md | 6 +- frontend/src/assets/main.css | 91 +++++++++ frontend/src/assets/maintenance.css | 54 ------ frontend/src/assets/tripLog.css | 44 +---- frontend/src/components/ChipGroup.vue | 52 ++++++ frontend/src/components/ClimateDetailCard.vue | 173 ++++++------------ frontend/src/components/DetailListSwitch.vue | 28 +++ .../components/MaintenanceItemDetailModal.vue | 27 ++- .../components/MaintenanceItemFormModal.vue | 40 ++-- frontend/src/components/RangeControl.vue | 50 +++++ frontend/src/components/TripLogItem.vue | 44 ++--- .../components/__tests__/ChipGroup.spec.ts | 77 ++++++++ .../components/__tests__/RangeControl.spec.ts | 58 ++++++ .../__tests__/useDistanceField.spec.ts | 48 ----- .../__tests__/useMeasureField.spec.ts | 60 ++++++ frontend/src/composables/useDistanceField.ts | 32 ---- frontend/src/composables/useMeasureField.ts | 32 ++++ frontend/src/utils/chips.ts | 9 + frontend/src/views/MaintenanceView.vue | 2 +- .../Endpoints/VehicleEndpoints.cs | 27 +-- .../Hubs/CommandResultMessage.cs | 1 + src/GarageStack.Api/Program.cs | 1 + src/GarageStack.Api/VehicleCommandSender.cs | 46 +++++ .../Helpers}/VehicleCommands.cs | 31 ++-- .../VehicleCommandSenderTests.cs | 75 ++++++++ .../VehicleCommandValidationTests.cs | 21 +-- src/GarageStack.Tests/VehicleCommandsTests.cs | 2 +- 27 files changed, 737 insertions(+), 394 deletions(-) create mode 100644 frontend/src/components/ChipGroup.vue create mode 100644 frontend/src/components/DetailListSwitch.vue create mode 100644 frontend/src/components/RangeControl.vue create mode 100644 frontend/src/components/__tests__/ChipGroup.spec.ts create mode 100644 frontend/src/components/__tests__/RangeControl.spec.ts delete mode 100644 frontend/src/composables/__tests__/useDistanceField.spec.ts create mode 100644 frontend/src/composables/__tests__/useMeasureField.spec.ts delete mode 100644 frontend/src/composables/useDistanceField.ts create mode 100644 frontend/src/composables/useMeasureField.ts create mode 100644 frontend/src/utils/chips.ts create mode 100644 src/GarageStack.Api/VehicleCommandSender.cs rename src/{GarageStack.Api => GarageStack.Core/Helpers}/VehicleCommands.cs (79%) create mode 100644 src/GarageStack.Tests/VehicleCommandSenderTests.cs diff --git a/documentation/ARCHITECTURE.md b/documentation/ARCHITECTURE.md index bd92bae1..57db4556 100644 --- a/documentation/ARCHITECTURE.md +++ b/documentation/ARCHITECTURE.md @@ -55,7 +55,7 @@ Messages from the official MG app are the one notification the Worker does not w ## Sending a command to the car (the reverse path) -The frontend calls `POST /api/vehicles/{vin}/commands/{command}` on the Api, which publishes an MQTT message via `MqttPublisher` ([src/GarageStack.Api/Services/MqttPublisher.cs](../src/GarageStack.Api/Services/MqttPublisher.cs)) - a separate MQTT client from the Worker's, since the Api only ever publishes and the Worker only ever consumes. `saic-mqtt-gateway` picks the message up and relays it to the SAIC cloud API. `VehicleCommandGate` ([src/GarageStack.Api/VehicleCommandGate.cs](../src/GarageStack.Api/VehicleCommandGate.cs)) serializes commands per VIN server-side, since the real gateway only processes one command at a time. The command names and their gateway topics live in `VehicleCommands`, one table for both directions. +The frontend calls `POST /api/vehicles/{vin}/commands/{command}` on the Api, which publishes an MQTT message via `MqttPublisher` ([src/GarageStack.Api/Services/MqttPublisher.cs](../src/GarageStack.Api/Services/MqttPublisher.cs)) - a separate MQTT client from the Worker's, since the Api only ever publishes and the Worker only ever consumes. `saic-mqtt-gateway` picks the message up and relays it to the SAIC cloud API. `VehicleCommandGate` ([src/GarageStack.Api/VehicleCommandGate.cs](../src/GarageStack.Api/VehicleCommandGate.cs)) serializes commands per VIN server-side, since the real gateway only processes one command at a time. The command names and their gateway topics live in `VehicleCommands` ([src/GarageStack.Core/Helpers/VehicleCommands.cs](../src/GarageStack.Core/Helpers/VehicleCommands.cs)), one table for both directions, and `VehicleCommandSender` is the one place a command is checked and published. The gateway answers every command on the same topic with `/set` swapped for `/result`: `Success`, or `Failed: ` once the SAIC API gives up (it polls for up to 30 seconds). The Worker parses that (`GatewayCommandResult`) and raises `command_result`. The Api releases the command gate for that VIN and pushes a `commandResult` event over SignalR, named by the command the browser sent. `useVehicleCommand` then settles the matching pending command: a refusal clears it and shows the reason. A success clears a command that no telemetry reports (the horn, the charge current limit). A command that telemetry does report stays pending until the telemetry shows it, but a batch can already move on to its next command. Without an answer, the 45-second hold on both sides still applies, which is also what happens in demo mode. @@ -231,8 +231,8 @@ The API speaks metric only (km, km/h, bar, °C, litres, L/100 km), and so does t chosen units live in the UI settings store and are applied at the last moment by `UnitFormatter` in `utils/units.ts`, reached through the `useUnits` composable: a value on screen goes through `units.format(quantity, metricValue)` (or `measure` when the number and its unit are -styled apart), never through a hand-written `km` or `bar`. A distance typed into a form goes back -through `useDistanceField`, which returns the untouched kilometres it loaded rather than a +styled apart), never through a hand-written `km` or `bar`. A distance or temperature typed into a form goes back +through `useMeasureField`, which returns the untouched metric value it loaded rather than a round-tripped approximation. Dashboard cards are described once, in `frontend/src/cards/registry.ts`: each entry carries the card's icon, whether it is visible by default for a given drivetrain, and whether the current telemetry has anything to show. The card ids, the default layout and the "does this card have data" checks are all derived from that list, so a new card is one entry plus its markup in `DashboardCardContent.vue`. The map's point-of-interest layers work the same way: `composables/poiTileLayer.ts` holds the fetch-by-tile, cache and cluster logic, and charging stations, fuel stations, service areas and speed cameras are four configurations of it. diff --git a/frontend/src/assets/main.css b/frontend/src/assets/main.css index 5038c7b0..831446f3 100644 --- a/frontend/src/assets/main.css +++ b/frontend/src/assets/main.css @@ -323,6 +323,13 @@ body, /* ── Status card grid ─────────────────────────────────────── */ +/* Status cards as a single-column list of items (maintenance, climate schedules). */ +.status-card-list { + display: flex; + flex-direction: column; + gap: 0.6rem; +} + .status-grid { display: grid; grid-template-columns: repeat(4, 1fr); @@ -1104,6 +1111,82 @@ body, left: 23px; } +/* Labelled fields stacked in a modal form: a group is a label above its field, a row puts + two groups side by side (one above the other on phones, see the mobile breakpoint). */ +.form-stack { + display: flex; + flex-direction: column; + gap: 0.9rem; +} + +.form-row { + display: grid; + grid-template-columns: 1fr 1fr; + gap: 0.9rem; +} + +.form-group { + display: flex; + flex-direction: column; + gap: 0.3rem; +} + +.form-group label { + font-size: 0.78rem; + color: var(--color-text-muted); +} + +.form-control { + width: 100%; + padding: 0.4rem 0.6rem; + font-size: 0.85rem; + border-radius: 6px; + border: 1px solid var(--color-border); + background-color: var(--color-surface-2); + color: var(--color-text); +} + +textarea.form-control { + resize: vertical; + font-family: inherit; +} + +/* ── Toggle chips (one-of-N or several-of-N choices) ──────── */ + +.toggle-chip-group { + display: flex; + flex-wrap: wrap; + gap: 0.35rem; +} + +.toggle-chip { + --chip-color: var(--color-primary); + + display: inline-flex; + align-items: center; + justify-content: center; + gap: 0.35rem; + padding: 0.3rem 0.6rem; + font-size: 0.78rem; + border: 1px solid var(--color-border); + border-radius: 6px; + background: var(--color-surface-2); + color: var(--color-text-muted); + cursor: pointer; +} + +.toggle-chip--active { + border-color: var(--chip-color); + background: color-mix(in srgb, var(--chip-color) 18%, var(--color-surface-2)); + color: var(--color-text); + font-weight: 600; +} + +.toggle-chip:disabled { + cursor: not-allowed; + opacity: 0.6; +} + /* ── Settings ─────────────────────────────────────────────── */ .settings-section { @@ -1451,6 +1534,10 @@ body, gap: 0.7rem; } +.range-control--grow { + flex: 1; +} + /* ── Card info overlay ────────────────────────────────────── */ .card-info-wrap { @@ -1670,6 +1757,10 @@ body, /* ── Mobile breakpoint ────────────────────────────────────── */ @media (width <= 767px) { + .form-row { + grid-template-columns: 1fr; + } + /* Show topbar, hide desktop sidebar brand */ .mobile-topbar { display: flex; diff --git a/frontend/src/assets/maintenance.css b/frontend/src/assets/maintenance.css index 5e6aeb82..3bd37aec 100644 --- a/frontend/src/assets/maintenance.css +++ b/frontend/src/assets/maintenance.css @@ -1,51 +1,3 @@ -/* ── List ─────────────────────────────────────────────────── */ - -.maintenance-list { - display: flex; - flex-direction: column; - gap: 0.6rem; -} - -/* ── Form fields ──────────────────────────────────────────── */ - -.maintenance-form { - display: flex; - flex-direction: column; - gap: 0.9rem; -} - -.maintenance-field-row { - display: grid; - grid-template-columns: 1fr 1fr; - gap: 0.9rem; -} - -.maintenance-field-group { - display: flex; - flex-direction: column; - gap: 0.3rem; -} - -.maintenance-field-group label { - font-size: 0.78rem; - color: var(--color-text-muted); -} - -.maintenance-field { - width: 100%; - padding: 0.4rem 0.6rem; - font-size: 0.85rem; - border-radius: 6px; - border: 1px solid var(--color-border); - background-color: var(--color-surface-2); - color: var(--color-text); -} - -.maintenance-field--textarea { - resize: vertical; - font-family: inherit; -} - /* ── Detail modal ─────────────────────────────────────────── */ .maintenance-detail { @@ -95,9 +47,3 @@ .maintenance-history__delete { margin-left: auto; } - -@media (width <= 767px) { - .maintenance-field-row { - grid-template-columns: 1fr; - } -} diff --git a/frontend/src/assets/tripLog.css b/frontend/src/assets/tripLog.css index f0568af3..aed45f53 100644 --- a/frontend/src/assets/tripLog.css +++ b/frontend/src/assets/tripLog.css @@ -122,11 +122,6 @@ align-items: center; } -.trip-log-item__purpose { - display: flex; - gap: 0.35rem; -} - .trip-log-item__notes { flex: 1 1 12rem; min-width: 0; @@ -138,42 +133,15 @@ color: var(--color-text); } -/* ── Purpose buttons ──────────────────────────────────────── */ +/* ── Purpose chips (ChipGroup, main.css) ─────────────────── */ -.trip-log-purpose { - --purpose-color: var(--color-primary); - - display: inline-flex; - align-items: center; - justify-content: center; - gap: 0.35rem; - padding: 0.3rem 0.6rem; - font-size: 0.78rem; - border: 1px solid var(--color-border); - border-radius: 6px; - background: var(--color-surface-2); - color: var(--color-text-muted); - cursor: pointer; -} - -.trip-log-purpose--commute { - --purpose-color: var(--color-info); -} - -.trip-log-purpose--private { - --purpose-color: var(--color-success); -} - -.trip-log-purpose--active { - border-color: var(--purpose-color); - background: color-mix(in srgb, var(--purpose-color) 18%, var(--color-surface-2)); - color: var(--color-text); - font-weight: 600; +/* Two classes, so the colour wins over .toggle-chip whichever stylesheet loads last. */ +.toggle-chip.trip-log-purpose--commute { + --chip-color: var(--color-info); } -.trip-log-purpose:disabled { - cursor: not-allowed; - opacity: 0.6; +.toggle-chip.trip-log-purpose--private { + --chip-color: var(--color-success); } /* ── Phones ───────────────────────────────────────────────── */ diff --git a/frontend/src/components/ChipGroup.vue b/frontend/src/components/ChipGroup.vue new file mode 100644 index 00000000..9538c191 --- /dev/null +++ b/frontend/src/components/ChipGroup.vue @@ -0,0 +1,52 @@ + + + diff --git a/frontend/src/components/ClimateDetailCard.vue b/frontend/src/components/ClimateDetailCard.vue index bde9e4ed..609a83fc 100644 --- a/frontend/src/components/ClimateDetailCard.vue +++ b/frontend/src/components/ClimateDetailCard.vue @@ -3,6 +3,8 @@ import { computed, ref, watch } from 'vue' import { useI18n } from 'vue-i18n' import ExpandableStatusCard from './ExpandableStatusCard.vue' import DetailListItem from './DetailListItem.vue' +import DetailListSwitch from './DetailListSwitch.vue' +import RangeControl from './RangeControl.vue' import CommandFailure from './CommandFailure.vue' import { useVehicleCommand } from '@/composables/useVehicleCommand' import type { TelemetrySnapshot } from '@/services/vehicleApi' @@ -164,12 +166,17 @@ async function applyAll() { } } -function onSeatLeftChange(e: Event) { - seatLeftLocal.value = Number((e.target as HTMLInputElement).value) -} +const temperatureText = computed( + () => `${wholeDegrees(sliderTemp.value)} ${units.value.symbol('temperature')}`, +) +const temperatureEdges = computed((): [string, string] => [ + `${wholeDegrees(TEMP_MIN)}°`, + `${wholeDegrees(TEMP_MAX)}°`, +]) +const controlsDisabled = computed(() => isApplying.value || !props.vin) -function onSeatRightChange(e: Event) { - seatRightLocal.value = Number((e.target as HTMLInputElement).value) +function seatText(level: number): string { + return seatLabels.value[level] ?? '' } @@ -183,65 +190,33 @@ function onSeatRightChange(e: Event) { v-model:open="modalOpen" >
- -
- -
-
- {{ t('control.temperature') }} - {{ wholeDegrees(sliderTemp) }} {{ units.symbol('temperature') }} -
-
- {{ wholeDegrees(TEMP_MIN) }}° - - {{ wholeDegrees(TEMP_MAX) }}° -
-
-
+ v-model="sliderTemp" + icon="temperature-half" + :label="t('control.temperature')" + :value-text="temperatureText" + :min="TEMP_MIN" + :max="TEMP_MAX" + :edge-labels="temperatureEdges" + :disabled="controlsDisabled" + /> - -
- - {{ t('control.climate') }} -
- -
-
+ - -
- - {{ t('control.rearDefroster') }} -
- -
-
+ - -
- -
-
- {{ t('vehicle.climateDetail.seatLeft') }} - {{ seatLabels[seatLeftLocal] }} -
-
- -
-
- {{ label }} -
-
-
+ - -
- -
-
- {{ t('vehicle.climateDetail.seatRight') }} - {{ seatLabels[seatRightLocal] }} -
-
- -
-
- {{ label }} -
-
-
+
- - diff --git a/frontend/src/components/DetailListSwitch.vue b/frontend/src/components/DetailListSwitch.vue new file mode 100644 index 00000000..59873d29 --- /dev/null +++ b/frontend/src/components/DetailListSwitch.vue @@ -0,0 +1,28 @@ + + + diff --git a/frontend/src/components/MaintenanceItemDetailModal.vue b/frontend/src/components/MaintenanceItemDetailModal.vue index 51fbf6a5..c3120980 100644 --- a/frontend/src/components/MaintenanceItemDetailModal.vue +++ b/frontend/src/components/MaintenanceItemDetailModal.vue @@ -9,7 +9,7 @@ import { formatIntervalSummary } from '@/utils/maintenance' import { formatDate } from '@/utils/format' import { ODOMETER_FORMAT } from '@/utils/units' import { useUnits } from '@/composables/useUnits' -import { useDistanceField } from '@/composables/useDistanceField' +import { useMeasureField } from '@/composables/useMeasureField' import { useErrorMessage } from '@/composables/useErrorMessage' const props = defineProps<{ @@ -27,7 +27,7 @@ const units = useUnits() const errorMessage = useErrorMessage() const performedAt = ref('') -const odometer = useDistanceField(units) +const odometer = useMeasureField(units, 'distance') const logNotes = ref('') const logSaving = ref(false) const logError = ref(null) @@ -56,7 +56,7 @@ async function submitLog() { try { await store.logService(props.vin, props.item.id, { performedAt: performedAt.value, - odometerKm: odometer.km(), + odometerKm: odometer.metric(), notes: logNotes.value.trim() || null, }) if (store.actionError) { @@ -118,18 +118,13 @@ async function confirmDelete() {

{{ currentItem.notes }}

{{ t('maintenance.logService') }}

-
-
-
+ +
+
- +
-
+
@@ -138,17 +133,17 @@ async function confirmDelete() { v-model.number="odometer.shown" type="number" min="0" - class="maintenance-field" + class="form-control" />
-
+
diff --git a/frontend/src/components/MaintenanceItemFormModal.vue b/frontend/src/components/MaintenanceItemFormModal.vue index 92def271..2aef73bf 100644 --- a/frontend/src/components/MaintenanceItemFormModal.vue +++ b/frontend/src/components/MaintenanceItemFormModal.vue @@ -6,7 +6,7 @@ import { useMaintenanceStore } from '@/stores/maintenance' import type { MaintenanceItem } from '@/services/maintenanceApi' import { useUnits } from '@/composables/useUnits' import { useErrorMessage } from '@/composables/useErrorMessage' -import { useDistanceField } from '@/composables/useDistanceField' +import { useMeasureField } from '@/composables/useMeasureField' const props = defineProps<{ open: boolean @@ -23,10 +23,10 @@ const errorMessage = useErrorMessage() const name = ref('') const notes = ref('') -const interval = useDistanceField(units) +const interval = useMeasureField(units, 'distance') const intervalMonths = ref(null) const lastServiceDate = ref('') -const lastServiceOdometer = useDistanceField(units) +const lastServiceOdometer = useMeasureField(units, 'distance') const validationError = ref(null) const distanceUnit = computed(() => ({ unit: units.value.symbol('distance') })) const saving = ref(false) @@ -60,7 +60,7 @@ async function submit() { validationError.value = t('maintenance.form.validationNameRequired') return } - const intervalKm = interval.km() + const intervalKm = interval.metric() if (intervalKm == null && intervalMonths.value == null) { validationError.value = t('maintenance.form.validationIntervalRequired') return @@ -82,7 +82,7 @@ async function submit() { intervalKm, intervalMonths: intervalMonths.value, lastServiceDate: lastServiceDate.value || null, - lastServiceOdometerKm: lastServiceOdometer.km(), + lastServiceOdometerKm: lastServiceOdometer.metric(), }) } if (store.actionError) { @@ -98,32 +98,32 @@ async function submit() {