From a88ba6d9a8423cc72668344b7f8aef51d401ab19 Mon Sep 17 00:00:00 2001 From: Mo <83806223+Xmoo26@users.noreply.github.com> Date: Sat, 19 Sep 2026 18:00:47 +0200 Subject: [PATCH] =?UTF-8?q?GeoPos:=20pan=20continuously=20across=200=C2=B0?= =?UTF-8?q?=20lat/lon=20(#3317)=20(#3323)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * GeoPos: pan continuously across 0 deg lat/lon (#3317) The DMS coordinate fields store an unsigned magnitude with the sign in a separate hemisphere field (#3283). That made (-1, 0) representable but left interactive panning unable to cross 0: at magnitude zero the minute/ degree carry clamps instead of flipping the hemisphere, so a 1-degree band along the equator and the Greenwich meridian is skipped by the wheel. Route each DMS field's encoder through a signed arcsecond model: a turn moves the coordinate along the number line, and the hemisphere flip plus magnitude reflection fall out of one set of fields being rewritten. E.g. 0 deg 0' 42" E minus one minute now gives 0 deg 0' 18" W as expected. The old per-field on_wrap carry handlers are removed; the fields no longer apply the turn locally. * Correct latitude adjustment reporting logic Fix adjustment logic to only report position if latitude changes. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * GeoPos: apply the clamp-report guard to longitude too The latitude fix (Copilot review) left adjust_lon() reporting a pan even when set_lon_arcseconds() clamped at +-180 deg and changed nothing. Guard it the same way: report only when the signed arcseconds actually changed. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- firmware/application/ui/ui_geomap.cpp | 128 ++++++++++++++++---------- firmware/application/ui/ui_geomap.hpp | 46 +++++++-- 2 files changed, 119 insertions(+), 55 deletions(-) diff --git a/firmware/application/ui/ui_geomap.cpp b/firmware/application/ui/ui_geomap.cpp index 0f13368d2..9043cd5a2 100644 --- a/firmware/application/ui/ui_geomap.cpp +++ b/firmware/application/ui/ui_geomap.cpp @@ -64,19 +64,11 @@ GeoPos::GeoPos( set_lon(0); const auto changed_fn = [this](int32_t) { - // Convert degrees/minutes/seconds fields to decimal (floating point) lat/lon degree - float lat_value = lat(); - float lon_value = lon(); - - text_lat_decimal.set(to_string_decimal(lat_value, 5)); - text_lon_decimal.set(to_string_decimal(lon_value, 5)); - - if (on_change && report_change) - on_change(altitude(), lat_value, lon_value, speed()); + report_position(); }; - const auto changed_hemisphere_fn = [changed_fn](size_t, OptionsField::value_t) { - changed_fn(0); + const auto changed_hemisphere_fn = [this](size_t, OptionsField::value_t) { + report_position(); }; field_altitude.on_change = changed_fn; @@ -90,44 +82,15 @@ GeoPos::GeoPos( field_lon_minutes.on_change = changed_fn; field_lon_seconds.on_change = changed_fn; - const auto wrapped_lat_seconds = [this](int32_t v) { - const auto old_minutes = field_lat_minutes.value(); - field_lat_minutes.on_encoder(v); - if (field_lat_minutes.value() == old_minutes) { - field_lat_seconds.set_value((v > 0) ? 59 : 0); - } - }; - - // 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(v); - if (field_lat_degrees.value() == old_degrees) { - field_lat_minutes.set_value((v > 0) ? 59 : 0); - } - }; - - const auto wrapped_lon_seconds = [this](int32_t v) { - const auto old_minutes = field_lon_minutes.value(); - field_lon_minutes.on_encoder(v); - if (field_lon_minutes.value() == old_minutes) { - field_lon_seconds.set_value((v > 0) ? 59 : 0); - } - }; - - const auto wrapped_lon_minutes = [this](int32_t v) { - const auto old_degrees = field_lon_degrees.value(); - field_lon_degrees.on_encoder(v); - if (field_lon_degrees.value() == old_degrees) { - field_lon_minutes.set_value((v > 0) ? 59 : 0); - } - }; - - field_lat_seconds.on_wrap = wrapped_lat_seconds; - field_lat_minutes.on_wrap = wrapped_lat_minutes; - field_lon_seconds.on_wrap = wrapped_lon_seconds; - field_lon_minutes.on_wrap = wrapped_lon_minutes; + // Route each DMS field's encoder through the signed arcsecond model so a turn moves + // the coordinate along the number line and crosses 0 correctly (issue #3317). One + // second is the base unit; minutes and degrees step by 60 and 3600 of it. + field_lat_degrees.on_delta = [this](int32_t d) { adjust_lat(d * 3600); }; + field_lat_minutes.on_delta = [this](int32_t d) { adjust_lat(d * 60); }; + field_lat_seconds.on_delta = [this](int32_t d) { adjust_lat(d); }; + field_lon_degrees.on_delta = [this](int32_t d) { adjust_lon(d * 3600); }; + field_lon_minutes.on_delta = [this](int32_t d) { adjust_lon(d * 60); }; + field_lon_seconds.on_delta = [this](int32_t d) { adjust_lon(d); }; text_alt_unit.set(altitude_unit_ ? "m" : "ft"); if (speed_unit_ == KMPH) text_speed_unit.set("kmph"); @@ -199,6 +162,73 @@ float GeoPos::lon() { return (field_lon_hemisphere.selected_index_value() != 0) ? -magnitude : magnitude; }; +int32_t GeoPos::lat_arcseconds() { + int32_t magnitude = field_lat_degrees.value() * 3600 + field_lat_minutes.value() * 60 + field_lat_seconds.value(); + return (field_lat_hemisphere.selected_index_value() != 0) ? -magnitude : magnitude; +} + +int32_t GeoPos::lon_arcseconds() { + int32_t magnitude = field_lon_degrees.value() * 3600 + field_lon_minutes.value() * 60 + field_lon_seconds.value(); + return (field_lon_hemisphere.selected_index_value() != 0) ? -magnitude : magnitude; +} + +void GeoPos::set_lat_arcseconds(int32_t arcseconds) { + if (arcseconds > lat_arcsecond_limit) arcseconds = lat_arcsecond_limit; + if (arcseconds < -lat_arcsecond_limit) arcseconds = -lat_arcsecond_limit; + bool south = arcseconds < 0; + int32_t magnitude = south ? -arcseconds : arcseconds; + field_lat_hemisphere.set_by_value(south ? 1 : 0); + field_lat_degrees.set_value(magnitude / 3600); + field_lat_minutes.set_value((magnitude / 60) % 60); + field_lat_seconds.set_value(magnitude % 60); +} + +void GeoPos::set_lon_arcseconds(int32_t arcseconds) { + if (arcseconds > lon_arcsecond_limit) arcseconds = lon_arcsecond_limit; + if (arcseconds < -lon_arcsecond_limit) arcseconds = -lon_arcsecond_limit; + bool west = arcseconds < 0; + int32_t magnitude = west ? -arcseconds : arcseconds; + field_lon_hemisphere.set_by_value(west ? 1 : 0); + field_lon_degrees.set_value(magnitude / 3600); + field_lon_minutes.set_value((magnitude / 60) % 60); + field_lon_seconds.set_value(magnitude % 60); +} + +// Rewrite the fields from a single signed value so a hemisphere flip and the magnitude +// reflection happen together. report_change is held off while the four fields settle, +// then the final position is reported once. +void GeoPos::adjust_lat(int32_t arcsecond_delta) { + const int32_t old_arcseconds = lat_arcseconds(); + bool previous = report_change; + report_change = false; + set_lat_arcseconds(old_arcseconds + arcsecond_delta); + report_change = previous; + if (lat_arcseconds() != old_arcseconds) + report_position(); +} + +void GeoPos::adjust_lon(int32_t arcsecond_delta) { + const int32_t old_arcseconds = lon_arcseconds(); + bool previous = report_change; + report_change = false; + set_lon_arcseconds(old_arcseconds + arcsecond_delta); + report_change = previous; + if (lon_arcseconds() != old_arcseconds) + report_position(); +} + +void GeoPos::report_position() { + // Convert degrees/minutes/seconds fields to decimal (floating point) lat/lon degree + float lat_value = lat(); + float lon_value = lon(); + + text_lat_decimal.set(to_string_decimal(lat_value, 5)); + text_lon_decimal.set(to_string_decimal(lon_value, 5)); + + if (on_change && report_change) + on_change(altitude(), lat_value, lon_value, speed()); +} + int32_t GeoPos::altitude() { return field_altitude.value(); }; diff --git a/firmware/application/ui/ui_geomap.hpp b/firmware/application/ui/ui_geomap.hpp index 30759d5cc..a136506f0 100644 --- a/firmware/application/ui/ui_geomap.hpp +++ b/firmware/application/ui/ui_geomap.hpp @@ -172,6 +172,27 @@ class BMPFileCache { uint16_t stamp_{0}; }; +// A degrees/minutes/seconds sub-field whose encoder turns are handed to the owning +// GeoPos instead of being applied locally. GeoPos keeps the coordinate as a signed +// arcsecond value, so panning stays continuous across 0 - including the (-1, 0) band +// along the equator and the Greenwich meridian that a magnitude field cannot cross on +// its own (see issue #3317). The field still displays an unsigned magnitude; the sign +// lives in the hemisphere field next to it. +class GeoPosField : public NumberField { + public: + using NumberField::NumberField; + + std::function on_delta{}; + + bool on_encoder(const EncoderEvent delta) override { + if (on_delta) { + on_delta(delta); + return true; + } + return NumberField::on_encoder(delta); + } +}; + class GeoPos : public View { public: enum alt_unit { @@ -206,6 +227,19 @@ class GeoPos : public View { void set_report_change(bool v); private: + // Signed arcsecond view of the DMS fields. The encoder handlers work on these so a + // turn moves the coordinate along the number line, flipping the hemisphere and + // reflecting the magnitude when it crosses 0 (issue #3317). + static constexpr int32_t lat_arcsecond_limit = 90 * 3600; + static constexpr int32_t lon_arcsecond_limit = 180 * 3600; + int32_t lat_arcseconds(); + int32_t lon_arcseconds(); + void set_lat_arcseconds(int32_t arcseconds); + void set_lon_arcseconds(int32_t arcseconds); + void adjust_lat(int32_t arcsecond_delta); + void adjust_lon(int32_t arcsecond_delta); + void report_position(); + bool read_only{false}; bool report_change{true}; alt_unit altitude_unit_{}; @@ -247,21 +281,21 @@ class GeoPos : public View { 1, {{"N", 0}, {"S", 1}}}; - NumberField field_lat_degrees{ + GeoPosField field_lat_degrees{ {6 * 8, 1 * 16}, 3, {0, 90}, 1, ' ', false}; - NumberField field_lat_minutes{ + GeoPosField field_lat_minutes{ {10 * 8, 1 * 16}, 2, {0, 59}, 1, ' ', true}; - NumberField field_lat_seconds{ + GeoPosField field_lat_seconds{ {13 * 8, 1 * 16}, 2, {0, 59}, @@ -277,21 +311,21 @@ class GeoPos : public View { 1, {{"E", 0}, {"W", 1}}}; - NumberField field_lon_degrees{ + GeoPosField field_lon_degrees{ {6 * 8, 2 * 16}, 3, {0, 180}, 1, ' ', false}; - NumberField field_lon_minutes{ + GeoPosField field_lon_minutes{ {10 * 8, 2 * 16}, 2, {0, 59}, 1, ' ', true}; - NumberField field_lon_seconds{ + GeoPosField field_lon_seconds{ {13 * 8, 2 * 16}, 2, {0, 59},