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

Ios mute fix after discussion #296

Merged
merged 17 commits into from
Oct 29, 2024
Merged

Ios mute fix after discussion #296

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
249d977
ios mute button
Oct 12, 2024
64cf978
adjustment made based on dicsussion for Ios Mute button
Oct 13, 2024
fae9ad2
fixed the in the file
Oct 14, 2024
d76959f
changed name of file so the names are consistent
Oct 14, 2024
60bb06b
adjusted based on the other buttons
Oct 16, 2024
bd4b7f8
Clean up and fixes to get this ready
ianthetechie Oct 25, 2024
b71ea85
Use a boring ObservableObject and use UDF
ianthetechie Oct 28, 2024
35df836
Simplified and tested mute button:
Archdoog Oct 29, 2024
583acea
Delete .gitmodules
ianthetechie Oct 29, 2024
45ee6c8
Delete extraneous submodule
ianthetechie Oct 29, 2024
10c83d9
Try to fix CI
ianthetechie Oct 29, 2024
d64dd25
Attempt to improve UI responsiveness
ianthetechie Oct 29, 2024
f93e9e1
Improve speed limit display
ianthetechie Oct 29, 2024
4c284a2
Hide when not navigating
ianthetechie Oct 29, 2024
1d3cc67
Add missing docs
ianthetechie Oct 29, 2024
57e6f89
Swiftformat
ianthetechie Oct 29, 2024
7e01782
Update snapshots
ianthetechie Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion apple/DemoApp/Demo/DemoNavigationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ struct DemoNavigationView: View {
private let navigationDelegate = NavigationDelegate()
// NOTE: This is probably not ideal but works for demo purposes.
// This causes a thread performance checker warning log.
private let spokenInstructionObserver = AVSpeechSpokenInstructionObserver(isMuted: false)
@State private var spokenInstructionObserver = AVSpeechSpokenInstructionObserver(isMuted: false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look quite right, but it's probably a result of our AVSpeechSpokenInstructionObserver. That class doesn't really have a good way to publish the isMuted value.

Here's an updated version which you could copy & replace to the apple/Sources/FerrostarCore/Speech.swift file:

public protocol SpokenInstructionObserver {
    
    /// Mute or unmute the speech engine.
    ///
    /// - Parameter mute: Mute the speech engine if true, unmute if false.
    func setMute(_ mute: Bool)
    
    /// Handles spoken instructions as they are triggered.
    ///
    /// As long as it is used with the supplied ``FerrostarCore`` class,
    /// implementors may assume this function will never be called twice
    /// for the same instruction during a navigation session.
    func spokenInstructionTriggered(_ instruction: SpokenInstruction)

    /// Stops speech and clears the queue of spoken utterances.
    func stopAndClearQueue()

    /// If the speech synthisizer is currently muted.
    var isMuted: Bool { get }
}

public class AVSpeechSpokenInstructionObserver: ObservableObject, SpokenInstructionObserver {
    
    @Published public private(set) var isMuted: Bool
    
    private let synthesizer = AVSpeechSynthesizer()

    public init(isMuted: Bool) {
        self.isMuted = isMuted
    }

    public func setMute(_ mute: Bool) {
        guard isMuted != mute else {
            return
        }
        
        // Immediately set the publisher. This will avoid delays updating UI.
        isMuted = mute
        
        if mute && synthesizer.isSpeaking {
            synthesizer.stopSpeaking(at: .immediate)
        }
    }
    
    public func spokenInstructionTriggered(_ instruction: FerrostarCoreFFI.SpokenInstruction) {
        guard !isMuted else {
            return
        }

        let utterance: AVSpeechUtterance = if #available(iOS 16.0, *),
                                              let ssml = instruction.ssml,
                                              let ssmlUtterance = AVSpeechUtterance(ssmlRepresentation: ssml)
        {
            ssmlUtterance
        } else {
            AVSpeechUtterance(string: instruction.text)
        }

        synthesizer.speak(utterance)
    }

    public func stopAndClearQueue() {
        synthesizer.stopSpeaking(at: .immediate)
    }
}

I'd be happy to also post a PR with this change if that'd help.

The key difference is the AVSpeechSpokenInstructionObserver will actually have an isMuted publisher on it that's designed to be used in a SwiftUI instead of a simple bool. A simple bool won't trigger updates to the button, causing the problem you were probably hoping to solve with @State.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is tricky. You're right that the current protocol isn't going to just drop in to a SwiftUI setup.... but I'm not sure this actually fixes the problem 😅 It handles one half (publishing updates), but it doesn't actually get us to a two-way binding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In a SwiftUI app, I would not expect a @Binding var to actually alter the behavior of a service even though you're correct that it offers a two way street which could be used for that. Having a bespoke func setMute(_ mute: Bool) and a published state that SwiftUI can easily use seems to capture the best of a button running a behavior and a state being published for the visual UI.

A get set pattern was nice, but in this case seems like the wrong solution given the goals and SwiftUI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I see your point. Though I think @Binding is at least somewhat intended for this purpose.

The problem I hit with the method-based approach, which is how you'd do it in Jetpack Compose to preserve UDF, is that things get extremely messy with the Swift type system.

At A high level, here's what I think you're suggesting:

SpokenInstructionObserver:

  • Make isMuted get-only
  • Add func setMuted(...)

MuteUIButton:

  • Switch from @Binding to @ObservedObject and observe the SpokenInstructionObserver directly
  • Rather than toggling the binding, invoke spokenInstructionObserver.setMuted(...)
  • Since spokenInstructionObserver.isMuted is published, observe that value in the image composition rather than the binding

In order to use a @Published property and observe the object directly, we would need to make SpokenInstructionObserver conform to ObservableObject. This sounds trivial, but this opens up Pandora's box with errors like Type 'any SpokenInstructionObserver' cannot conform to 'ObservableObject' that highlight the flaws of the Swift type system...

I am unable to find a resolution to this at the moment. Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay... after banging my head against the wall a bit more, I think I have something that works. It's nominally better... Have a look over it and lemme know what you think. I don't really like the way it forces us to include generic bounds. And I couldn't get it to work with an optional T in the constructor, since the desirable default, nil is going to have an unknown type 😂 And any types don't work as noted. So, the only option remaining that seems to work OK is to define a dummy default, which you can override.


private var locationProvider: LocationProviding
@ObservedObject private var ferrostarCore: FerrostarCore
@State private var isMuted: Bool = false

@State private var isFetchingRoutes = false
@State private var routes: [Route]?
Expand Down Expand Up @@ -78,6 +79,7 @@ struct DemoNavigationView: View {
styleURL: style,
camera: $camera,
navigationState: ferrostarCore.state,
isMuted: $isMuted,
onTapExit: { stopNavigation() },
makeMapContent: {
let source = ShapeSource(identifier: "userLocation") {
Expand Down Expand Up @@ -148,6 +150,18 @@ struct DemoNavigationView: View {
.task {
await getRoutes()
}

// .overlay(alignment: .topTrailing) {
// //if ferrostarCore.state?.isNavigating == true { // will be used after PR275 is finished
// if case .navigating = ferrostarCore.state?.tripState {
// MuteUIButton(isMuted: $spokenInstructionObserver.isMuted)
// .padding(.trailing, 18) // Right
// .padding(.top, 112)
// }
// }



}
}

Expand Down
12 changes: 10 additions & 2 deletions apple/Sources/FerrostarMapLibreUI/Views/DynamicallyOrientingNavigationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
let styleURL: URL
@Binding var camera: MapViewCamera
let navigationCamera: MapViewCamera
@Binding var isMuted: Bool // Add the isMuted binding


private var navigationState: NavigationState?
private let userLayers: [StyleLayerDefinition]
Expand Down Expand Up @@ -47,12 +49,14 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
navigationCamera: MapViewCamera = .automotiveNavigation(),
navigationState: NavigationState?,
minimumSafeAreaInsets: EdgeInsets = EdgeInsets(top: 16, leading: 16, bottom: 16, trailing: 16),
isMuted: Binding<Bool>,
onTapExit: (() -> Void)? = nil,
@MapViewContentBuilder makeMapContent: () -> [StyleLayerDefinition] = { [] }
) {
self.styleURL = styleURL
self.navigationState = navigationState
self.minimumSafeAreaInsets = minimumSafeAreaInsets
self._isMuted = isMuted
self.onTapExit = onTapExit

userLayers = makeMapContent()
Expand Down Expand Up @@ -89,6 +93,7 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
onZoomOut: { camera.incrementZoom(by: -1) },
showCentering: !camera.isTrackingUserLocationWithCourse,
onCenter: { camera = navigationCamera },
isMuted: $isMuted,
onTapExit: onTapExit
)
.innerGrid {
Expand All @@ -109,6 +114,7 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
onZoomOut: { camera.incrementZoom(by: -1) },
showCentering: !camera.isTrackingUserLocationWithCourse,
onCenter: { camera = navigationCamera },
isMuted: $isMuted,
onTapExit: onTapExit
)
.innerGrid {
Expand Down Expand Up @@ -146,7 +152,8 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
return DynamicallyOrientingNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false)
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
Expand All @@ -165,7 +172,8 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn
return DynamicallyOrientingNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false)
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
11 changes: 9 additions & 2 deletions apple/Sources/FerrostarMapLibreUI/Views/LandscapeNavigationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public struct LandscapeNavigationView: View, CustomizableNavigatingInnerGridView
let styleURL: URL
@Binding var camera: MapViewCamera
let navigationCamera: MapViewCamera
@Binding var isMuted: Bool // Add mute binding to the view


private var navigationState: NavigationState?
private let userLayers: [StyleLayerDefinition]
Expand Down Expand Up @@ -47,12 +49,14 @@ public struct LandscapeNavigationView: View, CustomizableNavigatingInnerGridView
navigationCamera: MapViewCamera = .automotiveNavigation(),
navigationState: NavigationState?,
minimumSafeAreaInsets: EdgeInsets = EdgeInsets(top: 16, leading: 16, bottom: 16, trailing: 16),
isMuted: Binding<Bool>,
onTapExit: (() -> Void)? = nil,
@MapViewContentBuilder makeMapContent: () -> [StyleLayerDefinition] = { [] }
) {
self.styleURL = styleURL
self.navigationState = navigationState
self.minimumSafeAreaInsets = minimumSafeAreaInsets
self._isMuted = isMuted
self.onTapExit = onTapExit

userLayers = makeMapContent()
Expand Down Expand Up @@ -83,6 +87,7 @@ public struct LandscapeNavigationView: View, CustomizableNavigatingInnerGridView
onZoomOut: { camera.incrementZoom(by: -1) },
showCentering: !camera.isTrackingUserLocationWithCourse,
onCenter: { camera = navigationCamera },
isMuted: $isMuted,
onTapExit: onTapExit
)
.innerGrid {
Expand Down Expand Up @@ -115,7 +120,8 @@ public struct LandscapeNavigationView: View, CustomizableNavigatingInnerGridView
return LandscapeNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false) // Fix: Provide isMuted binding
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
Expand All @@ -136,7 +142,8 @@ public struct LandscapeNavigationView: View, CustomizableNavigatingInnerGridView
return LandscapeNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false)
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
7 changes: 6 additions & 1 deletion apple/Sources/FerrostarMapLibreUI/Views/Overylays/LandscapeNavigationOverlayView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ struct LandscapeNavigationOverlayView: View, CustomizableNavigatingInnerGridView
private var navigationState: NavigationState?

@State private var isInstructionViewExpanded: Bool = false
@Binding var isMuted: Bool // Add the isMuted binding


var topCenter: (() -> AnyView)?
var topTrailing: (() -> AnyView)?
Expand All @@ -34,6 +36,7 @@ struct LandscapeNavigationOverlayView: View, CustomizableNavigatingInnerGridView
onZoomOut: @escaping () -> Void = {},
showCentering: Bool = false,
onCenter: @escaping () -> Void = {},
isMuted: Binding<Bool>,
onTapExit: (() -> Void)? = nil
) {
self.navigationState = navigationState
Expand All @@ -43,6 +46,7 @@ struct LandscapeNavigationOverlayView: View, CustomizableNavigatingInnerGridView
self.onZoomOut = onZoomOut
self.showCentering = showCentering
self.onCenter = onCenter
self._isMuted = isMuted
self.onTapExit = onTapExit
}

Expand Down Expand Up @@ -87,7 +91,8 @@ struct LandscapeNavigationOverlayView: View, CustomizableNavigatingInnerGridView
onZoomIn: onZoomIn,
onZoomOut: onZoomOut,
showCentering: showCentering,
onCenter: onCenter
onCenter: onCenter,
isMuted: $isMuted
)
.innerGrid {
topCenter?()
Expand Down
7 changes: 6 additions & 1 deletion apple/Sources/FerrostarMapLibreUI/Views/Overylays/PortraitNavigationOverlayView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ struct PortraitNavigationOverlayView: View, CustomizableNavigatingInnerGridView

@State private var isInstructionViewExpanded: Bool = false
@State private var instructionsViewSizeWhenNotExpanded: CGSize = .zero
@Binding var isMuted: Bool // Add the isMuted binding


var topCenter: (() -> AnyView)?
var topTrailing: (() -> AnyView)?
Expand All @@ -35,6 +37,7 @@ struct PortraitNavigationOverlayView: View, CustomizableNavigatingInnerGridView
onZoomOut: @escaping () -> Void = {},
showCentering: Bool = false,
onCenter: @escaping () -> Void = {},
isMuted: Binding<Bool>,
onTapExit: (() -> Void)? = nil
) {
self.navigationState = navigationState
Expand All @@ -44,6 +47,7 @@ struct PortraitNavigationOverlayView: View, CustomizableNavigatingInnerGridView
self.onZoomOut = onZoomOut
self.showCentering = showCentering
self.onCenter = onCenter
self._isMuted = isMuted
self.onTapExit = onTapExit
}

Expand All @@ -62,7 +66,8 @@ struct PortraitNavigationOverlayView: View, CustomizableNavigatingInnerGridView
onZoomIn: onZoomIn,
onZoomOut: onZoomOut,
showCentering: showCentering,
onCenter: onCenter
onCenter: onCenter,
isMuted: $isMuted
)
.innerGrid {
topCenter?()
Expand Down
11 changes: 9 additions & 2 deletions apple/Sources/FerrostarMapLibreUI/Views/PortraitNavigationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public struct PortraitNavigationView: View, CustomizableNavigatingInnerGridView

@Binding var camera: MapViewCamera
let navigationCamera: MapViewCamera
@Binding var isMuted: Bool // Add mute binding to the view


var onTapExit: (() -> Void)?

Expand All @@ -48,12 +50,14 @@ public struct PortraitNavigationView: View, CustomizableNavigatingInnerGridView
navigationCamera: MapViewCamera = .automotiveNavigation(),
navigationState: NavigationState?,
minimumSafeAreaInsets: EdgeInsets = EdgeInsets(top: 16, leading: 16, bottom: 16, trailing: 16),
isMuted: Binding<Bool>,
onTapExit: (() -> Void)? = nil,
@MapViewContentBuilder makeMapContent: () -> [StyleLayerDefinition] = { [] }
) {
self.styleURL = styleURL
self.navigationState = navigationState
self.minimumSafeAreaInsets = minimumSafeAreaInsets
self._isMuted = isMuted
self.onTapExit = onTapExit

userLayers = makeMapContent()
Expand Down Expand Up @@ -85,6 +89,7 @@ public struct PortraitNavigationView: View, CustomizableNavigatingInnerGridView
onZoomOut: { camera.incrementZoom(by: -1) },
showCentering: !camera.isTrackingUserLocationWithCourse,
onCenter: { camera = navigationCamera },
isMuted: $isMuted,
onTapExit: onTapExit
)
.innerGrid {
Expand Down Expand Up @@ -116,7 +121,8 @@ public struct PortraitNavigationView: View, CustomizableNavigatingInnerGridView
return PortraitNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false)
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
Expand All @@ -136,7 +142,8 @@ public struct PortraitNavigationView: View, CustomizableNavigatingInnerGridView
return PortraitNavigationView(
styleURL: URL(string: "https://demotiles.maplibre.org/style.json")!,
camera: .constant(.center(userLocation.clLocation.coordinate, zoom: 12)),
navigationState: state
navigationState: state,
isMuted: .constant(false)
)
.navigationFormatterCollection(FoundationFormatterCollection(distanceFormatter: formatter))
}
29 changes: 29 additions & 0 deletions apple/Sources/FerrostarSwiftUI/Views/Controls/MuteUIButton.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import SwiftUI

public struct MuteUIButton: View {
@Binding public var isMuted: Bool

public init(isMuted: Binding<Bool>) {
self._isMuted = isMuted
}

public var body: some View {
Button(action: {
isMuted.toggle()
}) {
Image(systemName: isMuted ? "speaker.slash.fill" : "speaker.2.fill")
.resizable()
.aspectRatio(contentMode: .fit)
.frame(width: 18, height: 18)
.padding()
.foregroundColor(.black)
.background(Color.white)
.clipShape(Circle())
.shadow(radius: 10)
}
}
}

#Preview {
MuteUIButton(isMuted: .constant(false))
}
19 changes: 16 additions & 3 deletions apple/Sources/FerrostarSwiftUI/Views/GridViews/NavigatingInnerGridView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView
public var midLeading: (() -> AnyView)?
public var bottomTrailing: (() -> AnyView)?

@Binding var isMuted: Bool

/// The default navigation inner grid view.
///
/// This view provides all default navigation UI views that are used in the open map area. This area is defined as
Expand All @@ -43,14 +45,17 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView
onZoomIn: @escaping () -> Void = {},
onZoomOut: @escaping () -> Void = {},
showCentering: Bool = false,
onCenter: @escaping () -> Void = {}
onCenter: @escaping () -> Void = {},
isMuted: Binding<Bool>

) {
self.speedLimit = speedLimit
self.showZoom = showZoom
self.onZoomIn = onZoomIn
self.onZoomOut = onZoomOut
self.showCentering = showCentering
self.onCenter = onCenter
self._isMuted = isMuted
}

public var body: some View {
Expand All @@ -65,7 +70,13 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView
}
},
topCenter: { topCenter?() },
topTrailing: { topTrailing?() },
topTrailing: {

MuteUIButton(isMuted: $isMuted)
.shadow(radius: 8)
.padding(.top,16)

},
midLeading: { midLeading?() },
midCenter: {
// This view does not allow center content.
Expand Down Expand Up @@ -110,7 +121,9 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView
NavigatingInnerGridView(
speedLimit: .init(value: 55, unit: .milesPerHour),
showZoom: true,
showCentering: true
showCentering: true,
isMuted: .constant(false)

)
.padding(.horizontal, 16)

Expand Down