From 923dbbefee3b6009d1d204a0e0d9192b881068f2 Mon Sep 17 00:00:00 2001 From: Mikhail Gorbushin Date: Mon, 28 Oct 2019 14:21:46 +0300 Subject: [routing] Refactor RouterDelegate --- routing/async_router.hpp | 2 +- routing/bicycle_directions.cpp | 7 ++-- routing/index_router.cpp | 24 ++++++------- routing/index_router.hpp | 2 +- routing/router_delegate.cpp | 48 ++++++++------------------ routing/router_delegate.hpp | 29 ++++++---------- routing/routes_builder/routes_builder.hpp | 3 +- routing/routing_helpers.cpp | 1 + routing/routing_tests/async_router_test.cpp | 4 +-- routing/routing_tests/routing_algorithm.cpp | 5 +-- routing/routing_tests/routing_session_test.cpp | 26 +++++++++----- routing/turns_generator.cpp | 4 +-- routing/turns_generator.hpp | 6 ++-- 13 files changed, 71 insertions(+), 90 deletions(-) (limited to 'routing') diff --git a/routing/async_router.hpp b/routing/async_router.hpp index 14cf55bbbc..7716da46ab 100644 --- a/routing/async_router.hpp +++ b/routing/async_router.hpp @@ -53,7 +53,7 @@ public: NeedMoreMapsCallback const & needMoreMapsCallback, RemoveRouteCallback const & removeRouteCallback, ProgressCallback const & progressCallback, - uint32_t timeoutSec); + uint32_t timeoutSec = RouterDelegate::kNoTimeout); /// Interrupt routing and clear buffers void ClearState(); diff --git a/routing/bicycle_directions.cpp b/routing/bicycle_directions.cpp index 9fd8023a80..9a8798dadc 100644 --- a/routing/bicycle_directions.cpp +++ b/routing/bicycle_directions.cpp @@ -178,7 +178,7 @@ bool BicycleDirectionsEngine::Generate(IndexRoadGraph const & graph, vector::Params params( jointStarter, jointStarter.GetStartJoint(), jointStarter.GetFinishJoint(), nullptr /* prevRoute */, - delegate, onVisitJunctionJoints, checkLength); + delegate.GetCancellable(), onVisitJunctionJoints, checkLength); set const mwmIds = starter.GetMwms(); RouterResultCode const result = FindPath(params, mwmIds, routingResult, mode); @@ -647,7 +647,7 @@ RouterResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoints, RoutingResult routingResult; AStarAlgorithm::Params params( starter, starter.GetStartSegment(), starter.GetFinishSegment(), nullptr /* prevRoute */, - delegate, onVisitJunction, checkLength); + delegate.GetCancellable(), onVisitJunction, checkLength); set const mwmIds = starter.GetMwms(); RouterResultCode const result = FindPath(params, mwmIds, routingResult, mode); @@ -745,8 +745,9 @@ RouterResultCode IndexRouter::AdjustRoute(Checkpoints const & checkpoints, AStarAlgorithm algorithm; AStarAlgorithm::Params params(starter, starter.GetStartSegment(), - {} /* finalVertex */, &prevEdges, delegate, - onVisitJunction, checkLength); + {} /* finalVertex */, &prevEdges, + delegate.GetCancellable(), onVisitJunction, + checkLength); RoutingResult result; auto const resultCode = ConvertResult(algorithm.AdjustRoute(params, result)); @@ -780,7 +781,7 @@ RouterResultCode IndexRouter::AdjustRoute(Checkpoints const & checkpoints, route.SetCurrentSubrouteIdx(checkpoints.GetPassedIdx()); route.SetSubroteAttrs(move(subroutes)); - auto const redressResult = RedressRoute(result.m_path, delegate, starter, route); + auto const redressResult = RedressRoute(result.m_path, delegate.GetCancellable(), starter, route); if (redressResult != RouterResultCode::NoError) return redressResult; @@ -1173,8 +1174,7 @@ RouterResultCode IndexRouter::ProcessLeapsJoints(vector const & input, AStarAlgorithm::Params params( jointStarter, jointStarter.GetStartJoint(), jointStarter.GetFinishJoint(), - nullptr /* prevRoute */, delegate, - onVisitJunctionJoints, checkLength); + nullptr /* prevRoute */, delegate.GetCancellable(), onVisitJunctionJoints, checkLength); resultCode = FindPath(params, mwmIds, routingResult, mode); return resultCode; @@ -1290,7 +1290,7 @@ RouterResultCode IndexRouter::ProcessLeapsJoints(vector const & input, } RouterResultCode IndexRouter::RedressRoute(vector const & segments, - RouterDelegate const & delegate, + base::Cancellable const & cancellable, IndexGraphStarter & starter, Route & route) const { CHECK(!segments.empty(), ()); @@ -1321,8 +1321,8 @@ RouterResultCode IndexRouter::RedressRoute(vector const & segments, } CHECK(m_directionsEngine, ()); - ReconstructRoute(*m_directionsEngine, roadGraph, m_trafficStash, delegate, junctions, move(times), - route); + ReconstructRoute(*m_directionsEngine, roadGraph, m_trafficStash, cancellable, junctions, + move(times), route); CHECK(m_numMwmIds, ()); auto & worldGraph = starter.GetGraph(); @@ -1353,7 +1353,7 @@ RouterResultCode IndexRouter::RedressRoute(vector const & segments, FillSpeedCamProhibitedMwms(segments, speedCamProhibited); route.SetMwmsPartlyProhibitedForSpeedCams(move(speedCamProhibited)); - if (delegate.IsCancelled()) + if (cancellable.IsCancelled()) return RouterResultCode::Cancelled; if (!route.IsValid()) diff --git a/routing/index_router.hpp b/routing/index_router.hpp index 8d5eb7e915..83d6c459d2 100644 --- a/routing/index_router.hpp +++ b/routing/index_router.hpp @@ -153,7 +153,7 @@ private: IndexGraphStarter & starter, AStarProgress & progress, std::vector & output); RouterResultCode RedressRoute(std::vector const & segments, - RouterDelegate const & delegate, IndexGraphStarter & starter, + base::Cancellable const & cancellable, IndexGraphStarter & starter, Route & route) const; bool AreMwmsNear(IndexGraphStarter const & starter) const; diff --git a/routing/router_delegate.cpp b/routing/router_delegate.cpp index accde1bbd1..06ff26a4a0 100644 --- a/routing/router_delegate.cpp +++ b/routing/router_delegate.cpp @@ -1,12 +1,14 @@ #include "routing/router_delegate.hpp" +#include + namespace routing { namespace { void DefaultProgressFn(float /* progress */) {} void DefaultPointFn(m2::PointD const & /* point */) {} -} // namespace +} // namespace RouterDelegate::RouterDelegate() { @@ -14,56 +16,34 @@ RouterDelegate::RouterDelegate() m_pointCallback = DefaultPointFn; } -void RouterDelegate::SetProgressCallback(ProgressCallback const & progressCallback) -{ - m_progressCallback = progressCallback ? progressCallback : DefaultProgressFn; -} - -void RouterDelegate::SetPointCheckCallback(PointCheckCallback const & pointCallback) -{ - m_pointCallback = pointCallback ? pointCallback : DefaultPointFn; -} - void RouterDelegate::OnProgress(float progress) const { - std::lock_guard l(m_guard); - if (!IsCancelled()) + if (!m_cancellable.IsCancelled()) m_progressCallback(progress); } -void RouterDelegate::Reset() -{ - std::lock_guard l(m_guard); - TimeoutCancellable::Reset(); -} - void RouterDelegate::OnPointCheck(m2::PointD const & point) const { - std::lock_guard l(m_guard); - if (!IsCancelled()) + if (!m_cancellable.IsCancelled()) m_pointCallback(point); } -TimeoutCancellable::TimeoutCancellable() : m_timeoutSec(0) +void RouterDelegate::SetProgressCallback(ProgressCallback const & progressCallback) { + m_progressCallback = progressCallback ? progressCallback : DefaultProgressFn; } -bool TimeoutCancellable::IsCancelled() const +void RouterDelegate::SetPointCheckCallback(PointCheckCallback const & pointCallback) { - if (m_timeoutSec && m_timer.ElapsedSeconds() > m_timeoutSec) - return true; - return Cancellable::IsCancelled(); + m_pointCallback = pointCallback ? pointCallback : DefaultPointFn; } -void TimeoutCancellable::Reset() +void RouterDelegate::SetTimeout(uint32_t timeoutSec) { - m_timeoutSec = 0; - Cancellable::Reset(); -} + if (timeoutSec == kNoTimeout) + return; -void TimeoutCancellable::SetTimeout(uint32_t timeoutSec) -{ - m_timeoutSec = timeoutSec; - m_timer.Reset(); + std::chrono::steady_clock::duration const timeout = std::chrono::seconds(timeoutSec); + m_cancellable.SetDeadline(std::chrono::steady_clock::now() + timeout); } } // namespace routing diff --git a/routing/router_delegate.hpp b/routing/router_delegate.hpp index 65cc0c3da3..7b227d4f66 100644 --- a/routing/router_delegate.hpp +++ b/routing/router_delegate.hpp @@ -10,26 +10,11 @@ namespace routing { -class TimeoutCancellable : public base::Cancellable +class RouterDelegate { public: - TimeoutCancellable(); + static auto constexpr kNoTimeout = std::numeric_limits::max(); - /// Sets timeout before cancellation. 0 means an infinite timeout. - void SetTimeout(uint32_t timeoutSec); - - // Cancellable overrides: - bool IsCancelled() const override; - void Reset() override; - -private: - base::Timer m_timer; - uint32_t m_timeoutSec; -}; - -class RouterDelegate : public TimeoutCancellable -{ -public: RouterDelegate(); /// Set routing progress. Waits current progress status from 0 to 100. @@ -39,11 +24,17 @@ public: void SetProgressCallback(ProgressCallback const & progressCallback); void SetPointCheckCallback(PointCheckCallback const & pointCallback); - void Reset() override; + void SetTimeout(uint32_t timeoutSec); + + base::Cancellable const & GetCancellable() const { return m_cancellable; } + void Reset() { return m_cancellable.Reset(); } + void Cancel() { return m_cancellable.Cancel(); } + bool IsCancelled() const { return m_cancellable.IsCancelled(); } private: - mutable std::mutex m_guard; ProgressCallback m_progressCallback; PointCheckCallback m_pointCallback; + + base::Cancellable m_cancellable; }; } // namespace routing diff --git a/routing/routes_builder/routes_builder.hpp b/routing/routes_builder/routes_builder.hpp index c0754bd251..c8e38e4d53 100644 --- a/routing/routes_builder/routes_builder.hpp +++ b/routing/routes_builder/routes_builder.hpp @@ -4,6 +4,7 @@ #include "routing/checkpoints.hpp" #include "routing/index_router.hpp" +#include "routing/router_delegate.hpp" #include "routing/routing_callbacks.hpp" #include "routing/segment.hpp" #include "routing/vehicle_mask.hpp" @@ -60,7 +61,7 @@ public: VehicleType m_type = VehicleType::Car; Checkpoints m_checkpoints; - uint32_t m_timeoutSeconds = 0; + uint32_t m_timeoutSeconds = RouterDelegate::kNoTimeout; }; struct Route diff --git a/routing/routing_helpers.cpp b/routing/routing_helpers.cpp index eb9e065507..9f321f06a2 100644 --- a/routing/routing_helpers.cpp +++ b/routing/routing_helpers.cpp @@ -1,4 +1,5 @@ #include "routing/routing_helpers.hpp" + #include "routing/road_point.hpp" #include "routing/segment.hpp" diff --git a/routing/routing_tests/async_router_test.cpp b/routing/routing_tests/async_router_test.cpp index a680258694..db2f89d192 100644 --- a/routing/routing_tests/async_router_test.cpp +++ b/routing/routing_tests/async_router_test.cpp @@ -121,7 +121,7 @@ UNIT_CLASS_TEST(AsyncGuiThreadTest, NeedMoreMapsSignalTest) async.CalculateRoute(Checkpoints({1, 2} /* start */, {5, 6} /* finish */), {3, 4}, false, bind(ref(resultCallback), _1, _2) /* readyCallback */, bind(ref(resultCallback), _1, _2) /* needMoreMapsCallback */, - nullptr /* removeRouteCallback */, nullptr /* progressCallback */, 0); + nullptr /* removeRouteCallback */, nullptr /* progressCallback */); resultCallback.WaitFinish(); @@ -143,7 +143,7 @@ UNIT_CLASS_TEST(AsyncGuiThreadTest, StandardAsyncFogTest) async.SetRouter(move(router), move(fetcher)); async.CalculateRoute(Checkpoints({1, 2} /* start */, {5, 6} /* finish */), {3, 4}, false, bind(ref(resultCallback), _1, _2), nullptr /* needMoreMapsCallback */, - nullptr /* progressCallback */, nullptr /* removeRouteCallback */, 0); + nullptr /* progressCallback */, nullptr /* removeRouteCallback */); resultCallback.WaitFinish(); diff --git a/routing/routing_tests/routing_algorithm.cpp b/routing/routing_tests/routing_algorithm.cpp index ae32d60051..0bec7a4e12 100644 --- a/routing/routing_tests/routing_algorithm.cpp +++ b/routing/routing_tests/routing_algorithm.cpp @@ -192,9 +192,10 @@ TestAStarBidirectionalAlgo::Result TestAStarBidirectionalAlgo::CalculateRoute( RoutingResult & path) { RoadGraph roadGraph(graph); - base::Cancellable const cancellable; + base::Cancellable cancellable; Algorithm::Params params(roadGraph, startPos, finalPos, {} /* prevRoute */, - cancellable, {} /* onVisitJunctionFn */, {} /* checkLength */); + cancellable, {} /* onVisitJunctionFn */, + {} /* checkLength */); Algorithm::Result const res = Algorithm().FindPathBidirectional(params, path); return Convert(res); } diff --git a/routing/routing_tests/routing_session_test.cpp b/routing/routing_tests/routing_session_test.cpp index f9fd641b96..fd08022ee8 100644 --- a/routing/routing_tests/routing_session_test.cpp +++ b/routing/routing_tests/routing_session_test.cpp @@ -153,8 +153,10 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestRouteBuilding) LOG(LINFO, ("Ready")); timedSignal.Signal(); }, - nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, + nullptr /* removeRouteCallback */); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); }); // Manual check of the routeBuilding mutex to avoid spurious results. @@ -185,7 +187,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestRouteRebuildingMovingA [&alongTimedSignal](Route const &, RouterResultCode) { alongTimedSignal.Signal(); }, nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::RouterDelegate::kNoTimeout); }); TEST(alongTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); TEST_EQUAL(counter, 1, ()); @@ -219,7 +222,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestRouteRebuildingMovingA {SessionState::OnRoute, SessionState::RoutingNotActive, SessionState::RouteBuilding}, *m_session); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); } }); TEST(oppositeTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); @@ -265,7 +269,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestRouteRebuildingMovingT [&alongTimedSignal](Route const &, RouterResultCode) { alongTimedSignal.Signal(); }, nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); }); TEST(alongTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); TEST_EQUAL(counter, 1, ()); @@ -314,7 +319,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestFollowRouteFlagPersist [&alongTimedSignal](Route const &, RouterResultCode) { alongTimedSignal.Signal(); }, nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); }); TEST(alongTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); @@ -342,7 +348,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestFollowRouteFlagPersist [&oppositeTimedSignal](Route const &, RouterResultCode) { oppositeTimedSignal.Signal(); }, nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); }); TEST(oppositeTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); @@ -367,7 +374,7 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestFollowRouteFlagPersist m_session->RebuildRoute( kTestRoute.front(), [&rebuildTimedSignal](Route const &, RouterResultCode) { rebuildTimedSignal.Signal(); }, - nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */, 0, + nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */, RouterDelegate::kNoTimeout, SessionState::RouteBuilding, false /* adjust */); }); TEST(rebuildTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); @@ -401,7 +408,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadTestWithRoutingSession, TestFollowRoutePercentTest [&alongTimedSignal](Route const &, RouterResultCode) { alongTimedSignal.Signal(); }, nullptr /* rebuildReadyCallback */, nullptr /* needMoreMapsCallback */, nullptr /* removeRouteCallback */); - m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), 0); + m_session->BuildRoute(Checkpoints(kTestRoute.front(), kTestRoute.back()), + RouterDelegate::kNoTimeout); }); TEST(alongTimedSignal.WaitUntil(steady_clock::now() + kRouteBuildingMaxDuration), ("Route was not built.")); diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 70074de54d..e071713a5b 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -569,13 +569,13 @@ bool GetNextRoutePointIndex(IRoutingResult const & result, RoutePointIndex const } RouterResultCode MakeTurnAnnotation(IRoutingResult const & result, NumMwmIds const & numMwmIds, - RouterDelegate const & delegate, + base::Cancellable const & cancellable, vector & junctions, Route::TTurns & turnsDir, Route::TStreets & streets, vector & segments) { LOG(LDEBUG, ("Shortest th length:", result.GetPathLength())); - if (delegate.IsCancelled()) + if (cancellable.IsCancelled()) return RouterResultCode::Cancelled; // Annotate turns. size_t skipTurnSegments = 0; diff --git a/routing/turns_generator.hpp b/routing/turns_generator.hpp index 0698e19655..06dd9e7901 100644 --- a/routing/turns_generator.hpp +++ b/routing/turns_generator.hpp @@ -94,9 +94,9 @@ bool GetNextRoutePointIndex(IRoutingResult const & result, RoutePointIndex const * \return routing operation result code. */ RouterResultCode MakeTurnAnnotation(IRoutingResult const & result, NumMwmIds const & numMwmIds, - RouterDelegate const & delegate, std::vector & points, - Route::TTurns & turnsDir, Route::TStreets & streets, - std::vector & segments); + base::Cancellable const & cancellable, + std::vector & points, Route::TTurns & turnsDir, + Route::TStreets & streets, std::vector & segments); // Returns the distance in meractor units for the path of points for the range [startPointIndex, endPointIndex]. double CalculateMercatorDistanceAlongPath(uint32_t startPointIndex, uint32_t endPointIndex, -- cgit v1.2.3