Merge changes from topic "liboemservice-improve-testing" into main

* changes:
  dump_modemlog: always move modem logs
  modem_log_constants: System props for logging
  modem/modem_log_constants: create common folder
  gs-common/modem: clock manager interface
  dump_modemlog: move android_property_manager
  dump_modemlog: move all files to subdirectory

modem/Android.bp

@@ -1,44 +0,0 @@
-package {
-    default_applicable_licenses: ["Android-Apache-2.0"],
-}
-
-sh_binary {
-    name: "dump_modem.sh",
-    src: "dump_modem.sh",
-    vendor: true,
-    sub_dir: "dump",
-}
-
-cc_defaults {
-    name: "dump_modemlog_defaults",
-    srcs: ["modem_log_dumper.cpp"],
-    local_include_dirs: ["include"],
-    shared_libs: ["liblog"],
-}
-
-cc_binary {
-    name: "dump_modemlog",
-    srcs: ["dump_modemlog.cpp"],
-    cflags: [
-        "-Wall",
-        "-Wextra",
-        "-Werror",
-    ],
-    shared_libs: [
-        "libbase",
-        "libdump",
-        "liblog",
-    ],
-    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"],
-    vendor: true,
-}

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,
+}

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,
+}

modem/android_property_manager/fake/fake_android_property_manager.cpp

@@ -0,0 +1,80 @@
+#include "fake_android_property_manager.h"
+
+#include <android-base/parseint.h>
+#include <android-base/result.h>
+
+#include <cerrno>
+#include <map>
+#include <string>
+
+#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<int>((*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<std::string> 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

modem/android_property_manager/fake/include/fake_android_property_manager.h

@@ -0,0 +1,54 @@
+#pragma once
+
+#include <map>
+#include <string>
+
+#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<std::string> GetProperty(const std::string& key);
+
+  std::map<std::string, std::string> property_map_;
+  bool modem_logging_has_been_off_ = false;
+  bool modem_logging_has_restarted_ = false;
+};
+
+}  // namespace pixel_modem

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,
+}

modem/android_property_manager/impl/android_property_manager_impl.cpp

@@ -0,0 +1,29 @@
+#include "android_property_manager_impl.h"
+
+#include <android-base/properties.h>
+
+#include <string>
+
+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

modem/android_property_manager/impl/include/android_property_manager_impl.h

@@ -0,0 +1,25 @@
+#pragma once
+
+#include <string>
+
+#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

modem/android_property_manager/include/android_property_manager.h

@@ -1,9 +1,14 @@
+
 #pragma once
 
 #include <string>
+#include <string_view>
 
-namespace modem {
-namespace 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.
@@ -11,11 +16,12 @@ namespace 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 logging
-}  // namespace modem
+
+}  // namespace pixel_modem

modem/clock_manager/Android.bp

@@ -0,0 +1,9 @@
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_library {
+    name: "modem_clock_manager",
+    export_include_dirs: [ "include" ],
+    vendor_available: true,
+}

modem/clock_manager/fake/Android.bp

@@ -0,0 +1,15 @@
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_defaults {
+    name: "fake_modem_clock_manager_defaults",
+    shared_libs: [ "modem_clock_manager" ],
+}
+
+cc_library_static {
+    name: "fake_modem_clock_manager",
+    export_include_dirs: [ "include" ],
+    defaults: [ "fake_modem_clock_manager_defaults" ],
+    vendor_available: true,
+}

modem/clock_manager/fake/include/fake_clock_manager.h

@@ -0,0 +1,21 @@
+#include "clock_manager.h"
+
+namespace pixel_modem {
+
+/**
+ * @brief Fake implementation of clock manager that doesn't actually sleep.
+ *
+ * A lot of vendor code don't have return values and instead force the client
+ * codes to sleep for a specified period of time before checking some system
+ * properties. However, since unit tests don't rely on the real vendor
+ * implementations, these sleeps should be ignored and so a fake clock should be
+ * used.
+ *
+ * Since this definition is unlikely to change, it will be defined in the header
+ * and not an implementation file.
+ */
+struct FakeClockManager : public ClockManager {
+  void Sleep(size_t /*seconds*/) const override{};
+};
+
+}  // namespace pixel_modem

modem/clock_manager/impl/Android.bp

@@ -0,0 +1,16 @@
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+modem_clock_manager_impl_public_deps = [
+    "modem_clock_manager",
+]
+
+cc_library {
+    name: "modem_clock_manager_impl",
+    export_include_dirs: [ "include" ],
+    srcs: [ "clock_manager_impl.cpp" ],
+    shared_libs: modem_clock_manager_impl_public_deps,
+    export_shared_lib_headers: modem_clock_manager_impl_public_deps,
+    vendor_available: true,
+}

modem/clock_manager/impl/clock_manager_impl.cpp

@@ -0,0 +1,9 @@
+#include "clock_manager_impl.h"
+
+#include <unistd.h>
+
+namespace pixel_modem {
+
+void ClockManagerImpl::Sleep(size_t seconds) const { sleep(seconds); }
+
+}  // namespace pixel_modem

modem/clock_manager/impl/include/clock_manager_impl.h

@@ -0,0 +1,13 @@
+#pragma once
+
+#include <cstddef>
+
+#include "clock_manager.h"
+
+namespace pixel_modem {
+
+struct ClockManagerImpl : public ClockManager {
+  void Sleep(size_t seconds) const override;
+};
+
+}  // namespace pixel_modem

modem/clock_manager/include/clock_manager.h

@@ -0,0 +1,28 @@
+#pragma once
+
+#include <cstddef>
+
+namespace pixel_modem {
+
+/**
+ * @brief Interface for time based operations.
+ *
+ * This interface was intentionally not called `Clock`, like the Java side
+ * counterpart since it's likely that clients would call the local variable
+ * `clock(_)`, which would clash with the C defined `clock` method.
+ */
+struct ClockManager {
+  virtual ~ClockManager() = default;
+
+  /**
+   * @brief Sleep the thread for a given number of seconds.
+   *
+   * @param seconds Minimum number of seconds to sleep for. Note, this is
+   * different than the Java android clock which accepts seconds. This was done
+   * because C++ developers are likely more familiar with the `sleep` command,
+   * which accepts seconds.
+   */
+  virtual void Sleep(size_t seconds) const = 0;
+};
+
+}  // namespace pixel_modem

modem/dump_modemlog/Android.bp

@@ -0,0 +1,79 @@
+package {
+    default_applicable_licenses: [ "Android-Apache-2.0" ],
+}
+
+sh_binary {
+    name: "dump_modem.sh",
+    src: "dump_modem.sh",
+    vendor: true,
+    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: "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",
+        "modem_log_constants",
+    ],
+}
+
+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" ],
+    cflags: [
+        "-Wall",
+        "-Wextra",
+        "-Werror",
+    ],
+    shared_libs: [
+        "libbase",
+        "libdump",
+        "liblog",
+        "modem_android_property_manager_impl",
+        "modem_log_dumper",
+    ],
+    vendor: true,
+    relative_install_path: "dump",
+}
+
+cc_test {
+    name: "dump_modemlog_test",
+    srcs: [ "modem_log_dumper_test.cpp" ],
+    defaults: [
+        "modem_log_dumper_defaults",
+        "modem_android_property_manager_fake_defaults",
+    ],
+    static_libs: [
+        "modem_android_property_manager",
+        "modem_android_property_manager_fake",
+        "modem_log_constants",
+        "modem_log_dumper",
+        "libgmock",
+    ],
+    vendor: true,
+    // Shared libs in vendor folder are guarded by SEPolicy, so tests need root
+    // access to run them.
+    require_root: true,
+}

modem/dump_modemlog/dump_modemlog.cpp

@@ -16,33 +16,11 @@
 #include <android-base/properties.h>
 #include <dump/pixel_dump.h>
 
+#include "android_property_manager_impl.h"
 #include "dumper.h"
 #include "modem_log_dumper.h"
 
-namespace modem {
-namespace 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);
-  };
-};
+namespace pixel_modem::logging {
 
 /**
  * @brief Implementation of Dumper that directly forwards to their corresponding
@@ -59,13 +37,12 @@ class DumperImpl : public Dumper {
   }
 };
 
-}  // namespace logging
-}  // namespace modem
+}  // namespace pixel_modem::logging
 
 int main() {
-  modem::logging::DumperImpl dumper_impl;
-  modem::logging::AndroidPropertyManagerImpl android_property_manager_impl;
-  modem::logging::ModemLogDumper modem_log_dumper(
+  pixel_modem::logging::DumperImpl dumper_impl;
+  pixel_modem::AndroidPropertyManagerImpl android_property_manager_impl;
+  pixel_modem::logging::ModemLogDumper modem_log_dumper(
       dumper_impl, android_property_manager_impl);
 
   modem_log_dumper.DumpModemLogs();

modem/dump_modemlog/dump_modemlog.mk

@@ -0,0 +1,5 @@
+BOARD_VENDOR_SEPOLICY_DIRS += device/google/gs-common/modem/dump_modemlog/sepolicy
+
+PRODUCT_PACKAGES += dump_modem.sh
+PRODUCT_PACKAGES += dump_modemlog
+

modem/dump_modemlog/include/bugreport_constants.h

@@ -0,0 +1,33 @@
+#pragma once
+
+#include <string_view>
+
+#include "dumper.h"
+
+namespace pixel_modem::logging {
+
+inline constexpr std::string_view kBugreportPackingDirectory =
+    "/data/vendor/radio/logs/always-on/all_logs";
+
+inline constexpr LogDumpInfo kLogDumpInfo[] = {
+    {.src_dir = "/data/vendor/radio/extended_logs",
+     .dest_dir = kBugreportPackingDirectory,
+     .limit = 20,
+     .prefix = "extended_log_"},
+    {.src_dir = "/data/vendor/radio/sim/",
+     .dest_dir = kBugreportPackingDirectory,
+     .limit = 1,
+     .prefix = "sim_poweron_log_"},
+    {.src_dir = "data/vendor/radio/logs/history",
+     .dest_dir = kBugreportPackingDirectory,
+     .limit = 2,
+     .prefix = "Logging"}};
+
+constexpr FileCopyInfo kFileCopyInfo[] = {
+    {.src_dir = "/mnt/vendor/efs/nv_normal.bin",
+     .dest_dir = "/data/vendor/radio/logs/always-on/all_logs/nv_normal.bin"},
+    {.src_dir = "/mnt/vendor/efs/nv_protected.bin",
+     .dest_dir =
+         "/data/vendor/radio/logs/always-on/all_logs/nv_protected.bin"}};
+
+}  // namespace pixel_modem::logging

modem/dump_modemlog/include/dumper.h

@@ -3,8 +3,7 @@
 #include <ostream>
 #include <string_view>
 
-namespace modem {
-namespace logging {
+namespace pixel_modem::logging {
 
 /**
  * @brief Data object for information about dumpings logs.
@@ -67,5 +66,5 @@ class Dumper {
   virtual void DumpLogs(const LogDumpInfo& log_dump_info);
   virtual void CopyFile(const FileCopyInfo& file_copy_info);
 };
-}  // namespace logging
-}  // namespace modem
+
+}  // namespace pixel_modem::logging

modem/dump_modemlog/include/modem_log_dumper.h

@@ -3,8 +3,7 @@
 #include "android_property_manager.h"
 #include "dumper.h"
 
-namespace modem {
-namespace logging {
+namespace pixel_modem::logging {
 
 /**
  * @brief Responsible for dumping all relevant modem logs.
@@ -77,5 +76,4 @@ class ModemLogDumper {
   AndroidPropertyManager& android_property_manager_;
 };
 
-}  // namespace logging
-}  // namespace modem
+}  // namespace pixel_modem::logging

modem/dump_modemlog/modem_log_dumper.cpp

@@ -2,11 +2,11 @@
 
 #include <log/log.h>
 
+#include "bugreport_constants.h"
 #include "dumper.h"
 #include "modem_log_constants.h"
 
-namespace modem {
-namespace logging {
+namespace pixel_modem::logging {
 
 void ModemLogDumper::DumpModemLogs() {
   bool shouldRestartModemLogging =
@@ -15,7 +15,11 @@ void ModemLogDumper::DumpModemLogs() {
       kModemLoggingNumberBugreportProperty.data(),
       kDefaultBugreportNumberFiles);
 
-  if (shouldRestartModemLogging) {
+  // Should always trigger `stopModemLogging`. This is because currently copying
+  // modem logs and stopping modem logging are entangled.
+  // TODO: b/289435256 - Always copy logs and return this to checking if logging
+  // is actively running.
+  if (allowedToStopModemLogging()) {
     // If modem logging is running at time of bugreport, it needs to be stopped
     // to ensure that the most recent logs are included in the bugreport. If
     // this command fails, only older log files will be included, as seen in
@@ -76,5 +80,5 @@ void ModemLogDumper::startModemLogging() {
   android_property_manager_.SetProperty(kModemLoggingEnabledProperty.data(),
                                         "true");
 }
-}  // namespace logging
-}  // namespace modem
+
+}  // namespace pixel_modem::logging

modem/dump_modemlog/modem_log_dumper_test.cpp

@@ -2,13 +2,15 @@
 
 #include <string_view>
 
+#include "android_property_manager.h"
+#include "bugreport_constants.h"
 #include "dumper.h"
 #include "fake_android_property_manager.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "modem_log_constants.h"
 
-namespace modem {
-namespace logging {
+namespace pixel_modem::logging {
 namespace {
 
 using ::testing::Eq;
@@ -23,9 +25,8 @@ inline constexpr static LogDumpInfo kAlwaysOnLogDumpInfo = {
 
 void StartModemLogging(
     FakeAndroidPropertyManager& fake_android_property_manager) {
-  fake_android_property_manager.SetProperty(
-      kModemLoggingEnabledProperty.data(),
-      FakeAndroidPropertyManager::kTruthString.data());
+  fake_android_property_manager.SetProperty(kModemLoggingEnabledProperty.data(),
+                                            kTruthString.data());
 }
 
 class MockDumper : public Dumper {
@@ -101,6 +102,6 @@ TEST_F(ModemLogDumperTest,
 
   EXPECT_FALSE(fake_android_property_manager.ModemLoggingHasRestarted());
 }
+
 }  // namespace
-}  // namespace logging
-}  // namespace modem
+}  // namespace pixel_modem::logging

modem/include/modem_log_constants.h

@@ -1,56 +0,0 @@
-#pragma once
-#include <string>
-
-#include "dumper.h"
-
-namespace modem {
-namespace logging {
-
-// Modem related Android System Properties
-
-// Controls triggering `modem_logging_start` and `modem_logging_stop`.
-inline constexpr static std::string_view kModemLoggingEnabledProperty =
-    "vendor.sys.modem.logging.enable";
-// Signals the current modem logging state. This will be set to
-// `vendor.sys.modem.logging.enable` when `modem_log_start` or `modem_log_stop`
-// terminates.
-inline constexpr static std::string_view kModemLoggingStatusProperty =
-    "vendor.sys.modem.logging.status";
-// Int which specifies how many files to include in the bugreport.
-inline constexpr static std::string_view kModemLoggingNumberBugreportProperty =
-    "persist.vendor.sys.modem.logging.br_num";
-// Signals the current location that is being logged to. This can be used to
-// determine the logging type.
-inline constexpr static std::string_view kModemLoggingPathProperty =
-    "vendor.sys.modem.logging.log_path";
-
-// Bugreport constants
-inline constexpr static int kDefaultBugreportNumberFiles = 100;
-inline constexpr static std::string_view kModemAlwaysOnLogDirectory =
-    "/data/vendor/radio/logs/always-on";
-inline constexpr static std::string_view kModemLogPrefix = "sbuff_";
-inline constexpr static std::string_view kBugreportPackingDirectory =
-    "/data/vendor/radio/logs/always-on/all_logs";
-
-inline constexpr static LogDumpInfo kLogDumpInfo[] = {
-    {.src_dir = "/data/vendor/radio/extended_logs",
-     .dest_dir = kBugreportPackingDirectory,
-     .limit = 20,
-     .prefix = "extended_log_"},
-    {.src_dir = "/data/vendor/radio/sim/",
-     .dest_dir = kBugreportPackingDirectory,
-     .limit = 1,
-     .prefix = "sim_poweron_log_"},
-    {.src_dir = "data/vendor/radio/logs/history",
-     .dest_dir = kBugreportPackingDirectory,
-     .limit = 2,
-     .prefix = "Logging"}};
-
-constexpr static FileCopyInfo kFileCopyInfo[] = {
-    {.src_dir = "/mnt/vendor/efs/nv_normal.bin",
-     .dest_dir = "/data/vendor/radio/logs/always-on/all_logs/nv_normal.bin"},
-    {.src_dir = "/mnt/vendor/efs/nv_protected.bin",
-     .dest_dir =
-         "/data/vendor/radio/logs/always-on/all_logs/nv_protected.bin"}};
-}  // namespace logging
-}  // namespace modem

modem/modem.mk

@@ -1,5 +1 @@
-BOARD_VENDOR_SEPOLICY_DIRS += device/google/gs-common/modem/sepolicy
-
-PRODUCT_PACKAGES += dump_modem.sh
-PRODUCT_PACKAGES += dump_modemlog
-
+include device/google/gs-common/modem/dump_modemlog/dump_modemlog.mk

modem/modem_log_constants/Android.bp

@@ -0,0 +1,9 @@
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_library {
+    name: "modem_log_constants",
+    export_include_dirs: [ "include" ],
+    vendor_available: true,
+}

modem/modem_log_constants/include/modem_log_constants.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <string_view>
+
+namespace pixel_modem::logging {
+
+// Modem related Android System Properties
+
+// Controls triggering `modem_logging_start` and `modem_logging_stop`.
+inline constexpr std::string_view kModemLoggingEnabledProperty =
+    "vendor.sys.modem.logging.enable";
+// Signals the current modem logging state. This will be set to
+// `vendor.sys.modem.logging.enable` when `modem_log_start` or `modem_log_stop`
+// terminates.
+inline constexpr std::string_view kModemLoggingStatusProperty =
+    "vendor.sys.modem.logging.status";
+// Int which specifies how many files to include in the bugreport.
+inline constexpr std::string_view kModemLoggingNumberBugreportProperty =
+    "persist.vendor.sys.modem.logging.br_num";
+// Signals the current location that is being logged to. This can be used to
+// determine the logging type.
+inline constexpr std::string_view kModemLoggingPathProperty =
+    "vendor.sys.modem.logging.log_path";
+inline constexpr std::string_view kModemLoggingLogCountProperty =
+    "vendor.sys.modem.logging.log_count";
+inline constexpr std::string_view kModemLoggingLogPath =
+    "vendor.sys.modem.logging.log_path";
+
+// Bugreport constants
+inline constexpr int kDefaultBugreportNumberFiles = 100;
+inline constexpr std::string_view kModemAlwaysOnLogDirectory =
+    "/data/vendor/radio/logs/always-on";
+inline constexpr std::string_view kModemLogPrefix = "sbuff_";
+
+}  // namespace pixel_modem::logging

modem/test/include/fake_android_property_manager.h

@@ -1,77 +0,0 @@
-
-
-#include <map>
-#include <string>
-#include <string_view>
-
-#include "android_property_manager.h"
-#include "modem_log_constants.h"
-
-namespace modem {
-namespace 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<std::string, std::string> property_map_;
-  bool modem_logging_has_been_off_ = false;
-  bool modem_logging_has_restarted_ = false;
-};
-}  // namespace logging
-}  // namespace modem