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

Expose exits on RouteSteps #385

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahmedre
Copy link
Contributor

@ahmedre ahmedre commented Dec 3, 2024

No description provided.

@ahmedre
Copy link
Contributor Author

ahmedre commented Dec 3, 2024

I was checking and exits can be in 2 places - one is here (which already exists), and the second is inside of the various BannerInstruction.BannerContent (for primary, secondary, and sub). For the second place, I can open another PR - should we expose BannerContents instead?

In a previous patch, we mapped various fields / types within the BannerContent into lanes, so wondering if this is the approach we should continue (i.e. add fields denoting the exit-numbers fields in addition to lanes, etc), or if we want to keep lanes and also add the actual info, or if we want to remove lanes altogether in favor of just exposing what is now parsed as Vec<BannerContentComponent>?

i.e. option 1:

pub struct VisualInstructionContent {
    pub text: String,
    pub maneuver_type: Option<ManeuverType>,
    pub maneuver_modifier: Option<ManeuverModifier>,
    pub roundabout_exit_degrees: Option<u16>,
    pub lane_info: Option<Vec<LaneInfo>>,
    pub exit_numbers: Option<Vec<String>>,
}

or option 2:

pub struct VisualInstructionContent {
    pub text: String,
    pub maneuver_type: Option<ManeuverType>,
    pub maneuver_modifier: Option<ManeuverModifier>,
    pub roundabout_exit_degrees: Option<u16>,
    pub lane_info: Option<Vec<LaneInfo>>,
    pub details: Option<Vec<BannerContentComponent>>,
}

or option 3 being option 2 with removing lane_info.

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.

1 participant