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

iOS/Apple Annotations for entire Route #320

Open
Archdoog opened this issue Oct 29, 2024 · 2 comments
Open

iOS/Apple Annotations for entire Route #320

Archdoog opened this issue Oct 29, 2024 · 2 comments
Labels
iOS UI/UX Related to the UI/UX (on any platform)

Comments

@Archdoog
Copy link
Collaborator

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
Copy link
Contributor

engali94 commented Oct 29, 2024

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
Copy link
Collaborator Author

Archdoog commented Oct 30, 2024

@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 Apple Annotation parsing #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 😅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS UI/UX Related to the UI/UX (on any platform)
Projects
None yet
Development

No branches or pull requests

2 participants