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

pass flags into process_segment #6658

Merged
merged 6 commits into from
Apr 6, 2024

Conversation

mattwigway
Copy link
Contributor

Issue

This partially addresses #6145, by making the extractor edge flags available to process_segment as segment.flags. This allows accessing road classification, start point, etc. in process_segment. I'm using it by tagging bridges/tunnels as non-startpoints and then not applying elevation weighting to them since they are flat (ish) where I am but often cross deep valleys.

I'm not quite sure how to write an automated test for this. Maybe a profile that uses the flags information in a cucumber test?

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@mattwigway mattwigway marked this pull request as ready for review August 2, 2023 13:11
Copy link
Member

@mjjbell mjjbell left a comment

Choose a reason for hiding this comment

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

I'm not quite sure how to write an automated test for this. Maybe a profile that uses the flags information in a cucumber test?

Yes, take a look at https://github.com/Project-OSRM/osrm-backend/blob/master/features/options/extract/lua.feature and https://github.com/Project-OSRM/osrm-backend/blob/master/features/options/extract/turn_function.feature.

You could write a test that uses the flags within the profile's process_segment function and confirm you get the expected result.

include/extractor/extraction_segment.hpp Outdated Show resolved Hide resolved
include/extractor/extraction_segment.hpp Outdated Show resolved Hide resolved
@mattwigway
Copy link
Contributor Author

I'm having some trouble getting the Cucumber tests running to finish this, which seems to have to do the node OSRM library. It looks like it's trying to find it locally, failing, and then trying to grab it from github, which fails as well since it's a non-release version. Do I need to do something to build the Node.js bindings locally?

@mattwigway
Copy link
Contributor Author

Okay, I figured out the Cucumber tests and added a test that confirms that segment flags get passed through to process_segment. @mjjbell let me know if anything else is needed and my apologies for the delay.

Copy link
Member

@mjjbell mjjbell left a comment

Choose a reason for hiding this comment

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

LGTM. Please add a changelog entry.

@mattwigway
Copy link
Contributor Author

@mjjbell I added the changelog entry and rebased to avoid merge conflicts in changelog. Thanks!

@mjjbell mjjbell merged commit 7ebd21f into Project-OSRM:master Apr 6, 2024
20 checks passed
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.

2 participants