From 686fb44b02bd858795e87885ed656be19ec644f3 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Fri, 21 Dec 2018 17:43:31 +0300 Subject: [PATCH] power manager review fixes --- android/jni/com/mapswithme/maps/Framework.cpp | 10 +- android/jni/com/mapswithme/maps/Framework.hpp | 4 +- .../src/com/mapswithme/maps/Framework.java | 4 +- map/framework.cpp | 2 +- map/framework.hpp | 4 +- map/map_tests/power_manager_tests.cpp | 80 +++++------ map/power_manager/power_manager.cpp | 126 +++++++++--------- map/power_manager/power_manager.hpp | 33 +++-- 8 files changed, 133 insertions(+), 130 deletions(-) diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index bd551283b6..cfb53a1516 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -681,13 +681,13 @@ void Framework::LogLocalAdsEvent(local_ads::EventType type, double lat, double l m_work.GetLocalAdsManager().GetStatistics().RegisterEvent(std::move(event)); } -void Framework::OnFacilityStateChanged(PowerManager::Facility const facility, bool state) +void Framework::OnPowerFacilityChanged(PowerManager::Facility const facility, bool enabled) { // Dummy // TODO: provide information for UI Properties. } -void Framework::OnConfigChanged(PowerManager::Config const actualConfig) +void Framework::OnPowerSchemeChanged(PowerManager::Scheme const actualScheme) { // Dummy // TODO: provide information for UI Properties. @@ -1981,12 +1981,12 @@ Java_com_mapswithme_maps_Framework_nativeSetPowerManagerFacility(JNIEnv *, jclas jint facilityType, jboolean state) { frm()->GetPowerManager().SetFacility(static_cast(facilityType), - static_cast(state)); + static_cast(state)); } JNIEXPORT void JNICALL -Java_com_mapswithme_maps_Framework_nativeSetPowerManagerConfig(JNIEnv *, jclass, jint configType) +Java_com_mapswithme_maps_Framework_nativeSetPowerManagerScheme(JNIEnv *, jclass, jint schemeType) { - frm()->GetPowerManager().SetConfig(static_cast(configType)); + frm()->GetPowerManager().SetScheme(static_cast(schemeType)); } } // extern "C" diff --git a/android/jni/com/mapswithme/maps/Framework.hpp b/android/jni/com/mapswithme/maps/Framework.hpp index 28ecbc0139..cc2325ad65 100644 --- a/android/jni/com/mapswithme/maps/Framework.hpp +++ b/android/jni/com/mapswithme/maps/Framework.hpp @@ -223,8 +223,8 @@ namespace android void LogLocalAdsEvent(local_ads::EventType event, double lat, double lon, uint16_t accuracy); // PowerManager::Subscriber overrides: - void OnFacilityStateChanged(PowerManager::Facility const facility, bool state) override; - void OnConfigChanged(PowerManager::Config const actualConfig) override; + void OnPowerFacilityChanged(PowerManager::Facility const facility, bool enabled) override; + void OnPowerSchemeChanged(PowerManager::Scheme const actualScheme) override; }; } diff --git a/android/src/com/mapswithme/maps/Framework.java b/android/src/com/mapswithme/maps/Framework.java index 10e719ba2a..0e16db002d 100644 --- a/android/src/com/mapswithme/maps/Framework.java +++ b/android/src/com/mapswithme/maps/Framework.java @@ -513,8 +513,6 @@ public class Framework public static native int nativeGetCurrentTipsApi(); - private static native void nativeTipsShown(int tipType, int event); - private static native void nativeDisableAdProvider(int provider, int bannerPlace); public static native void nativeBindUser(@NonNull UserBindingListener listener); @@ -528,5 +526,5 @@ public class Framework public static native void nativeOnBatteryLevelChanged(int level); public static native void nativeSetPowerManagerFacility(int facilityType, boolean state); - public static native void nativeSetPowerManagerConfig(int configType); + public static native void nativeSetPowerManagerScheme(int schemeType); } diff --git a/map/framework.cpp b/map/framework.cpp index f0ff87718e..fb54257ecd 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -3828,7 +3828,7 @@ booking::AvailabilityParams Framework::GetLastBookingAvailabilityParams() const return m_bookingAvailabilityParams; } -void Framework::OnFacilityStateChanged(PowerManager::Facility const facility, bool state) +void Framework::OnPowerFacilityChanged(PowerManager::Facility const facility, bool enabled) { // Dummy. // TODO: process facilities which do not have switch in UI. diff --git a/map/framework.hpp b/map/framework.hpp index 7ac2eb56b9..62a5fe24fd 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -145,7 +145,7 @@ class Framework : public SearchAPI::Delegate, public RoutingManager::Delegate, public TipsApi::Delegate, public notifications::NotificationManager::Delegate, - public PowerManager::Subscriber + private PowerManager::Subscriber { DISALLOW_COPY(Framework); @@ -927,5 +927,5 @@ public: PowerManager & GetPowerManager() { return m_powerManager; } // PowerManager::Subscriber override. - void OnFacilityStateChanged(PowerManager::Facility const facility, bool state) override; + void OnPowerFacilityChanged(PowerManager::Facility const facility, bool enabled) override; }; diff --git a/map/map_tests/power_manager_tests.cpp b/map/map_tests/power_manager_tests.cpp index bae08da9ee..2b34a42af8 100644 --- a/map/map_tests/power_manager_tests.cpp +++ b/map/map_tests/power_manager_tests.cpp @@ -13,24 +13,24 @@ struct SubscriberForTesting : public PowerManager::Subscriber { public: // PowerManager::Subscriber overrides: - void OnFacilityStateChanged(PowerManager::Facility const facility, bool state) override + void OnPowerFacilityChanged(PowerManager::Facility const facility, bool enabled) override { - m_onFacilityEvents.push_back({facility, state}); + m_onFacilityEvents.push_back({facility, enabled}); } - void OnConfigChanged(PowerManager::Config const actualConfig) override + void OnPowerSchemeChanged(PowerManager::Scheme const actualConfig) override { - m_onConfigEvents.push_back(actualConfig); + m_onShemeEvents.push_back(actualConfig); } - struct facilityState + struct FacilityState { PowerManager::Facility m_facility; bool m_state; }; - std::vector m_onFacilityEvents; - std::vector m_onConfigEvents; + std::vector m_onFacilityEvents; + std::vector m_onShemeEvents; }; UNIT_TEST(PowerManager_SetFacility) @@ -41,33 +41,33 @@ UNIT_TEST(PowerManager_SetFacility) manager.Subscribe(&subscriber); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::ThreeDimensionalBuildings), true, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), true, ()); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::Normal, ()); - manager.SetFacility(PowerManager::Facility::ThreeDimensionalBuildings, false); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::ThreeDimensionalBuildings), false, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), true, ()); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::None, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::Buildings3d), true, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), true, ()); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::Normal, ()); + manager.SetFacility(PowerManager::Facility::Buildings3d, false); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::Buildings3d), false, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), true, ()); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::None, ()); TEST_EQUAL(subscriber.m_onFacilityEvents.size(), 1, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_facility, - PowerManager::Facility::ThreeDimensionalBuildings, ()); + PowerManager::Facility::Buildings3d, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_state, false, ()); - TEST_EQUAL(subscriber.m_onConfigEvents.size(), 1, ()); - TEST_EQUAL(subscriber.m_onConfigEvents[0], PowerManager::Config::None, ()); + TEST_EQUAL(subscriber.m_onShemeEvents.size(), 1, ()); + TEST_EQUAL(subscriber.m_onShemeEvents[0], PowerManager::Scheme::None, ()); subscriber.m_onFacilityEvents.clear(); - subscriber.m_onConfigEvents.clear(); + subscriber.m_onShemeEvents.clear(); manager.SetFacility(PowerManager::Facility::TrackRecord, false); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), false, ()); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::Economy, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), false, ()); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::Economy, ()); TEST_EQUAL(subscriber.m_onFacilityEvents.size(), 1, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_facility, PowerManager::Facility::TrackRecord, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_state, false, ()); - TEST_EQUAL(subscriber.m_onConfigEvents.size(), 1, ()); - TEST_EQUAL(subscriber.m_onConfigEvents[0], PowerManager::Config::Economy, ()); + TEST_EQUAL(subscriber.m_onShemeEvents.size(), 1, ()); + TEST_EQUAL(subscriber.m_onShemeEvents[0], PowerManager::Scheme::Economy, ()); } UNIT_TEST(PowerManager_SetConfig) @@ -78,36 +78,36 @@ UNIT_TEST(PowerManager_SetConfig) manager.Subscribe(&subscriber); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::Normal, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::ThreeDimensionalBuildings), true, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), true, ()); - manager.SetConfig(PowerManager::Config::Economy); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::Economy, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::ThreeDimensionalBuildings), false, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), false, ()); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::Normal, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::Buildings3d), true, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), true, ()); + manager.SetScheme(PowerManager::Scheme::Economy); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::Economy, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::Buildings3d), false, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), false, ()); - TEST_EQUAL(subscriber.m_onConfigEvents.size(), 1, ()); - TEST_EQUAL(subscriber.m_onConfigEvents[0], PowerManager::Config::Economy, ()); + TEST_EQUAL(subscriber.m_onShemeEvents.size(), 1, ()); + TEST_EQUAL(subscriber.m_onShemeEvents[0], PowerManager::Scheme::Economy, ()); TEST_EQUAL(subscriber.m_onFacilityEvents.size(), 2, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_facility, - PowerManager::Facility::ThreeDimensionalBuildings, ()); + PowerManager::Facility::Buildings3d, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_state, false, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[1].m_facility, PowerManager::Facility::TrackRecord, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[1].m_state, false, ()); subscriber.m_onFacilityEvents.clear(); - subscriber.m_onConfigEvents.clear(); + subscriber.m_onShemeEvents.clear(); - manager.SetConfig(PowerManager::Config::Normal); - TEST_EQUAL(manager.GetConfig(), PowerManager::Config::Normal, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::ThreeDimensionalBuildings), true, ()); - TEST_EQUAL(manager.GetFacility(PowerManager::Facility::TrackRecord), true, ()); + manager.SetScheme(PowerManager::Scheme::Normal); + TEST_EQUAL(manager.GetScheme(), PowerManager::Scheme::Normal, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::Buildings3d), true, ()); + TEST_EQUAL(manager.IsFacilityEnabled(PowerManager::Facility::TrackRecord), true, ()); - TEST_EQUAL(subscriber.m_onConfigEvents.size(), 1, ()); - TEST_EQUAL(subscriber.m_onConfigEvents[0], PowerManager::Config::Normal, ()); + TEST_EQUAL(subscriber.m_onShemeEvents.size(), 1, ()); + TEST_EQUAL(subscriber.m_onShemeEvents[0], PowerManager::Scheme::Normal, ()); TEST_EQUAL(subscriber.m_onFacilityEvents.size(), 2, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_facility, - PowerManager::Facility::ThreeDimensionalBuildings, ()); + PowerManager::Facility::Buildings3d, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[0].m_state, true, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[1].m_facility, PowerManager::Facility::TrackRecord, ()); TEST_EQUAL(subscriber.m_onFacilityEvents[1].m_state, true, ()); diff --git a/map/power_manager/power_manager.cpp b/map/power_manager/power_manager.cpp index dfa4356c7f..fc22e89389 100644 --- a/map/power_manager/power_manager.cpp +++ b/map/power_manager/power_manager.cpp @@ -15,40 +15,52 @@ namespace { -std::unordered_map const kConfigToState = +using Subscribers = std::vector; + +std::unordered_map const kSchemeToState = { - {PowerManager::Config::Normal, {{true, true}}}, - {PowerManager::Config::Economy, {{false, false}}} + {PowerManager::Scheme::Normal, {{true, true}}}, + {PowerManager::Scheme::Economy, {{false, false}}} }; std::string GetFilePath() { return base::JoinPath(GetPlatform().SettingsDir(), "power_manager_config"); } + +void NotifySubscribers(Subscribers & subscribers, PowerManager::Scheme const scheme) +{ + for (auto & subscriber : subscribers) + subscriber->OnPowerSchemeChanged(scheme); +} + +void NotifySubscribers(Subscribers & subscribers, PowerManager::Facility const facility, + bool enabled) +{ + for (auto & subscriber : subscribers) + subscriber->OnPowerFacilityChanged(facility, enabled); +} } // namespace void PowerManager::Load() { - Data result; try { FileReader reader(GetFilePath()); NonOwningReaderSource source(reader); coding::DeserializerJson des(source); + Config result; des(result); - m_data = result; - for (size_t i = 0; i < m_data.m_facilities.size(); ++i) + m_config = result; + for (size_t i = 0; i < m_config.m_facilities.size(); ++i) { - CHECK_NOT_EQUAL(static_cast(i), Facility::Count, ()); - - for (auto & subscriber : m_subscribers) - subscriber->OnFacilityStateChanged(static_cast(i), m_data.m_facilities[i]); + NotifySubscribers(m_subscribers, static_cast(i), m_config.m_facilities[i]); } - for (auto & subscriber : m_subscribers) - subscriber->OnConfigChanged(m_data.m_config); + NotifySubscribers(m_subscribers, m_config.m_scheme); + return; } catch (base::Json::Exception & ex) { @@ -60,86 +72,80 @@ void PowerManager::Load() } // Reset to default state. - m_data = {}; + m_config = {}; } -void PowerManager::SetFacility(Facility const facility, bool state) +void PowerManager::SetFacility(Facility const facility, bool enabled) { CHECK_NOT_EQUAL(facility, Facility::Count, ()); - if (m_data.m_facilities[static_cast(facility)] == state) + if (m_config.m_facilities[static_cast(facility)] == enabled) return; - m_data.m_facilities[static_cast(facility)] = state; + m_config.m_facilities[static_cast(facility)] = enabled; - bool isConfigChanged = BalanceConfig(); + auto const isSchemeChanged = BalanceScheme(); if (!Save()) return; - for (auto & subscriber : m_subscribers) - subscriber->OnFacilityStateChanged(facility, state); + NotifySubscribers(m_subscribers, facility, enabled); - if (isConfigChanged) - { - for (auto & subscriber : m_subscribers) - subscriber->OnConfigChanged(m_data.m_config); - } + if (isSchemeChanged) + NotifySubscribers(m_subscribers, m_config.m_scheme); } -void PowerManager::SetConfig(Config const config) +void PowerManager::SetScheme(Scheme const scheme) { - if (m_data.m_config == config) + if (m_config.m_scheme == scheme) return; - m_data.m_config = config; + m_config.m_scheme = scheme; - if (m_data.m_config == Config::None || m_data.m_config == Config::Auto) + if (m_config.m_scheme == Scheme::None || m_config.m_scheme == Scheme::Auto) return; - auto actualState = kConfigToState.at(config); + auto actualState = kSchemeToState.at(scheme); - if (m_data.m_facilities == actualState) + if (m_config.m_facilities == actualState) return; - std::swap(m_data.m_facilities, actualState); + std::swap(m_config.m_facilities, actualState); if (!Save()) return; - for (auto & subscriber : m_subscribers) - subscriber->OnConfigChanged(m_data.m_config); + NotifySubscribers(m_subscribers, m_config.m_scheme); for (size_t i = 0; i < actualState.size(); ++i) { - if (m_data.m_facilities[i] != actualState[i]) - { - for (auto & subscriber : m_subscribers) - subscriber->OnFacilityStateChanged(static_cast(i), m_data.m_facilities[i]); - } + if (m_config.m_facilities[i] != actualState[i]) + NotifySubscribers(m_subscribers, static_cast(i), m_config.m_facilities[i]); } } -bool PowerManager::GetFacility(Facility const facility) const +bool PowerManager::IsFacilityEnabled(Facility const facility) const { CHECK_NOT_EQUAL(facility, Facility::Count, ()); - return m_data.m_facilities[static_cast(facility)]; + return m_config.m_facilities[static_cast(facility)]; } PowerManager::FacilitiesState const & PowerManager::GetFacilities() const { - return m_data.m_facilities; + return m_config.m_facilities; } -PowerManager::Config const & PowerManager::GetConfig() const +PowerManager::Scheme const & PowerManager::GetScheme() const { - return m_data.m_config; + return m_config.m_scheme; } void PowerManager::OnBatteryLevelChanged(uint8_t level) { - if (m_data.m_config != Config::Auto) + CHECK_LESS_OR_EQUAL(level, 100, ()); + + if (m_config.m_scheme != Scheme::Auto) return; // TODO. @@ -156,27 +162,27 @@ void PowerManager::UnsubscribeAll() m_subscribers.clear(); } -bool PowerManager::BalanceConfig() +bool PowerManager::BalanceScheme() { bool found = false; - Config actualConfig = m_data.m_config; - for (auto const & item : kConfigToState) + Scheme actualScheme = m_config.m_scheme; + for (auto const & item : kSchemeToState) { - if (item.second == m_data.m_facilities) + if (item.second == m_config.m_facilities) { - actualConfig = item.first; + actualScheme = item.first; found = true; break; } } if (!found) - actualConfig = Config::None; + actualScheme = Scheme::None; - if (m_data.m_config == actualConfig) + if (m_config.m_scheme == actualScheme) return false; - m_data.m_config = actualConfig; + m_config.m_scheme = actualScheme; return true; } @@ -188,7 +194,7 @@ bool PowerManager::Save() { FileWriter writer(fileName); coding::SerializerJson ser(writer); - ser(m_data); + ser(m_config); return true; } catch (base::Json::Exception & ex) @@ -215,19 +221,19 @@ std::string DebugPrint(PowerManager::Facility const facility) { switch (facility) { - case PowerManager::Facility::ThreeDimensionalBuildings: return "ThreeDimensionalBuildings"; + case PowerManager::Facility::Buildings3d: return "Buildings3d"; case PowerManager::Facility::TrackRecord: return "TrackRecord"; case PowerManager::Facility::Count: return "Count"; } } -std::string DebugPrint(PowerManager::Config const config) +std::string DebugPrint(PowerManager::Scheme const scheme) { - switch (config) + switch (scheme) { - case PowerManager::Config::None: return "None"; - case PowerManager::Config::Normal: return "Normal"; - case PowerManager::Config::Economy: return "Economy"; - case PowerManager::Config::Auto: return "Auto"; + case PowerManager::Scheme::None: return "None"; + case PowerManager::Scheme::Normal: return "Normal"; + case PowerManager::Scheme::Economy: return "Economy"; + case PowerManager::Scheme::Auto: return "Auto"; } } diff --git a/map/power_manager/power_manager.hpp b/map/power_manager/power_manager.hpp index 229dc7ffba..c4acdf909a 100644 --- a/map/power_manager/power_manager.hpp +++ b/map/power_manager/power_manager.hpp @@ -16,7 +16,7 @@ public: // Note: do not use Facility::Count in external code, this value for internal use only. enum class Facility : uint8_t { - ThreeDimensionalBuildings, + Buildings3d, TrackRecord, Count @@ -24,7 +24,7 @@ public: using FacilitiesState = std::array(Facility::Count)>; - enum class Config : uint8_t + enum class Scheme : uint8_t { None, Normal, @@ -37,17 +37,16 @@ public: public: virtual ~Subscriber() = default; - virtual void OnFacilityStateChanged(Facility const facility, bool state) {} - - virtual void OnConfigChanged(Config const actualConfig) {} + virtual void OnPowerFacilityChanged(Facility const facility, bool enabled) {} + virtual void OnPowerSchemeChanged(Scheme const actualScheme) {} }; void Load(); - void SetFacility(Facility const facility, bool state); - void SetConfig(Config const config); - bool GetFacility(Facility const facility) const; + void SetFacility(Facility const facility, bool enabled); + void SetScheme(Scheme const scheme); + bool IsFacilityEnabled(Facility const facility) const; FacilitiesState const & GetFacilities() const; - Config const & GetConfig() const; + Scheme const & GetScheme() const; void OnBatteryLevelChanged(uint8_t level); @@ -55,22 +54,22 @@ public: void UnsubscribeAll(); private: - struct Data + struct Config { - DECLARE_VISITOR(visitor(m_facilities, "current_state"), visitor(m_config, "config")); + DECLARE_VISITOR(visitor(m_facilities, "current_state"), visitor(m_scheme, "scheme")); - FacilitiesState m_facilities = {true, true}; - Config m_config = Config::Normal; + FacilitiesState m_facilities = {{true, true}}; + Scheme m_scheme = Scheme::Normal; }; - // Returns true when config was changed. - bool BalanceConfig(); + // Returns true when scheme was changed. + bool BalanceScheme(); bool Save(); std::vector m_subscribers; - Data m_data; + Config m_config; }; std::string DebugPrint(PowerManager::Facility const facility); -std::string DebugPrint(PowerManager::Config const config); +std::string DebugPrint(PowerManager::Scheme const scheme);