Review fixes.

This commit is contained in:
Daria Volvenkova 2016-12-01 17:47:42 +03:00
parent fb61ac89e6
commit 694410c11b
4 changed files with 106 additions and 93 deletions

View file

@ -1,7 +1,5 @@
#include "map/traffic_manager.hpp"
#include "platform/platform.hpp"
#include "routing/routing_helpers.hpp"
#include "drape_frontend/drape_engine.hpp"
@ -10,6 +8,8 @@
#include "indexer/ftypes_matcher.hpp"
#include "indexer/scales.hpp"
#include "platform/platform.hpp"
namespace
{
auto constexpr kUpdateInterval = minutes(1);
@ -19,10 +19,28 @@ auto constexpr kNetworkErrorTimeout = minutes(20);
auto constexpr kMaxRetriesCount = 5;
} // namespace
TrafficManager::CacheEntry::CacheEntry()
: m_isLoaded(false)
, m_dataSize(0)
, m_retriesCount(0)
, m_isWaitingForResponse(false)
, m_lastAvailability(traffic::TrafficInfo::Availability::Unknown)
{}
TrafficManager::CacheEntry::CacheEntry(time_point<steady_clock> const & requestTime)
: m_isLoaded(false)
, m_dataSize(0)
, m_lastRequestTime(requestTime)
, m_retriesCount(0)
, m_isWaitingForResponse(false)
, m_lastAvailability(traffic::TrafficInfo::Availability::Unknown)
{}
TrafficManager::TrafficManager(GetMwmsByRectFn const & getMwmsByRectFn, size_t maxCacheSizeBytes)
: m_getMwmsByRectFn(getMwmsByRectFn)
, m_currentDataVersion(0)
, m_state(TrafficState::Disabled)
, m_notifyStateChanged(false)
, m_maxCacheSizeBytes(maxCacheSizeBytes)
, m_isRunning(true)
, m_thread(&TrafficManager::ThreadRoutine, this)
@ -33,7 +51,7 @@ TrafficManager::TrafficManager(GetMwmsByRectFn const & getMwmsByRectFn, size_t m
TrafficManager::~TrafficManager()
{
{
lock_guard<mutex> lock(m_requestedMwmsLock);
lock_guard<mutex> lock(m_mutex);
m_isRunning = false;
}
m_condition.notify_one();
@ -51,7 +69,15 @@ void TrafficManager::SetStateListener(TrafficStateChangedFn const & onStateChang
void TrafficManager::SetEnabled(bool enabled)
{
{
lock_guard<mutex> lock(m_requestedMwmsLock);
lock_guard<mutex> lock(m_mutex);
if (enabled == IsEnabled())
{
LOG(LWARNING, ("Invalid attempt to", enabled ? "enable" : "disable",
"traffic manager, it's already", enabled ? "enabled" : "disabled",
", doing nothing."));
return;
}
Clear();
ChangeState(enabled ? TrafficState::Enabled : TrafficState::Disabled);
}
@ -66,8 +92,6 @@ void TrafficManager::SetEnabled(bool enabled)
if (m_currentPosition.second)
UpdateMyPosition(m_currentPosition.first);
}
NotifyStateChanged();
}
void TrafficManager::Clear()
@ -92,10 +116,8 @@ void TrafficManager::OnDestroyGLContext()
if (!IsEnabled())
return;
{
lock_guard<mutex> lock(m_requestedMwmsLock);
Clear();
}
lock_guard<mutex> lock(m_mutex);
Clear();
}
void TrafficManager::OnRecoverGLContext()
@ -111,7 +133,7 @@ void TrafficManager::OnRecoverGLContext()
void TrafficManager::UpdateViewport(ScreenBase const & screen)
{
m_currentModelView = {screen, true};
m_currentModelView = {screen, true /* initialized */};
if (!IsEnabled() || IsInvalidState())
return;
@ -123,7 +145,7 @@ void TrafficManager::UpdateViewport(ScreenBase const & screen)
auto mwms = m_getMwmsByRectFn(screen.ClipRect());
{
lock_guard<mutex> lock(m_requestedMwmsLock);
lock_guard<mutex> lock(m_mutex);
m_activeMwms.clear();
for (auto const & mwm : mwms)
@ -134,13 +156,11 @@ void TrafficManager::UpdateViewport(ScreenBase const & screen)
RequestTrafficData();
}
NotifyStateChanged();
}
void TrafficManager::UpdateMyPosition(MyPosition const & myPosition)
{
m_currentPosition = {myPosition, true};
m_currentPosition = {myPosition, true /* initialized */};
if (!IsEnabled() || IsInvalidState())
return;
@ -170,8 +190,6 @@ void TrafficManager::ThreadRoutine()
LOG(LWARNING, ("Traffic request failed. Mwm =", mwm));
OnTrafficRequestFailed(info);
}
NotifyStateChanged();
}
mwms.clear();
}
@ -179,7 +197,7 @@ void TrafficManager::ThreadRoutine()
bool TrafficManager::WaitForRequest(vector<MwmSet::MwmId> & mwms)
{
unique_lock<mutex> lock(m_requestedMwmsLock);
unique_lock<mutex> lock(m_mutex);
bool const timeout = !m_condition.wait_for(lock, kUpdateInterval, [this]
{
@ -189,8 +207,11 @@ bool TrafficManager::WaitForRequest(vector<MwmSet::MwmId> & mwms)
if (!m_isRunning)
return false;
if (timeout && IsEnabled() && !IsInvalidState())
if (timeout)
{
if (!IsEnabled() || IsInvalidState())
return true;
mwms.reserve(m_activeMwms.size());
for (auto const & mwmId : m_activeMwms)
mwms.push_back(mwmId);
@ -248,7 +269,7 @@ void TrafficManager::RequestTrafficData(MwmSet::MwmId const & mwmId)
void TrafficManager::OnTrafficRequestFailed(traffic::TrafficInfo const & info)
{
lock_guard<mutex> lock(m_requestedMwmsLock);
lock_guard<mutex> lock(m_mutex);
auto it = m_mwmCache.find(info.GetMwmId());
if (it == m_mwmCache.end())
@ -257,24 +278,22 @@ void TrafficManager::OnTrafficRequestFailed(traffic::TrafficInfo const & info)
it->second.m_isWaitingForResponse = false;
it->second.m_lastAvailability = info.GetAvailability();
if (info.GetAvailability() == traffic::TrafficInfo::Availability::Unknown)
if (info.GetAvailability() == traffic::TrafficInfo::Availability::Unknown &&
!it->second.m_isLoaded)
{
if (!it->second.m_isLoaded)
if (m_activeMwms.find(info.GetMwmId()) != m_activeMwms.end())
{
if (m_activeMwms.find(info.GetMwmId()) != m_activeMwms.end())
if (it->second.m_retriesCount < kMaxRetriesCount)
{
if (it->second.m_retriesCount < kMaxRetriesCount)
{
it->second.m_lastRequestTime = steady_clock::now();
it->second.m_isWaitingForResponse = true;
RequestTrafficData(info.GetMwmId());
}
++it->second.m_retriesCount;
}
else
{
it->second.m_retriesCount = 0;
it->second.m_lastRequestTime = steady_clock::now();
it->second.m_isWaitingForResponse = true;
RequestTrafficData(info.GetMwmId());
}
++it->second.m_retriesCount;
}
else
{
it->second.m_retriesCount = 0;
}
}
@ -283,7 +302,7 @@ void TrafficManager::OnTrafficRequestFailed(traffic::TrafficInfo const & info)
void TrafficManager::OnTrafficDataResponse(traffic::TrafficInfo const & info)
{
lock_guard<mutex> lock(m_requestedMwmsLock);
lock_guard<mutex> lock(m_mutex);
auto it = m_mwmCache.find(info.GetMwmId());
if (it == m_mwmCache.end())
@ -355,7 +374,7 @@ void TrafficManager::UpdateState()
bool waiting = false;
bool networkError = false;
bool expiredApp = false;
bool expiredMwm = false;
bool expiredData = false;
bool noData = false;
for (auto const & mwmId : m_activeMwms)
@ -370,7 +389,7 @@ void TrafficManager::UpdateState()
else
{
expiredApp |= it->second.m_lastAvailability == traffic::TrafficInfo::Availability::ExpiredApp;
expiredMwm |= it->second.m_lastAvailability == traffic::TrafficInfo::Availability::ExpiredMwm;
expiredData |= it->second.m_lastAvailability == traffic::TrafficInfo::Availability::ExpiredData;
noData |= it->second.m_lastAvailability == traffic::TrafficInfo::Availability::NoData;
}
@ -392,7 +411,7 @@ void TrafficManager::UpdateState()
ChangeState(TrafficState::WaitingData);
else if (expiredApp)
ChangeState(TrafficState::ExpiredApp);
else if (expiredMwm)
else if (expiredData)
ChangeState(TrafficState::ExpiredData);
else if (noData)
ChangeState(TrafficState::NoData);
@ -408,19 +427,10 @@ void TrafficManager::ChangeState(TrafficState newState)
return;
m_state = newState;
m_notifyStateChanged = true;
}
void TrafficManager::NotifyStateChanged()
{
if (m_notifyStateChanged)
GetPlatform().RunOnGuiThread([this, newState]()
{
m_notifyStateChanged = false;
GetPlatform().RunOnGuiThread([this]()
{
if (m_onStateChangedFn != nullptr)
m_onStateChangedFn(m_state);
});
}
if (m_onStateChangedFn != nullptr)
m_onStateChangedFn(newState);
});
}

View file

@ -75,18 +75,19 @@ private:
void ThreadRoutine();
bool WaitForRequest(vector<MwmSet::MwmId> & mwms);
void RequestTrafficData();
void RequestTrafficData(MwmSet::MwmId const & mwmId);
void OnTrafficDataResponse(traffic::TrafficInfo const & info);
void OnTrafficRequestFailed(traffic::TrafficInfo const & info);
private:
// This is a group of methods that haven't their own synchronization inside.
void RequestTrafficData();
void RequestTrafficData(MwmSet::MwmId const & mwmId);
void Clear();
void CheckCacheSize();
void UpdateState();
void ChangeState(TrafficState newState);
void NotifyStateChanged();
bool IsInvalidState() const;
bool IsEnabled() const;
@ -94,33 +95,31 @@ private:
GetMwmsByRectFn m_getMwmsByRectFn;
ref_ptr<df::DrapeEngine> m_drapeEngine;
int64_t m_currentDataVersion = 0;
atomic<int64_t> m_currentDataVersion;
// These fields have a flag of their initialization.
pair<MyPosition, bool> m_currentPosition = {MyPosition(), false};
pair<ScreenBase, bool> m_currentModelView = {ScreenBase(), false};
atomic<TrafficState> m_state;
atomic<bool> m_notifyStateChanged;
TrafficStateChangedFn m_onStateChangedFn;
struct CacheEntry
{
CacheEntry() = default;
CacheEntry();
CacheEntry(time_point<steady_clock> const & requestTime);
CacheEntry(time_point<steady_clock> const & requestTime) : m_lastRequestTime(requestTime) {}
bool m_isLoaded = false;
size_t m_dataSize = 0;
bool m_isLoaded;
size_t m_dataSize;
time_point<steady_clock> m_lastSeenTime;
time_point<steady_clock> m_lastRequestTime;
time_point<steady_clock> m_lastResponseTime;
uint32_t m_retriesCount = 0;
bool m_isWaitingForResponse = false;
int m_retriesCount;
bool m_isWaitingForResponse;
traffic::TrafficInfo::Availability m_lastAvailability =
traffic::TrafficInfo::Availability::Unknown;
traffic::TrafficInfo::Availability m_lastAvailability;
};
size_t m_maxCacheSizeBytes;
@ -134,6 +133,6 @@ private:
set<MwmSet::MwmId> m_activeMwms;
vector<MwmSet::MwmId> m_requestedMwms;
mutex m_requestedMwmsLock;
mutex m_mutex;
thread m_thread;
};

View file

@ -4,12 +4,14 @@
#include "coding/bit_streams.hpp"
#include "coding/reader.hpp"
#include "coding/url_encode.hpp"
#include "coding/varint.hpp"
#include "coding/write_to_sink.hpp"
#include "coding/writer.hpp"
#include "base/assert.hpp"
#include "base/logging.hpp"
#include "base/string_utils.hpp"
#include "std/algorithm.hpp"
#include "std/string.hpp"
@ -22,18 +24,21 @@ namespace traffic
{
namespace
{
bool ReadRemoteFile(string const & url, string & result, int & errorCode)
bool ReadRemoteFile(string const & url, vector<uint8_t> & contents, int & errorCode)
{
platform::HttpClient request(url);
if (!request.RunHttpRequest())
{
LOG(LINFO, ("Couldn't run traffic request", url));
errorCode = -1;
errorCode = request.ErrorCode();
LOG(LINFO, ("Couldn't run traffic request", url, ". Error:", errorCode));
return false;
}
errorCode = request.ErrorCode();
result = request.ServerResponse();
string const & result = request.ServerResponse();
contents.resize(result.size());
memcpy(contents.data(), result.data(), result.size());
if (errorCode != 200)
{
@ -53,7 +58,7 @@ string MakeRemoteURL(string const & name, uint64_t version)
ss << TRAFFIC_DATA_BASE_URL;
if (version != 0)
ss << version << "/";
ss << name << TRAFFIC_FILE_EXTENSION;
ss << UrlEncode(name) << TRAFFIC_FILE_EXTENSION;
return ss.str();
}
} // namespace
@ -67,7 +72,7 @@ TrafficInfo::RoadSegmentId::RoadSegmentId(uint32_t fid, uint16_t idx, uint8_t di
}
// TrafficInfo --------------------------------------------------------------------------------
TrafficInfo::TrafficInfo(MwmSet::MwmId const & mwmId, int64_t const & currentDataVersion)
TrafficInfo::TrafficInfo(MwmSet::MwmId const & mwmId, int64_t currentDataVersion)
: m_mwmId(mwmId)
, m_currentDataVersion(currentDataVersion)
{}
@ -83,33 +88,31 @@ bool TrafficInfo::ReceiveTrafficData()
if (url.empty())
return false;
string result;
vector<uint8_t> contents;
int errorCode;
if (!ReadRemoteFile(url, result, errorCode))
if (!ReadRemoteFile(url, contents, errorCode))
{
if (errorCode == 404)
{
int64_t version = atoi(result.c_str());
string const result(reinterpret_cast<char*>(contents.data()), contents.size());
int64_t version = 0;
strings::to_int64(result.c_str(), version);
if (version > info->GetVersion() && version <= m_currentDataVersion)
m_availabilityStatus = Availability::ExpiredMwm;
m_availability = Availability::ExpiredData;
else if (version > m_currentDataVersion)
m_availabilityStatus = Availability::ExpiredApp;
m_availability = Availability::ExpiredApp;
else
m_availabilityStatus = Availability::NoData;
m_availability = Availability::NoData;
}
else
{
m_availabilityStatus = Availability::Unknown;
m_availability = Availability::Unknown;
}
return false;
}
vector<uint8_t> contents;
contents.resize(result.size());
for (size_t i = 0; i < result.size(); ++i)
contents[i] = static_cast<uint8_t>(result[i]);
Coloring coloring;
try
{
@ -117,12 +120,13 @@ bool TrafficInfo::ReceiveTrafficData()
}
catch (Reader::Exception const & e)
{
LOG(LINFO, ("Could not read traffic data received from server. MWM:", info->GetCountryName(),
"Version:", info->GetVersion()));
m_availability = Availability::NoData;
LOG(LWARNING, ("Could not read traffic data received from server. MWM:", info->GetCountryName(),
"Version:", info->GetVersion()));
return false;
}
m_coloring.swap(coloring);
m_availabilityStatus = Availability::IsAvailable;
m_availability = Availability::IsAvailable;
return true;
}

View file

@ -20,7 +20,7 @@ public:
{
IsAvailable,
NoData,
ExpiredMwm,
ExpiredData,
ExpiredApp,
Unknown
};
@ -65,7 +65,7 @@ public:
TrafficInfo() = default;
TrafficInfo(MwmSet::MwmId const & mwmId, int64_t const & currentDataVersion);
TrafficInfo(MwmSet::MwmId const & mwmId, int64_t currentDataVersion);
// Fetches the latest traffic data from the server and updates the coloring.
// Construct the url by passing an MwmId.
@ -77,7 +77,7 @@ public:
MwmSet::MwmId const & GetMwmId() const { return m_mwmId; }
Coloring const & GetColoring() const { return m_coloring; }
Availability GetAvailability() const { return m_availabilityStatus; }
Availability GetAvailability() const { return m_availability; }
static void SerializeTrafficData(Coloring const & coloring, vector<uint8_t> & result);
@ -87,7 +87,7 @@ private:
// The mapping from feature segments to speed groups (see speed_groups.hpp).
Coloring m_coloring;
MwmSet::MwmId m_mwmId;
Availability m_availabilityStatus = Availability::Unknown;
Availability m_availability = Availability::Unknown;
int64_t m_currentDataVersion = 0;
};
} // namespace traffic