Add encoded value for mountainbike route relation handling #2904
Conversation
|
I used the following way as a real-world testcase: https://www.openstreetmap.org/way/169096949. It is part of a regional bike relation and of two mountainbike relations:
|
There was a problem hiding this comment.
Do I understand it correctly that mtb_network is introduced but not used for the mtb parsers?
Couldn't we omit the mtb_network handling in the parser but create a custom model mtb.json and use this with mtb_network and bike_network? Or even better try to get rid of mtb parsers completely so that we only have a custom model? Shall I try this and propose a change?
|
|
||
| import com.graphhopper.routing.ev.RouteNetwork; | ||
|
|
||
| public class BikeNetworkParser { |
There was a problem hiding this comment.
Can you move the determine method into OSMBikeNetworkTagParser? (Or rename this to BikeNetworkParserHelper as I would avoid the "Parser" naming here as it does not implement TagParser)
There was a problem hiding this comment.
I choose to rename it into BikeNetworkParserHelper as suggested
Yes. Usage is supposed to be in a custom model. I tested with
As I do not quite understand what you mean and I don't see the relation to the PR I would ask for a proposal. Isn't this something unrelated? |
| @@ -15,16 +15,16 @@ public class MountainBikePriorityParser extends BikeCommonPriorityParser { | |||
| public MountainBikePriorityParser(EncodedValueLookup lookup, PMap properties) { | |||
| this(lookup.getDecimalEncodedValue(VehicleSpeed.key(properties.getString("name", "mtb"))), | |||
| lookup.getDecimalEncodedValue(VehiclePriority.key(properties.getString("name", "mtb"))), | |||
| lookup.getEnumEncodedValue(BikeNetwork.KEY, RouteNetwork.class)); | |||
There was a problem hiding this comment.
Yes. Usage is supposed to be in a custom model.
But have a look into this change. Shouldn't we keep using BikeNetwork here then? Otherwise normal (and more frequent) route=bicycle tags will be ignored.
There was a problem hiding this comment.
Now that you ask I agree. I was unsure about it when writing this line. But it makes sense to keep the current behaviour in regards of prioritization for the more frequent route=bicycle tags. I changed it in the PR.
There was a problem hiding this comment.
Ok, thanks for your explanation! Can you also revert the change in MountainBikePriorityParser or is this somehow required?
There was a problem hiding this comment.
Thank you, I forgot that. Now I reverted the network priority changes in MountainBikePriorityParser.
Triggered by a user request in https://discuss.graphhopper.com/t/mtb-network-is-missing/8259/2 I implemented support for mountainbike relation handling in this PR