#368 improve structure analysis for roundabouts

server/src/main/scala/kpn/core/analysis/Link.scala

@@ -13,6 +13,10 @@ case class Link(
  isOnewayTail: Boolean,
 ) {
 
+ def isBidirectional: Boolean = {
+ !isOnewayLoopForwardPart && !isOnewayLoopBackwardPart
+ }
+
  def name: String = {
  val directionLetter = direction match {
  case LinkDirection.Forward => "f"

server/src/main/scala/kpn/core/tools/next/support/RouteAnalysisCompareTool.scala

@@ -27,9 +27,9 @@ class RouteAnalysisCompareTool(config: AnalysisStartConfiguration) {
 
  def analyze(): Unit = {
  log.info("Collecting routeIds")
- // val routeIds = config.oldDatabase.oldRoutes.ids()
+ val routeIds = config.oldDatabase.oldRoutes.ids()
  // val routeIds = readRouteIds("logs/mismatch-ids-7.txt")
- val routeIds = Seq(4275L)
+ // val routeIds = Seq(5880L)
  log.info(s"Comparing ${routeIds.size} routes")
  routeIds.zipWithIndex.foreach { case (routeId, index) =>
  if (index % 50 == 0) {

server/src/main/scala/kpn/core/tools/next/support/RouteAnalysisTool.scala

@@ -70,7 +70,7 @@ class RouteAnalysisTool(config: AnalysisStartConfiguration) {
  // analyzeRoutes(essenOkRouteIds)
  // analyzeRoutes(law9)
  // analyzeRoutes(Seq(13844575L))
- analyzeRoutes(Seq(4275L)) // exception during structure analysis
+ analyzeRoutes(Seq(5880L)) // exception during structure analysis
  // analyzeRoutes(Seq(3952592)) // broken route
  // analyzeRoutes(Seq(3963819)) // route with roundabout
  buildTiles()

server/src/main/scala/kpn/core/tools/next/support/compare/CompareFacts.scala

@@ -7,6 +7,10 @@ import kpn.core.util.Log
 class CompareFacts(oldRouteDoc: OldRouteDoc, newRouteDoc: RouteDetailDoc, log: Log) {
 
  def compare(): Unit = {
+ if (newRouteDoc.segments.size != 1) {
+ // cannot compare facts
+ return
+ }
  if (oldRouteDoc.facts.toSet != newRouteDoc.facts.toSet) {
  val oldFacts = s"""old-facts=${oldRouteDoc.facts.mkString(", ")}"""
  val newFacts = s"""new-facts=${newRouteDoc.facts.mkString(", ")}"""

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/analyzers/RouteStructureAnalyzer.scala

@@ -10,6 +10,7 @@ import kpn.api.custom.Fact.RouteNotOneWay
 import kpn.api.custom.Fact.RouteOneWay
 import kpn.api.custom.Fact.RouteUnusedSegments
 import kpn.api.custom.Fact.RouteWithoutNodes
+import kpn.core.analysis.LinkDirection
 import kpn.server.analyzer.engine.analysis.route.domain.RouteDetailAnalysisContext
 import kpn.server.analyzer.engine.analysis.route.structure.StructureAnalyzer
 
@@ -47,11 +48,13 @@ class RouteStructureAnalyzer(context: RouteDetailAnalysisContext) {
  }
  }
  else {
- if (oneWayRoute || oneWayRouteForward) {
- facts += RouteOneWay
- }
- else {
- facts += RouteNotBackward
+ if (!isSingleWayRoundabout()) {
+ if (oneWayRoute || oneWayRouteForward) {
+ facts += RouteOneWay
+ }
+ else {
+ facts += RouteNotBackward
+ }
  }
  }
  }
@@ -72,9 +75,11 @@ class RouteStructureAnalyzer(context: RouteDetailAnalysisContext) {
  }
 
  if (!Seq(RouteNodeMissingInWays, RouteOneWay).exists(facts.contains)) {
- if (structure.forwardPath.isEmpty || /* segmentAnalysis.structure.forwardPath.get.broken ||*/
- structure.backwardPath.isEmpty /*|| segmentAnalysis.structure.backwardPath.get.broken*/ ) {
- facts += RouteNotContinious
+ if (!isSingleWayRoundabout()) {
+ if (structure.forwardPath.isEmpty || /* segmentAnalysis.structure.forwardPath.get.broken ||*/
+ structure.backwardPath.isEmpty /*|| segmentAnalysis.structure.backwardPath.get.broken*/ ) {
+ facts += RouteNotContinious
+ }
  }
  }
 
@@ -90,4 +95,17 @@ class RouteStructureAnalyzer(context: RouteDetailAnalysisContext) {
  facts = facts.toSeq,
  )
  }
+
+ private def isSingleWayRoundabout(): Boolean = {
+ if (context.segments.size == 1) {
+ val elements = context.segments.head.elements
+ if (elements.size == 1) {
+ val fragments = elements.head.fragments
+ if (fragments.size == 1) {
+ return fragments.head.link.direction == LinkDirection.RoundaboutRight
+ }
+ }
+ }
+ false
+ }
 }

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/structure/RouteSegmentAnalyzer.scala

@@ -4,7 +4,6 @@ import kpn.core.analysis.LinkDirection
 import kpn.core.util.Triplet
 import kpn.core.util.Util
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteAnalyzer
-import kpn.server.analyzer.engine.analysis.route.domain
 import kpn.server.analyzer.engine.analysis.route.domain.RouteAnalysisElement
 import kpn.server.analyzer.engine.analysis.route.domain.RouteAnalysisFragment
 import kpn.server.analyzer.engine.analysis.route.domain.RouteAnalysisNode
@@ -36,7 +35,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
 
  def analyze(): Seq[RouteAnalysisSegment] = {
  Triplet.slide(context.links.routeLinkWays).foreach { case Triplet(previousRouteLinkWayOption, currentRouteLinkWay, nextRouteLinkWayOption) =>
- if (currentRouteLinkWay.link.direction == LinkDirection.RoundaboutRight && currentRouteLinkWay.isClosedLoop && currentRouteLinkWay.link.hasNext) {
+ if (isRoundabout(currentRouteLinkWay)) {
  handleRoundabout(previousRouteLinkWayOption, currentRouteLinkWay, nextRouteLinkWayOption)
  }
  else {
@@ -81,7 +80,12 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  segments.toSeq
  }
 
- private def handleRoundabout(previousRouteLinkWayOption: Option[RouteLinkWay], currentRouteLinkWay: RouteLinkWay, nextRouteLinkWayOption: Option[RouteLinkWay]) = {
+ private def handleRoundabout(
+ previousRouteLinkWayOption: Option[RouteLinkWay],
+ currentRouteLinkWay: RouteLinkWay,
+ nextRouteLinkWayOption: Option[RouteLinkWay]
+ ) = {
+
  finalizeSegmentElement()
 
  nextRouteLinkWayOption match {
@@ -160,7 +164,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  lastBackwardElement.fromNodeId
  }
 
- val nextLinkOption = context.links.routeLinkWays.find(link => link.id > currentRouteLinkWay.id && link.link.isOnewayLoopBackwardPart)
+ val nextLinkOption = context.links.routeLinkWays.find(link => link.id > currentRouteLinkWay.id && (link.link.isBidirectional || link.link.isOnewayLoopBackwardPart))
  nextLinkOption match {
  case None =>
 
@@ -180,6 +184,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  case None =>
  // TODO redesign ???
  println("")
+ ???
  }
  }
  }
@@ -189,7 +194,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  private def buildSegment(id: Long, elements: Seq[RouteAnalysisElement]): RouteAnalysisSegment = {
  val fromNodeId = elements.head.fromNodeId
  val toNodeId = elements.last.toNodeId
- domain.RouteAnalysisSegment(
+ RouteAnalysisSegment(
  id,
  fromNodeId,
  toNodeId,
@@ -205,7 +210,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  val fromNodeId = fragments.head.fromNodeId
  val toNodeId = fragments.last.toNodeId
 
- val direction: RoutePathDirection = if (fragments.head.link.isOnewayLoopForwardPart) {
+ val direction: RoutePathDirection = if (fragments.head.link.isOnewayLoopForwardPart || fragments.head.link.direction == LinkDirection.RoundaboutRight) {
  RoutePathDirection.Forward
  }
  else if (fragments.head.link.isOnewayLoopBackwardPart) {
@@ -281,7 +286,7 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  fragments: Seq[RouteAnalysisFragment]
  ): RouteAnalysisElement = {
  val fragmentGroups = SurfaceFragmentSplitter.split(context.networkTypes, fragments)
- domain.RouteAnalysisElement(
+ RouteAnalysisElement(
  elementIds.next(),
  direction,
  fromNetworkNode,
@@ -291,4 +296,8 @@ class RouteSegmentAnalyzer(context: RouteDetailAnalysisContext) {
  fragmentGroups
  )
  }
+
+ private def isRoundabout(routeLinkWay: RouteLinkWay): Boolean = {
+ routeLinkWay.link.direction == LinkDirection.RoundaboutRight && routeLinkWay.isClosedLoop && routeLinkWay.link.hasNext
+ }
 }

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/structure/StructureAnalyzer.scala

@@ -228,7 +228,7 @@ class StructureAnalyzer(context: RouteDetailAnalysisContext, traceEnabled: Boole
  val elements = findNonNodeNetworkRouteForwardPath(Seq.empty, context.segments.flatMap(_.elements))
  if (elements.nonEmpty) {
  Some(
- domain.StructurePath(
+ StructurePath(
  pathIds.next(),
  elements.head.startNodeId,
  elements.last.endNodeId,

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/StructureTestSetup.scala

@@ -3,19 +3,14 @@ package kpn.server.analyzer.engine.analysis.route.structure.test
 import kpn.api.common.route.RouteNodes
 import kpn.api.custom.Relation
 import kpn.core.data.Data
-import kpn.server.analyzer.engine.analysis.location.LocationAnalyzer
 import kpn.server.analyzer.engine.analysis.route.RouteDetailMainAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteCountryAnalyzerMock
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteLocationAnalyzerMock
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteTileAnalyzer
 import kpn.server.analyzer.engine.analysis.route.structure.RouteDetailAnalysisTestContext
 import kpn.server.analyzer.engine.analysis.route.structure.StructureElementAnalyzer
 import kpn.server.analyzer.engine.tile.LineSegmentTileCalculatorImpl
-import kpn.server.analyzer.engine.tile.OldLinesTileCalculatorImpl
-import kpn.server.analyzer.engine.tile.OldTileCalculatorImpl
-import kpn.server.analyzer.engine.tile.RouteTileCalculatorImpl
 import kpn.server.analyzer.engine.tile.TileCalculatorImpl
-import kpn.server.repository.RouteRepository
 import org.scalamock.scalatest.MockFactory
 
 class StructureTestSetup(val data: Data) extends MockFactory {
@@ -39,14 +34,9 @@ class StructureTestSetup(val data: Data) extends MockFactory {
  // }
 
  def analyze(traceEnabled: Boolean = false): RouteDetailAnalysisTestContext = {
- val oldTileCalculator = new OldTileCalculatorImpl()
  val tileCalculator = new TileCalculatorImpl()
- val linesTileCalculator = new OldLinesTileCalculatorImpl(oldTileCalculator)
  val lineSegmentTileCalculator = new LineSegmentTileCalculatorImpl(tileCalculator)
- val routeTileCalculator = new RouteTileCalculatorImpl(lineSegmentTileCalculator)
  val routeTileAnalyzer = new RouteTileAnalyzer(lineSegmentTileCalculator)
- val locationAnalyzer = stub[LocationAnalyzer]
- val routeRepository = stub[RouteRepository]
  val routeCountryAnalyzer = new RouteCountryAnalyzerMock()
  val routeLocationAnalyzer = new RouteLocationAnalyzerMock()
  val routeAnalyzer = new RouteDetailMainAnalyzer(

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/Structure_04_SingleWayRoundaboutLoopTest.scala

@@ -21,17 +21,15 @@ class Structure_04_SingleWayRoundaboutLoopTest extends UnitTest {
  context.segments.shouldMatchTo(
  Seq(
  "segment-1 1>1",
- " element-1 1>1 ↔ nodes=1, 2, 3, 4, 1",
+ " element-1 1>1 → nodes=1, 2, 3, 4, 1",
  " way-11 p n loop fp bp head tail d roundaboutright",
  )
  )
 
  context.paths.shouldMatchTo(
  Seq(
  "forward=1>1 nodes=1, 2, 3, 4, 1",
- "backward=1>1 nodes=1, 4, 3, 2, 1", // TODO redesign - NOK
  )
  )
- pending
  }
 }

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/Structure_51_RoundaboutTest.scala

@@ -21,17 +21,15 @@ class Structure_51_RoundaboutTest extends UnitTest {
  context.segments.shouldMatchTo(
  Seq(
  "segment-1 1>1",
- " element-1 1>1 ↔ nodes=1, 2, 3, 4, 1",
+ " element-1 1>1 → nodes=1, 2, 3, 4, 1",
  " way-11 p n loop fp bp head tail d roundaboutright",
  )
  )
 
  context.paths.shouldMatchTo(
  Seq(
  "forward=1>1 nodes=1, 2, 3, 4, 1",
- "backward=1>1 nodes=1, 4, 3, 2, 1", // TODO redesign
  )
  )
- pending
  }
 }

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/Structure_52_RoundaboutRoundaboutTest.scala

@@ -27,17 +27,16 @@ class Structure_52_RoundaboutRoundaboutTest extends UnitTest {
  " way-11 p n ■ loop ■ fp bp head tail d roundaboutright",
  " element-2 1>3 ← nodes=1, 4, 3",
  " way-11 p n ■ loop ■ fp bp head tail d roundaboutright",
- " element-3 3>3 ↔ nodes=3, 5, 6, 7, 3",
+ " element-3 3>3 → nodes=3, 5, 6, 7, 3",
  " way-12 p ■ n loop ■ fp bp head tail d roundaboutright",
  )
  )
 
  context.paths.shouldMatchTo(
  Seq(
  "forward=1>3 nodes=1, 2, 3, 5, 6, 7, 3",
- "backward=3>1 nodes=3, 7, 6, 5, 3, 4, 1", // TODO redesign nodeIds = Seq(3, 5, 6, 7, 3, 4, 1)
+ "backward=3>1 nodes=3, 4, 1",
  )
  )
- pending
  }
 }