Warn about direction=forward/backward on invalid nodes.

[skyper]

What steps will reproduce the problem?

1. Have an child end node or a child middle node of more than one way with [high|rail|water]way] and ^[.+:]?direction$=[forward|backward]
2. run validator

What is the expected result?

A warning about invalid values of direction for the geometry

What happens instead?

No warning

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

See #20013 for the origin.

Replying to Klumbumbus:

I think the validator should warn if a node with direction=* is

• on a ways first/last node (possible in mapcss)
• belongs to two different highway=* ways (not possible in mapcss)

• Only warn about values forward and backward. Other values are valid.
• Include railway and waterway in both test.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-10-31 01:21:14 +0100 (Sat, 31 Oct 2020)
Revision:17288
Build-Date:2020-10-31 02:30:50
URL:https://josm.openstreetmap.de/svn/trunk

[Famlam]

Personally I have this in my custom rules, perhaps it helps? It also covers the 'two-different-highway' case with just mapcss.

node[direction=~/^(forward|backward)$/]:connection,
node["traffic_signals:direction"=~/^(forward|backward)$/][highway=traffic_signals]:connection {
  throwWarning: tr("Node with {0} on a connection of multiple ways", "{0.key}");
}

p.s.: obviously it can be made more generic using just a single regex; something like (untested):
node[/(^|:)direction$/=~/^(forward|backward)$/]:connection

[GerdP]

Didn't try it, but the rule doesn't care about the tags of the parent ways. It probably produces false positives when ways are sharing nodes? For example when landuse=* polygons are glued to highways?

[Famlam]

Indeed, but I don't expect many (i.e.) grass fields or residential borders to have (i.e.) (directional) traffic signals to enter or leave that landuse area, so probably those are all false positives. (Except "keep off the grass" signs perhaps, but even then direction only refers to the highway-way direction, unless anticlockwise grass-area walking is permitted ;) )

[GerdP]

Here is an example for what I had in mind:
​https://www.openstreetmap.org/node/1465196576

[Famlam]

Hmm, ok, personally I would argue give_way is unrelated to (and thus shouldn't be connected to) the landuse. But I see your point, and unfortunately mapcss indeed lacks count_parents_with_tag or such.

[Klumbumbus]

Replying to GerdP:

Here is an example for what I had in mind:
​https://www.openstreetmap.org/node/1465196576

Yes, thats what I meant. The validator should't warn about this node.

[GerdP]

So its going to be a new test coded in java.

[GerdP]

I've started to implement this.
I think if we find a node with .*[:]?direction=[forward|backward] and no parent way this should also be flagged:
Unconnected node with {0}
if the node is inside a download area. I am not sure what to do if there are parent ways but none has a key that matches the pattern [high|rail|water]way]. Ignore them to avoid false positives? Or create an information?

[GerdP]

alpha version, please suggest improvements regarding the i18n strings

[GerdP]

+        super(tr("Direction nodes"), tr("Check for nodes which have a direction"));
+                        .message(tr("Unconnected node with {0}", key)).build());
+                        .message(tr("Node with {0} on end of way", key)).build());
+                    .message(tr("Node with {0} on a connection of multiple ways", key)).build());

• I tend to think that it would be better to show the full tag instead of only the key in the error messages, since values other than forward/backward are allowed.
• I don't like "Direction node" but found no better phrase yet.
• What about closed highways where the node is the start+end node?

[skyper]

Please, always warn about new nodes (id=0)

If the node is new and/or inside download area it needs be a middle node of only one way with special primary key.

Have to check data but so far I only found man_made=pipeline and power=* as possible additional candidates. What other tags exist for infrastructure on ways?

[GerdP]

• reports full tag
• handles new nodes
• adds man_made=pipeline as allowed parent way
• complains if no allowed parent way was found (as "Unconnected node", searching for a better text)

I don't yet understand power=*. Do you have an example?

[Famlam]

Minor thing: MULLTIPLE_WAYS_CODE typo in the variable name ;)

[GerdP]

Thanks, will correct that.
BTW: Here is a link to find some test data with overpass: ​http://overpass-turbo.eu/s/ZJI

[GerdP]

The regex doesn't work yet, it also complains about xyzdirection=forward.

[GerdP]

20019.3.patch:

• combine warnings in group Invalid usage of direction on node
• only check keys if equal to direction or ending with :direction
• suppress Unconnected node with {0} message when node is connected to ways which are not "suitable":

      private static boolean isSuitableParentWay(Way w) {
          return w.hasKey("highway", "railway", "waterway") || w.hasTag("man_made", "pipeline");
      }
  

[Famlam]

The regex doesn't work yet, it also complains about xyzdirection=forward.

The regex should've been (^|:)direction, meaning "starts with OR contains ':', followed by 'direction', but this looks like a good solution too :)

Not sure what the overpass query checks, but it marks nearly every traffic light with direction in Nijmegen (NL), also good ones?

[GerdP]

The overpass query checks nothing, it just collects input for the test. Once the basic code is accepted I'll try to prepare a test file for unit tests.

[skyper]

Replying to GerdP:

I don't yet understand power=*. Do you have an example?

No, do not have an example. I was thinking about infrastructure on ways with nodes describing a lock or gate in one direction. As there are electric components like diodes, I thought that there is a chance one of these tags is used on nodes with a parent way power=*.

Have to dig around in France as I did not find a query to run on Europe or the globe, yet.

[francois.lacombe]

Thank you for notice :)

I think there is not such object on power distribution nor transmission networks. Power could go forward and backward depending on loads. On meshed networks it's totally intermittent.
Power flows are usually directed by adjusting the impedance of lines not with diodes or one-way other component of any kind.

It's ok on pipelines since we find non-return valves.

[skyper]

How about suggesting: Use cardinal directions or numbers, instead, or move relevant tags onto a middle way node connected to only one way with tags highway=*, railway=*, waterway=* or man_made=pipeline.

[Famlam]

How about suggesting: Use cardinal directions or numbers, instead, or move relevant tags onto a middle way node connected to only one way with tags highway=*, railway=*, waterway=* or man_made=pipeline.

I'm not sure if I would understand middle way node or the sentence at all, sorry :)
I personally think the error messages are clear as they are in the patch

[GerdP]

Cardinal numbers would be wrong. They are used when you map the position of the traffic-sign as a standalone node. Here we map the meaning of the sign for the way.

[skyper]

Convinced me, both of you. Always forget that I cannot treat direction and *:direction equally.

[Don-vip]

Milestone renamed

[GerdP]

In 17349/josm:

see #20019: Warn about direction=forward/backward on invalid nodes.

• implement new test DirectionNodes which uses error codes 4000-4099

TODO: unit test

[GerdP]

In 17352/josm:

see #20019: Warn about direction=forward/backward on invalid nodes.

• fix parenthesis

[stoecker]

In 17368/josm:

fix i18n, see #20019

[GerdP]

Oops, thanks!

[stoecker]

jenkins/job/JOSM-i18n/1950/ ;-)

[GerdP]

I started to collect test cases for unit tests. I wonder what to with traffic_sign=city_limit like ​https://www.openstreetmap.org/node/427123883. It produces
Invalid usage of direction on node - Node with direction=backward on a connection of multiple ways (1)
I think this is a false positive, but maybe I'm wrong?

I also see lots of railway=buffer_stop with further tags like ​https://www.openstreetmap.org/node/656590415 which produce either

• Node with railway:signal:direction=forward on end of way (161) or
• Node with railway:signal:direction=backward on end of way (64)

I don't know much about railway mapping, so maybe this needs an exception?

[skyper]

What happens if you reverse the direction of one connected way. Is it denied or do I get a warning?

Still think forward/backward on nodes are ticking time bombs and instead of adjusting each editor software a proper tagging and/or tagging system is the cleaner solution.

city_limits are even more special as they can be valid for two directions. Still forward/backward will not work unless both ways are reversed and if these ways have several nodes with forward/backward more ways have to be taken into account which might be even not downloaded.

[GerdP]

Replying to skyper:

What happens if you reverse the direction of one connected way. Is it denied or do I get a warning?

A dialog pops up, but it doesn't warn that the node is connected to other ways.

Still think forward/backward on nodes are ticking time bombs and instead of adjusting each editor software a proper tagging and/or tagging system is the cleaner solution.

city_limits are even more special as they can be valid for two directions. Still forward/backward will not work unless both ways are reversed and if these ways have several nodes with forward/backward more ways have to be taken into account which might be even not downloaded.

I've also asked in the German Forum because it was discussed this year, see ​https://forum.openstreetmap.org/viewtopic.php?id=70180

[aceman]

Hi, I get this new warning on a junction of a footway and a road where there is a traffic_lights crossing. It is common to tag that shared now with highway=traffic_signals and crossing=traffic_signals. The traffic_signals:direction should only apply to the road direction, not for pedestrians. If this warning is to stay, what is the correct way to tag this?

[GerdP]

I also think the wording is too aggressive. In a discussion in the German forum I suggested to change it to something like "Disputed usage" instead of "Invalid usage".

[GerdP]

In 17411/josm:

fix #20019: Warn about direction=forward/backward on invalid nodes.

• show error for unconnected node Unconnected node with {0}. Use angle or cardinal direction
• show warning for node that is connected, but not to a suitable way Node with {0} should be connected to a linear way
• show information for Node with {0} on end of way and Node with {0} on a connection of multiple ways, both in group Disputed usage of direction on node
• special handling for highways: if there is a major highway as defined in Highways.CLASSIFIED_HIGHWAYS, ignore minor ways like footway, path to reduce false positives at traffic lights.

[GerdP]

In 17412/josm:

see #20019: Warn about direction=forward/backward on invalid nodes.

• simplify source
• improve test coverage

[GerdP]

In 17413/josm:

see #20019: Warn about direction=forward/backward on invalid nodes.

• revert changes in DirectionNodes.java from 17412
• improve test coverage (nodes with positive ids)

[GerdP]

Please try trunk/nodist/data/direction-nodes.osm​ and also some real world data.
At the moment, the value in i18n Strings {0} contains the tag. Maybe it would be better to use tag key + =forward/backward?

[skyper]

Actually, I like the tags in the string, as I do not care about railway tagging that much and you have individual warnings to add to the ignore list.

[GerdP]

I don't understand this comment. How is railway tagging related? The strings are not relevant for the ignore list.

[skyper]

It makes a difference if railway:signal:direction or traffic_signals:direction is warned about. Maybe the warning could be split into several depending on the major tag of the parent ways?

[GerdP]

I think you got comment:42 wrong. I wonder if we should report two different error messages for forward and backward.
You probably never want to ignore only the forward message?

[skyper]

Yes, you are right, sorry. We only need the key and not the tag (key_value). The values can be directly in the message or even left out.

[GerdP]

I think we need them in the message to distinguish fron the angle or cardinal direction

[GerdP]

In 17415/josm:

see #20019: Warn about direction=forward/backward on invalid nodes.

• use tag key + "=forward|backward" in messages so that we don't get separate messages for each direction