Attempt at addressing: Announce GPS accuracy #103
Summary
In reference to soundscape-community/soundscape#103, I’ve implemented a new behavior that improves user awareness of GPS accuracy during startup and wake events.
Implementation Details
Startup/Wake Behavior: On app startup or wake, the system now checks the GPS accuracy.
If the accuracy is better than the defined threshold (±10 meters), the app will announce that GPS accuracy is good.
If the accuracy is poor, the app will inform the user and suggest moving around to improve signal quality.
Once the accuracy improves beyond the threshold, the app announces that the GPS accuracy has improved.
User Preference Setting: A new setting has been added to allow users to enable or disable GPS accuracy announcements.
By default, this setting is on false to prevent extra notification anncouncments from the GPS Accuracy feature for those who dont want.
But for users who prefer the GPS Acc. updates they can though enable this option to hear accuracy feedback on startup or wake.
Manual Accuracy Check Button: A new button allows users to manually check the current GPS accuracy at any time.
This gives users more control while minimizing potential over-announcements.
Ovreall this aims to adress issue #103 by improve accessibility for users who want GPS feedback, while being flexibile for those who prefer a quieter experience and dont care for it being alwasy on startup.
Any feedback or suggestions for improvement would be greatly appreciated.
Summary by CodeRabbit
-
New Features
- GPS accuracy announcements: spoken accuracy at startup and when improved, plus a new toggle in Settings → Callouts and updated settings UI/table layout.
-
Documentation
- Added/expanded localizations for GPS announcements, FAQ two‑finger double‑tap tip, mail providers and place/landmark categories across multiple languages.
-
Bug Fixes
- Minor localization formatting cleanups; one locale has a duplicated key that should be resolved.
Walkthrough
Adds GPS accuracy announcements: new AutomaticGenerator source and Xcode project entry; settings toggle and notification; settings UI, storyboard and localization entries; a new AnnounceGPSAccuracyEvent; and wiring into behavior initialization and build phases.
Changes
| Cohort / File(s) | Summary |
|---|---|
Project Configurationapps/ios/GuideDogs.xcodeproj/project.pbxproj |
Adds file reference and build-phase entry for GPSAccuracyAnnouncementGenerator.swift. |
Generator Sourceapps/ios/GuideDogs/Code/Behaviors/Default/GPSAccuracyAnnouncementGenerator.swift |
New internal GPSAccuracyAnnouncementGenerator implementing startup/wake/improved accuracy announcement flows, state tracking, unit formatting, Combine subscriptions, enqueueing callouts, and APIs (init, respondsTo, handle, cancelCalloutsForEntity). |
Settings Backendapps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift |
Adds gpsAnnouncementsEnabled key & default, Notification.Name .gpsAnnouncementsEnabledChanged, and a persisted computed property that posts notifications on change. |
Behavior Registration & Event Typesapps/ios/GuideDogs/Code/Behaviors/Default/SoundscapeBehavior.swift, <br>apps/ios/GuideDogs/Code/Behaviors/Default/SystemGenerator.swift` |
Inserts GPSAccuracyAnnouncementGenerator into autoGenerators; adds AnnounceGPSAccuracyEvent and registers it in SystemGenerator's event types. |
Settings UI Logicapps/ios/GuideDogs/Code/Visual UI/Controls/Settings/CalloutSettingsCellView.swift, <br>apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift` |
Adds gpsAnnouncements enum case; binds switch to SettingsContext.shared.gpsAnnouncementsEnabled; updates cell configuration, indices, and emits telemetry on toggle. |
Storyboard Layoutapps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard |
Adds GPS Announcements table cell (labels and switch), wires outlet/action; many frame/constraint adjustments and removal of designable preview blocks to reflow layout. |
Localization — GPS & Related (en-US / en-GB)apps/ios/GuideDogs/Assets/Localization/en-US.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/en-GB.lproj/Localizable.strings` |
Adds multiple GPS accuracy localization keys (announce/improved in feet/meters, unavailable, check & hint, good/poor/ok variants, settings title/info). en-GB also adds mail & OSM tag keys; en-GB contains a duplicated GPS block. |
Localization — FAQ tip translationsapps/ios/GuideDogs/Assets/Localization/da-DK.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/el-GR.lproj/Localizable.strings, apps/ios/GuideDogs/Assets/Localization/fr-FR.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/it-IT.lproj/Localizable.strings, apps/ios/GuideDogs/Assets/Localization/pt-BR.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/pt-PT.lproj/Localizable.strings` |
Adds faq.tip.two_finger_double_tap translations for multiple locales. |
Localization — Minor formatting editsapps/ios/GuideDogs/Assets/Localization/de-DE.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/es-ES.lproj/Localizable.strings, apps/ios/GuideDogs/Assets/Localization/fa-IR.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/fi-FI.lproj/Localizable.strings, apps/ios/GuideDogs/Assets/Localization/ja-JP.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/nb-NO.lproj/Localizable.strings` |
Minor whitespace/blank-line adjustments (added/removed trailing blank lines). |
Localization — Other additions / issuesapps/ios/GuideDogs/Assets/Localization/nl-NL.lproj/Localizable.strings, <br>apps/ios/GuideDogs/Assets/Localization/sv-SE.lproj/Localizable.strings` |
Adds mail.spark in nl-NL; sv-SE contains a duplicate faq.tip.two_finger_double_tap entry (potential conflict). |
Misc — Non-functional whitespaceapps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonGeometry.swift |
Minor blank-line insertion only; no logic change. |
Sequence Diagram(s)
sequenceDiagram
participant App as App Lifecycle
participant Gen as GPSAccuracyAnnouncementGenerator
participant Settings as SettingsContext
participant Loc as LocationManager
participant Callout as CalloutSystem
Note over App,Gen: Initialization
App->>Gen: init(settings, motionActivity)
Gen->>Settings: subscribe to gpsAnnouncementsEnabled
Note over App,Gen: App events (launch / wake)
App->>Gen: GlyphEvent (appLaunch / wake)
Gen->>Gen: set pending (startup/wake)
Note over Loc,Gen: Location updates drive announcements
Loc->>Gen: LocationUpdatedEvent
alt Settings.gpsAnnouncementsEnabled == false
Gen->>Gen: ignore events
else
alt First GPS update for pending
Gen->>Gen: compute accuracy (m/ft)
Gen->>Callout: enqueue initial accuracy message
Gen->>Gen: mark pending -> accurate/done
else Improved accuracy detected
Gen->>Callout: enqueue improved accuracy message (once)
Gen->>Gen: mark improved announced
end
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
- Focus review on:
- apps/ios/GuideDogs/Code/Behaviors/Default/GPSAccuracyAnnouncementGenerator.swift — state machine, Combine subscriptions, race conditions when toggling the setting, locale/unit formatting, and enqueue semantics.
- apps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard — verify outlets/actions and layout reflow across device sizes.
- apps/ios/GuideDogs/Code/Behaviors/Default/SoundscapeBehavior.swift — ordering of
autoGeneratorsand potential event consumption conflicts. - apps/ios/GuideDogs/Assets/Localization/sv-SE.lproj/Localizable.strings and apps/ios/GuideDogs/Assets/Localization/en-GB.lproj/Localizable.strings — duplicate localization keys to resolve.
Poem
🐰
I hopped where satellites hum and sing,
I count the feet and meters ring.
A tiny toggle wakes my voice,
I whisper distance when it's wise.
Hooray — a gentle guiding choice!
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and specifically describes the main change: implementing GPS accuracy announcements to address issue #103, which aligns with the substantial changes throughout the changeset (new generator class, localization strings, settings UI, etc.). |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between c1e7e9e2de3355756eeb8225d1b3f826c9369195 and aadea52bf5416b1a124063ab1c079e97c7892f86.
📒 Files selected for processing (1)
apps/ios/GuideDogs/Code/Behaviors/Default/GPSAccuracyAnnouncementGenerator.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/ios/GuideDogs/Code/Behaviors/Default/GPSAccuracyAnnouncementGenerator.swift (1)
apps/ios/GuideDogs/Code/App/AppContext.swift (1)
snooze(307-319)
🪛 SwiftLint (0.57.0)
apps/ios/GuideDogs/Code/Behaviors/Default/GPSAccuracyAnnouncementGenerator.swift
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 49-49: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 93-93: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 169-169: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 250-250: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 280-280: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
In addition to the comments left by the AI, I think you should change the logic for the localization for the GPS accuracy announcements. Rather than checking if the language and region is U.S. or other countries, you should check if the "Units of Measure" setting is set to Imperial or Metric. Also the GPS callout toggle switch and the accessibility hint should be rewritten to match the existing callout switches. Instead of "GPS Accuracy Announcements," the callout switch should simply say "GPS Accuracy" since a callout is understood to be an announcement. The accessibility hint reads "Toggles GPS accuracy announcements on or off." It should read something like this: "Announces GPS accuracy when the app is launched". Voiceover will already let the user know that it is a toggle switch. The period should not be part of the hint because a comma will automatically be put at the end of the hint, and Voiceover may read the hint funny if the user sets the punctuation in Voiceover to All. As for the GPS accuracy announcement button for manual checking, that may not be necessary because the GPS accuracy can already be displayed in the "Troubleshooting" section and read by voiceover. I think that Jchudge explained in issue 103 that we just need the announcement to play when the app is launched or when woken up, the user can look it up later in Settings, and the way to look it up is already in the Troubleshooting section. This means that the manual check button is redundant to the current implementation.
@ajiteshbankulaa Thanks for working on this.
To generate the strings for the announcements, you can use LanguageFormatter.string(from: distance, rounded: false which will automatically use the user's preference.
I'm not sure about including machine generated translations in a Pr. Ideally we would have a translation service setup on weblate and the strings would be marked as machine translated for native speakers to review, see #170 .
If anyone wants to look into automatic translation on weblate, let me know and I'll give you admin access.
cool thanks for the feedback, I'll try and fix all these issues soon
Hi @ajiteshbankulaa did you mean to close this? It's good work and almost ready to merge.
I think RD Murray is right. Either reopen it, or open a brand-new pull request with a clean version. All we need to do is remove the button to announce GPS accuracy manually, and fix the automatic announcement punctuations. In this case the AI is right. The automatic announcement needs a period to separate the statements so we can hear a pause. These announcements will be called out by Soundscape, not reviewed with Voiceover. And Soundscape doesn't verbally announce punctuations in the first place. I was only referring to the callout switch in the "Manage Callouts" section where the Voiceover hint shouldn't have a period. The "Manage Callouts" hint will be reviewed by Voiceover, and the period may make the formatting look weird. Once this gets merged, I'll get to work and translate everything to Spanish.
Since this is a fast action navigation app, I think we could shorten the announcement. If you think the current format is good enough, and separators like commas and periods get added so that Soundscape pauses properly, I suggest the poor accuracy announcement goes like this, "GPS accuracy is poor, about %@ meters. Move around for better accuracy." The ± sign may not be pronounced correctly by Soundscape, and the word "about" is faster to say. Now if we want to make the announcement even shorter, we could rewrite it this way, "Poor GPS accuracy, about %@ meters, move for better accuracy." This shorter, more urgent announcement suggests that the user needs to find some place else quickly. The good accuracy announcement should say "GPS accuracy is good, about %@ meters." I believe that rewriting the good accuracy message "Good GPS accuracy, about %@ meters" is more confusing, but the shorter poor accuracy message above makes the user feel like there's something wrong.
Hi I just closed because I didn't mean to commit this into this yet. I am almost done so yea I'll open a new pull probably or I might just reopen. I will also do the rewriting suggestions for the +- part and the periods.
I think I have another idea for the poor accuracy announcement. Since the troubleshooting says that under 10 meters or 30 feet is good, under 20 meters or 60 feet is ok, anything else is poor, we could rewrite the poor accuracy announcement to be more consistent with the troubleshooting explanation. This new version could sound like this, "Poor GPS accuracy, exceeded 20 meter limit, move for better accuracy." The imperial version would say "Poor GPS accuracy, exceeded 60 foot limit, move for better accuracy." The user probably doesn't need to know the exact number once we get over 20 meters or over 60 feet. The user could still see the exact number in "Troubleshooting" displayed on screen if they want to.
Now we may need an announcement for the accuracy between 10 and 20 meters (between 30 and 60 feet) to line up with the Troubleshooting description. This would say "GPS accuracy is ok, about %@ meters." Now here's another suggestion, you may not like it though, it's just another possibility. We could forget the good, ok, poor distinction and just announce the accuracy in all cases like this, "GPS accuracy: %@ meters", and the user can learn what to do by reading the troubleshooting section. Either way is fine with me, maybe RD Murray or Jchudge could chime in.
I like the idea here: "Poor GPS accuracy, exceeded 20 meter limit, move for better accuracy.". It makes sense I dought people will care for the specific number. So I did change that. I also added the GPS accuracy is ok part also.
The button is removed now and also fixed all the labeling and whatnot issues form before I believe.
as for the all cases annouce as: GPS accuracy: %@ meters
Im personaly open to anything based on what everyone thinks would be better. Another thing is I could also put another toggle or something in settings so the user can decide which way they would like it. It should be pretty simple to do so, but I don't know if it would feel janky then. But i'm overall open to whatever.
The "ok" accuracy message is still missing the comma found in the "good" accuracy message. That should look more like this: "GPS accuracy is ok, about %d meters." I think you need to change the ranges for "good," "ok," and "poor" to match the existing explanation in the "Troubleshooting" section. "Less than +-10 meters is good. Less than +-20 meters is ok. Anything else is poor". That means that the poor message should be above 20 meters or above 60 feet. The OK range should be between 10 and 20 meters, between 30 and 60 feet.
On second thought, the simple version of just announcing the GPS accuracy without the good, fair, poor scale isn't as beginner friendly. Voice Vista already announces accuracy like that, and we want Soundscape to be more beginner friendly than Voice Vista, but without sounding patronizing for experienced users. We're continuing to follow the direction from when Soundscape used to be a Microsoft App, and I think our current version looks like a Microsoft feature in spirit. Don't worry about adding a button to change the announcement style. We'll have too many buttons to overcomplicate the UI.
It looks like the UK message for poor GPS accuracy still says "10 metres". That needs to be updated.
The messages look pretty good now. So let's move on to the imperial and metric conversion itself. I explained that there might be a way to get the number from Soundscape itself so you don't need to do the calculations. I think RD Murray left a comment to use LanguageFormatter to look up the value. The AI explained that this should shorten the code, and you don't have to repeat some code.
Not sure if LanguageFormatter is better. I can change to that but I just updated to use: let feet = Measurement(value: acc, unit: UnitLength.meters).converted(to: UnitLength.feet).value I did this because I saw this was what was being used in the troubleshooting section. I will though look into the LanguageFormatter thing also and see if there is any benefit to it.
Thanks, I think I already put in my two cents. I'll leave it up to anybody else to look over what needs to be done.
We should definitely use LanguageFormatter. It is already used for distances in other callouts, it uses the user's preference for measurement units and avoids the need to duplicate translation strings. It also handles rounding so it will output 10 meters or 30 feet automatically.