soundscape icon indicating copy to clipboard operation
soundscape copied to clipboard

Attempt at addressing: Announce GPS accuracy #103

Open ajiteshbankulaa opened this issue 2 months ago • 18 comments

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.

GPS Accuracy Setting

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.

Manual Accuracy Button Notes

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.

ajiteshbankulaa avatar Oct 14 '25 21:10 ajiteshbankulaa

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 Configuration
apps/ios/GuideDogs.xcodeproj/project.pbxproj
Adds file reference and build-phase entry for GPSAccuracyAnnouncementGenerator.swift.
Generator Source
apps/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 Backend
apps/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 Types
apps/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 Logic
apps/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 Layout
apps/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 translations
apps/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 edits
apps/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 / issues
apps/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 whitespace
apps/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 autoGenerators and 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.

❤️ Share

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

coderabbitai[bot] avatar Oct 14 '25 21:10 coderabbitai[bot]

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.

JJGatchalian avatar Oct 14 '25 22:10 JJGatchalian

@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.

RDMurray avatar Oct 15 '25 12:10 RDMurray

cool thanks for the feedback, I'll try and fix all these issues soon

ajiteshbankulaa avatar Oct 16 '25 01:10 ajiteshbankulaa

Hi @ajiteshbankulaa did you mean to close this? It's good work and almost ready to merge.

RDMurray avatar Nov 01 '25 20:11 RDMurray

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.

JJGatchalian avatar Nov 03 '25 01:11 JJGatchalian

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.

JJGatchalian avatar Nov 03 '25 12:11 JJGatchalian

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.

ajiteshbankulaa avatar Nov 04 '25 21:11 ajiteshbankulaa

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.

JJGatchalian avatar Nov 04 '25 22:11 JJGatchalian

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.

JJGatchalian avatar Nov 04 '25 22:11 JJGatchalian

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.

ajiteshbankulaa avatar Nov 07 '25 22:11 ajiteshbankulaa

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.

JJGatchalian avatar Nov 07 '25 23:11 JJGatchalian

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.

JJGatchalian avatar Nov 07 '25 23:11 JJGatchalian

It looks like the UK message for poor GPS accuracy still says "10 metres". That needs to be updated.

JJGatchalian avatar Nov 11 '25 22:11 JJGatchalian

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.

JJGatchalian avatar Nov 12 '25 15:11 JJGatchalian

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.

ajiteshbankulaa avatar Nov 13 '25 19:11 ajiteshbankulaa

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.

JJGatchalian avatar Nov 13 '25 23:11 JJGatchalian

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.

RDMurray avatar Nov 14 '25 10:11 RDMurray