From d47713516d713de12de565a9f50e913de2f57540 Mon Sep 17 00:00:00 2001 From: mvglasow Date: Sun, 8 Jun 2025 19:28:27 +0300 Subject: [PATCH] [traffxml] Ensure decoder uses newly-added maps Signed-off-by: mvglasow --- traffxml/traff_decoder.cpp | 89 ++++++++++++++++++++++---------------- traffxml/traff_decoder.hpp | 27 +++++++++++- 2 files changed, 77 insertions(+), 39 deletions(-) diff --git a/traffxml/traff_decoder.cpp b/traffxml/traff_decoder.cpp index a67ec2d91..632354b7d 100644 --- a/traffxml/traff_decoder.cpp +++ b/traffxml/traff_decoder.cpp @@ -528,30 +528,38 @@ RoutingTraffDecoder::RoutingTraffDecoder(DataSource & dataSource, CountryInfoGet std::map & messageCache) : TraffDecoder(dataSource, countryInfoGetter, countryParentNameGetter, messageCache) { + m_dataSource.AddObserver(*this); InitRouter(); } +void RoutingTraffDecoder::OnMapRegistered(platform::LocalCountryFile const & localFile) +{ + std::lock_guard lock(m_mutex); + // register with our router instance, unless it is World or WorldCoasts. + if (!localFile.GetCountryName().starts_with(WORLD_FILE_NAME)) + m_numMwmIds->RegisterFile(localFile.GetCountryFile()); +} + bool RoutingTraffDecoder::InitRouter() { + std::lock_guard lock(m_mutex); if (m_router) return true; - // code mostly from RoutingManager::SetRouterImpl(RouterType) /* - * RoutingManager::SetRouterImpl(RouterType) calls m_delegate.RegisterCountryFilesOnRoute(numMwmIds). - * m_delegate is the framework, and the routine cycles through the countries in storage. - * As we don’t have access to storage, we get our country files from the data source. + * Code based on RoutingManager::SetRouterImpl(RouterType), which calls + * m_delegate.RegisterCountryFilesOnRoute(numMwmIds); m_delegate being the framework instance. + * RegisterCountryFilesOnRoute() is protected and uses a private `Storage` instance. + * We therefore have to resort to populating `m_numMwmIds` from `m_dataSource`. Unlike the + * “original”, this will only include MWMs loaded on startup, not those added later. + * For these, we register ourselves as an MwmSet::Observer and add maps to `m_numMwms` as they + * are registered. + * World and WorldCoasts must be excluded (as in the original routine), as they would cause the + * router to return bogus routes. Just like the original, we use a string comparison for this. */ std::vector> mwmsInfo; m_dataSource.GetMwmsInfo(mwmsInfo); - /* TODO this should include all countries (whether we have the MWM or not), except World and WorldCoasts. - * Excluding World and WorldCoasts is important, else the router will return bogus routes. - * Storage uses a string comparison for filtering, we do the same here. - storage.ForEachCountry([&](storage::Country const & country) - { - numMwmIds->RegisterFile(country.GetFile()); - }); - */ + for (auto mwmInfo : mwmsInfo) if (!mwmInfo->GetCountryName().starts_with(WORLD_FILE_NAME)) m_numMwmIds->RegisterFile(mwmInfo->GetLocalFile().GetCountryFile()); @@ -702,41 +710,46 @@ void RoutingTraffDecoder::DecodeLocationDirection(traffxml::TraffMessage & messa */ routing::RouterDelegate delegate; delegate.SetTimeout(kRouterTimeoutSec); + routing::RouterResultCode code; + std::shared_ptr route; - if (!m_router && !InitRouter()) - return; + { + std::lock_guard lock(m_mutex); - /* + if (!m_router && !InitRouter()) + return; + + /* * TODO is that for following a track? If so, can we use that with just 2–3 reference points? * – Doesn’t look like it, m_guides only seems to get used in test functions */ - //router->SetGuides(std::move(m_guides)); - //m_guides.clear(); + //router->SetGuides(std::move(m_guides)); + //m_guides.clear(); - auto route = std::make_shared(m_router->GetName(), routeId); - routing::RouterResultCode code; + route = std::make_shared(m_router->GetName(), routeId); - base::Timer timer; - double elapsedSec = 0.0; + base::Timer timer; + double elapsedSec = 0.0; - try - { - LOG(LINFO, ("Calculating the route of direct length", checkpoints.GetSummaryLengthBetweenPointsMeters(), - "m. checkpoints:", checkpoints, "startDirection:", startDirection, "router name:", m_router->GetName())); + try + { + LOG(LINFO, ("Calculating the route of direct length", checkpoints.GetSummaryLengthBetweenPointsMeters(), + "m. checkpoints:", checkpoints, "startDirection:", startDirection, "router name:", m_router->GetName())); - // Run basic request. - code = m_router->CalculateRoute(checkpoints, startDirection, adjustToPrevRoute, - delegate, *route); - m_router->SetGuides({}); - elapsedSec = timer.ElapsedSeconds(); // routing time - LogCode(code, elapsedSec); - LOG(LINFO, ("ETA:", route->GetTotalTimeSec(), "sec.")); - } - catch (RootException const & e) - { - code = routing::RouterResultCode::InternalError; - LOG(LERROR, ("Exception happened while calculating route:", e.Msg())); - return; + // Run basic request. + code = m_router->CalculateRoute(checkpoints, startDirection, adjustToPrevRoute, + delegate, *route); + m_router->SetGuides({}); + elapsedSec = timer.ElapsedSeconds(); // routing time + LogCode(code, elapsedSec); + LOG(LINFO, ("ETA:", route->GetTotalTimeSec(), "sec.")); + } + catch (RootException const & e) + { + code = routing::RouterResultCode::InternalError; + LOG(LERROR, ("Exception happened while calculating route:", e.Msg())); + return; + } } if (code == routing::RouterResultCode::NoError) diff --git a/traffxml/traff_decoder.hpp b/traffxml/traff_decoder.hpp index 4207932bc..81524f06c 100644 --- a/traffxml/traff_decoder.hpp +++ b/traffxml/traff_decoder.hpp @@ -3,6 +3,7 @@ #include "traffxml/traff_model.hpp" #include "indexer/data_source.hpp" +#include "indexer/mwm_set.hpp" #include "openlr/openlr_decoder.hpp" #include "openlr/openlr_model.hpp" @@ -173,7 +174,8 @@ private: /** * @brief A `TraffDecoder` implementation which internally uses the routing engine. */ -class RoutingTraffDecoder : public TraffDecoder +class RoutingTraffDecoder : public TraffDecoder, + public MwmSet::Observer { public: class DecoderRouter : public routing::IndexRouter @@ -283,6 +285,18 @@ public: const CountryParentNameGetterFn & countryParentNameGetter, std::map & messageCache); + /** + * @brief Called when a map is registered for the first time and can be used. + */ + void OnMapRegistered(platform::LocalCountryFile const & localFile) override; + + /** + * @brief Called when a map is deregistered and can no longer be used. + * + * This implementation does nothing, as `NumMwmIds` does not support removal. + */ + virtual void OnMapDeregistered(platform::LocalCountryFile const & /* localFile */) {} + protected: /** * @brief Initializes the router. @@ -327,6 +341,17 @@ protected: private: static void LogCode(routing::RouterResultCode code, double const elapsedSec); + /** + * @brief Mutex for access to shared members. + * + * This is to prevent adding newly-registered maps while the router is in use. + * + * @todo As per the `MwmSet::Observer` documentation, implementations should be quick and lean, + * as they may be called from any thread. Locking a mutex may be in conflict with this, as it may + * mean locking up the caller while a location is being decoded. + */ + std::mutex m_mutex; + std::shared_ptr m_numMwmIds = std::make_shared(); std::unique_ptr m_router; std::optional m_message = std::nullopt;