-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
highway=path and bicycle=yes should not get speed for pushing section #1892
highway=path and bicycle=yes should not get speed for pushing section #1892
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that we should increase speed for bicycle=yes & highway=path but should we really use such a high speed of 14km/h on a highway=footway only because bicycle is allowed there?
I.e. should we make a difference to bicycle=designated?
There is a difference to bicycle=designated, here we expect 18km/h, which is the cycleway speed. This seems correct to me. If you feel 14km/h is too much for bicycle=yes, what would you suggest? |
Yes, the sign is correct. I reduced the speed for this classification to 10 km/h as suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
Can you re-format the code in BikeCommonFlagEncoder.java? (some missing spaces and some too much)
if (way.hasTag("bicycle", "yes")) | ||
speed = 10; | ||
else | ||
speed = highwaySpeeds.get("cycleway"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For nested if-else blocks can you add the brackets?
I added brackets for the nested ifs and improved the identations. |
See https://discuss.graphhopper.com/t/bike-profil-highway-pathpath-and-bicycle-yes-should-not-get-speed-for-pushing-section/5148