From 366634284ae1ee9371afbc257b651fd9bee96cfb Mon Sep 17 00:00:00 2001 From: Dkx <45234736+dkxmercury@users.noreply.github.com> Date: Sat, 11 Jul 2026 13:02:25 +0500 Subject: [PATCH 1/5] fix(position): convert fixed altitude between feet and meters (#1051) The fixed-altitude field is labelled Feet when the display units are set to Imperial, but the value was stored and loaded as raw meters with no conversion, so entering 1025 stored 1025 m instead of ~312 m. Add feetToMeters/metersToFeet helpers in core/utils/geo.ts and convert at the form load, submit, and geolocation-fill edges when units are Imperial. Altitude is rounded to integer meters to match the int32 protobuf field. Fixes #1051 --- .../PageComponents/Settings/Position.tsx | 50 ++++++++++++------- apps/web/src/core/utils/geo.test.ts | 35 +++++++++++++ apps/web/src/core/utils/geo.ts | 13 +++++ 3 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 apps/web/src/core/utils/geo.test.ts diff --git a/apps/web/src/components/PageComponents/Settings/Position.tsx b/apps/web/src/components/PageComponents/Settings/Position.tsx index c61674695..ac6361e6a 100644 --- a/apps/web/src/components/PageComponents/Settings/Position.tsx +++ b/apps/web/src/components/PageComponents/Settings/Position.tsx @@ -16,6 +16,7 @@ import { import { useMyNodeAsProto } from "@core/hooks/useNodesAsProto.ts"; import { useToast } from "@core/hooks/useToast.ts"; import { useDevice } from "@core/stores"; +import { feetToMeters, metersToFeet } from "@core/utils/geo.ts"; import { Protobuf } from "@meshtastic/sdk"; import { useConfigEditor, useSignal } from "@meshtastic/sdk-react"; import { LocateFixed } from "lucide-react"; @@ -32,7 +33,7 @@ interface PositionConfigProps { * via navigator.geolocation and writes lat/lng/altitude into the form. * No-op without a geolocation API (e.g. insecure context). */ -function UseBrowserLocationButton() { +function UseBrowserLocationButton({ isImperial }: { isImperial: boolean }) { const { setValue } = useFormContext(); const { toast } = useToast(); const { t } = useTranslation("config"); @@ -57,9 +58,14 @@ function UseBrowserLocationButton() { pos.coords.altitude !== null && !Number.isNaN(pos.coords.altitude) ) { - setValue("altitude", Math.round(pos.coords.altitude), { - shouldDirty: true, - }); + // Geolocation altitude is in meters; show feet when Imperial. + setValue( + "altitude", + isImperial + ? Math.round(metersToFeet(pos.coords.altitude)) + : Math.round(pos.coords.altitude), + { shouldDirty: true }, + ); } }, (err) => { @@ -113,8 +119,12 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { const currentPosition = myNode?.position; const displayUnits = getEffectiveConfig("display")?.units; + const isImperial = + displayUnits === Protobuf.Config.Config_DisplayConfig_DisplayUnits.IMPERIAL; const formValues = useMemo(() => { + // Firmware stores altitude in meters; the field shows feet when Imperial. + const altitudeMeters = currentPosition?.altitude ?? 0; return { ...config.position, ...effectivePosition, @@ -124,9 +134,11 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { longitude: currentPosition?.longitudeI ? currentPosition.longitudeI / 1e7 : undefined, - altitude: currentPosition?.altitude ?? 0, + altitude: isImperial + ? Math.round(metersToFeet(altitudeMeters)) + : altitudeMeters, } as PositionValidation; - }, [config.position, effectivePosition, currentPosition]); + }, [config.position, effectivePosition, currentPosition, isImperial]); const onSubmit = (data: PositionValidation) => { const { @@ -149,13 +161,23 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { data.latitude !== undefined && data.longitude !== undefined ) { + // The altitude field is in feet when Imperial; the protobuf expects + // integer meters, so convert back and round. Guard against a + // cleared/NaN field. + const altitudeInput = Number.isFinite(data.altitude) + ? (data.altitude as number) + : 0; + const altitudeMeters = isImperial + ? Math.round(feetToMeters(altitudeInput)) + : Math.round(altitudeInput); + const message = create(Protobuf.Admin.AdminMessageSchema, { payloadVariant: { case: "setFixedPosition", value: create(Protobuf.Mesh.PositionSchema, { latitudeI: Math.round(data.latitude * 1e7), longitudeI: Math.round(data.longitude * 1e7), - altitude: data.altitude || 0, + altitude: altitudeMeters, time: Math.floor(Date.now() / 1000), }), }, @@ -220,7 +242,7 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { { label: t("position.deviceGps.label"), description: t("position.deviceGps.description"), - footer: , + footer: , fields: [ { type: "toggle", @@ -257,19 +279,11 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { name: "altitude", label: t("position.fixedPosition.altitude.label"), description: t("position.fixedPosition.altitude.description", { - unit: - displayUnits === - Protobuf.Config.Config_DisplayConfig_DisplayUnits.IMPERIAL - ? "Feet" - : "Meters", + unit: isImperial ? "Feet" : "Meters", }), properties: { step: 0.0000001, - suffix: - displayUnits === - Protobuf.Config.Config_DisplayConfig_DisplayUnits.IMPERIAL - ? "Feet" - : "Meters", + suffix: isImperial ? "Feet" : "Meters", }, disabledBy: [{ fieldName: "fixedPosition" }], }, diff --git a/apps/web/src/core/utils/geo.test.ts b/apps/web/src/core/utils/geo.test.ts new file mode 100644 index 000000000..41773fe65 --- /dev/null +++ b/apps/web/src/core/utils/geo.test.ts @@ -0,0 +1,35 @@ +import { + feetToMeters, + METERS_PER_FOOT, + metersToFeet, +} from "@core/utils/geo.ts"; +import { describe, expect, it } from "vitest"; + +// Regression test for meshtastic/web#1051: +// The fixed-position altitude field is labelled "Feet" when the display units +// are Imperial, but the firmware stores altitude as an integer number of +// meters. The value entered in feet must be converted to meters before it is +// written to the protobuf. +describe("altitude unit conversion (geo)", () => { + it("uses the exact meters-per-foot factor", () => { + expect(METERS_PER_FOOT).toBe(0.3048); + }); + + it("converts feet to meters", () => { + expect(feetToMeters(1025)).toBeCloseTo(312.42, 2); + expect(feetToMeters(0)).toBe(0); + }); + + it("converts meters to feet", () => { + expect(metersToFeet(312)).toBeCloseTo(1023.62, 2); + }); + + it("stores 1025 ft as 312 m (rounded to int32 meters) — issue #1051", () => { + // A user entering 1025 in the feet-labelled field must not store 1025 m. + expect(Math.round(feetToMeters(1025))).toBe(312); + }); + + it("round-trips back to the ~1024 ft the reporter observed", () => { + expect(Math.round(metersToFeet(312))).toBe(1024); + }); +}); diff --git a/apps/web/src/core/utils/geo.ts b/apps/web/src/core/utils/geo.ts index a3017bd94..1439e7a3d 100644 --- a/apps/web/src/core/utils/geo.ts +++ b/apps/web/src/core/utils/geo.ts @@ -7,6 +7,19 @@ export type Bounds = [[number, number], [number, number]]; const INT_DEG = 1e7; const EARTH_RADIUS = 6378137; +/** + * Firmware/protobuf store altitude as an integer number of meters + * (mesh.proto: `optional int32 altitude = 3`). The Position config UI shows + * altitude in feet when the display units are set to Imperial, so we convert + * at the edges (form load / form submit). + */ +export const METERS_PER_FOOT = 0.3048; + +export const feetToMeters = (feet: number): number => feet * METERS_PER_FOOT; + +export const metersToFeet = (meters: number): number => + meters / METERS_PER_FOOT; + export const toLngLat = (position?: { latitudeI?: number; longitudeI?: number; From 45260234148178abefd0393a520c2165386fa713 Mon Sep 17 00:00:00 2001 From: Dkx <45234736+dkxmercury@users.noreply.github.com> Date: Sat, 11 Jul 2026 13:30:02 +0500 Subject: [PATCH 2/5] test(position): cover feet/meters altitude conversion in Position form (#1051) Render the real Position form and assert the conversion at each edge: Imperial load shows meters as feet, submitting feet queues integer meters into the setFixedPosition message, metric values pass through unchanged, and the browser-geolocation fill converts meters to feet. --- .../PageComponents/Settings/Position.test.tsx | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 apps/web/src/components/PageComponents/Settings/Position.test.tsx diff --git a/apps/web/src/components/PageComponents/Settings/Position.test.tsx b/apps/web/src/components/PageComponents/Settings/Position.test.tsx new file mode 100644 index 000000000..10c31add4 --- /dev/null +++ b/apps/web/src/components/PageComponents/Settings/Position.test.tsx @@ -0,0 +1,219 @@ +import { create } from "@bufbuild/protobuf"; +import { Protobuf } from "@meshtastic/sdk"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { Position } from "./Position.tsx"; + +// Integration/component test for meshtastic/web#1051. +// +// The fixed-position altitude field is labelled in Feet when the display units +// are Imperial, but the firmware/protobuf store altitude as integer meters. +// This test renders the real form (DynamicForm + react-hook-form + +// the zod resolver) and asserts the conversion happens at the edges: +// - form load (meters -> feet for display when Imperial) +// - form submit (feet -> meters into the queued setFixedPosition admin msg) +// - browser geolocation fill (meters -> feet for display when Imperial) +// +// The only external seam we assert on is the admin message handed to the +// config editor's queueAdminMessage(), which is exactly what the firmware +// receives. + +// --- Mutable per-test state (read lazily by the mock factories below) -------- +// Names are prefixed with `mock` so vitest's vi.mock hoisting whitelist allows +// the factories to reference them. +let mockDisplayUnits: Protobuf.Config.Config_DisplayConfig_DisplayUnits; +let mockPositionConfig: Protobuf.Config.Config_PositionConfig; +let mockMyNode: Protobuf.Mesh.NodeInfo | undefined; + +const mockSetRadioSection = vi.fn(); +const mockQueueAdminMessage = vi.fn(); +const mockToast = vi.fn(); +const mockGetCurrentPosition = vi.fn(); + +// A minimal signal-shaped object; the mocked useSignal just returns `.value`. +const mockEditor = { + radio: { value: {}, peek: () => ({}), subscribe: () => () => {} }, + setRadioSection: mockSetRadioSection, + queueAdminMessage: mockQueueAdminMessage, +}; + +const mockGetEffectiveConfig = (configCase: string) => { + if (configCase === "display") { + return { units: mockDisplayUnits }; + } + if (configCase === "position") { + return mockPositionConfig; + } + return undefined; +}; + +// useWaitForConfig throws a suspense promise until config is present; the render +// path here supplies config directly, so it is a no-op in tests. +vi.mock("@app/core/hooks/useWaitForConfig", () => ({ + useWaitForConfig: () => {}, +})); + +vi.mock("@core/stores", () => ({ + useDevice: () => ({ + config: { position: mockPositionConfig }, + getEffectiveConfig: mockGetEffectiveConfig, + }), +})); + +vi.mock("@meshtastic/sdk-react", () => ({ + useConfigEditor: () => mockEditor, + useSignal: (signal: { value?: unknown }) => signal?.value ?? {}, +})); + +vi.mock("@core/hooks/useNodesAsProto.ts", () => ({ + useMyNodeAsProto: () => mockMyNode, +})); + +vi.mock("@core/hooks/useToast.ts", () => ({ + useToast: () => ({ toast: mockToast }), +})); + +// --- Helpers ----------------------------------------------------------------- +const IMPERIAL = Protobuf.Config.Config_DisplayConfig_DisplayUnits.IMPERIAL; +const METRIC = Protobuf.Config.Config_DisplayConfig_DisplayUnits.METRIC; + +// San Francisco, so the fixed-position submit path (lat/lng defined) is taken. +const LATITUDE_I = 377_749_000; +const LONGITUDE_I = -1_224_194_000; + +interface SetupOptions { + units: Protobuf.Config.Config_DisplayConfig_DisplayUnits; + altitudeMeters: number; +} + +function setup({ units, altitudeMeters }: SetupOptions) { + mockDisplayUnits = units; + // A full, valid PositionConfig (defaults for every schema field) with a + // fixed position enabled so the altitude field is editable and the submit + // takes the setFixedPosition path. + mockPositionConfig = create(Protobuf.Config.Config_PositionConfigSchema, { + fixedPosition: true, + }); + mockMyNode = create(Protobuf.Mesh.NodeInfoSchema, { + num: 1, + position: create(Protobuf.Mesh.PositionSchema, { + latitudeI: LATITUDE_I, + longitudeI: LONGITUDE_I, + altitude: altitudeMeters, + }), + }); +} + +function renderPosition() { + return render( {}} />); +} + +function altitudeInput(container: HTMLElement): HTMLInputElement { + const input = container.querySelector("#altitude"); + if (!input) { + throw new Error("altitude field not found"); + } + return input; +} + +function lastQueuedPosition(): Protobuf.Mesh.Position { + const message = mockQueueAdminMessage.mock.calls.at( + -1, + )?.[0] as Protobuf.Admin.AdminMessage; + expect(message.payloadVariant.case).toBe("setFixedPosition"); + return message.payloadVariant.value as Protobuf.Mesh.Position; +} + +describe("Position (altitude unit conversion, issue #1051)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + // Remove any geolocation override so the button does not leak into other + // tests (the component renders it only when navigator.geolocation exists). + if ("geolocation" in navigator) { + Reflect.deleteProperty(navigator, "geolocation"); + } + }); + + it("Imperial: loads a 312 m altitude as ~1024 ft in the field", () => { + setup({ units: IMPERIAL, altitudeMeters: 312 }); + const { container } = renderPosition(); + + // metersToFeet(312) = 1023.62 -> rounded 1024 + expect(altitudeInput(container).value).toBe("1024"); + // The field is labelled in Feet, not Meters. + expect(screen.getByText("Feet")).toBeInTheDocument(); + expect(screen.queryByText("Meters")).not.toBeInTheDocument(); + }); + + it("Imperial: submitting 1025 ft queues setFixedPosition with altitude 312 m", async () => { + setup({ units: IMPERIAL, altitudeMeters: 312 }); + const { container } = renderPosition(); + + // DynamicForm submits on change (submitType defaults to "onChange"). + fireEvent.change(altitudeInput(container), { target: { value: "1025" } }); + + await waitFor(() => expect(mockQueueAdminMessage).toHaveBeenCalled()); + + const position = lastQueuedPosition(); + // feetToMeters(1025) = 312.42 -> rounded 312 (int32 meters), NOT 1025. + expect(position.altitude).toBe(312); + // Lat/lng round-trip untouched (still integer degrees * 1e7). + expect(position.latitudeI).toBe(LATITUDE_I); + expect(position.longitudeI).toBe(LONGITUDE_I); + }); + + it("Metric: submitting 312 queues setFixedPosition with altitude 312 m unchanged", async () => { + // Start from a different metric altitude so the change event fires. + setup({ units: METRIC, altitudeMeters: 100 }); + const { container } = renderPosition(); + + // Metric field shows raw meters, no conversion. + expect(altitudeInput(container).value).toBe("100"); + expect(screen.getByText("Meters")).toBeInTheDocument(); + + fireEvent.change(altitudeInput(container), { target: { value: "312" } }); + + await waitFor(() => expect(mockQueueAdminMessage).toHaveBeenCalled()); + + expect(lastQueuedPosition().altitude).toBe(312); + }); + + it("Imperial: browser geolocation fills the field in feet (100 m -> 328 ft)", async () => { + setup({ units: IMPERIAL, altitudeMeters: 312 }); + + // Geolocation reports meters; component must convert to feet for display. + mockGetCurrentPosition.mockImplementation((success: PositionCallback) => + success({ + coords: { + latitude: 37.7749, + longitude: -122.4194, + altitude: 100, + accuracy: 5, + altitudeAccuracy: null, + heading: null, + speed: null, + }, + } as GeolocationPosition), + ); + Object.defineProperty(navigator, "geolocation", { + configurable: true, + value: { getCurrentPosition: mockGetCurrentPosition }, + }); + + const { container } = renderPosition(); + + // Sanity: starts at the loaded 312 m -> 1024 ft. + expect(altitudeInput(container).value).toBe("1024"); + + fireEvent.click( + screen.getByRole("button", { name: /use browser location/i }), + ); + + // metersToFeet(100) = 328.08 -> rounded 328 + await waitFor(() => expect(altitudeInput(container).value).toBe("328")); + expect(mockGetCurrentPosition).toHaveBeenCalledTimes(1); + }); +}); From e617b0fe177f73a98305fe34e9843d9d80eb1f3a Mon Sep 17 00:00:00 2001 From: Dkx <45234736+dkxmercury@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:03:00 +0500 Subject: [PATCH 3/5] i18n(position): translate the altitude unit label instead of hardcoding (#1051) Add a `unit.foot` entry to the English common.json (mirroring `unit.meter`) and use `t("unit.foot.plural")` / `t("unit.meter.plural")` for the altitude field's unit label and suffix. Other locales are handled by Crowdin, which uploads only the en sources. --- apps/web/public/i18n/locales/en/common.json | 1 + .../src/components/PageComponents/Settings/Position.tsx | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/web/public/i18n/locales/en/common.json b/apps/web/public/i18n/locales/en/common.json index 49c01e22d..47f8f4689 100644 --- a/apps/web/public/i18n/locales/en/common.json +++ b/apps/web/public/i18n/locales/en/common.json @@ -57,6 +57,7 @@ "kilohertz": "kHz", "raw": "raw", "meter": { "one": "Meter", "plural": "Meters", "suffix": "m" }, + "foot": { "one": "Foot", "plural": "Feet", "suffix": "ft" }, "kilometer": { "one": "Kilometer", "plural": "Kilometers", "suffix": "km" }, "minute": { "one": "Minute", "plural": "Minutes" }, "hour": { "one": "Hour", "plural": "Hours" }, diff --git a/apps/web/src/components/PageComponents/Settings/Position.tsx b/apps/web/src/components/PageComponents/Settings/Position.tsx index ac6361e6a..d8c37288b 100644 --- a/apps/web/src/components/PageComponents/Settings/Position.tsx +++ b/apps/web/src/components/PageComponents/Settings/Position.tsx @@ -279,11 +279,15 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { name: "altitude", label: t("position.fixedPosition.altitude.label"), description: t("position.fixedPosition.altitude.description", { - unit: isImperial ? "Feet" : "Meters", + unit: isImperial + ? t("unit.foot.plural") + : t("unit.meter.plural"), }), properties: { step: 0.0000001, - suffix: isImperial ? "Feet" : "Meters", + suffix: isImperial + ? t("unit.foot.plural") + : t("unit.meter.plural"), }, disabledBy: [{ fieldName: "fixedPosition" }], }, From 4a99400af4426f344f64e4a235c21878010e4c61 Mon Sep 17 00:00:00 2001 From: Dkx <45234736+dkxmercury@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:03:06 +0500 Subject: [PATCH 4/5] test(position): assert a cleared altitude field does not queue a submit (#1051) A cleared number field becomes NaN, which fails the altitude zod validation, so the form never submits. Document that no setFixedPosition message is queued. --- .../PageComponents/Settings/Position.test.tsx | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/apps/web/src/components/PageComponents/Settings/Position.test.tsx b/apps/web/src/components/PageComponents/Settings/Position.test.tsx index 10c31add4..f32fef73f 100644 --- a/apps/web/src/components/PageComponents/Settings/Position.test.tsx +++ b/apps/web/src/components/PageComponents/Settings/Position.test.tsx @@ -216,4 +216,32 @@ describe("Position (altitude unit conversion, issue #1051)", () => { await waitFor(() => expect(altitudeInput(container).value).toBe("328")); expect(mockGetCurrentPosition).toHaveBeenCalledTimes(1); }); + + it("clearing the altitude field does not queue a setFixedPosition", async () => { + // Regression coverage requested by CodeRabbit. Clearing a number field + // yields NaN (FormInput does Number.parseFloat("").toString() === "NaN"), + // which fails altitude's `z.coerce.number().optional()` validation. Because + // DynamicForm submits via handleSubmit(onSubmit) on change, an invalid form + // never calls onSubmit, so nothing is queued. The `Number.isFinite(...) ? ... + // : 0` guard in onSubmit is thus defensive and unreachable through the form. + setup({ units: METRIC, altitudeMeters: 100 }); + const { container } = renderPosition(); + + // Baseline: a valid edit queues exactly one setFixedPosition, proving the + // form is wired up and submits on change. + fireEvent.change(altitudeInput(container), { target: { value: "150" } }); + await waitFor(() => expect(mockQueueAdminMessage).toHaveBeenCalledTimes(1)); + expect(lastQueuedPosition().altitude).toBe(150); + + // Clear the field -> NaN -> invalid -> must NOT queue anything. + fireEvent.change(altitudeInput(container), { target: { value: "" } }); + + // Recover with another valid edit and synchronize on its value landing. + // Then assert exactly two queues total (150 then 200): if the clear had + // queued, the count would be three. Synchronizing on the settled value + // (not a bare negative assertion) keeps this non-racy. + fireEvent.change(altitudeInput(container), { target: { value: "200" } }); + await waitFor(() => expect(lastQueuedPosition().altitude).toBe(200)); + expect(mockQueueAdminMessage).toHaveBeenCalledTimes(2); + }); }); From ebb27be8b5932650512d1a2ee1e6d4a79838a7d5 Mon Sep 17 00:00:00 2001 From: Dkx <45234736+dkxmercury@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:31:12 +0500 Subject: [PATCH 5/5] refactor(position): use a type guard and simplify the altitude naming (#1051) Address review feedback: replace the `as number` cast with a typeof guard that narrows the type, and rename altitudeMeters to altitude. --- .../components/PageComponents/Settings/Position.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/PageComponents/Settings/Position.tsx b/apps/web/src/components/PageComponents/Settings/Position.tsx index d8c37288b..8f466468f 100644 --- a/apps/web/src/components/PageComponents/Settings/Position.tsx +++ b/apps/web/src/components/PageComponents/Settings/Position.tsx @@ -164,10 +164,11 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { // The altitude field is in feet when Imperial; the protobuf expects // integer meters, so convert back and round. Guard against a // cleared/NaN field. - const altitudeInput = Number.isFinite(data.altitude) - ? (data.altitude as number) - : 0; - const altitudeMeters = isImperial + const altitudeInput = + typeof data.altitude === "number" && Number.isFinite(data.altitude) + ? data.altitude + : 0; + const altitude = isImperial ? Math.round(feetToMeters(altitudeInput)) : Math.round(altitudeInput); @@ -177,7 +178,7 @@ export const Position = ({ onFormInit }: PositionConfigProps) => { value: create(Protobuf.Mesh.PositionSchema, { latitudeI: Math.round(data.latitude * 1e7), longitudeI: Math.round(data.longitude * 1e7), - altitude: altitudeMeters, + altitude, time: Math.floor(Date.now() / 1000), }), },