From 9067e007b659a9adb2af62baa92b0aa02d1469c2 Mon Sep 17 00:00:00 2001 From: MsfPablo <129399053+MsfPablo@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:18:18 +0200 Subject: [PATCH] Fix GeoPos losing the sign of coordinates between 0 and -1 (#3283) GeoPos stored a coordinate's sign only in its degrees NumberField, which is an int32_t and therefore has no negative zero. Any coordinate whose integer degrees part is 0 but which is negative (i.e. strictly between 0.0 and -1.0) could not be represented at all: - set_lat()/set_lon() passed the raw float to the degrees field, so -0.2933 truncated to 0 and the sign was gone before it reached the widget. - lat()/lon() then decided the sign with `field_lon_degrees.value() < 0`, which is false for 0, so the value came back positive. The reporter's airport is at longitude -0.2933, which was impossible to enter in the ADSB Tx app. Give each coordinate an explicit hemisphere field (N/S and E/W) and make the degrees field an unsigned magnitude: - The hemisphere OptionsField is the single source of the sign, so "negative with zero degrees" is now representable. - set_lat()/set_lon() derive the hemisphere from the sign of the input and feed the fields the magnitude. - lat()/lon() read the hemisphere instead of inferring the sign from the degrees value. - The minutes on_wrap handlers no longer need to flip the carry direction based on the degrees sign, since degrees is now a magnitude; the carry is the same in both hemispheres. - The degrees fields no longer loop, so carrying below 0 clamps at 0 instead of wrapping round to 90/180. The hemisphere indicator occupies the column that the 4-wide signed degrees field used for its minus sign, so the row layout, the degree symbol and the decimal readout all stay where they were. Fixes #3234 --- firmware/application/ui/ui_geomap.cpp | 46 ++++++++++++++++----------- firmware/application/ui/ui_geomap.hpp | 31 +++++++++++++----- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/firmware/application/ui/ui_geomap.cpp b/firmware/application/ui/ui_geomap.cpp index dac0fadb1..0f13368d2 100644 --- a/firmware/application/ui/ui_geomap.cpp +++ b/firmware/application/ui/ui_geomap.cpp @@ -46,10 +46,12 @@ GeoPos::GeoPos( &field_speed, &text_alt_unit, &text_speed_unit, + &field_lat_hemisphere, &field_lat_degrees, &field_lat_minutes, &field_lat_seconds, &text_lat_decimal, + &field_lon_hemisphere, &field_lon_degrees, &field_lon_minutes, &field_lon_seconds, @@ -73,8 +75,14 @@ GeoPos::GeoPos( on_change(altitude(), lat_value, lon_value, speed()); }; + const auto changed_hemisphere_fn = [changed_fn](size_t, OptionsField::value_t) { + changed_fn(0); + }; + field_altitude.on_change = changed_fn; field_speed.on_change = changed_fn; + field_lat_hemisphere.on_change = changed_hemisphere_fn; + field_lon_hemisphere.on_change = changed_hemisphere_fn; field_lat_degrees.on_change = changed_fn; field_lat_minutes.on_change = changed_fn; field_lat_seconds.on_change = changed_fn; @@ -90,9 +98,11 @@ GeoPos::GeoPos( } }; + // Degrees now holds a magnitude, so a minutes wrap always carries in the + // same direction regardless of hemisphere. const auto wrapped_lat_minutes = [this](int32_t v) { const auto old_degrees = field_lat_degrees.value(); - field_lat_degrees.on_encoder((old_degrees >= 0) ? v : -v); + field_lat_degrees.on_encoder(v); if (field_lat_degrees.value() == old_degrees) { field_lat_minutes.set_value((v > 0) ? 59 : 0); } @@ -108,7 +118,7 @@ GeoPos::GeoPos( const auto wrapped_lon_minutes = [this](int32_t v) { const auto old_degrees = field_lon_degrees.value(); - field_lon_degrees.on_encoder((old_degrees >= 0) ? v : -v); + field_lon_degrees.on_encoder(v); if (field_lon_degrees.value() == old_degrees) { field_lon_minutes.set_value((v > 0) ? 59 : 0); } @@ -162,31 +172,31 @@ void GeoPos::set_speed(int32_t speed) { } void GeoPos::set_lat(float lat) { - field_lat_degrees.set_value(lat); - field_lat_minutes.set_value((uint32_t)abs(lat / (1.0 / 60)) % 60); - field_lat_seconds.set_value((uint32_t)abs(lat / (1.0 / 3600)) % 60); + bool south = lat < 0; + float magnitude = south ? -lat : lat; + field_lat_hemisphere.set_by_value(south ? 1 : 0); + field_lat_degrees.set_value((int32_t)magnitude); + field_lat_minutes.set_value((uint32_t)(magnitude * 60) % 60); + field_lat_seconds.set_value((uint32_t)(magnitude * 3600) % 60); } void GeoPos::set_lon(float lon) { - field_lon_degrees.set_value(lon); - field_lon_minutes.set_value((uint32_t)abs(lon / (1.0 / 60)) % 60); - field_lon_seconds.set_value((uint32_t)abs(lon / (1.0 / 3600)) % 60); + bool west = lon < 0; + float magnitude = west ? -lon : lon; + field_lon_hemisphere.set_by_value(west ? 1 : 0); + field_lon_degrees.set_value((int32_t)magnitude); + field_lon_minutes.set_value((uint32_t)(magnitude * 60) % 60); + field_lon_seconds.set_value((uint32_t)(magnitude * 3600) % 60); } float GeoPos::lat() { - if (field_lat_degrees.value() < 0) { - return -1 * (-1 * field_lat_degrees.value() + (field_lat_minutes.value() / 60.0) + (field_lat_seconds.value() / 3600.0)); - } else { - return field_lat_degrees.value() + (field_lat_minutes.value() / 60.0) + (field_lat_seconds.value() / 3600.0); - } + float magnitude = field_lat_degrees.value() + (field_lat_minutes.value() / 60.0) + (field_lat_seconds.value() / 3600.0); + return (field_lat_hemisphere.selected_index_value() != 0) ? -magnitude : magnitude; }; float GeoPos::lon() { - if (field_lon_degrees.value() < 0) { - return -1 * (-1 * field_lon_degrees.value() + (field_lon_minutes.value() / 60.0) + (field_lon_seconds.value() / 3600.0)); - } else { - return field_lon_degrees.value() + (field_lon_minutes.value() / 60.0) + (field_lon_seconds.value() / 3600.0); - } + float magnitude = field_lon_degrees.value() + (field_lon_minutes.value() / 60.0) + (field_lon_seconds.value() / 3600.0); + return (field_lon_hemisphere.selected_index_value() != 0) ? -magnitude : magnitude; }; int32_t GeoPos::altitude() { diff --git a/firmware/application/ui/ui_geomap.hpp b/firmware/application/ui/ui_geomap.hpp index 5dfbbaa8e..30759d5cc 100644 --- a/firmware/application/ui/ui_geomap.hpp +++ b/firmware/application/ui/ui_geomap.hpp @@ -239,12 +239,21 @@ class GeoPos : public View { {25 * 8, UI_POS_Y(0), 5 * 8, 16}, ""}; - NumberField field_lat_degrees{ + // Sign is held by the hemisphere field, not by the degrees field: an + // int32_t degrees field has no negative zero, so a coordinate in + // (-1, 0) could not otherwise be represented (see issue #3234). + OptionsField field_lat_hemisphere{ {5 * 8, 1 * 16}, - 4, - {-90, 90}, 1, - ' '}; + {{"N", 0}, + {"S", 1}}}; + NumberField field_lat_degrees{ + {6 * 8, 1 * 16}, + 3, + {0, 90}, + 1, + ' ', + false}; NumberField field_lat_minutes{ {10 * 8, 1 * 16}, 2, @@ -263,12 +272,18 @@ class GeoPos : public View { {17 * 8, 1 * 16, 13 * 8, 1 * 16}, ""}; - NumberField field_lon_degrees{ + OptionsField field_lon_hemisphere{ {5 * 8, 2 * 16}, - 4, - {-180, 180}, 1, - ' '}; + {{"E", 0}, + {"W", 1}}}; + NumberField field_lon_degrees{ + {6 * 8, 2 * 16}, + 3, + {0, 180}, + 1, + ' ', + false}; NumberField field_lon_minutes{ {10 * 8, 2 * 16}, 2,