Modify

Opened 17 months ago

Closed 12 months ago

Last modified 12 months ago

#20442 closed enhancement (fixed)

[PATCH] Normal highway=traffic_signals on junction=roundabout is never correct

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 21.06
Component: Core validator Version:
Keywords: template_report Cc:

Description

What steps will reproduce the problem?

  1. Create closed way with highway=primary junction=roundabout
  2. Tag one of its nodes with highway=traffic_signals
  3. Run validator

What is the expected result?

Validator complains "Normal traffic signals are impossible on roundabout"

What happens instead?

Nothing

Please provide any additional information below. Attach a screenshot if possible.

"A roundabout is a generally circular (self-intersecting) road junction where the traffic on the roundabout has always right of way." - so traffic signals are impossible there.

It is a common mistake to tag thing called "Roundabout ZYX" for historical reasons (because such junction used to be a roundabout) as junction=roundabout

See https://www.openstreetmap.org/changeset/98319133 where I fixed such incorrect tagging.

Note that traffic_signals with value other than signal should be skipped.

Note traffic_signals=continuous_green, traffic_signals=emergency that definitely can appear on roundabout - maybe also other?

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-01-21 23:33:21 +0100 (Thu, 21 Jan 2021)
Revision:17474
Build-Date:2021-01-22 02:30:49
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (17474 en) Linux Ubuntu 20.04.1 LTS
Memory Usage: 714 MB / 3974 MB (290 MB allocated, but free)
Java version: 11.0.9.1+1-Ubuntu-0ubuntu1.20.04, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Desktop environment: LXQt
Java package: openjdk-11-jre:amd64-11.0.9.1+1-0ubuntu1~20.04
Java ATK Wrapper package: libatk-wrapper-java:all-0.37.1-1
Environment variable LANG: en_US.UTF-8
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:-
Dataset consistency test: No problems found

Plugins:
+ reverter (35688)
+ todo (30306)

Validator rules:
+ https://josm.openstreetmap.de/josmfile?page=Rules/OSMLint&zip=1
+ ${HOME}/Documents/install_moje/OSM software/manual editing and discussions/josm/resources/data/validator/deprecated.mapcss

Attachments (3)

20442.patch (829 bytes) - added by reichg 13 months ago.
20442_v2.patch (819 bytes) - added by reichg 13 months ago.
switched logic so child node would be selected.
20442_v3.patch (857 bytes) - added by Gabe 13 months ago.
moved to highway.mapcss

Download all attachments as: .zip

Change History (18)

comment:1 Changed 13 months ago by reichg

So only nodes on a roundabout with highway=traffic_signals or traffic_signals=signal should be warned about?

comment:2 in reply to:  1 Changed 13 months ago by reichg

Replying to reichg:

So only nodes on a roundabout with highway=traffic_signals or traffic_signals=signal should be warned about?

Can we confirm all of the tag combinations that need to be warned about?

comment:3 Changed 13 months ago by Famlam

Something like this?

way[highway][junction=roundabout] > node[highway=traffic_signals][traffic_signals=signal],
way[highway][junction=roundabout] > node[highway][highway=~/^(traffic_signals|stop|give_way)$/][!traffic_signals] {
  throwWarning: tr("{0} on a road with {1}", "{0.tag}", "junction=roundabout");
  group: tr("suspicious tag combination");
  suggestAlternative: "junction=circular"
}

comment:4 in reply to:  3 Changed 13 months ago by reichg

Replying to Famlam:

Something like this?

way[highway][junction=roundabout] > node[highway=traffic_signals][traffic_signals=signal],
way[highway][junction=roundabout] > node[highway][highway=~/^(traffic_signals|stop|give_way)$/][!traffic_signals] {
  throwWarning: tr("{0} on a road with {1}", "{0.tag}", "junction=roundabout");
  group: tr("suspicious tag combination");
  suggestAlternative: "junction=circular"
}

Yup this looks good. Tested it moderately. Had to look into "give_way" specifically for yield signs at roundabout entrances but found they should not be ON the roundabout but on a node PRIOR to entering the roundabout. I will add the patch if there is nothing else for this ticket

Changed 13 months ago by reichg

Attachment: 20442.patch added

comment:5 Changed 13 months ago by reichg

Summary: Normal highway=traffic_signals on junction=roundabout is never correct[PATCH] Normal highway=traffic_signals on junction=roundabout is never correct

comment:6 Changed 13 months ago by skyper

I would select the way and not the node, e.g. using parent selector < instead of the child selector.

Searched for an option to select both but did not find any within mapcss. Was hoping to get the trick with osm_id() and parent_osm_id() but I miss a selection command.

comment:7 in reply to:  6 Changed 13 months ago by skyper

Replying to skyper:

I would select the way and not the node, e.g. using parent selector < instead of the child selector.

Searched for an option to select both but did not find any within mapcss. Was hoping to get the trick with osm_id() and parent_osm_id() but I miss a selection command.

Wait a minute, it is working with the parent selector:

/* roundabout without right of way, see #20442 */
node[highway=traffic_signals][traffic_signals=signal] < way[highway][junction=roundabout],
node[highway][highway=~/^(traffic_signals|stop|give_way)$/][!traffic_signals] < way[highway][junction=roundabout] {
  throwWarning: tr("{0} without right of way", "{1.tag}");
  group: tr("suspicious tag combination");
  suggestAlternative: "junction=circular"
}
Last edited 13 months ago by skyper (previous) (diff)

comment:8 Changed 13 months ago by mkoniecz

So only nodes on a roundabout with highway=traffic_signals or traffic_signals=signal should be warned about?

When I was looking at docs any other traffic signals were theoretically possible.

Though I would warn also about highway=traffic_signals without traffic_signals value, as traffic_signals=signal is added mostly by iD users I think.

And highway=traffic_signals is a common way to mark a regular signal. Or maybe not if it would be potentially confusing and unclear that more specific value can be tagged on it?

Last edited 13 months ago by mkoniecz (previous) (diff)

comment:9 Changed 13 months ago by mkoniecz

throwWarning: tr("{0} without right of way", "{1.tag}");

Will it show "junction=roundabout without right of way" or something else?

comment:10 Changed 13 months ago by Famlam

@mkoniecz,
All your concerns are dealt with in either patch already. And yes, it'll show that.

@Skyper,

node[highway=traffic_signals][traffic_signals=signal] < way[highway][junction=roundabout],
node[highway][highway=~/^(traffic_signals|stop|give_way)$/][!traffic_signals] < way[highway][junction=roundabout] {
  throwWarning: tr("{0} without right of way", "{1.tag}");
  group: tr("suspicious tag combination");
  suggestAlternative: "junction=circular";
}

Looks good to me!
(I added a semicolon after the last line; my fault that it was missing...)

Changed 13 months ago by reichg

Attachment: 20442_v2.patch added

switched logic so child node would be selected.

comment:11 Changed 13 months ago by skyper

@Gabe:
Thanks for the patch file but as this is directly related to highway, it might better fit in highway.mapcss like the first patch.

comment:12 in reply to:  11 Changed 13 months ago by anonymous

Replying to skyper:

@Gabe:
Thanks for the patch file but as this is directly related to highway, it might better fit in highway.mapcss like the first patch.

Yes of course! My mistake!

Changed 13 months ago by Gabe

Attachment: 20442_v3.patch added

moved to highway.mapcss

comment:13 Changed 13 months ago by reichg

Can this patch be merged?

comment:14 Changed 12 months ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 17941/josm:

fix #20442 - Warn about traffic signals or similar on roundabouts (patch by mkoniecz, Famlam, reichg, skyper and Gabe :))

comment:15 Changed 12 months ago by Klumbumbus

Milestone: 21.06

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.