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

Refactor restriction parsing to consider all tags on a relation #2833

Closed
karenzshea opened this issue Sep 1, 2016 · 1 comment
Closed

Refactor restriction parsing to consider all tags on a relation #2833

karenzshea opened this issue Sep 1, 2016 · 1 comment
Assignees
Milestone

Comments

@karenzshea
Copy link
Contributor

From #2802 (comment)

Today in some circumstances (restriction=no_* lead to false and then restriction:<>=only__ leads to true) the last restriction value of type only will override the previous ones regarding the variable is_only_restriction (which will be set as true). With this patch it will be even more visible and the last restriction will always override is_only_restriction. To solve this already existing case will require some additional steps/a rewriting of the whole algorithm. But at a first step and to close this issue ignoring the second if and adding only the else statement will solve the current issue (i.e. ignore not-supported restrictions) without changing the existing behavior.

Turn restriction extraction currently has some "last processed restriction wins" logic. My understanding is that in a scenario where a relation is tagged with both a restriction=no_turn and a restriction:bicycle=only_right_turn, if the restriction=no_turn is looped over last, the relation will be understood as a restricted turn, even if it's actually a valid turn for bicycles.

Solution would be to holistically consider tags on a relation to determine a restriction for the given profile.

@daniel-j-h
Copy link
Member

I think the issue actually goes deeper. We handle all tags prefixed with restriction as in restriction*= instead of only restriction=, have a look at the KeyPrefixFilter here:

osmium::tags::KeyPrefixFilter filter(false);
filter.add(true, "restriction");

This has to change completely based on the profile we're using if we want to support restriction:<type>= tags.

Quick fix is to change the KeyPrefixFilter to a KeyFilter.
And only support restriction={no_,only_}*.

Seems like there are not that many restrictions following this scheme anyway e.g. for the tag in question restriction:bicycle: http://taginfo.openstreetmap.org/keys/restriction%3Abicycle

@daniel-j-h daniel-j-h added this to the 5.4.0 milestone Sep 2, 2016
@MoKob MoKob modified the milestones: 5.5.0, 5.4.0 Sep 6, 2016
daniel-j-h added a commit that referenced this issue Sep 9, 2016
Takes a stricter aproach for whitelisting / blacklisting restrictions:

- uses `restriction=`
- uses more specific `restriction:<type>=`
- uses `except=<type>` to invert

Where `type` is the type of transportation to restrict, e.g. `motorcar`.

#2833
daniel-j-h added a commit that referenced this issue Sep 9, 2016
Takes a stricter aproach for whitelisting / blacklisting restrictions:

- uses `restriction=`
- uses more specific `restriction:<type>=`
- uses `except=<type>` to invert

Where `type` is the type of transportation to restrict, e.g. `motorcar`.

#2833
daniel-j-h added a commit that referenced this issue Sep 20, 2016
Takes a stricter aproach for whitelisting / blacklisting restrictions:

- uses `restriction=`
- uses more specific `restriction:<type>=`
- uses `except=<type>` to invert

Where `type` is the type of transportation to restrict, e.g. `motorcar`.

#2833
daniel-j-h added a commit that referenced this issue Sep 27, 2016
Takes a stricter aproach for whitelisting / blacklisting restrictions:

- uses `restriction=`
- uses more specific `restriction:<type>=`
- uses `except=<type>` to invert

Where `type` is the type of transportation to restrict, e.g. `motorcar`.

#2833
@MoKob MoKob closed this as completed in bbbbacb Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants