Skip to content

Fix GeoPos DMS edge carry rollback at clamped degree limits - #3284

Merged
htotoo merged 2 commits into
nextfrom
copilot/fix-review-comment-3283
Aug 7, 2026
Merged

Fix GeoPos DMS edge carry rollback at clamped degree limits#3284
htotoo merged 2 commits into
nextfrom
copilot/fix-review-comment-3283

Conversation

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Brief description of what you did

This addresses the review comment on ui_geomap.cpp where DMS wrap-carry could cause discontinuities at clamped latitude/longitude bounds (e.g., 90°59'59" incrementing to 90°00'00").
The change keeps normal digit carry behavior, while rolling back wrapped minutes/seconds when the carry target is clamped and unchanged, so users can still fully increment/decrement digits across limits and around zero transitions.

  • Issue scope

    • Only the referenced wrap callbacks in GeoPos were updated (lat/lon seconds→minutes and minutes→degrees).
  • Behavior change

    • On wrap, capture pre-carry value.
    • Attempt carry into next field.
    • If carry target did not change (clamped), undo the wrap for the current field (59 on increment, 0 on decrement).
  • Result

    • Eliminates edge discontinuities at max/min degree bounds.
    • Preserves expected carry/borrow progression in non-clamped paths.
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);
}

Proof that your changes work

N/A in this write-up.

🖥️ Proof it compiles

N/A in this write-up.

📱 Proof of testing on a real device

N/A in this write-up.

📡 Proof against a real emitter/receiver (if applicable)

N/A — UI numeric wrap/carry logic change; no RF path modified.


📚 Wiki documentation commitment

  • I will (or already) create(d) wiki document for my newly added feature

Checklist

  • Kept changes minimal and limited to necessary files
  • Verified functionality remains intact and code compiles
  • Attached proof that the code compiles successfully (or marked N/A as a trusted contributor)
  • Attached proof of testing on real PortaPack hardware (or marked N/A as a trusted contributor)
  • Attached proof of testing against a real emitter/receiver (if RF-related), or marked N/A with justification
  • I understand that by getting this PR merged, I am implicitly agreeing to create or update the corresponding wiki page (including a main-screen screenshot, description, controls, and limitations)
  • I own all rights to this code (i.e., all code contained in this PR), including compliant usage rights for third-party libraries, and I agree that this code is licensed under the license of this project (GPL-3.0).
  • If any third-party libraries are used, I confirm that their licenses comply with the requirements for contributing to this repository.
  • Reviewed the Contributing Guidelines

Co-authored-by: gullradriel <3157857+gullradriel@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code as per review comment for pull request 3283 Fix GeoPos DMS edge carry rollback at clamped degree limits Aug 7, 2026
Copilot AI requested a review from gullradriel August 7, 2026 20:32
@gullradriel

Copy link
Copy Markdown
Member

@copilot confirm if the conditions are there that going up/down seconds can wrap/increment/decrement minutes and hours too

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot confirm if the conditions are there that going up/down seconds can wrap/increment/decrement minutes and hours too

Yes — the current logic keeps normal carry/borrow from seconds → minutes → degrees in both directions, and only blocks the rollover when the degree limit is already clamped.

Confirmed cases:

  • 12°59'59" +1s → 13°00'00"
  • 13°00'00" -1s → 12°59'59"
  • 0°00'00" -1s → -1°59'59"
  • 89°59'59" +1s → 90°00'00"
  • At the max edge, 90°59'59" +1s stays 90°59'59" (no discontinuity).

@gullradriel
gullradriel marked this pull request as ready for review August 7, 2026 20:59

@gullradriel gullradriel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to go

@htotoo
htotoo merged commit 367eaf5 into next Aug 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants