From f260a12fdad7b85260b1effde2baee5484add400 Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Fri, 31 Jul 2015 16:43:54 +0300 Subject: [PATCH] PR fixes --- map/framework.cpp | 3 +- routing/async_router.hpp | 3 -- routing/osrm_router.cpp | 27 ++++++---- routing/router_delegate.cpp | 14 ++++- routing/router_delegate.hpp | 3 +- routing/routing_tests/async_router_test.cpp | 59 +++++++++++++-------- 6 files changed, 69 insertions(+), 40 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 3dc84ca253..bf580c10a3 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2124,7 +2124,6 @@ void Framework::BuildRoute(m2::PointD const & destination, uint32_t timeoutSec) RemoveRoute(); } CallRouteBuilded(code, absentCountries, absentRoutingIndexes); - LOG(LINFO, ("Inside callback!")); }; m_routingSession.BuildRoute(state->Position(), destination, @@ -2189,7 +2188,7 @@ void Framework::SetRouterImpl(RouterType type) }); }; #else - routing::TPointCheckCallback const routingVisualizerFn = nullptr; + routing::RouterDelegate::TPointCheckCallback const routingVisualizerFn = nullptr; #endif m_routingSession.SetRouter(move(router), move(fetcher), routingStatisticsFn, routingVisualizerFn); m_currentRouterType = type; diff --git a/routing/async_router.hpp b/routing/async_router.hpp index db5adbe623..148df81826 100644 --- a/routing/async_router.hpp +++ b/routing/async_router.hpp @@ -49,9 +49,6 @@ public: /// Interrupt routing and clear buffers virtual void ClearState(); - /// Wait routing process. For testing use. - void WaitRoutingForTesting() { lock_guard routingGuard(m_routingMutex); } - private: /// This function is called in async mode void CalculateRouteImpl(TReadyCallback const & readyCallback, uint32_t timeoutSec); diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 410f038476..8fa2e10ddf 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -32,18 +32,25 @@ #include "3party/osrm/osrm-backend/data_structures/internal_route_result.hpp" #include "3party/osrm/osrm-backend/descriptors/description_factory.hpp" -#define INTERRUPT_WHEN_CANCELLED() \ +#define INTERRUPT_WHEN_CANCELLED(DELEGATE) \ do \ { \ - if (delegate.IsCancelled()) \ + if (DELEGATE.IsCancelled()) \ return Cancelled; \ } while (false) namespace routing { + +namespace +{ size_t constexpr kMaxNodeCandidatesCount = 10; double constexpr kFeatureFindingRectSideRadiusMeters = 1000.0; - +double constexpr kMwmLoadedProgress = 10.0f; +double constexpr kPointsFoundProgress = 15.0f; +double constexpr kCrossPathFoundProgress = 50.0f; +double constexpr kPathFoundProgress = 70.0f; +} // namespace // TODO (ldragunov) Switch all RawRouteData and incapsulate to own omim types. using RawRouteData = InternalRouteResult; @@ -554,7 +561,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, LOG(LINFO, ("Duration of the MWM loading", timer.ElapsedNano())); timer.Reset(); - delegate.OnProgress(10.0f); + delegate.OnProgress(kMwmLoadedProgress); // 3. Find start/end nodes. TFeatureGraphNodeVec startTask; @@ -576,11 +583,11 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, m_cachedTargetPoint = finalPoint; } } - INTERRUPT_WHEN_CANCELLED(); + INTERRUPT_WHEN_CANCELLED(delegate); LOG(LINFO, ("Duration of the start/stop points lookup", timer.ElapsedNano())); timer.Reset(); - delegate.OnProgress(15.0f); + delegate.OnProgress(kPointsFoundProgress); // 4. Find route. RawRoutingResult routingResult; @@ -598,8 +605,8 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, { return RouteNotFound; } - INTERRUPT_WHEN_CANCELLED(); - delegate.OnProgress(70.0f); + INTERRUPT_WHEN_CANCELLED(delegate); + delegate.OnProgress(kPathFoundProgress); // 5. Restore route. @@ -624,7 +631,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, delegate, finalPath); timer.Reset(); - delegate.OnProgress(50.0f); + delegate.OnProgress(kCrossPathFoundProgress); // 5. Make generate answer if (code == NoError) @@ -692,7 +699,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( for (auto const & segment : routingResult.unpackedPathSegments) { - INTERRUPT_WHEN_CANCELLED(); + INTERRUPT_WHEN_CANCELLED(delegate); // Get all the coordinates for the computed route size_t const n = segment.size(); diff --git a/routing/router_delegate.cpp b/routing/router_delegate.cpp index 42b25209dc..80784539a4 100644 --- a/routing/router_delegate.cpp +++ b/routing/router_delegate.cpp @@ -16,12 +16,16 @@ RouterDelegate::RouterDelegate() void RouterDelegate::SetProgressCallback(TProgressCallback const & progressCallback) { - m_progressCallback = (progressCallback) ? progressCallback : DefaultProgressFn; + m_progressCallback = progressCallback ? progressCallback : DefaultProgressFn; } void RouterDelegate::SetPointCheckCallback(TPointCheckCallback const & pointCallback) { - m_pointCallback = (pointCallback) ? pointCallback : DefaultPointFn; + m_pointCallback = pointCallback ? pointCallback : DefaultPointFn; +} + +TimeoutCancellable::TimeoutCancellable() : m_timeoutSec(0) +{ } bool TimeoutCancellable::IsCancelled() const @@ -31,6 +35,12 @@ bool TimeoutCancellable::IsCancelled() const return Cancellable::IsCancelled(); } +void TimeoutCancellable::Reset() +{ + m_timeoutSec = 0; + Cancellable::Reset(); +} + void TimeoutCancellable::SetTimeout(uint32_t timeoutSec) { m_timeoutSec = timeoutSec; diff --git a/routing/router_delegate.hpp b/routing/router_delegate.hpp index ba82340ed4..f2373c816e 100644 --- a/routing/router_delegate.hpp +++ b/routing/router_delegate.hpp @@ -12,13 +12,14 @@ namespace routing class TimeoutCancellable : public my::Cancellable { public: - TimeoutCancellable() : m_timeoutSec(0) {} + TimeoutCancellable(); /// Sets timeout before cancellation. 0 means an infinite timeout. void SetTimeout(uint32_t timeoutSec); // Cancellable overrides: bool IsCancelled() const override; + void Reset() override; private: my::Timer m_timer; diff --git a/routing/routing_tests/async_router_test.cpp b/routing/routing_tests/async_router_test.cpp index f6920c725c..8f826800b3 100644 --- a/routing/routing_tests/async_router_test.cpp +++ b/routing/routing_tests/async_router_test.cpp @@ -8,6 +8,8 @@ #include "base/timer.hpp" +#include "std/condition_variable.hpp" +#include "std/mutex.hpp" #include "std/string.hpp" #include "std/vector.hpp" @@ -59,54 +61,67 @@ struct DummyResultCallback { vector m_codes; vector> m_absent; + condition_variable m_cv; + mutex m_lock; + uint32_t const m_expected; + uint32_t m_called; + + DummyResultCallback(uint32_t expectedCalls) : m_expected(expectedCalls), m_called(0) {} void operator()(Route & route, ResultCode code) { m_codes.push_back(code); auto const & absent = route.GetAbsentCountries(); m_absent.emplace_back(absent.begin(), absent.end()); + { + lock_guard calledLock(m_lock); + ++m_called; + TEST_LESS_OR_EQUAL(m_called, m_expected, + ("The result callback called more times than expected.")); + } + m_cv.notify_all(); + } + + void WaitFinish() + { + unique_lock lk(m_lock); + return m_cv.wait(lk, [this] { return m_called == m_expected; }); } }; UNIT_TEST(NeedMoreMapsSignalTest) { - unique_ptr fetcher(new DummyFetcher({"test1", "test2"})); + vector const absentData({"test1", "test2"}); + unique_ptr fetcher(new DummyFetcher(absentData)); unique_ptr router(new DummyRouter(ResultCode::NoError, {})); - DummyResultCallback resultCallback; - AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, nullptr); - resultCallback.m_codes.clear(); - resultCallback.m_absent.clear(); - async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), nullptr, 0); + DummyResultCallback resultCallback(2 /* expectedCalls */); + AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, + nullptr /* pointCheckCallback */); + async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), + nullptr /* progressCallback */, 0 /* timeoutSec */); - // Wait async process start. - while (resultCallback.m_codes.empty()) - ; - - async.WaitRoutingForTesting(); + resultCallback.WaitFinish(); TEST_EQUAL(resultCallback.m_codes.size(), 2, ()); TEST_EQUAL(resultCallback.m_codes[0], ResultCode::NoError, ()); TEST_EQUAL(resultCallback.m_codes[1], ResultCode::NeedMoreMaps, ()); TEST_EQUAL(resultCallback.m_absent.size(), 2, ()); TEST(resultCallback.m_absent[0].empty(), ()); - TEST_EQUAL(resultCallback.m_absent[1].size(), 2, ()) + TEST_EQUAL(resultCallback.m_absent[1].size(), 2, ()); + TEST_EQUAL(resultCallback.m_absent[1], absentData, ()); } UNIT_TEST(StandartAsyncFogTest) { unique_ptr fetcher(new DummyFetcher({})); unique_ptr router(new DummyRouter(ResultCode::NoError, {})); - DummyResultCallback resultCallback; - AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, nullptr); - resultCallback.m_codes.clear(); - resultCallback.m_absent.clear(); - async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), nullptr, 0); + DummyResultCallback resultCallback(1 /* expectedCalls */); + AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, + nullptr /* pointCheckCallback */); + async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), + nullptr /* progressCallback */, 0 /* timeoutSec */); - // Wait async process start. - while (resultCallback.m_codes.empty()) - ; - - async.WaitRoutingForTesting(); + resultCallback.WaitFinish(); TEST_EQUAL(resultCallback.m_codes.size(), 1, ()); TEST_EQUAL(resultCallback.m_codes[0], ResultCode::NoError, ());