Modify

Opened 2 years ago

Last modified 6 weeks ago

#20832 new defect

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?

  1. 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 (2)

r12657026.osm (4.7 KB) - added by simon04 2 years ago.
20832.patch (3.2 KB) - added by taylor.smock 20 months ago.
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.

Download all attachments as: .zip

Change History (15)

Changed 2 years ago by simon04

Attachment: r12657026.osm added

comment:1 Changed 2 years ago by simon04

Cc: GerdP added

Related: #17709

comment:2 Changed 2 years ago by Famlam

Also: #19722

comment:3 Changed 2 years ago by skyper

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.

Last edited 2 years ago by skyper (previous) (diff)

comment:4 Changed 2 years ago by Famlam

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 Changed 2 years ago by skyper

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 Changed 2 years ago by Famlam

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.

comment:7 Changed 21 months ago by skyper

Ticket #20109 has been marked as a duplicate of this ticket.

comment:8 Changed 20 months ago by skyper

Ticket #21363 has been marked as a duplicate of this ticket.

Changed 20 months ago by taylor.smock

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:9 Changed 20 months ago by skyper

I think it makes more sense to (re-)write this test in mapcss.

comment:10 Changed 20 months ago by GerdP

Owner: changed from team to GerdP

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 Changed 20 months ago by skyper

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.

Last edited 20 months ago by skyper (previous) (diff)

comment:12 Changed 20 months ago by skyper

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.

Last edited 20 months ago by skyper (previous) (diff)

comment:13 Changed 6 weeks ago by gaben

Cc: gaben added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain GerdP.
as The resolution will be set.
to The owner will be changed from GerdP to the specified user.
The owner will change to simon04
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from GerdP to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.