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

[WIP] Add text instruction parsing via osrm-text-instructions #187

Merged
merged 2 commits into from
Sep 21, 2016

Conversation

freenerd
Copy link
Member

@freenerd freenerd commented Sep 7, 2016

First pass at integrating https://github.com/Project-OSRM/osrm-text-instructions.js

Todo:

@freenerd
Copy link
Member Author

freenerd commented Sep 7, 2016

For comparison, here are old (left) and new (right) instructions side by side:

screen shot 2016-09-07 at 18 28 36

@freenerd freenerd force-pushed the osrm-text-instructions branch 2 times, most recently from 4a9bd1a to 034bc83 Compare September 20, 2016 14:55
@freenerd
Copy link
Member Author

osrm-text-instructions has made progress in the last weeks and feels fairly stable now. There are def more cases we want to support, but it's good enough for the basic case here. To get more feedback we'd love to get it live on map.project-osrm.org now.

I also continued on the hacky path to get lanes support in.

It's not pretty, but works a bit. The problem with integrating with LRM is that during the HTML building, I need access to the original step object from OSRM to decide on lanes. Since the other supported routing engines do not support lanes, Im not sure if this should become a LRM-supported function. If so, it will need quite a lot of refactoring on the LRM side.

Old Left, New Right

screen shot 2016-09-20 at 16 48 48

@freenerd freenerd force-pushed the osrm-text-instructions branch from 0fc638b to 6a43796 Compare September 21, 2016 11:14
@freenerd
Copy link
Member Author

I released osrm-text-instructions@0.0.1 which includes destinations as well, so I think it's ready to go for now.

screen shot 2016-09-21 at 14 25 30

@perliedman I'd love to find ways of getting this work upstream into LRM. Likely integrating osrm-text-instructions will be easy-ish, since it's only some text. But lanes are harder, since they need the original step.intersections.lanes while building the html. I couldn't find easy ways of integrating them, so need to defer back to you on that.

I'll merge this hacky version for now to get the feature out. Let's continue to discussion on how to make it better :)

@freenerd freenerd merged commit 7bf96cb into gh-pages Sep 21, 2016
@freenerd freenerd deleted the osrm-text-instructions branch September 21, 2016 12:27
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