Modify

Opened 6 years ago

Closed 5 weeks ago

Last modified 5 weeks ago

#9836 closed defect (fixed)

crossing barrier and highway is not reported

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 20.01
Component: Core validator Version:
Keywords: barrier highway crossing regression Cc:

Description (last modified by skyper)

According to Barriers "If a linear barrier crosses a highway, they must connect with a node at intersection. Always tag this node with the appropriate node-barrier tag" but this is currently not reported. I attach osm testcase.

Attachments (4)

testcase.osm (804 bytes) - added by mkoniecz 6 years ago.
9836.patch (13.2 KB) - added by GerdP 5 weeks ago.
detect crossings barrier/building, barrier/highway, barrier/railway, barrier/waterway, and barrier/way
9836-v2.patch (11.9 KB) - added by GerdP 5 weeks ago.
cleaned patch
9836-v3.patch (12.7 KB) - added by GerdP 5 weeks ago.
improved unit test

Download all attachments as: .zip

Change History (20)

Changed 6 years ago by mkoniecz

Attachment: testcase.osm added

comment:1 Changed 6 years ago by skyper

Of course, they need to have the same layer value.

comment:2 Changed 6 years ago by Don-vip

Component: CoreCore validator
Summary: crossing barrier and highway is not reported by validatorcrossing barrier and highway is not reported

comment:3 Changed 6 weeks ago by skyper

Description: modified (diff)
Keywords: barrier highway crossing added
Version: tested

comment:4 Changed 5 weeks ago by skyper

Version: tested

Still reproducible

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-01-13 00:44:06 +0100 (Mon, 13 Jan 2020)
Revision:15697
Build-Date:2020-01-13 02:30:54
URL:https://josm.openstreetmap.de/svn/trunk

comment:5 Changed 5 weeks ago by GerdP

I think there is actually no code to detect this case. We report crossing barriers and crossing highways but not a mix of both.
@Dirk: Maybe it was reported before r6581. Do you remember why you added a special test for barriers?

comment:6 Changed 5 weeks ago by stoecker

#8220. ;-)

Highway and barrier was not asked for yet I'd say :-)

comment:7 in reply to:  6 Changed 5 weeks ago by stoecker

Replying to stoecker:

#8220. ;-)

Highway and barrier was not asked for yet I'd say :-)

Well, as this ticket is 6 years old I'd reword this to "was not implemented yet".

comment:8 Changed 5 weeks ago by GerdP

I think it was implemented before r6581, but probably the old implementation caused too many false positives?

Changed 5 weeks ago by GerdP

Attachment: 9836.patch added

detect crossings barrier/building, barrier/highway, barrier/railway, barrier/waterway, and barrier/way

comment:9 Changed 5 weeks ago by GerdP

Hmm, not sure about barrier/way. Probably never happens because of the filter implemented with ignoreWaySegmentCombination()

comment:10 Changed 5 weeks ago by GerdP

Milestone: 20.01

comment:11 in reply to:  8 Changed 5 weeks ago by skyper

Should the changes on src/org/openstreetmap/josm/gui/NavigatableComponent.java be part of the patch ?

Replying to GerdP:

I think it was implemented before r6581, but probably the old implementation caused too many false positives?

Yes, was implemented years ago.

Changed 5 weeks ago by GerdP

Attachment: 9836-v2.patch added

cleaned patch

comment:12 Changed 5 weeks ago by stoecker

Hmm. Isn't the idea of unit tests that you expand them when changing code instead of reducing them?

Last edited 5 weeks ago by stoecker (previous) (diff)

Changed 5 weeks ago by GerdP

Attachment: 9836-v3.patch added

improved unit test

comment:13 Changed 5 weeks ago by GerdP

Not sure if this is a false positive? way 617592269 & way 617380123 give Crossing barrier/highway
barrier=retaining_wall / highway=steps

I see a lot of new crossing warnings in Genua.

comment:14 in reply to:  13 Changed 5 weeks ago by skyper

Replying to GerdP:

Not sure if this is a false positive? way 617592269 & way 617380123 give Crossing barrier/highway
barrier=retaining_wall / highway=steps

I would say no. Either a common node with barrier=entrance or a spit of the barrier=* in two pieces is needed.
Alternativly, layer=*could be used.

I see a lot of new crossing warnings in Genua.

The test was silent for years!

comment:15 Changed 5 weeks ago by GerdP

Resolution: fixed
Status: newclosed

In 15704/josm:

fix #9836: crossing barrier and highway is not reported (regressin of r6581)

  • detect crossings between barrier/building, barrier/highway, barrier/railway, and barrier/waterway
  • remove CrossingWays.Barrier class and add check barrier in standard test CrossingWays.Ways

comment:16 Changed 5 weeks ago by GerdP

Keywords: regression added

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.