stadiamaps / ferrostar

A FOSS navigation SDK built from the ground up for the future
https://stadiamaps.github.io/ferrostar/
Other
180 stars 25 forks source link

iOS/Apple Annotations for entire Route #320

Open Archdoog opened 3 weeks ago

Archdoog commented 3 weeks ago

Add functionality to apply or publish annotation data for the entire Route. The existing iOS annotation publisher just publishes the current annotations. This would add the ability for the annotation publishing & publisher to add congestion data long the entire route (or route steps).

This could be used for things like congestion/traffic.

engali94 commented 3 weeks ago

Hi @Archdoog I beileve this can be easly added as a computed properity this way:

extension Route {
    public var annotations: [ValhallaOsrmAnnotation] {
        let decoder = JSONDecoder()

        return steps
            .compactMap(\.annotations)
            .flatMap { annotations in
                annotations.compactMap { annotationString in
                    guard let data = annotationString.data(using: .utf8) else {
                        return nil
                    }
                    return try? decoder.decode(ValhallaOsrmAnnotation.self, from: data)
                }
            }
    }
}

what do you think?

Archdoog commented 3 weeks ago

@engali94 Here are some thoughts I have on full route annotations, including congestion (versus current annotation at the user's location as published for iOS in #287):

  1. We should probably start with a PR to introduce something like a CongestionRouteOverlay (or similarly named). Which would be placed roughly at NavigationMapView.swift#L60. This would likely include the data structure for a CongestionSegment or similar and let us refine what we actually need to pass down to the MapView. It could be passed down as a simple argument like congestion: [CongestionSegment]? = nil.
  2. Using the findings in that PR, we should decide where to put the parsing behavior. I like attaching it to the Route as you've posted for simplicity. But as we saw with my early work on #287, this pattern isn't nice when using a custom annotation Codable. For that reason, it may be best to put this on the AnnotationPublishing. This would enable any developer using ferrostar to simply build their own AnnotationPublisher<MyServersCustomAnnotationData> and decode/map to a @Published var congestion: [CongestionSegment]? if they need it. Further, they could create their own AnnotationPublishing that uses whatever data they want if desired (e.g. protobuf or whatever).

    2.a. This would allow us to ensure the system calculate's congestion once per route change. Whereas on the route itself, it may be easier for a developer to misuse and run this calculate many times. 2.b. We could then just pass congestion: [CongestionSegment]? = nil into the NavigationMapView from the top just like speed limit.

Side conversation:

This may actually be a good exercise into whether it's a good idea that we hold the route inside navigation state. I wonder if might be better served as a standalone @Published var route: Route? on FerrostarCore. Thoughts @ianthetechie? Detaching the Route may make it easier to handle annotation logic, display the route before navigating, etc. The AnnotationPublishing might even become the RoutePublishing? (Just brainstorming 😅)