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.