[PATCH] No warning about barrier with inappropriate access tags on highway

[skyper]

What steps will reproduce the problem?

1. Download a motorway way
2. Add a node with barrier=gate
3. Run validator

What is the expected result?

Some kind of warning

What happens instead?

No warning

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

The example is on one edge of the spectrum of tags. Some barriers might be ok for some kind of highways.
Usually, I would expect barriers only on minor roads at least below tertiary but maybe even below residential and unclassified.
But in reality we find barriers on national borders and toll infrastructure, too.

So the access values need to be taken into account.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-04-09 23:18:01 +0200 (Fri, 09 Apr 2021)
Revision:17736
Build-Date:2021-04-10 01:30:58
URL:https://josm.openstreetmap.de/svn/trunk

[reichg]

Can we start a discussion on this? This sounds like it could be pretty complicated with specific requirements.

[reichg]

Not exactly sure what needs to be done here still. Would like a discussion about potential modifications.

[reichg]

Still need a discussion on this to determine what changes need to be made.

[reichg]

Not sure how we can proceed with this if there aren't concrete requirements for this warning.

Are you proposing that on a highway above Tertiary, if barrier=* is present then there also needs an access=* tag accompanying it?

[skyper]

I see two major scenarios here:

• a barrier on a way's middle node where the access tags conflict
• barrier on shared ways' end nodes

Conflicts are not value conflicts but rather the opposite and the access tags on the highway can be stricter than on the barrier but not upside down.

As end nodes are tricky, let's focus on middle nodes. With any highway for motorcars I would expect some access tag on the barrier or the access has to be denied on the highway. I'll see, if I can make up some examples.

I don't expect this test to dip that deep into the matter but finding the easily recognizable main problems could help. The rest is better integrated in #18364.

[reichg]

Replying to skyper:

I see two major scenarios here:

• a barrier on a way's middle node where the access tags conflict
• barrier on shared ways' end nodes

Conflicts are not value conflicts but rather the opposite and the access tags on the highway can be stricter than on the barrier but not upside down.

As end nodes are tricky, let's focus on middle nodes. With any highway for motorcars I would expect some access tag on the barrier or the access has to be denied on the highway. I'll see, if I can make up some examples.

I don't expect this test to dip that deep into the matter but finding the easily recognizable main problems could help. The rest is better integrated in #18364.

Yes, some examples would be good. I am not sure I am understanding the conflict part between access and barrier.

[skyper]

some simple examples

[skyper]

Ok, I've attached some examples​:

• A1 is the simplest case with missing access tag on barrier or highway
• A2 is ok regarding motor_vehicle
• A3 and A4 are ok

Now some examples with end nodes.

• B1 looks suspicious
• B2-B4 are rather ok

What should be warned about are barriers on intersections which is usually wrong, see C.

Hope this helps a bit.

[reichg]

very basic patch 20742_v1 here creating the "suspicious barrier tags" test group. Checks for some basic combinations regarding barrier tag.

The intersection logic might need to be added in java. How can this patch be improved?

[reichg]

added requirement that node must be connected to parent way WITH highway tag.

[Famlam]

This would also trigger on i.e. a bollard on a cycleway, or a cycle barrier on a footway, which doesn't require more tags. I think you must make explicit to only involve major ways (tertiary and above). (For lower car-roads, i.e. residential, it's not uncommon that it contains a bollard midway to prevent traffic from using it as a shortcut; at least in NL)

[reichg]

Are there any other combinations to look out for that I should ignore? Easy fix.

[reichg]

restricts barrier test to major_roads (teriary_link and above).

[reichg]

@Skyper, how detailed do you want this barrier test to be? Keep it at tertiary and above or look into residential/unclassified as well?

[reichg]

Also, can someone explain why patch 20742_v2 does not work when I set major_roads as tertiary and above tags and refer to it in the test? Looks like everything should work? It works as intended when I move the regex highway=~/^(motorway|motorway_link|trunk|trunk_link|primary|primary_link|secondary|secondary_link|tertiary|tertiary_link)$/ into the test without the reference.

[reichg]

moved major highways into the test as a long regex entry.

[reichg]

patch 20742_v2 does not work with separate regex variable. but patch 20742_v3 works with regex entry hard coded without a variable. Little confused.

[skyper]

Replying to reichg:

@Skyper, how detailed do you want this barrier test to be? Keep it at tertiary and above or look into residential/unclassified as well?

Keep it simply, for now, and let us start with tertiary and above, only.

Replying to reichg:

patch 20742_v2 does not work with separate regex variable. but patch 20742_v3 works with regex entry hard coded without a variable. Little confused.

Think I had similar problems working with the external rules PublicTransportGtfs and did not understand it either. Maybe it does not work on the left side? Bug?

[Famlam]

You've discovered #10215 ;)

I would also add [!vehicle] to the node check, to catch (on major highways probably rare) cases where i.e. most vehicles are not permitted, but certain and i.e. foot are. Such a case would be i.e. a busway that transfers into a tertiary

[reichg]

added [!vehicle] to node

[anonymous]

May I propose to also change
throwWarning: tr("{0} tag on node missing complimentary tags such as {1}, {2}, or {3}.", "barrier", "access", "motor_vehicle", "vehicle");
to
throwWarning: tr("{0} without access tags such as {1}, {2}, or {3}.", "{0.tag}", "access", "motor_vehicle", "vehicle");

I.e., a 'bad' bollard would yield:
barrier=bollard without access tags such as access, motor_vehicle, or vehicle.

[Famlam]

And also to be excluded:
barrier=border_control|height_restrictor|toll_booth

[reichg]

adjusted warning

[reichg]

whoops need to address last comment

[reichg]

added barrier!~/border_control|height_restrictor|toll_booth/

[Famlam]

May I propose to combine
[barrier!=entrance][barrier!~/border_control|height_restrictor|toll_booth/]
to
[barrier!~/entrance|border_control|height_restrictor|toll_booth/]?
(or [barrier!~/^(entrance|border_control|height_restrictor|toll_booth)$/])

[skyper]

Replying to anonymous:

May I propose to also change
throwWarning: tr("{0} tag on node missing complimentary tags such as {1}, {2}, or {3}.", "barrier", "access", "motor_vehicle", "vehicle");
to
throwWarning: tr("{0} without access tags such as {1}, {2}, or {3}.", "{0.tag}", "access", "motor_vehicle", "vehicle");

Even following might work:

throwWarning: tr("{0} without {1} tags such as {1}, {2}, or {3}", "{0.tag}", "{1.key}", "{2.key}", "{3.key}");

I.e., a 'bad' bollard would yield:
barrier=bollard without access tags such as access, motor_vehicle, or vehicle.

But, wait a minute. This rule is more about a suspicious barrier or access tags missing on either the barrier or the highway:

throwWarning: tr("Suspicious {0} on {1} without {2} tags such as {2}, {3}, or {4}", "{0.tag}", "highway", "{1.key}", "{2.key}", "{3.key}");

[Famlam]

But, wait a minute. This rule is more about a suspicious barrier or access tags missing on either the barrier or the highway:

This part I don't get? It's just checking the barriers (and doesn't warn if the highway itself is already limited, like access=private on the highway, in which case it's not strictly needed to repeat it on the i.e. bollards)?

[skyper]

Replying to Famlam:

But, wait a minute. This rule is more about a suspicious barrier or access tags missing on either the barrier or the highway:

This part I don't get? It's just checking the barriers (and doesn't warn if the highway itself is already limited, like access=private on the highway, in which case it's not strictly needed to repeat it on the i.e. bollards)?

Damn, I mix-up > and <. < might be the better choice as it selects the barrier and the highway.

[Famlam]

But why would you select the highway? ( I'm still confused)

[skyper]

Sorry, I forgot about the simplicity of this rule and was still thinking about complex cases where access tag mismatch or a highway does not have access tags but the barrier has e.g. motor_vehicle=private.

[reichg]

Replying to Famlam:

May I propose to combine
[barrier!=entrance][barrier!~/border_control|height_restrictor|toll_booth/]
to
[barrier!~/entrance|border_control|height_restrictor|toll_booth/]?
(or [barrier!~/^(entrance|border_control|height_restrictor|toll_booth)$/])

Yeah easy add, would this complete the ticket or is there anything else?

[Famlam]

Yeah easy add, would this complete the ticket or is there anything else?

Thanks!

---

Well, in terms of wording for the group: I don't know if the "barrier tags" are suspicious (which barrier tag*s*, note the plural, as there's only one barrier tag). Perhaps simply "suspicious barrier", as the barrier is the suspicious object

---

I don't think you can do assert(No)Match with a node on a way*; I'd just leave those asserts out.
(* That is, I haven't figured it out)

[reichg]

suspicious barrier and removed asserts.

[skyper]

• I'd still like to get a warning if on a way's middle node, the barrier has access restrictions but the highway does not have any
• Barriers on intersections with more than two highways or on T-intersections (one way's middle node plus one end node) are suspicious, too.

[Famlam]

@reichg, looks good to me :)

@skyper, I think it doesn't prevent this patch from being merged, right?
Both of those should only apply to major roads too:

• a residential with a bollard in the middle is not uncommon; it's just a normal dead-end-for-cars from two sides
• a bollard on a cycleway where a footway connects is also not odd; the cycleway may be split there due to i.e. relations or different values of surface or such, causing false positives.

And then I can only think of very few cases where the current issue wouldn't already say there's something odd at the location of the barrier. Do you have an example perhaps?

[skyper]

No, the patch is fine and one part which can be applied as the first step, thanks.

• please, remove the leading space of the search expression
• as already mentioned the warning can include more parts of the expression:

      throwWarning: tr("{0} without {1} tags such as {1}, {2}, or {3}.", "{0.tag}", "{2.key}", "{3.key}", "{4.key}");
  

For the barrier on middle node of highway=tertiary, I have to dig through my CS-comments. I remember at least two cases:

1. a mapping error using barrier=lift_gate instead of railway=level_crossing, crossing:barrier=*. This is covered by the patch.
2. a mapping error adding barrier=gate, access=private on the intersection with a highway=service instead of on a different node only part of the service road.

Maybe, barrier=bollard is a special case but at least some barriers are usually not placed at the intersection. How about a white list instead of a blacklist?
I can see, that we can get problems with highway=service and below but a residential road with a service road leading of and a barrier=gate at the intersection is odd.

[skyper]

Just found ​this node even without a barrier tag.

[Famlam]

Replying to skyper:

Just found ​this node even without a barrier tag.

Made #21341 for that (as it's a different issue).

---

a mapping error adding barrier=gate, access=private on the intersection with a highway=service instead of on a different node only part of the service road.

Something like this?

way[highway=~/^(footway|path|bridleway|cycleway|service)$/] >node[barrier]:connection {
        set .barrierSmallRoadConnection;
}
way[highway=~/^(unclassified|residential)$/] >[index = 1] node.barrierSmallRoadConnection,
way[highway=~/^(unclassified|residential)$/] >[index = -1] node.barrierSmallRoadConnection {
        set .barrierAllowedAtConnection;
}
/* 20742; warnings for major roads set by other rule, also issue 20742 */
way[highway=~/^(unclassified|residential)$/] > node[barrier][barrier!=bollard][!access][!access:conditional][!vehicle][!vehicle:conditional][!motor_vehicle][!motor_vehicle:conditional].barrierSmallRoadConnection!.barrierAllowedAtConnection {
        throwWarning: tr("Suspicious {0} on a connection of a small highway with a larger highway", "{0.tag}");
        set .hasWarningForBarrierOnWay;
}
way[highway=~/^(unclassified|residential)$/] > node[barrier].barrierSmallRoadConnection!.hasWarningForBarrierOnWay {
        throwOther: tr("Suspicious {0} on a connection of a small highway with a larger highway", "{0.tag}");
}

[reichg]

a mapping error adding barrier=gate, access=private on the intersection with a highway=service instead of on a different node only part of the service road.

Something like this?
[EDIT FAMLAM: I cut the code out to make sure the correct one is copied]

Has this been tested? I can test some of this and add it to the patch.

[Famlam]

I've updated it (in the original comment), as there was an issue with when a residential continued as cycleway, for instance. It would be nice to test it and put it in the patch indeed!

[reichg]

a mapping error adding barrier=gate, access=private on the intersection with a highway=service instead of on a different node only part of the service road.

Something like this?

way[highway=~/^(footway|path|bridleway|cycleway|service)$/] >node[barrier]:connection {
        set .barrierSmallRoadConnection;
}
way[highway=~/^(unclassified|residential)$/] >[index = 1] node.barrierSmallRoadConnection,
way[highway=~/^(unclassified|residential)$/] >[index = -1] node.barrierSmallRoadConnection {
        set .barrierAllowedAtConnection;
}
/* 20742; warnings for major roads set by other rule, also issue 20742 */
way[highway=~/^(unclassified|residential)$/] > node[barrier][barrier!=bollard][!access][!access:conditional][!vehicle][!vehicle:conditional][!motor_vehicle][!motor_vehicle:conditional].barrierSmallRoadConnection!.barrierAllowedAtConnection {
        throwWarning: tr("Suspicious {0} on a connection of a small highway with a larger highway", "{0.tag}");
        set .hasWarningForBarrierOnWay;
}
way[highway=~/^(unclassified|residential)$/] > node[barrier].barrierSmallRoadConnection!.hasWarningForBarrierOnWay {
        throwOther: tr("Suspicious {0} on a connection of a small highway with a larger highway", "{0.tag}");
}

One suggestion would be to adjust the warning to be more descriptive? Isn't this also an issue of having barriers on nodes which are not end nodes? Should that be mentioned in the warning?

I have looked at a few scenarios and this seems to be working as expected.
Need another pair of eyes to look at this and test it.

[reichg]

including lower class roads that famlam introduced

[Famlam]

Isn't this also an issue of having barriers on nodes which are not end nodes? Should that be mentioned in the warning?

A barrier in the middle of a (non-major) road is not per se an issue. Think of i.e. a cycle barrier on a footway or a bollard on a residential. Besides, by suggesting it should be an end node, people will just cut the road in two, thereby not resolving any issue.

including lower class roads that famlam introduced

Thanks! I realized I forgot to add:
group: tr("suspicious barrier");
after the throwWarning and throwOther, sorry :S

[skyper]

@Gabe
Please, use as much placeholders as possible in the warning:
throwWarning: tr("{0} without {1} tags such as {1}, {2}, or {3}.", "{0.tag}", "{2.key}", "{3.key}", "{4.key}");

Replying to Famlam:

including lower class roads that famlam introduced

Thanks! I realized I forgot to add:
group: tr("suspicious barrier");
after the throwWarning and throwOther, sorry :S

Sorry, I already included this in my local copy but never wrote about it.
I think, common practice is to add the name with set without a leading . which is optional.
Maybe we can even display the highway values in the warnings with parent_tags() similar to trunk/resources/data/validator/geometry.mapcss?rev=18175#L234​

[Famlam]

Maybe we can even display the highway values in the warnings with parent_tags() similar to trunk/resources/data/validator/geometry.mapcss?rev=18175#L234​

In this case, with connecting roads, there will be multiple parents; I'm not sure how to extract the correct ones in "mapcss language"

[reichg]

Everything except the usage of parent tags. Not sure how to get the proper parent tag if there are multiple parents.

[reichg]

Replying to skyper:

@Gabe
Please, use as much placeholders as possible in the warning:
throwWarning: tr("{0} without {1} tags such as {1}, {2}, or {3}.", "{0.tag}", "{2.key}", "{3.key}", "{4.key}");

I totally forgot how these worked. :) Got it now.

[Famlam]

With group on the second part of the patch & indentation made consistent :)

[reichg]

do we all agree that this last patch satisfies the ticket? Not sure what we can do about adding the parent tags into the warning message.

[skyper]

highway values are not important and I spend my time on #20530 instead of playing around with filtering the output of parent_tags() nor deeper testing these new rules.

The indentation is still off at the second line with the leading space but besides of this glitch, I think it is ready for commit.

[reichg]

indentation fix

[Don-vip]

In 18231/josm:

fix #20742 - warns about barrier with inappropriate access tags on highway (patch by reichg)

[Famlam]

Poor me, not listed :P

[aceman]

Replying to skyper:

• I'd still like to get a warning if on a way's middle node, the barrier has access restrictions but the highway does not have any

That is not strictly an error, there may be a gate or a bollard in the middle of the roads, while cars can still freely come to the barrier from both sides (thus no access tags on the highway). Would you require to split the road to silence the warning?

[skyper]

Replying to aceman:

Replying to skyper:

• I'd still like to get a warning if on a way's middle node, the barrier has access restrictions but the highway does not have any

That is not strictly an error, there may be a gate or a bollard in the middle of the roads, while cars can still freely come to the barrier from both sides (thus no access tags on the highway). Would you require to split the road to silence the warning?

Yes, but only on minor roads, right?
Do you find false positives with r18235?

[Don-vip]

Milestone renamed

[skyper]

I get several warnings for one node now, see #21548.

[skyper]

Replying to skyper:

I get several warnings for one node now, see #21548.

Sorry, user fault. Works as expected with proper setting.