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

Publish reframed Annotations on Current Step #261

Merged
merged 24 commits into from
Oct 1, 2024

Conversation

Archdoog
Copy link
Collaborator

@Archdoog Archdoog commented Sep 23, 2024

  • Removes hard coded annotation type in favor of BTreeMap for annotations. This gives us the flexibility of any json attribute an API wants to return plus keeps the json keys sorted for consistency and testability (since we're passing this as a json string through the FFI)
  • Reshapes the original annotations format of many arrays of a single value into one array with an object of key value pairs (making it easy to pass to a client platform).

Closes #89

Two follow up tickets are:

  1. Parsing the data on the client side is here: Convert AnnotationValue to parsed type instead of Map in Android & iOS #271.
  2. Convert MaxSpeed to something reasonable: Convert MaxSpeed to consistent JSON format (a float of meters per second) #272

Copy link
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

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

Here's a first pass over it. Honestly I think this is fine methodology-wise. We can debug the indexing issues tomorrow, but I think the approach is probably fine so you can proceed with refinements and clean-up.

common/ferrostar/src/models.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/osrm/algorithms.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/osrm/models.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/osrm/models.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/osrm/models.rs Outdated Show resolved Hide resolved
@Archdoog Archdoog self-assigned this Sep 26, 2024
@Archdoog Archdoog marked this pull request as ready for review September 26, 2024 03:23
@Archdoog Archdoog changed the title WIP: Publish reframed Annotations on Current Step Publish reframed Annotations on Current Step Sep 26, 2024
Copy link
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

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

Few more comments... We're really close on this :D

common/ferrostar/src/models.rs Outdated Show resolved Hide resolved
common/ferrostar/src/models.rs Show resolved Hide resolved
common/ferrostar/src/models.rs Show resolved Hide resolved
common/ferrostar/src/models.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/error.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/algorithms.rs Outdated Show resolved Hide resolved
common/ferrostar/src/routing_adapters/osrm/mod.rs Outdated Show resolved Hide resolved
@ianthetechie ianthetechie merged commit 13be012 into main Oct 1, 2024
14 checks passed
@ianthetechie ianthetechie deleted the feat/common/current-attrs branch October 1, 2024 03:45
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.

Telemetry/CurrentAttributes State from Core
3 participants