Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only require background audio when using speech synthesis #64

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

michaelkirk
Copy link
Collaborator

@michaelkirk michaelkirk commented Jun 14, 2024

Fixes #63

I'm not currently using voice instructions. I'd like to add it at some point, but my app is still useful without this feature.

The SDK requires the background "Audio" entitlement at runtime, but without the voice instructions, there's no reason for the background "Audio" entitlement.

The AppStore may not accept an application with the background audio entitlement that doesn't actually use background audio. (speaking from experience here 😬)

RouteVoiceController.verifyBackgroundAudio is called upon init, which happens when instantiating a NavigationViewController. Instead we should wait until we actually use the audio before verifying this permission.

@@ -193,6 +191,9 @@ open class RouteVoiceController: NSObject, AVSpeechSynthesizerDelegate {
- parameter ignoreProgress: A `Bool` that indicates if the routeProgress is added to the instruction.
*/
open func speak(_ instruction: SpokenInstruction, with locale: Locale?, ignoreProgress: Bool = false) {
// Don't require background audio unless the implementer is actually using speech APIs
self.verifyBackgroundAudio()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that this ultimately calls an assertionFailure so it shouldn't have any bearing on release builds one way or the other.

Copy link
Collaborator

@boldtrn boldtrn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 🎉

@michaelkirk michaelkirk force-pushed the mkirk/dont-require-background-audio branch from 55529f4 to 5b14aad Compare June 20, 2024 21:35
@michaelkirk
Copy link
Collaborator Author

Rebased.

This has been approved for a few days and is pretty small, so I'm going to merge. LMK if there's a different process I should follow in the future.

@michaelkirk michaelkirk merged commit 7c02808 into main Jun 20, 2024
2 checks passed
@michaelkirk michaelkirk deleted the mkirk/dont-require-background-audio branch July 15, 2024 17:13
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.

Make background Audio (and RouteVoiceController?) optional
2 participants