Opened 4 years ago
Last modified 5 months ago
#20832 new defect
[patch] Unknown turn restriction for restriction:hgv:conditional=no_left_turn
Reported by: | simon04 | Owned by: | GerdP |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core validator | Version: | |
Keywords: | template_report turn restriction conditional | Cc: | GerdP, gaben |
Description
What steps will reproduce the problem?
- Validate https://www.openstreetmap.org/relation/12657026 (
type=restriction
restriction:hgv:conditional=no_left_turn @ (20:00 - 08:00)
)
What is the expected result?
No validation error?
What happens instead?
Error "Unknown turn restriction"
Please provide any additional information below. Attach a screenshot if possible.
Reported via https://app.element.io/#/room/#_oftc_#josm:matrix.org/$1619901120674395AzAOS:matrix.org
Revision:17855 Is-Local-Build:true Build-Date:2021-05-02 11:46:51 Identification: JOSM/1.5 (17855 SVN en) Mac OS X 11.3 OS Build number: macOS 11.3 (20E232) Memory Usage: 235 MB / 2048 MB (55 MB allocated, but free) Java version: 16.0.1+9, Azul Systems, Inc., OpenJDK 64-Bit Server VM Look and Feel: com.formdev.flatlaf.FlatLightLaf Screen: Display 1 1440×900 (scaling 2.00×2.00) Display 2 3008×1692 (scaling 2.00×2.00) Maximum Screen Size: 3008×1692 Best cursor sizes: 16×16→16×16, 32×32→32×32 Environment variable LANG: en_IE.UTF-8 System property file.encoding: UTF-8 System property sun.jnu.encoding: UTF-8 Locale info: en_AT Numbers with default locale: 1234567890 -> 1234567890
Attachments (4)
Change History (20)
by , 4 years ago
Attachment: | r12657026.osm added |
---|
comment:1 by , 4 years ago
Cc: | added |
---|
comment:3 by , 4 years ago
Is it really useful to have a tag for every vehicle type?
E.g. restriction:conditional=no_left_turn @ (hgv AND 20:00-24:00,00:00-08:00)
does say the same or does it not?
Probably will introduce restriction:conditional
in the scope of #20833.
comment:4 by , 4 years ago
Is it really useful to have a tag for every vehicle type?
This is the same as oneway. Hypothetically all i.e. oneway:bicycle= X
could be converted to oneway:conditional = X @ (bicycle)
(provided it fits the 255 char length limitation). Nevertheless, for clarity, I prefer i.e. oneway:bicycle
over oneway:conditional
. I guess the same applies to restriction:xxx
and restriction:xxx:conditional
versus restriction:conditional = ... @ (xxx)
.
So if you'd ask: is it really strictly necessary, then: no, there are alternatives, indeed. However, is it useful: I'd say definitely yes.
Example:
restriction:hgv
-> 1038 cases
restriction:hgv:conditional
-> 75 cases
restriction:conditional
containing hgv
-> only 6 cases
(similar ratios for i.e. bicycle
and bus
)
comment:5 by , 4 years ago
Interesting, restriction:bicycle=give_way
is dominant (1,686 and 95.58% key's value).
If we would cover all possibility the preset will get too big or we need to split it per vehicle type as we need two additional tags per transport mode, e.g. restriction:motor_vehicle=*
and restriction:motor_vehicle:conditional=*
Another question, are :conditional
type values valid for restriction:[vehicle_type]=*
, e.g. restriction:hgv=no_left_turn @ (00:00-08:00,10:00-24:00)
?
comment:6 by , 4 years ago
If we would cover all possibility the preset will get too big
I don't think the preset has to cover all possibilities as they're quite rare. I think it's sufficient to silence the validator (and definitely don't throw an error) for those cases.
by , 3 years ago
Attachment: | 20832.patch added |
---|
Modify TurnrestrictionTest to account for restriction:mode:conditional
and restriction:mode
. Note: supported modes are currently hardcoded (see #18383, needs update) to a limited set and no tests are written yet.
comment:10 by , 3 years ago
Owner: | changed from | to
---|
I think I added this test to avoid confusing detail messages for unknown restrictions, so the test itself is probably needed anyhow. If we code this again in MapCSS we just have duplicate code.
comment:11 by , 3 years ago
I thought following should work
relation[type=restriction][/^restriction(:|$)/][/^restriction(:(agricultural|bicycle|bus|caravan|foot|hazmat|hgv|horse|motor_vehicle|motorcar|motorcycle|psv|vehicle))?(:condition)?$/ !~ /^(only|no)_(straight_on|u_turn|(left|right)_turn|no_(entry|exit)/] { throwWarning: tr("Unknown turn restriction");
We also have an error (java) and a warning (mapcss) for missing ^restriction(:|$)
, atm. Should raise the warning to error if the java part is dropped.
comment:12 by , 3 years ago
I see, we need both lists for the role checks in the java test. To get rid of duplicates how about an error warning (mapcss) for missing restriction*=*
(#19722) and the rest in java.
comment:13 by , 22 months ago
Cc: | added |
---|
by , 12 months ago
Attachment: | 20832.2.patch added |
---|
comment:14 by , 12 months ago
improved patch, based on 20832.patch,
- catches also invalid vehicle types like restriction:hvg:conditional=*
- special handling for documented restriction:bicycle=stop and restriction:bicycle=give_way
by , 12 months ago
Attachment: | 20832.3.patch added |
---|
comment:15 by , 12 months ago
- add only_u_turn (in wiki since 2020-09-05)
- add some correct examples in unit test data
comment:16 by , 5 months ago
Summary: | Unknown turn restriction for restriction:hgv:conditional=no_left_turn → [patch] Unknown turn restriction for restriction:hgv:conditional=no_left_turn |
---|
Related: #17709