fix: prevent conditional tags from overriding implicit pedestrian roa…#3324
Open
pgrigolli wants to merge 1 commit intographhopper:masterfrom
Open
fix: prevent conditional tags from overriding implicit pedestrian roa…#3324pgrigolli wants to merge 1 commit intographhopper:masterfrom
pgrigolli wants to merge 1 commit intographhopper:masterfrom
Conversation
Author
|
@karussell could you please check if this is ok? |
Member
|
Hi! Thanks, but I think this isn't it. Technically speaking, your second test already succeeds before your patch (we are not in fact opening anything for "delivery" exemptions here, despite what it looks like in the ticket), and your second test only tests one side (that we aren't opening "part-time" pedestrian zones anymore), but it doesn't show what we do open. Less technically: What's the difference between a pedestrian zone with "motor_vehicle=no" and one without? Shouldn't they be treated the same when a conditional like this hits them? The question is more like should a pedestrian zone that is only open for cars for parts of the day be open or closed if we can only pick one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3308
highway=pedestrian roads were being routed for cars when a motor_vehicle:conditional tag was present, even without any explicit access tag. For example, a road tagged with:
was being treated as accessible for cars, causing the router to send vehicles through pedestrian areas instead of calculating a detour.
Root cause
The if block responsible for blocking cars on pedestrian roads calls hasPermissiveTemporalRestriction as its third condition. This method was returning true whenever a conditional tag contained a value present in allowedValues (such as delivery) and a time range — regardless of whether the restriction on the road was explicit (access=no) or implicit (derived from highway=pedestrian). This caused the CAN_SKIP return to be skipped, leaving the road accessible for cars.
Fix
Added a private method hasPermissiveTemporalRestrictionForPedestrian in CarAccessParser that adds a guard before calling hasPermissiveTemporalRestriction — only considering conditional tags if an explicit restriction (access=no or motor_vehicle=no) is present on the way. The original hasPermissiveTemporalRestriction method was left untouched to avoid breaking other parsers that depend on it (bike, temporal access).