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

Additional Incident members #672

Merged
merged 3 commits into from Mar 31, 2022
Merged

Additional Incident members #672

merged 3 commits into from Mar 31, 2022

Conversation

Udumft
Copy link
Contributor

@Udumft Udumft commented Mar 31, 2022

This PR exposes missing Incident members as country codes, closed road flag, long description, etc.

@Udumft Udumft self-assigned this Mar 31, 2022

let value: Int
var clampedValue: Int? {
value == 101 ? nil : value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The doc says that value can range from 0 up to 101. Directions team clarified that 101 value indicates something like "invalid state" which effectively means that there is no congestion data available. So, for the sake of simplifying, and since both cases means the same for the end user, I've added this conversion.

Copy link

Choose a reason for hiding this comment

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

Oh I see, it's what's stated in the public doc you linked too.

Copy link
Contributor

@S2Ler S2Ler Mar 31, 2022

Choose a reason for hiding this comment

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

Worth having a named constant for 101 with a link to docs if possible.

/// List of roads names affected by the incident.
///
/// Alternate road names are separated by a /. The list is ordered from the first affected road to the last one that the incident lies on.
public var affectedRoadNames: [String]?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Android also maps lanesClearDesc property. I managed to find out that this is some kind of a value extracted from telemetry and mapped by NN "as is", but I could not find out what it exactly means, what values it may or may not contain and if it was meant for user display at all. Also, Directions object does not have any analog of that field.
Given that I decided not to expose a property which we cannot support or even explain even if Android does so.

@Udumft Udumft marked this pull request as ready for review March 31, 2022 12:12
@Udumft Udumft requested a review from a team March 31, 2022 12:12
@mapbox mapbox deleted a comment from purew Mar 31, 2022
@Udumft Udumft merged commit 9daac3f into main Mar 31, 2022
@Udumft Udumft deleted the vk/1040-incidents-fields branch March 31, 2022 15:06
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.

None yet

3 participants