From 1eece0bef3428cd1394d74841e98b823ef6ef490 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Mon, 21 Jan 2019 15:59:56 +0300 Subject: [PATCH] [core][power_manager] review fixes --- map/CMakeLists.txt | 2 -- map/power_management/power_manager.cpp | 10 +++--- map/power_management/power_manager.hpp | 7 ++-- platform/CMakeLists.txt | 2 ++ .../battery_tracker.cpp | 36 ++++++++----------- .../battery_tracker.hpp | 10 +++--- platform/platform.cpp | 3 ++ platform/platform.hpp | 8 +++++ platform/platform_ios.mm | 4 +++ platform/platform_linux.cpp | 1 + platform/platform_mac.mm | 1 + 11 files changed, 47 insertions(+), 37 deletions(-) rename {map/power_management => platform}/battery_tracker.cpp (59%) rename {map/power_management => platform}/battery_tracker.hpp (69%) diff --git a/map/CMakeLists.txt b/map/CMakeLists.txt index d151c8f3b6..1cf150c7b1 100644 --- a/map/CMakeLists.txt +++ b/map/CMakeLists.txt @@ -87,8 +87,6 @@ set( notifications/notification_queue_storage.hpp place_page_info.cpp place_page_info.hpp - power_management/battery_tracker.cpp - power_management/battery_tracker.hpp power_management/power_manager.cpp power_management/power_manager.hpp power_management/power_management_schemas.cpp diff --git a/map/power_management/power_manager.cpp b/map/power_management/power_manager.cpp index e0fe42ff1b..0a54082f36 100644 --- a/map/power_management/power_manager.cpp +++ b/map/power_management/power_manager.cpp @@ -51,7 +51,7 @@ void PowerManager::Load() m_config = result; if (m_config.m_scheme == Scheme::Auto) - m_batteryTracker.Subscribe(this); + GetPlatform().GetBatteryTracker().Subscribe(this); for (size_t i = 0; i < m_config.m_facilities.size(); ++i) { @@ -86,7 +86,7 @@ void PowerManager::SetFacility(Facility const facility, bool enabled) auto const isSchemeChanged = m_config.m_scheme != Scheme::None; if (m_config.m_scheme == Scheme::Auto) - m_batteryTracker.Unsubscribe(this); + GetPlatform().GetBatteryTracker().Unsubscribe(this); m_config.m_scheme = Scheme::None; @@ -107,9 +107,9 @@ void PowerManager::SetScheme(Scheme const scheme) m_config.m_scheme = scheme; if (m_config.m_scheme == Scheme::Auto) - m_batteryTracker.Subscribe(this); + GetPlatform().GetBatteryTracker().Subscribe(this); else - m_batteryTracker.Unsubscribe(this); + GetPlatform().GetBatteryTracker().Unsubscribe(this); if (m_config.m_scheme == Scheme::None || m_config.m_scheme == Scheme::Auto) { @@ -160,8 +160,6 @@ void PowerManager::OnBatteryLevelReceived(uint8_t level) { CHECK_LESS_OR_EQUAL(level, 100, ()); - GetPlatform().RunDelayedTask(Platform::Thread::Background, std::chrono::minutes(10), [] {}); - if (m_config.m_scheme != Scheme::Auto) return; diff --git a/map/power_management/power_manager.hpp b/map/power_management/power_manager.hpp index 1c3ff71b55..fb78deb941 100644 --- a/map/power_management/power_manager.hpp +++ b/map/power_management/power_manager.hpp @@ -1,6 +1,7 @@ #pragma once -#include "map/power_management/battery_tracker.hpp" +#include "platform/battery_tracker.hpp" + #include "map/power_management/power_management_schemas.hpp" #include "base/visitor.hpp" @@ -12,7 +13,7 @@ namespace power_management { // Note: this class is NOT thread-safe. -class PowerManager : public BatteryLevelTracker::Subscriber +class PowerManager : public platform::BatteryLevelTracker::Subscriber { public: class Subscriber @@ -32,6 +33,7 @@ public: FacilitiesState const & GetFacilities() const; Scheme const & GetScheme() const; + // BatteryLevelTracker::Subscriber overrides: void OnBatteryLevelReceived(uint8_t level) override; void Subscribe(Subscriber * subscriber); @@ -53,6 +55,5 @@ private: std::vector m_subscribers; Config m_config; - BatteryLevelTracker m_batteryTracker; }; } // namespace power_management diff --git a/platform/CMakeLists.txt b/platform/CMakeLists.txt index 36cdf3c03b..5f66116e94 100644 --- a/platform/CMakeLists.txt +++ b/platform/CMakeLists.txt @@ -6,6 +6,8 @@ set(CMAKE_AUTOMOC ON) set( SRC + battery_tracker.cpp + battery_tracker.hpp chunks_download_strategy.cpp chunks_download_strategy.hpp constants.hpp diff --git a/map/power_management/battery_tracker.cpp b/platform/battery_tracker.cpp similarity index 59% rename from map/power_management/battery_tracker.cpp rename to platform/battery_tracker.cpp index ac3fe1c1e5..a80ce09eed 100644 --- a/map/power_management/battery_tracker.cpp +++ b/platform/battery_tracker.cpp @@ -1,4 +1,4 @@ -#include "map/power_management/battery_tracker.hpp" +#include "platform/battery_tracker.hpp" #include "platform/platform.hpp" @@ -6,27 +6,27 @@ namespace { auto const kBatteryTrackingInterval = std::chrono::minutes(10); -bool IsLevelExpired(std::chrono::system_clock::time_point lastRequestedTime) +bool IsLevelExpired(std::chrono::system_clock::time_point lastRequestTime) { - return std::chrono::system_clock::now() - lastRequestedTime > kBatteryTrackingInterval; + return std::chrono::system_clock::now() - lastRequestTime > kBatteryTrackingInterval; } } // namespace -namespace power_management +namespace platform { void BatteryLevelTracker::Subscribe(Subscriber * subscriber) { m_subscribers.push_back(subscriber); - if (IsLevelExpired(m_lastRequestedTime)) + if (IsLevelExpired(m_lastRequestTime)) { - // Run periodic requests when first subscriber is added. + // Run periodic requests when the first subscriber is added. if (m_subscribers.size() == 1) RequestBatteryLevel(); } else { - subscriber->OnBatteryLevelReceived(m_lastReceivedlevel); + subscriber->OnBatteryLevelReceived(m_lastReceivedLevel); } } @@ -41,26 +41,20 @@ void BatteryLevelTracker::UnsubscribeAll() m_subscribers.clear(); } -uint8_t BatteryLevelTracker::GetBatteryLevel() -{ - if (IsLevelExpired(m_lastRequestedTime)) - { - m_lastReceivedlevel = GetPlatform().GetBatteryLevel(); - m_lastRequestedTime = std::chrono::system_clock::now(); - } - - return m_lastReceivedlevel; -} - void BatteryLevelTracker::RequestBatteryLevel() { if (m_subscribers.empty()) return; - auto const level = GetBatteryLevel(); + if (IsLevelExpired(m_lastRequestTime)) + { + m_lastReceivedLevel = GetPlatform().GetBatteryLevel(); + m_lastRequestTime = std::chrono::system_clock::now(); + } + for (auto s : m_subscribers) { - s->OnBatteryLevelReceived(level); + s->OnBatteryLevelReceived(m_lastReceivedLevel); } GetPlatform().RunDelayedTask(Platform::Thread::Background, kBatteryTrackingInterval, [this] @@ -71,4 +65,4 @@ void BatteryLevelTracker::RequestBatteryLevel() }); }); } -} // namespace power_management +} // namespace platform diff --git a/map/power_management/battery_tracker.hpp b/platform/battery_tracker.hpp similarity index 69% rename from map/power_management/battery_tracker.hpp rename to platform/battery_tracker.hpp index b34b3513de..65a6b35f3b 100644 --- a/map/power_management/battery_tracker.hpp +++ b/platform/battery_tracker.hpp @@ -4,8 +4,9 @@ #include #include -namespace power_management +namespace platform { +// Note: this class is NOT thread-safe. class BatteryLevelTracker { public: @@ -20,11 +21,10 @@ public: void UnsubscribeAll(); private: - uint8_t GetBatteryLevel(); void RequestBatteryLevel(); std::vector m_subscribers; - std::chrono::system_clock::time_point m_lastRequestedTime; - uint8_t m_lastReceivedlevel = 0; + std::chrono::system_clock::time_point m_lastRequestTime; + uint8_t m_lastReceivedLevel = 0; }; -} // power_management +} // platform diff --git a/platform/platform.cpp b/platform/platform.cpp index add77316ce..9303d4ad86 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -329,6 +329,9 @@ unsigned Platform::CpuCores() const void Platform::ShutdownThreads() { ASSERT(m_networkThread && m_fileThread && m_backgroundThread, ()); + + m_batteryTracker.UnsubscribeAll(); + m_networkThread->ShutdownAndJoin(); m_fileThread->ShutdownAndJoin(); m_backgroundThread->ShutdownAndJoin(); diff --git a/platform/platform.hpp b/platform/platform.hpp index 9d92513670..3f832bcd07 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -1,5 +1,6 @@ #pragma once +#include "platform/battery_tracker.hpp" #include "platform/country_defines.hpp" #include "platform/gui_thread.hpp" #include "platform/http_user_agent.hpp" @@ -131,6 +132,8 @@ protected: std::unique_ptr m_fileThread; std::unique_ptr m_backgroundThread; + platform::BatteryLevelTracker m_batteryTracker; + public: Platform(); virtual ~Platform() = default; @@ -291,6 +294,9 @@ public: static bool IsConnected() { return ConnectionStatus() != EConnectionType::CONNECTION_NONE; } static ChargingStatus GetChargingStatus(); + + // Returns current battery level. Possible values are from 0 to 100. + // Returns 100 when actual level is unknown. static uint8_t GetBatteryLevel(); void SetupMeasurementSystem() const; @@ -347,6 +353,8 @@ public: // Use this method for testing purposes only. void SetGuiThread(std::unique_ptr guiThread); + platform::BatteryLevelTracker & GetBatteryTracker() { return m_batteryTracker; } + private: void RunThreads(); void ShutdownThreads(); diff --git a/platform/platform_ios.mm b/platform/platform_ios.mm index cd8c068800..e88d565870 100644 --- a/platform/platform_ios.mm +++ b/platform/platform_ios.mm @@ -228,6 +228,10 @@ Platform::ChargingStatus Platform::GetChargingStatus() uint8_t Platform::GetBatteryLevel() { auto const level = UIDevice.currentDevice.batteryLevel; + + ASSERT_GREATER_OR_EQUAL(level, -1.0, ()); + ASSERT_LESS_OR_EQUAL(level, 1.0, ()); + if (level == -1.0) return 100; diff --git a/platform/platform_linux.cpp b/platform/platform_linux.cpp index 0f014e0a9d..8a0a1306b0 100644 --- a/platform/platform_linux.cpp +++ b/platform/platform_linux.cpp @@ -262,6 +262,7 @@ Platform::ChargingStatus Platform::GetChargingStatus() uint8_t Platform::GetBatteryLevel() { + // This value is always 100 for desktop. return 100; } diff --git a/platform/platform_mac.mm b/platform/platform_mac.mm index 63f1ff6957..8f353108db 100644 --- a/platform/platform_mac.mm +++ b/platform/platform_mac.mm @@ -176,6 +176,7 @@ Platform::ChargingStatus Platform::GetChargingStatus() uint8_t Platform::GetBatteryLevel() { + // This value is always 100 for desktop. return 100; }