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.

Sign up for GitHub

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

Jump to bottom

Don't speak over each other #1213

Merged
merged 2 commits into from Mar 8, 2018
Merged

Don't speak over each other #1213

merged 2 commits into from Mar 8, 2018

Conversation

bsudekum
Copy link
Contributor

@bsudekum bsudekum commented Mar 8, 2018

We have a few reports of the the RouteVoiceController and the MapboxVoiceController are speaking over each other in a few cases:

  1. Turned on my car in a parking lot, bluetooth connected, was playing from Audible
  2. Entered my address, got a route telling me to go around the block (Original route above)
  3. Realized I didn't need to go around because I could drive out of the parking lot directly onto SE 23rd Ave, which triggered a reroute (Rerouted route above)
  4. The reroute paused the Audible playback
  5. "Turn left onto Southeast Morrison Street" was spoken simultaneously by Polly and the fallback robot phone voice
  6. When the announcement was done, Audible did not start playing again

It's difficult to reproduce but these are changes that I see as pretty valid changes.

/cc @mapbox/navigation-ios @lyzidiamond

bsudekum
bsudekum commented Mar 8, 2018
View reviewed changes
MapboxNavigation/MapboxVoiceController.swift Outdated
@@ -171,6 +169,9 @@ open class MapboxVoiceController: RouteVoiceController {
Plays an audio file.
*/
@objc open func play(_ data: Data) {

guard !super.speechSynth.isSpeaking else { return }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or we could change this to:

super.speechSynth.stopSpeaking(at: .immediate)

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah let's do that.

bsudekum
bsudekum commented Mar 8, 2018
View reviewed changes
MapboxNavigation/MapboxVoiceController.swift
@@ -186,6 +187,7 @@ open class MapboxVoiceController: RouteVoiceController {
let played = self.audioPlayer?.play() ?? false

guard played else {
try super.unDuckAudio()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We ducked a few lines above, so unduck before playing super.speak.

@bsudekum
Copy link
Contributor Author

bsudekum commented Mar 8, 2018

@vincethecoder could you give this a review?

JThramer
JThramer approved these changes Mar 8, 2018
View reviewed changes
Copy link
Contributor

@JThramer JThramer left a comment

Choose a reason for hiding this comment

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

:shipit:

@bsudekum bsudekum merged commit 6aab907 into master Mar 8, 2018
@bsudekum bsudekum deleted the dont-speak branch March 8, 2018 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JThramer JThramer JThramer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bsudekum @JThramer