Skip to content

Commit

Permalink
do not ask whether oneway road is really also a oneway for cyclists i…
Browse files Browse the repository at this point in the history 
…f the contra-flow side remains to be not defined (fixes #5367)
  • Loading branch information
@westnordost
westnordost committed Nov 13, 2023
1 parent f024e3c commit 814f6f6
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 59 deletions.
31 changes: 20 additions & 11 deletions app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/Cycleway.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,36 @@ fun LeftAndRightCycleway.selectableOrNullValues(countryInfo: CountryInfo): LeftA
fun LeftAndRightCycleway.wasNoOnewayForCyclistsButNowItIs(tags: Map<String, String>, isLeftHandTraffic: Boolean): Boolean =
isOneway(tags)
&& isNotOnewayForCyclists(tags, isLeftHandTraffic)
&& !isNotOnewayForCyclistsNow(tags, isLeftHandTraffic)
&& isNotOnewayForCyclistsNow(tags, isLeftHandTraffic) == false

fun LeftAndRightCycleway.isNotOnewayForCyclistsNow(tags: Map<String, String>, isLeftHandTraffic: Boolean): Boolean {
/** Returns whether this is now not a oneway for cyclists. It returns null if the contra-flow side
* is null, i.e. no change is being made for the contra-flow side */
fun LeftAndRightCycleway.isNotOnewayForCyclistsNow(tags: Map<String, String>, isLeftHandTraffic: Boolean): Boolean? {
val onewayDir = when {
isForwardOneway(tags) -> FORWARD
isReversedOneway(tags) -> BACKWARD
else -> return true
}
val previous = createCyclewaySides(tags, isLeftHandTraffic)
val l = (left ?: previous?.left)
val r = (right ?: previous?.right)
/* "no cycleway" has no direction and should be ignored
"separate" should also be ignored because if the cycleway is mapped separately, the existence
of a separate way that enables cyclists to go in contra-flow-direction doesn't mean that they
can do the same on the main way for the road too (see #4715) */
val leftDir = l?.direction?.takeIf { l.cycleway != NONE && l.cycleway != SEPARATE }
val rightDir = r?.direction?.takeIf { r.cycleway != NONE && r.cycleway != SEPARATE }

return leftDir == BOTH || rightDir == BOTH ||
leftDir != null && leftDir != onewayDir ||
rightDir != null && rightDir != onewayDir
val leftDir = left?.direction?.takeIf { left.cycleway != NONE && left.cycleway != SEPARATE }
val rightDir = right?.direction?.takeIf { right.cycleway != NONE && right.cycleway != SEPARATE }

// any side goes in both directions
if (leftDir == BOTH || rightDir == BOTH) return true
// left side has contra-flow to oneway
if (leftDir != null && leftDir != onewayDir) return true
// right side has contra-flow to oneway
if (rightDir != null && rightDir != onewayDir) return true

// if no cycleway is defined for the side that is usually contra to the oneway flow, we
// return that we do not know it
val contraFlowSide = if (isForwardOneway(tags) xor isLeftHandTraffic) left else right
if (contraFlowSide == null) return null

return false
}

@Serializable
Expand Down
7 changes: 4 additions & 3 deletions app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ private fun Tags.expandBareTagIntoSide(key: String, suffix: String = "", side: S
}

private fun LeftAndRightCycleway.applyOnewayNotForCyclists(tags: Tags, isLeftHandTraffic: Boolean) {
if (isOneway(tags) && isNotOnewayForCyclistsNow(tags, isLeftHandTraffic)) {
tags["oneway:bicycle"] = "no"
} else {
val isNotOnewayForCyclistsNow = isNotOnewayForCyclistsNow(tags, isLeftHandTraffic)
if (!isOneway(tags) || isNotOnewayForCyclistsNow == false) {
if (tags["oneway:bicycle"] == "no") {
tags.remove("oneway:bicycle")
}
} else if(isNotOnewayForCyclistsNow == true) {

Check failure on line 91 in app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt

View workflow job for this annotation

GitHub Actions / Kotlin 

Missing spacing after "if"
tags["oneway:bicycle"] = "no"
}
}

Expand Down
135 changes: 90 additions & 45 deletions app/src/test/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayKtTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import de.westnordost.streetcomplete.osm.cycleway.Cycleway.*
import de.westnordost.streetcomplete.osm.cycleway.Direction.*
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertNull
import kotlin.test.assertTrue

class CyclewayKtTest {
Expand Down Expand Up @@ -62,12 +63,45 @@ class CyclewayKtTest {
)
}

@Test fun `not a oneway for cyclists is still not a oneway for cyclists when contra-flow side is not defined`() {
val noOnewayForCyclists = mapOf("oneway" to "yes", "oneway:bicycle" to "no")
val noOnewayForCyclistsReverse = mapOf("oneway" to "-1", "oneway:bicycle" to "no")
val forwardTrack = CyclewayAndDirection(TRACK, FORWARD)
val backwardTrack = CyclewayAndDirection(TRACK, BACKWARD)

assertNull(
LeftAndRightCycleway(null, null)
.isNotOnewayForCyclistsNow(noOnewayForCyclists, false)
)
assertNull(
LeftAndRightCycleway(null, forwardTrack)
.isNotOnewayForCyclistsNow(noOnewayForCyclists, false)
)
assertNull(
LeftAndRightCycleway(backwardTrack, null)
.isNotOnewayForCyclistsNow(noOnewayForCyclistsReverse, false)
)

assertNull(
LeftAndRightCycleway(null, null)
.isNotOnewayForCyclistsNow(noOnewayForCyclists, true)
)
assertNull(
LeftAndRightCycleway(forwardTrack, null)
.isNotOnewayForCyclistsNow(noOnewayForCyclists, true)
)
assertNull(
LeftAndRightCycleway(null, backwardTrack)
.isNotOnewayForCyclistsNow(noOnewayForCyclistsReverse, true)
)
}

@Test fun `not a oneway is no oneway for cyclists`() {
assertTrue(
LeftAndRightCycleway(
CyclewayAndDirection(NONE, BACKWARD),
CyclewayAndDirection(NONE, FORWARD)
).isNotOnewayForCyclistsNow(mapOf(), false)
).isNotOnewayForCyclistsNow(mapOf(), false)!!
)
}

Expand All @@ -76,7 +110,7 @@ class CyclewayKtTest {
LeftAndRightCycleway(
CyclewayAndDirection(NONE, BACKWARD),
CyclewayAndDirection(NONE, FORWARD)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)!!
)
}

Expand All @@ -85,7 +119,7 @@ class CyclewayKtTest {
LeftAndRightCycleway(
CyclewayAndDirection(NONE, BACKWARD),
CyclewayAndDirection(NONE, FORWARD)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)!!
)
}

Expand All @@ -94,70 +128,81 @@ class CyclewayKtTest {
LeftAndRightCycleway(
CyclewayAndDirection(NONE, BACKWARD),
CyclewayAndDirection(TRACK, BOTH)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)!!
)
assertTrue(
LeftAndRightCycleway(
CyclewayAndDirection(TRACK, BOTH),
CyclewayAndDirection(NONE, FORWARD)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
).isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)!!
)
}

@Test fun `oneway is no oneway for cyclists when any cycleway goes in contra-flow direction`() {
val forwardTrack = CyclewayAndDirection(TRACK, FORWARD)
val backwardTrack = CyclewayAndDirection(TRACK, BACKWARD)

assertTrue(
LeftAndRightCycleway(CyclewayAndDirection(TRACK, BACKWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
LeftAndRightCycleway(backwardTrack, null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)!!
)
assertTrue(
LeftAndRightCycleway(null, CyclewayAndDirection(TRACK, BACKWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
LeftAndRightCycleway(null, backwardTrack)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)!!
)
assertTrue(
LeftAndRightCycleway(CyclewayAndDirection(TRACK, FORWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
LeftAndRightCycleway(forwardTrack, null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)!!
)
assertTrue(
LeftAndRightCycleway(null, CyclewayAndDirection(TRACK, FORWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
LeftAndRightCycleway(null, forwardTrack)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)!!
)
}

@Test fun `oneway is a oneway for cyclists when separately mapped cycleway goes in contra-flow direction`() {
assertFalse(
LeftAndRightCycleway(CyclewayAndDirection(SEPARATE, BACKWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
)
assertFalse(
LeftAndRightCycleway(null, CyclewayAndDirection(SEPARATE, BACKWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
)
assertFalse(
LeftAndRightCycleway(CyclewayAndDirection(SEPARATE, FORWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
)
assertFalse(
LeftAndRightCycleway(null, CyclewayAndDirection(SEPARATE, FORWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
)
// ... because direction of separate cycleway and no cycleway is ignored
val separate = CyclewayAndDirection(SEPARATE, BOTH)
val none = CyclewayAndDirection(NONE, BOTH)

for (isLeftHandTraffic in listOf(false, true)) {
for (oneway in listOf(mapOf("oneway" to "yes"), mapOf("oneway" to "-1"))) {
assertFalse(
LeftAndRightCycleway(separate, none)
.isNotOnewayForCyclistsNow(oneway, isLeftHandTraffic)!!
)
assertFalse(
LeftAndRightCycleway(none, separate)
.isNotOnewayForCyclistsNow(oneway, isLeftHandTraffic)!!
)
}
}
}


Check failure on line 182 in app/src/test/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayKtTest.kt

View workflow job for this annotation

GitHub Actions / Kotlin 

Needless blank line(s)
@Test fun `oneway is oneway for cyclists when no cycleway goes in contra-flow direction`() {
assertFalse(
LeftAndRightCycleway(CyclewayAndDirection(TRACK, FORWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
)
assertFalse(
LeftAndRightCycleway(null, CyclewayAndDirection(TRACK, FORWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), false)
)
assertFalse(
LeftAndRightCycleway(CyclewayAndDirection(TRACK, BACKWARD), null)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
)
assertFalse(
LeftAndRightCycleway(null, CyclewayAndDirection(TRACK, BACKWARD))
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), false)
)
val forwardTrack = CyclewayAndDirection(TRACK, FORWARD)
val backwardTrack = CyclewayAndDirection(TRACK, BACKWARD)
// direction does not matter for NONE, best way to test that is to set BOTH
val none = CyclewayAndDirection(NONE, BOTH)

for (isLeftHandTraffic in listOf(false, true)) {
assertFalse(
LeftAndRightCycleway(forwardTrack, none)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), isLeftHandTraffic)!!
)
assertFalse(
LeftAndRightCycleway(none, forwardTrack)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "yes"), isLeftHandTraffic)!!
)
assertFalse(
LeftAndRightCycleway(backwardTrack, none)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), isLeftHandTraffic)!!
)
assertFalse(
LeftAndRightCycleway(none, backwardTrack)
.isNotOnewayForCyclistsNow(mapOf("oneway" to "-1"), isLeftHandTraffic)!!
)
}
}
}

0 comments on commit 814f6f6

Please sign in to comment.