Skip to content

fix(position-config): stop writing deprecated gpsEnabled on save (#2024)#2110

Open
bruschill wants to merge 2 commits into
meshtastic:mainfrom
bruschill:fix/position-config-drop-deprecated-gps-enabled-2024
Open

fix(position-config): stop writing deprecated gpsEnabled on save (#2024)#2110
bruschill wants to merge 2 commits into
meshtastic:mainfrom
bruschill:fix/position-config-drop-deprecated-gps-enabled-2024

Conversation

@bruschill

@bruschill bruschill commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What changed?

Stopped writing the deprecated PositionConfig.gpsEnabled field (gps_enabled = 4) when saving Position config. The save path now persists only pc.gpsMode. The read-side legacy bridge in setPositionValues() is retained, so gpsEnabled/deviceGpsEnabled arriving from older firmware is still tolerated and mapped onto the GPS mode UI (deviceGpsEnabled && gpsMode != 1gpsMode = 1).

Fixes #2024

Why did it change?

gps_enabled is deprecated upstream ([deprecated=true]); its successor gps_mode = 13 has been the source of truth since v2.2.21, and Android/upstream use gps_mode exclusively. The Apple app was re-emitting the deprecated field to all firmware on every save. This removes that while keeping backward-compatible reads.

How is this tested?

  • Confirmed via full-tree scan that this was the only outgoing write site for gpsEnabled (remaining hits are protobuf codegen and incoming device→app persistence).
  • Traced the round-trip: gpsMode is persisted as source of truth at both upsertPositionConfigPacket branches, so the GPS mode UI still resolves correctly after save + config read.
  • SourceKit-LSP: no diagnostics on the changed file.
  • Reviewed with the project's Swift reviewer — approved, no blocking issues.

Screenshots/Videos (when applicable)

N/A — no user-visible UI change (the GPS Mode picker, labels, and navigation are unchanged).

Checklist

  • My code adheres to the project's coding and style guidelines.
  • I have conducted a self-review of my code.
  • I have commented my code, particularly in complex areas.
  • I have verified whether these changes require updates to the in-app documentation under docs/user/ or docs/developer/. No doc update is needed (no user-visible UI/label/navigation change) — skip-docs-check label applies.
  • I have tested the change to ensure that it works as intended.

Known accepted tradeoff

Genuinely old firmware that honors only gps_enabled and ignores gps_mode will read the default false. This is the explicit intended tradeoff in #2024 (matching Android/upstream, which emit gps_mode exclusively).

…htastic#2024)

PositionConfig saves were still setting the deprecated pc.gpsEnabled field
(gps_enabled = 4) on every write, sending it to all firmware regardless.
Its successor gps_mode has been the source of truth since v2.2.21 and the
field is annotated [deprecated=true] upstream.

- Stop writing pc.gpsEnabled; persist only pc.gpsMode.
- Keep backward-compat on read: setPositionValues() still tolerates
  gpsEnabled/deviceGpsEnabled arriving from older firmware and maps it onto
  the GPS mode UI (deviceGpsEnabled && gpsMode != 1 -> gpsMode = 1).
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@bruschill, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 24 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f50de72f-5f8b-47d3-aa7a-186edc4ecd93

📥 Commits

Reviewing files that changed from the base of the PR and between 127388e and f4bce47.

📒 Files selected for processing (4)
  • Meshtastic/Resources/docs/index.json
  • Meshtastic/Resources/docs/markdown/user/settings.md
  • Meshtastic/Resources/docs/user/settings.html
  • docs/user/settings.md
📝 Walkthrough

Walkthrough

The position configuration save flow no longer writes deprecated gpsEnabled; it continues persisting the UI-selected gpsMode as the source of truth.

Changes

Position GPS mode migration

Layer / File(s) Summary
Persist GPS mode during configuration save
Meshtastic/Views/Settings/Config/PositionConfig.swift
The save closure continues assigning pc.gpsMode and no longer assigns deprecated pc.gpsEnabled.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: garthvh

Poem

A bunny hops through settings bright,
GPS mode is set just right.
The old field rests, no longer sent,
Clean configs leave the burrow content.
Hop, hop—migration’s done! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code change matches #2024 by removing deprecated gpsEnabled writes while preserving legacy read-side handling.
Out of Scope Changes check ✅ Passed No unrelated changes are indicated; the diff appears limited to the PositionConfig save path for the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: stopping deprecated gpsEnabled writes when saving PositionConfig.
Description check ✅ Passed The description matches the template and includes what changed, why, how tested, screenshots, and checklist items.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bruschill bruschill added the skip-docs-check Use this label to skip the automatic docs audit label Jul 18, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Meshtastic/Views/Settings/Config/PositionConfig.swift`:
- Around line 324-327: Update docs/user/settings.md to document the Position
settings GPS migration: gpsMode is the sole persisted/outgoing source of truth,
while legacy gpsEnabled is only retained for backward-compatible reads. Ensure
the documentation reflects this behavior alongside the existing GPS settings
description.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a7be9e5b-6df1-421c-82d7-ada6b86cfbfa

📥 Commits

Reviewing files that changed from the base of the PR and between e531f15 and 127388e.

📒 Files selected for processing (1)
  • Meshtastic/Views/Settings/Config/PositionConfig.swift

Comment thread Meshtastic/Views/Settings/Config/PositionConfig.swift
Address CodeRabbit review on meshtastic#2110: document in docs/user/settings.md that
gpsMode is the sole source of truth the app writes, while the deprecated
gpsEnabled field is read-only for backward compatibility with older firmware.
Regenerate the bundled HTML docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-docs-check Use this label to skip the automatic docs audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate off deprecated PositionConfig.gps_enabled (use gps_mode)

1 participant