Modify

Opened 7 years ago

Closed 10 months ago

Last modified 10 months 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 7 years ago.
9836.patch (13.2 KB) - added by GerdP 10 months ago.
detect crossings barrier/building, barrier/highway, barrier/railway, barrier/waterway, and barrier/way
9836-v2.patch (11.9 KB) - added by GerdP 10 months ago.
cleaned patch
9836-v3.patch (12.7 KB) - added by GerdP 10 months ago.
improved unit test

Download all attachments as: .zip

Change History (20)

Changed 7 years ago by mkoniecz

Attachment: testcase.osm added

comment:1 Changed 7 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 10 months ago by skyper

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

comment:4 Changed 10 months 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 10 months 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 10 months ago by stoecker

#8220. ;-)

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

comment:7 in reply to:  6 Changed 10 months 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 10 months ago by GerdP

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

Changed 10 months ago by GerdP

Attachment: 9836.patch added

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

comment:9 Changed 10 months ago by GerdP

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

comment:10 Changed 10 months ago by GerdP

Milestone: 20.01

comment:11 in reply to:  8 Changed 10 months 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 10 months ago by GerdP

Attachment: 9836-v2.patch added

cleaned patch

comment:12 Changed 10 months ago by stoecker

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

Last edited 10 months ago by stoecker (previous) (diff)

Changed 10 months ago by GerdP

Attachment: 9836-v3.patch added

improved unit test

comment:13 Changed 10 months 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 10 months 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 10 months 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 10 months 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.