[Telemetry SWX] Make all battery info fields optional

Make all battery info fields optional to be more flexible in the
future, e.g. we may filter some items due to PII filtering policy.

Bug: b:158658869
Test: unit_tests
Change-Id: I65dff7f637db70860060c79dab9b418b809ee73b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252242
Commit-Queue: Oleh Lamzin <lamzin@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Mahmoud Gawad <mgawad@google.com>
Cr-Commit-Position: refs/heads/master@{#781811}

chromeos/components/telemetry_extension_ui/mojom/probe_service.mojom

@@ -14,6 +14,12 @@
 
 // This is a subset of the cros_healthd probe service interface which is
 // located in src/platform2/diagnostics/mojo/cros_healthd_probe.mojom.
+//
+// What is different:
+//   1) Introduced DoubleValue and Int64Value structs since numeric primitives
+//      are not nullable in Mojo.
+//   2) Make all fields in BatteryInfo optional in case we want to filter them
+//      out later.
 
 module chromeos.health.mojom;
 
@@ -60,6 +66,20 @@ struct ProbeError {
   string msg;
 };
 
+// Optional double field. Since primitives numeric types cannot be optional,
+// wrap double in a struct that can be nulled.
+struct DoubleValue {
+  // The value of the double.
+  double value;
+};
+
+// Optional int64 field. Since primitives numeric types cannot be optional,
+// wrap int64 in a struct that can be nulled.
+struct Int64Value {
+  // The value of the int64.
+  int64 value;
+};
+
 // Optional uint64 field. Since primitives numeric types cannot be optional,
 // wrap uint64 in a struct that can be nulled.
 struct UInt64Value {
@@ -70,29 +90,29 @@ struct UInt64Value {
 // Information related to the main battery.
 struct BatteryInfo {
   // Cycle count.
-  int64 cycle_count;
+  Int64Value? cycle_count;
   // Current battery voltage (V)
-  double voltage_now;
+  DoubleValue? voltage_now;
   // Manufacturer of the battery
-  string vendor;
+  string? vendor;
   // Serial number of the battery
-  string serial_number;
+  string? serial_number;
   // Design capacity (Ah)
-  double charge_full_design;
+  DoubleValue? charge_full_design;
   // Full capacity (Ah)
-  double charge_full;
+  DoubleValue? charge_full;
   // Desired minimum output voltage (V)
-  double voltage_min_design;
+  DoubleValue? voltage_min_design;
   // Model name.
-  string model_name;
+  string? model_name;
   // Current battery charge (Ah)
-  double charge_now;
+  DoubleValue? charge_now;
   // Current battery current (A)
-  double current_now;
+  DoubleValue? current_now;
   // Technology of the battery
-  string technology;
+  string? technology;
   // Status of the battery
-  string status;
+  string? status;
 
   // The fields below are optionally included if the main battery is a Smart
   // Battery as defined in http://sbs-forum.org/specs/sbdat110.pdf.

chromeos/components/telemetry_extension_ui/probe_service_converters.cc

@@ -51,6 +51,14 @@ health::mojom::ProbeErrorPtr Convert(cros_healthd::mojom::ProbeErrorPtr input) {
                                         std::move(input->msg));
 }
 
+health::mojom::DoubleValuePtr Convert(double input) {
+  return health::mojom::DoubleValue::New(input);
+}
+
+health::mojom::Int64ValuePtr Convert(int64_t input) {
+  return health::mojom::Int64Value::New(input);
+}
+
 health::mojom::UInt64ValuePtr Convert(
     cros_healthd::mojom::UInt64ValuePtr input) {
   if (!input) {
@@ -64,18 +72,19 @@ health::mojom::BatteryInfoPtr Convert(
   if (!input) {
     return nullptr;
   }
+
   auto output = health::mojom::BatteryInfo::New();
 
-  output->cycle_count = input->cycle_count;
-  output->voltage_now = input->voltage_now;
+  output->cycle_count = Convert(input->cycle_count);
+  output->voltage_now = Convert(input->voltage_now);
   output->vendor = std::move(input->vendor);
   output->serial_number = std::move(input->serial_number);
-  output->charge_full_design = input->charge_full_design;
-  output->charge_full = input->charge_full;
-  output->voltage_min_design = input->voltage_min_design;
+  output->charge_full_design = Convert(input->charge_full_design);
+  output->charge_full = Convert(input->charge_full);
+  output->voltage_min_design = Convert(input->voltage_min_design);
   output->model_name = std::move(input->model_name);
-  output->charge_now = input->charge_now;
-  output->current_now = input->current_now;
+  output->charge_now = Convert(input->charge_now);
+  output->current_now = Convert(input->current_now);
   output->technology = std::move(input->technology);
   output->status = std::move(input->status);
 

chromeos/components/telemetry_extension_ui/probe_service_converters.h

@@ -30,6 +30,10 @@ health::mojom::ErrorType Convert(cros_healthd::mojom::ErrorType type);
 
 health::mojom::ProbeErrorPtr Convert(cros_healthd::mojom::ProbeErrorPtr input);
 
+health::mojom::DoubleValuePtr Convert(double input);
+
+health::mojom::Int64ValuePtr Convert(int64_t input);
+
 health::mojom::UInt64ValuePtr Convert(
     cros_healthd::mojom::UInt64ValuePtr input);
 

chromeos/components/telemetry_extension_ui/probe_service_converters_unittest.cc

@@ -52,12 +52,22 @@ TEST(ProbeServiceConvertors, ProbeErrorPtr) {
                 health::mojom::ErrorType::kFileReadError, kMsg));
 }
 
+TEST(ProbeServiceConvertors, DoubleValuePtr) {
+  constexpr double kValue = 100500.500100;
+  EXPECT_EQ(Convert(kValue), health::mojom::DoubleValue::New(kValue));
+}
+
+TEST(ProbeServiceConvertors, Int64ValuePtr) {
+  constexpr int64_t kValue = 100500;
+  EXPECT_EQ(Convert(kValue), health::mojom::Int64Value::New(kValue));
+}
+
 TEST(ProbeServiceConvertors, UInt64ValuePtrNull) {
   EXPECT_TRUE(Convert(cros_healthd::mojom::UInt64ValuePtr()).is_null());
 }
 
 TEST(ProbeServiceConvertors, UInt64ValuePtr) {
-  constexpr uint64_t kValue = 100500;
+  constexpr uint64_t kValue = -100500;
   EXPECT_EQ(Convert(cros_healthd::mojom::UInt64Value::New(kValue)),
             health::mojom::UInt64Value::New(kValue));
 }
@@ -103,12 +113,17 @@ TEST(ProbeServiceConvertors, BatteryInfoPtr) {
 
   // Here we intentionaly use health::mojom::BatteryInfo::New to don't
   // forget to test new fields.
-  EXPECT_EQ(Convert(std::move(battery_info)),
-            health::mojom::BatteryInfo::New(
-                kCycleCount, kVoltageNow, kVendor, kSerialNumber,
-                kChargeFullDesign, kChargeFull, kVoltageMinDesign, kModelName,
-                kChargeNow, kCurrentNow, kTechnology, kStatus, kManufactureDate,
-                health::mojom::UInt64Value::New(kTemperature)));
+  EXPECT_EQ(
+      Convert(battery_info.Clone()),
+      health::mojom::BatteryInfo::New(
+          health::mojom::Int64Value::New(kCycleCount),
+          health::mojom::DoubleValue::New(kVoltageNow), kVendor, kSerialNumber,
+          health::mojom::DoubleValue::New(kChargeFullDesign),
+          health::mojom::DoubleValue::New(kChargeFull),
+          health::mojom::DoubleValue::New(kVoltageMinDesign), kModelName,
+          health::mojom::DoubleValue::New(kChargeNow),
+          health::mojom::DoubleValue::New(kCurrentNow), kTechnology, kStatus,
+          kManufactureDate, health::mojom::UInt64Value::New(kTemperature)));
 }
 
 TEST(ProbeServiceConvertors, BatteryResultPtrNull) {
@@ -146,9 +161,12 @@ TEST(ProbeServiceConvertors, TelemetryInfoPtrHasBatteryResult) {
   ASSERT_TRUE(telemetry_info_output);
   ASSERT_TRUE(telemetry_info_output->battery_result);
   ASSERT_TRUE(telemetry_info_output->battery_result->is_battery_info());
-  EXPECT_EQ(
-      telemetry_info_output->battery_result->get_battery_info()->cycle_count,
-      kCycleCount);
+  ASSERT_TRUE(telemetry_info_output->battery_result->get_battery_info());
+  ASSERT_TRUE(
+      telemetry_info_output->battery_result->get_battery_info()->cycle_count);
+  EXPECT_EQ(telemetry_info_output->battery_result->get_battery_info()
+                ->cycle_count->value,
+            kCycleCount);
 }
 
 TEST(ProbeServiceConvertors, TelemetryInfoPtrWithNullFields) {

chromeos/components/telemetry_extension_ui/probe_service_unittest.cc

@@ -69,8 +69,9 @@ TEST_F(ProbeServiceTest, ProbeTelemetryInfoSuccess) {
         ASSERT_TRUE(ptr);
         ASSERT_TRUE(ptr->battery_result);
         ASSERT_TRUE(ptr->battery_result->is_battery_info());
-
-        EXPECT_EQ(ptr->battery_result->get_battery_info()->cycle_count,
+        ASSERT_TRUE(ptr->battery_result->get_battery_info());
+        ASSERT_TRUE(ptr->battery_result->get_battery_info()->cycle_count);
+        EXPECT_EQ(ptr->battery_result->get_battery_info()->cycle_count->value,
                   kCycleCount);
 
         run_loop.Quit();