From b0845659f28e45bd84cb0f942a68006b8f424d34 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 23 May 2018 16:23:57 +0300 Subject: [PATCH] Moving the extrapolator to RoutingManager switching it on only for car and bicycle following mode. --- map/extrapolation/extrapolator.cpp | 16 ++++++++++--- map/extrapolation/extrapolator.hpp | 7 +++++- map/framework.cpp | 5 +--- map/framework.hpp | 2 -- map/routing_manager.cpp | 38 +++++++++++++++++++----------- map/routing_manager.hpp | 4 ++++ 6 files changed, 48 insertions(+), 24 deletions(-) diff --git a/map/extrapolation/extrapolator.cpp b/map/extrapolation/extrapolator.cpp index ac66fc424e..bb44b7d73e 100644 --- a/map/extrapolation/extrapolator.cpp +++ b/map/extrapolation/extrapolator.cpp @@ -62,7 +62,9 @@ location::GpsInfo LinearExtrapolation(location::GpsInfo const & gpsInfo1, // @TODO(bykoianko) Now |result.m_bearing == gpsInfo2.m_bearing|. // In case of |gpsInfo1.HasBearing() && gpsInfo2.HasBearing() == true| // consider finding an average value between |gpsInfo1.m_bearing| and |gpsInfo2.m_bearing| - // taking into account that they are periodic. + // taking into account that they are periodic. It's important to implement it + // because current implementation leads to changing course by steps. It doesn't + // look nice when the road changes its direction. if (gpsInfo1.HasSpeed() && gpsInfo2.HasSpeed()) result.m_speed = e.Extrapolate(gpsInfo1.m_speed, gpsInfo2.m_speed); @@ -72,7 +74,7 @@ location::GpsInfo LinearExtrapolation(location::GpsInfo const & gpsInfo1, // Extrapolator ------------------------------------------------------------------------------------ Extrapolator::Extrapolator(ExtrapolatedLocationUpdateFn const & update) - : m_extrapolatedLocationUpdate(update) + : m_isEnabled(false), m_extrapolatedLocationUpdate(update) { GetPlatform().RunTask(Platform::Thread::Background, [this] { @@ -82,12 +84,20 @@ Extrapolator::Extrapolator(ExtrapolatedLocationUpdateFn const & update) void Extrapolator::OnLocationUpdate(location::GpsInfo const & gpsInfo) { + // @TODO Consider calling ExtrapolatedLocationUpdate() on background thread immediately + // after OnLocationUpdate() was called. lock_guard guard(m_mutex); m_beforeLastGpsInfo = m_lastGpsInfo; m_lastGpsInfo = gpsInfo; m_extrapolationCounter = 0; } +void Extrapolator::Enable(bool enabled) +{ + lock_guard guard(m_mutex); + m_isEnabled = enabled; +} + void Extrapolator::ExtrapolatedLocationUpdate() { location::GpsInfo gpsInfo; @@ -135,7 +145,7 @@ bool Extrapolator::DoesExtrapolationWork(uint64_t extrapolationTimeMs) const // It may happen in rare cases because GpsInfo::m_timestamp is not monotonic generally. // Please see comment in declaration of class GpsInfo for details. - if (m_extrapolationCounter == m_extrapolationCounterUndefined || + if (!m_isEnabled || m_extrapolationCounter == m_extrapolationCounterUndefined || !m_lastGpsInfo.IsValid() || !m_beforeLastGpsInfo.IsValid() || m_beforeLastGpsInfo.m_timestamp >= m_lastGpsInfo.m_timestamp) { diff --git a/map/extrapolation/extrapolator.hpp b/map/extrapolation/extrapolator.hpp index e8c3b7fd30..2a6de3b1a6 100644 --- a/map/extrapolation/extrapolator.hpp +++ b/map/extrapolation/extrapolator.hpp @@ -29,10 +29,15 @@ public: // @TODO(bykoianko) Gyroscope information should be taken into account as well for calculation // extrapolated position. - void ExtrapolatedLocationUpdate(); + void Enable(bool enabled); private: + /// \returns true if there's enough information for extrapolation and extrapolation is enabled. + /// \note This method should be called only when |m_mutex| is locked. bool DoesExtrapolationWork(uint64_t extrapolationTimeMs) const; + void ExtrapolatedLocationUpdate(); + + bool m_isEnabled; std::mutex m_mutex; ExtrapolatedLocationUpdateFn m_extrapolatedLocationUpdate; diff --git a/map/framework.cpp b/map/framework.cpp index 4e8886b505..8924a4b301 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -216,7 +216,7 @@ void Framework::OnLocationUpdate(GpsInfo const & info) GpsInfo rInfo(info); #endif - m_extrapolator.OnLocationUpdate(rInfo); + m_routingManager.OnLocationUpdate(rInfo); } void Framework::OnCompassUpdate(CompassInfo const & info) @@ -367,9 +367,6 @@ Framework::Framework(FrameworkParams const & params) }, [this]() -> StringsBundle const & { return m_stringsBundle; }), static_cast(*this)) - , m_extrapolator([this](location::GpsInfo const & gpsInfo) { - this->GetRoutingManager().OnLocationUpdate(gpsInfo); - }) , m_trafficManager(bind(&Framework::GetMwmsByRect, this, _1, false /* rough */), kMaxTrafficCacheSizeBytes, m_routingManager.RoutingSession()) , m_bookingFilterProcessor(m_model.GetIndex(), *m_bookingApi) diff --git a/map/framework.hpp b/map/framework.hpp index 686d5f4608..5bd31099a2 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -7,7 +7,6 @@ #include "map/bookmark_manager.hpp" #include "map/discovery/discovery_manager.hpp" #include "map/displacement_mode_manager.hpp" -#include "map/extrapolation/extrapolator.hpp" #include "map/feature_vec_model.hpp" #include "map/local_ads_manager.hpp" #include "map/mwm_url.hpp" @@ -208,7 +207,6 @@ protected: // Note. |m_routingManager| should be declared before |m_trafficManager| RoutingManager m_routingManager; - extrapolation::Extrapolator m_extrapolator; TrafficManager m_trafficManager; diff --git a/map/routing_manager.cpp b/map/routing_manager.cpp index 1a57ca8932..6ca3e533b7 100644 --- a/map/routing_manager.cpp +++ b/map/routing_manager.cpp @@ -220,6 +220,8 @@ RoutingManager::RoutingManager(Callbacks && callbacks, Delegate & delegate) , m_delegate(delegate) , m_trackingReporter(platform::CreateSocket(), TRACKING_REALTIME_HOST, TRACKING_REALTIME_PORT, tracking::Reporter::kPushDelayMs) + , m_extrapolator( + [this](location::GpsInfo const & gpsInfo) { this->OnExtrapolatedLocationUpdate(gpsInfo); }) , m_transitReadManager(m_callbacks.m_indexGetter(), m_callbacks.m_readFeaturesFn) { auto const routingStatisticsFn = [](map const & statistics) { @@ -327,6 +329,11 @@ void RoutingManager::OnRoutePointPassed(RouteMarkType type, size_t intermediateI SaveRoutePoints(); } +void RoutingManager::OnLocationUpdate(location::GpsInfo const & info) +{ + m_extrapolator.OnLocationUpdate(info); +} + RouterType RoutingManager::GetBestRouter(m2::PointD const & startPoint, m2::PointD const & finalPoint) const { @@ -557,6 +564,8 @@ void RoutingManager::FollowRoute() if (!m_routingSession.EnableFollowMode()) return; + // Switching on the extrapolatior only for following mode in car and bicycle navigation. + m_extrapolator.Enable(m_currentRouterType == RouterType::Vehicle || m_currentRouterType == RouterType::Bicycle); m_delegate.OnRouteFollow(m_currentRouterType); HideRoutePoint(RouteMarkType::Start); @@ -567,6 +576,7 @@ void RoutingManager::FollowRoute() void RoutingManager::CloseRouting(bool removeRoutePoints) { + m_extrapolator.Enable(false); // Hide preview. HidePreviewSegments(); @@ -918,20 +928,6 @@ void RoutingManager::MatchLocationToRoute(location::GpsInfo & location, m_routingSession.MatchLocationToRoute(location, routeMatchingInfo); } -void RoutingManager::OnLocationUpdate(location::GpsInfo const & info) -{ - location::GpsInfo gpsInfo(info); - if (!m_drapeEngine) - m_gpsInfoCache = my::make_unique(gpsInfo); - - auto routeMatchingInfo = GetRouteMatchingInfo(gpsInfo); - m_drapeEngine.SafeCall(&df::DrapeEngine::SetGpsInfo, gpsInfo, - m_routingSession.IsNavigable(), routeMatchingInfo); - - if (IsTrackingReporterEnabled()) - m_trackingReporter.AddLocation(gpsInfo, m_routingSession.MatchTraffic(routeMatchingInfo)); -} - location::RouteMatchingInfo RoutingManager::GetRouteMatchingInfo(location::GpsInfo & info) { location::RouteMatchingInfo routeMatchingInfo; @@ -1243,6 +1239,20 @@ vector RoutingManager::GetRoutePointsToSave() const return result; } +void RoutingManager::OnExtrapolatedLocationUpdate(location::GpsInfo const & info) +{ + location::GpsInfo gpsInfo(info); + if (!m_drapeEngine) + m_gpsInfoCache = my::make_unique(gpsInfo); + + auto routeMatchingInfo = GetRouteMatchingInfo(gpsInfo); + m_drapeEngine.SafeCall(&df::DrapeEngine::SetGpsInfo, gpsInfo, + m_routingSession.IsNavigable(), routeMatchingInfo); + + if (IsTrackingReporterEnabled()) + m_trackingReporter.AddLocation(gpsInfo, m_routingSession.MatchTraffic(routeMatchingInfo)); +} + void RoutingManager::DeleteSavedRoutePoints() { if (!HasSavedRoutePoints()) diff --git a/map/routing_manager.hpp b/map/routing_manager.hpp index 4da46cc9e6..34504894dd 100644 --- a/map/routing_manager.hpp +++ b/map/routing_manager.hpp @@ -1,6 +1,7 @@ #pragma once #include "map/bookmark_manager.hpp" +#include "map/extrapolation/extrapolator.hpp" #include "map/routing_mark.hpp" #include "map/transit/transit_display.hpp" #include "map/transit/transit_reader.hpp" @@ -281,6 +282,8 @@ private: std::vector GetRoutePointsToSave() const; + void OnExtrapolatedLocationUpdate(location::GpsInfo const & info); + RouteBuildingCallback m_routingCallback = nullptr; RouteRecommendCallback m_routeRecommendCallback = nullptr; Callbacks m_callbacks; @@ -291,6 +294,7 @@ private: Delegate & m_delegate; tracking::Reporter m_trackingReporter; BookmarkManager * m_bmManager = nullptr; + extrapolation::Extrapolator m_extrapolator; std::vector m_drapeSubroutes; mutable std::mutex m_drapeSubroutesMutex;