From 047f0aca49217d58b4af8c8ac8652585d5b29eb3 Mon Sep 17 00:00:00 2001 From: kierancyphus Date: Fri, 10 Nov 2023 15:02:57 +0800 Subject: [PATCH] dump_modemlog: move android_property_manager android_property_manager is moved to its own folder in the root of the modem folder. This is so that libeomservice proxy has a specific build target to include. Test: build, flash, check modem logs in bugreport Bug: 302435001 Change-Id: Ifc4a0c888717f5c28cf9b642d0b978b495be29d0 --- modem/android_property_manager/Android.bp | 9 +++ .../android_property_manager/fake/Android.bp | 23 ++++++ .../fake/fake_android_property_manager.cpp | 80 +++++++++++++++++++ .../include/fake_android_property_manager.h | 54 +++++++++++++ .../android_property_manager/impl/Android.bp | 18 +++++ .../impl/android_property_manager_impl.cpp | 29 +++++++ .../include/android_property_manager_impl.h | 25 ++++++ .../include/android_property_manager.h | 19 +++-- modem/dump_modemlog/Android.bp | 54 ++++++++++--- modem/dump_modemlog/dump_modemlog.cpp | 26 +----- .../include/fake_android_property_manager.h | 75 ----------------- 11 files changed, 296 insertions(+), 116 deletions(-) create mode 100644 modem/android_property_manager/Android.bp create mode 100644 modem/android_property_manager/fake/Android.bp create mode 100644 modem/android_property_manager/fake/fake_android_property_manager.cpp create mode 100644 modem/android_property_manager/fake/include/fake_android_property_manager.h create mode 100644 modem/android_property_manager/impl/Android.bp create mode 100644 modem/android_property_manager/impl/android_property_manager_impl.cpp create mode 100644 modem/android_property_manager/impl/include/android_property_manager_impl.h rename modem/{dump_modemlog => android_property_manager}/include/android_property_manager.h (53%) delete mode 100644 modem/dump_modemlog/test/include/fake_android_property_manager.h diff --git a/modem/android_property_manager/Android.bp b/modem/android_property_manager/Android.bp new file mode 100644 index 0000000..af89aec --- /dev/null +++ b/modem/android_property_manager/Android.bp @@ -0,0 +1,9 @@ +package { + default_applicable_licenses: ["Android-Apache-2.0"], +} + +cc_library { + name: "modem_android_property_manager", + export_include_dirs: [ "include" ], + vendor_available: true, +} diff --git a/modem/android_property_manager/fake/Android.bp b/modem/android_property_manager/fake/Android.bp new file mode 100644 index 0000000..247b97c --- /dev/null +++ b/modem/android_property_manager/fake/Android.bp @@ -0,0 +1,23 @@ +package { + default_applicable_licenses: ["Android-Apache-2.0"], +} + +// When `modem_android_property_manager_fake` is included statically, its +// dependencies are not transitively included, so the target will also have to +// include this default to restate them. +cc_defaults { + name: "modem_android_property_manager_fake_defaults", + static_libs: [ + "modem_android_property_manager", + "libbase", + "modem_log_constants", + ], +} + +cc_library_static { + name: "modem_android_property_manager_fake", + export_include_dirs: [ "include" ], + srcs: [ "fake_android_property_manager.cpp" ], + defaults: [ "modem_android_property_manager_fake_defaults" ], + vendor_available: true, +} diff --git a/modem/android_property_manager/fake/fake_android_property_manager.cpp b/modem/android_property_manager/fake/fake_android_property_manager.cpp new file mode 100644 index 0000000..d25d6da --- /dev/null +++ b/modem/android_property_manager/fake/fake_android_property_manager.cpp @@ -0,0 +1,80 @@ +#include "fake_android_property_manager.h" + +#include +#include + +#include +#include +#include + +#include "modem_log_constants.h" + +namespace pixel_modem { + +bool FakeAndroidPropertyManager::GetBoolProperty(const std::string& key, + bool default_value) { + auto value_result = GetProperty(key); + return value_result.ok() ? (*value_result) == kTruthString : default_value; +} + +std::string FakeAndroidPropertyManager::GetProperty( + const std::string& key, const std::string& default_value) { + auto value_result = GetProperty(key); + return value_result.ok() ? *value_result : default_value; +} + +int FakeAndroidPropertyManager::GetIntProperty(const std::string& key, + int default_value) { + int value = default_value; + + auto property_result = GetProperty(key); + if (property_result.ok()) { + android::base::ParseInt((*property_result).data(), &value); + } + return value; +} + +/** + * This function needs to copy the behaviour of `modem_logging_control` to + * ensure that the right properties are being set in order. + * + * More specifically, this function will also set the + * `kModemLoggingStatusProperty` whenever `kModemLoggingEnabledProperty` is + * set to simulate modem logging stopping / starting. + */ +bool FakeAndroidPropertyManager::SetProperty(const std::string& key, + const std::string& value) { + if (key == logging::kModemLoggingEnabledProperty) { + property_map_[logging::kModemLoggingStatusProperty.data()] = value; + + // need to track if modem logging has restarted or not + if (value == kFalseString) { + modem_logging_has_been_off_ = true; + } + if (modem_logging_has_been_off_ && (value == kTruthString)) { + modem_logging_has_restarted_ = true; + } + } + property_map_[key] = value; + return true; +} + +/** + * @brief Gets android system property if present. + * + * @param[in] key Name of property. + * + * @return Status of get operation and value if successful. + * @retval EINVAL Key not present in map. + */ +android::base::Result FakeAndroidPropertyManager::GetProperty( + const std::string& key) { + const auto it = property_map_.find(key); + if (it == property_map_.end()) { + return android::base::Error() + << "Property: " << key << " not found." << EINVAL; + } + return it->second; +} + +} // namespace pixel_modem diff --git a/modem/android_property_manager/fake/include/fake_android_property_manager.h b/modem/android_property_manager/fake/include/fake_android_property_manager.h new file mode 100644 index 0000000..eeea5a0 --- /dev/null +++ b/modem/android_property_manager/fake/include/fake_android_property_manager.h @@ -0,0 +1,54 @@ +#pragma once + +#include +#include + +#include "android-base/result.h" +#include "android_property_manager.h" + +namespace pixel_modem { + +/** + * @brief Fake Implementation of AndroidPropertyManager that mocks some of the + * property changing behaviour from pixellogger's `modem_logging_control`. + */ +class FakeAndroidPropertyManager : public AndroidPropertyManager { + public: + bool GetBoolProperty(const std::string& key, bool default_value) override; + + std::string GetProperty(const std::string& key, + const std::string& default_value) override; + + int GetIntProperty(const std::string& key, int default_value) override; + + /** + * This function needs to copy the behaviour of `modem_logging_control` to + * ensure that the right properties are being set in order. + * + * More specifically, this function will also set the + * `kModemLoggingStatusProperty` whenever `kModemLoggingEnabledProperty` is + * set to simulate modem logging stopping / starting. + */ + bool SetProperty(const std::string& key, const std::string& value) override; + + inline bool ModemLoggingHasRestarted() { + return modem_logging_has_restarted_; + } + + private: + /** + * @brief Gets android system property if present. + * + * @param[in] key Name of property. + * + * @return Status of get operation and value if successful. + * @retval EINVAL Key not present in map. + */ + android::base::Result GetProperty(const std::string& key); + + std::map property_map_; + bool modem_logging_has_been_off_ = false; + bool modem_logging_has_restarted_ = false; +}; + +} // namespace pixel_modem diff --git a/modem/android_property_manager/impl/Android.bp b/modem/android_property_manager/impl/Android.bp new file mode 100644 index 0000000..2023e8f --- /dev/null +++ b/modem/android_property_manager/impl/Android.bp @@ -0,0 +1,18 @@ +package { + default_applicable_licenses: ["Android-Apache-2.0"], +} + +modem_android_property_manager_impl_public_deps = [ + "modem_android_property_manager", +] + +cc_library_shared { + name: "modem_android_property_manager_impl", + export_include_dirs: [ "include" ], + srcs: [ "android_property_manager_impl.cpp" ], + shared_libs: modem_android_property_manager_impl_public_deps + [ + "libbase", + ], + export_shared_lib_headers: modem_android_property_manager_impl_public_deps, + vendor_available: true, +} diff --git a/modem/android_property_manager/impl/android_property_manager_impl.cpp b/modem/android_property_manager/impl/android_property_manager_impl.cpp new file mode 100644 index 0000000..b6b900a --- /dev/null +++ b/modem/android_property_manager/impl/android_property_manager_impl.cpp @@ -0,0 +1,29 @@ +#include "android_property_manager_impl.h" + +#include + +#include + +namespace pixel_modem { + +bool AndroidPropertyManagerImpl::GetBoolProperty(const std::string& key, + bool default_value) { + return android::base::GetBoolProperty(key, default_value); +} + +std::string AndroidPropertyManagerImpl::GetProperty( + const std::string& key, const std::string& default_value) { + return android::base::GetProperty(key, default_value); +} + +int AndroidPropertyManagerImpl::GetIntProperty(const std::string& key, + int default_value) { + return android::base::GetIntProperty(key, default_value); +} + +bool AndroidPropertyManagerImpl::SetProperty(const std::string& key, + const std::string& value) { + return android::base::SetProperty(key, value); +} + +} // namespace pixel_modem diff --git a/modem/android_property_manager/impl/include/android_property_manager_impl.h b/modem/android_property_manager/impl/include/android_property_manager_impl.h new file mode 100644 index 0000000..91cee25 --- /dev/null +++ b/modem/android_property_manager/impl/include/android_property_manager_impl.h @@ -0,0 +1,25 @@ +#pragma once + +#include + +#include "android_property_manager.h" + +namespace pixel_modem { + +/** + * @brief Implementation of AndroidPropertyManager that directly forwards to + * android base methods. + */ +class AndroidPropertyManagerImpl : public AndroidPropertyManager { + public: + bool GetBoolProperty(const std::string& key, bool default_value) override; + + std::string GetProperty(const std::string& key, + const std::string& default_value) override; + + int GetIntProperty(const std::string& key, int default_value) override; + + bool SetProperty(const std::string& key, const std::string& value) override; +}; + +} // namespace pixel_modem diff --git a/modem/dump_modemlog/include/android_property_manager.h b/modem/android_property_manager/include/android_property_manager.h similarity index 53% rename from modem/dump_modemlog/include/android_property_manager.h rename to modem/android_property_manager/include/android_property_manager.h index eb426c3..e41a28d 100644 --- a/modem/dump_modemlog/include/android_property_manager.h +++ b/modem/android_property_manager/include/android_property_manager.h @@ -1,8 +1,14 @@ + #pragma once #include +#include -namespace pixel_modem::logging { +namespace pixel_modem { + +// Used to set boolean parameters to true / false +inline constexpr std::string_view kTruthString = "true"; +inline constexpr std::string_view kFalseString = "false"; /** * @brief Interface for interacting with Android System Properties. @@ -10,11 +16,12 @@ namespace pixel_modem::logging { class AndroidPropertyManager { public: virtual ~AndroidPropertyManager() = default; - virtual bool GetBoolProperty(const std::string& key, bool default_value); + virtual bool GetBoolProperty(const std::string& key, bool default_value) = 0; virtual std::string GetProperty(const std::string& key, - const std::string& default_value); - virtual int GetIntProperty(const std::string& key, int default_value); - virtual void SetProperty(const std::string& key, const std::string& value); + const std::string& default_value) = 0; + virtual int GetIntProperty(const std::string& key, int default_value) = 0; + virtual bool SetProperty(const std::string& key, + const std::string& value) = 0; }; -} // namespace pixel_modem::logging +} // namespace pixel_modem diff --git a/modem/dump_modemlog/Android.bp b/modem/dump_modemlog/Android.bp index b264609..cad6e49 100644 --- a/modem/dump_modemlog/Android.bp +++ b/modem/dump_modemlog/Android.bp @@ -1,5 +1,5 @@ package { - default_applicable_licenses: ["Android-Apache-2.0"], + default_applicable_licenses: [ "Android-Apache-2.0" ], } sh_binary { @@ -9,16 +9,38 @@ sh_binary { sub_dir: "dump", } +// Modem Log Dumper + +modem_log_dumper_public_deps = [ + "modem_android_property_manager", +] + +// When `modem_log_dumper` is included statically, its dependencies are not +// transitively included, so the target will also have to include this default +// to restate them. cc_defaults { - name: "dump_modemlog_defaults", - srcs: [ "modem_log_dumper.cpp" ], - local_include_dirs: [ "include" ], - shared_libs: [ "liblog" ], + name: "modem_log_dumper_defaults", + shared_libs: modem_log_dumper_public_deps + [ + "libbase", + // liblog is not directly used by us, but it's a transitive dependency of libbase + "liblog", + ], } +cc_library { + name: "modem_log_dumper", + srcs: [ "modem_log_dumper.cpp" ], + defaults: [ "modem_log_dumper_defaults" ], + export_shared_lib_headers: modem_log_dumper_public_deps, + export_include_dirs: [ "include" ], + vendor_available: true, +} + +// dump_modemlog + cc_binary { name: "dump_modemlog", - srcs: ["dump_modemlog.cpp"], + srcs: [ "dump_modemlog.cpp" ], cflags: [ "-Wall", "-Wextra", @@ -28,17 +50,27 @@ cc_binary { "libbase", "libdump", "liblog", + "modem_android_property_manager_impl", + "modem_log_dumper", ], - defaults: [ "dump_modemlog_defaults" ], vendor: true, relative_install_path: "dump", } cc_test { name: "dump_modemlog_test", - srcs: [ "*_test.cpp" ], - defaults: [ "dump_modemlog_defaults" ], - local_include_dirs: [ "test/include" ], - static_libs: [ "libgmock" ], + srcs: [ "modem_log_dumper_test.cpp" ], + defaults: [ + "modem_log_dumper_defaults", + "modem_android_property_manager_fake_defaults", + ], + static_libs: [ + "modem_log_dumper", + "modem_android_property_manager_fake", + "libgmock", + ], vendor: true, + // Shared libs in vendor folder are guarded by SEPolicy, so tests need root + // access to run them. + require_root: true, } diff --git a/modem/dump_modemlog/dump_modemlog.cpp b/modem/dump_modemlog/dump_modemlog.cpp index 47181cb..bed936c 100644 --- a/modem/dump_modemlog/dump_modemlog.cpp +++ b/modem/dump_modemlog/dump_modemlog.cpp @@ -16,33 +16,12 @@ #include #include +#include "android_property_manager_impl.h" #include "dumper.h" #include "modem_log_dumper.h" namespace pixel_modem::logging { -/** - * @brief Implementation of AndroidPropertyManager that directly forwards to - * android base methods. - */ -class AndroidPropertyManagerImpl : public AndroidPropertyManager { - public: - bool GetBoolProperty(const std::string& key, bool default_value) override { - return android::base::GetBoolProperty(key, default_value); - }; - - std::string GetProperty(const std::string& key, - const std::string& default_value) override { - return android::base::GetProperty(key, default_value); - }; - int GetIntProperty(const std::string& key, int default_value) override { - return android::base::GetIntProperty(key, default_value); - }; - void SetProperty(const std::string& key, const std::string& value) override { - android::base::SetProperty(key, value); - }; -}; - /** * @brief Implementation of Dumper that directly forwards to their corresponding * dumpstate methods. @@ -62,8 +41,7 @@ class DumperImpl : public Dumper { int main() { pixel_modem::logging::DumperImpl dumper_impl; - pixel_modem::logging::AndroidPropertyManagerImpl - android_property_manager_impl; + pixel_modem::AndroidPropertyManagerImpl android_property_manager_impl; pixel_modem::logging::ModemLogDumper modem_log_dumper( dumper_impl, android_property_manager_impl); diff --git a/modem/dump_modemlog/test/include/fake_android_property_manager.h b/modem/dump_modemlog/test/include/fake_android_property_manager.h deleted file mode 100644 index 0d5731a..0000000 --- a/modem/dump_modemlog/test/include/fake_android_property_manager.h +++ /dev/null @@ -1,75 +0,0 @@ - - -#include -#include -#include - -#include "android_property_manager.h" -#include "modem_log_constants.h" - -namespace pixel_modem::logging { - -/** - * @brief Fake Implementation of AndroidPropertyManager that mocks some of the - * property changing behaviour from pixellogger's `modem_logging_control`. - */ -class FakeAndroidPropertyManager : public AndroidPropertyManager { - public: - inline constexpr static std::string_view kTruthString = "true"; - inline constexpr static std::string_view kFalseString = "false"; - - bool GetBoolProperty(const std::string& key, bool default_value) override { - return MapContainsKey(key) ? GetPropertyInternal(key) == kTruthString - : default_value; - }; - - std::string GetProperty(const std::string& key, - const std::string& default_value) override { - return MapContainsKey(key) ? GetPropertyInternal(key) : default_value; - ; - }; - - int GetIntProperty(const std::string& key, int default_value) override { - return MapContainsKey(key) ? std::stoi(GetPropertyInternal(key)) - : default_value; - }; - - /** - * This function needs to copy the behaviour of `modem_logging_control` to - * ensure that the right properties are being set in order. - * - * More specifically, this function will also set the - * `kModemLoggingStatusProperty` whenever `kModemLoggingEnabledProperty` is - * set to simulate modem logging stopping / starting. - */ - void SetProperty(const std::string& key, const std::string& value) override { - if (key == kModemLoggingEnabledProperty) { - property_map_[kModemLoggingStatusProperty.data()] = value; - - // need to track if modem logging has restarted or not - if (value == kFalseString) { - modem_logging_has_been_off_ = true; - } - if (modem_logging_has_been_off_ && (value == kTruthString)) { - modem_logging_has_restarted_ = true; - } - } - property_map_[key] = value; - }; - - bool ModemLoggingHasRestarted() { return modem_logging_has_restarted_; } - - private: - bool MapContainsKey(const std::string& key) { - return property_map_.find(key) != property_map_.end(); - } - std::string GetPropertyInternal(const std::string& key) { - return property_map_.find(key)->second; - } - - std::map property_map_; - bool modem_logging_has_been_off_ = false; - bool modem_logging_has_restarted_ = false; -}; - -} // namespace pixel_modem::logging