Skip to content

Commit

Permalink
cros_healthd: Update mojom interface to raname SystemInfoV2
Browse files Browse the repository at this point in the history
Rename SystemResultV2 to SystemResult.

|SystemInfoV2| can now be retrieved by the |kSystem| flag in chromium
14948.0.0. (CL:3698116, CL:3714953).

BUG=b:190459636
TEST=CQ
TEST=tast run ${DUT_IP} telemetryextension.API*.target_models_managed
TEST=deploy chrome on DUT and then open Diagnostics App to check the
system information (board name, version info...)

Change-Id: Ie281d912b6b9dfcc7586247c2137f4d6f02e68bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3759149
Reviewed-by: Oleh Lamzin <lamzin@google.com>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Michael Checo <michaelcheco@google.com>
Commit-Queue: Byron Lee <byronlee@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1028654}
  • Loading branch information
BaiiYuan authored and Chromium LUCI CQ committed Jul 27, 2022
1 parent 176ea85 commit 5e17526
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 124 deletions.
16 changes: 8 additions & 8 deletions ash/webui/diagnostics_ui/backend/cros_healthd_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ using ::chromeos::cros_healthd::mojom::NonInteractiveRoutineUpdatePtr;
using ::chromeos::cros_healthd::mojom::RoutineUpdate;
using ::chromeos::cros_healthd::mojom::RoutineUpdateUnion;
using ::chromeos::cros_healthd::mojom::RoutineUpdateUnionPtr;
using ::chromeos::cros_healthd::mojom::SystemInfoV2;
using ::chromeos::cros_healthd::mojom::SystemResultV2;
using ::chromeos::cros_healthd::mojom::SystemResultV2Ptr;
using ::chromeos::cros_healthd::mojom::SystemInfo;
using ::chromeos::cros_healthd::mojom::SystemResult;
using ::chromeos::cros_healthd::mojom::SystemResultPtr;
using ::chromeos::cros_healthd::mojom::TelemetryInfo;

template <typename TResult, typename TTag>
Expand Down Expand Up @@ -84,14 +84,14 @@ const MemoryInfo* GetMemoryInfo(const TelemetryInfo& info) {
return memory_result->get_memory_info().get();
}

const SystemInfoV2* GetSystemInfo(const TelemetryInfo& info) {
const SystemResultV2Ptr& system_result = info.system_result_v2;
if (!CheckResponse(system_result, SystemResultV2::Tag::kSystemInfoV2,
"system info v2")) {
const SystemInfo* GetSystemInfo(const TelemetryInfo& info) {
const SystemResultPtr& system_result = info.system_result;
if (!CheckResponse(system_result, SystemResult::Tag::kSystemInfo,
"system info")) {
return nullptr;
}

return system_result->get_system_info_v2().get();
return system_result->get_system_info().get();
}

const NonInteractiveRoutineUpdate* GetNonInteractiveRoutineUpdate(
Expand Down
4 changes: 2 additions & 2 deletions ash/webui/diagnostics_ui/backend/cros_healthd_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ const chromeos::cros_healthd::mojom::CpuInfo* GetCpuInfo(
const chromeos::cros_healthd::mojom::MemoryInfo* GetMemoryInfo(
const chromeos::cros_healthd::mojom::TelemetryInfo& info);

// Extracts SystemInfoV2 from |info|. Logs and returns a nullptr if SystemInfoV2
// Extracts SystemInfo from |info|. Logs and returns a nullptr if SystemInfo
// in not present.
const chromeos::cros_healthd::mojom::SystemInfoV2* GetSystemInfo(
const chromeos::cros_healthd::mojom::SystemInfo* GetSystemInfo(
const chromeos::cros_healthd::mojom::TelemetryInfo& info);

const chromeos::cros_healthd::mojom::NonInteractiveRoutineUpdate*
Expand Down
14 changes: 7 additions & 7 deletions ash/webui/diagnostics_ui/backend/system_data_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ constexpr int kCpuUsageRefreshIntervalInSeconds = 1;
constexpr int kMemoryUsageRefreshIntervalInSeconds = 10;
constexpr int kMilliampsInAnAmp = 1000;

void PopulateBoardName(const healthd::SystemInfoV2& system_info_v2,
void PopulateBoardName(const healthd::SystemInfo& system_info,
mojom::SystemInfo& out_system_info) {
out_system_info.board_name = system_info_v2.os_info->code_name;
out_system_info.board_name = system_info.os_info->code_name;
}

void PopulateMarketingName(const healthd::SystemInfoV2& system_info_v2,
void PopulateMarketingName(const healthd::SystemInfo& system_info,
mojom::SystemInfo& out_system_info) {
const absl::optional<std::string>& marketing_name =
system_info_v2.os_info->marketing_name;
system_info.os_info->marketing_name;

if (!marketing_name.has_value()) {
DVLOG(1) << "No marketing name in SystemInfoV2 response.";
DVLOG(1) << "No marketing name in SystemInfo response.";
return;
}

Expand Down Expand Up @@ -84,7 +84,7 @@ void PopulateCpuInfo(const healthd::CpuInfo& cpu_info,
total_max_ghz / physical_cpus[0]->logical_cpus.size();
}

void PopulateVersionInfo(const healthd::SystemInfoV2& system_info,
void PopulateVersionInfo(const healthd::SystemInfo& system_info,
mojom::SystemInfo& out_system_info) {
const std::string full_version =
system_info.os_info->os_version->release_milestone + '.' +
Expand Down Expand Up @@ -389,7 +389,7 @@ void SystemDataProvider::OnSystemInfoProbeResponse(
return;
}

const healthd::SystemInfoV2* system_info_ptr =
const healthd::SystemInfo* system_info_ptr =
diagnostics::GetSystemInfo(*info_ptr);
if (system_info_ptr) {
PopulateBoardName(*system_info_ptr, *system_info.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ namespace healthd_mojom = ::chromeos::cros_healthd::mojom;
void SetProbeTelemetryInfoResponse(healthd_mojom::BatteryInfoPtr battery_info,
healthd_mojom::CpuInfoPtr cpu_info,
healthd_mojom::MemoryInfoPtr memory_info,
healthd_mojom::SystemInfoV2Ptr system_info) {
healthd_mojom::SystemInfoPtr system_info) {
auto info = healthd_mojom::TelemetryInfo::New();
if (system_info) {
info->system_result_v2 =
healthd_mojom::SystemResultV2::NewSystemInfoV2(std::move(system_info));
info->system_result =
healthd_mojom::SystemResult::NewSystemInfo(std::move(system_info));
}
if (battery_info) {
info->battery_result =
Expand Down Expand Up @@ -76,7 +76,7 @@ void SetCrosHealthdSystemInfoResponse(const std::string& board_name,
os_info->marketing_name = marketing_name;
os_info->os_version = healthd_mojom::OsVersion::New(
milestone_version, build_number, patch_number, "unittest-channel");
auto system_info = healthd_mojom::SystemInfoV2::New();
auto system_info = healthd_mojom::SystemInfo::New();
system_info->os_info = std::move(os_info);

// Battery info
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1235,28 +1235,28 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
}

// Process SystemResultV2.
const auto& system_result_v2 = probe_result->system_result_v2;
if (!system_result_v2.is_null()) {
switch (system_result_v2->which()) {
case cros_healthd::SystemResultV2::Tag::kError: {
LOG(ERROR) << "cros_healthd: Error getting system info v2: "
<< system_result_v2->get_error()->msg;
// Process SystemResult.
const auto& system_result = probe_result->system_result;
if (!system_result.is_null()) {
switch (system_result->which()) {
case cros_healthd::SystemResult::Tag::kError: {
LOG(ERROR) << "cros_healthd: Error getting system info: "
<< system_result->get_error()->msg;
break;
}

// The System Info V2 tag is always called to get vendor, product name,
// The System Info tag is always called to get vendor, product name,
// and product version. Because of this, make sure to wrap additional
// data collection behind a policy, similar to bios version and os
// info below.
case cros_healthd::SystemResultV2::Tag::kSystemInfoV2: {
const auto& system_info_v2 = system_result_v2->get_system_info_v2();
case cros_healthd::SystemResult::Tag::kSystemInfo: {
const auto& system_info = system_result->get_system_info();

if (report_vpd_info || report_system_info) {
em::SystemStatus* const system_status_out =
response_params_.device_status->mutable_system_status();
if (report_vpd_info && !system_info_v2->vpd_info.is_null()) {
const auto& vpd_info = system_info_v2->vpd_info;
if (report_vpd_info && !system_info->vpd_info.is_null()) {
const auto& vpd_info = system_info->vpd_info;
if (vpd_info->activate_date.has_value()) {
system_status_out->set_first_power_date(
vpd_info->activate_date.value());
Expand All @@ -1279,8 +1279,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
}
if (report_system_info) {
if (!system_info_v2->dmi_info.is_null()) {
const auto& dmi_info = system_info_v2->dmi_info;
if (!system_info->dmi_info.is_null()) {
const auto& dmi_info = system_info->dmi_info;
if (dmi_info->bios_version.has_value()) {
system_status_out->set_bios_version(
dmi_info->bios_version.value());
Expand All @@ -1302,8 +1302,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
SetDeviceStatusReported();
}
}
if (!system_info_v2->os_info.is_null()) {
const auto& os_info = system_info_v2->os_info;
if (!system_info->os_info.is_null()) {
const auto& os_info = system_info->os_info;
if (os_info->marketing_name.has_value()) {
system_status_out->set_marketing_name(
os_info->marketing_name.value());
Expand All @@ -1316,8 +1316,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {

em::SmbiosInfo* const smbios_info_out =
response_params_.device_status->mutable_smbios_info();
if (!system_info_v2->dmi_info.is_null()) {
const auto& dmi_info = system_info_v2->dmi_info;
if (!system_info->dmi_info.is_null()) {
const auto& dmi_info = system_info->dmi_info;

// The vendor, product name, and product version should always be
// reported.
Expand All @@ -1337,10 +1337,10 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
SetDeviceStatusReported();
}
if (report_system_info && !system_info_v2->os_info.is_null()) {
if (report_system_info && !system_info->os_info.is_null()) {
em::BootInfo* const boot_info_out =
response_params_.device_status->mutable_boot_info();
const auto& os_info = system_info_v2->os_info;
const auto& os_info = system_info->os_info;
boot_info_out->set_boot_method(
static_cast<em::BootInfo::BootMethod>(os_info->boot_mode));
SetDeviceStatusReported();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,9 @@ cros_healthd::NonRemovableBlockDeviceResultPtr CreateBlockDeviceResult() {
std::move(storage_vector));
}

cros_healthd::SystemResultV2Ptr CreateSystemResultV2() {
return cros_healthd::SystemResultV2::NewSystemInfoV2(
cros_healthd::SystemInfoV2::New(
cros_healthd::SystemResultPtr CreateSystemResult() {
return cros_healthd::SystemResult::NewSystemInfo(
cros_healthd::SystemInfo::New(
cros_healthd::OsInfo::New(
kFakeOsInfoProductName, kFakeOsInfoMarketingName,
cros_healthd::OsVersion::New(
Expand Down Expand Up @@ -745,8 +745,8 @@ cros_healthd::BusResultPtr CreateBusResult() {
void SetFakeCrosHealthdData() {
auto telemetry_info = cros_healthd::TelemetryInfo::New();
cros_healthd::TelemetryInfo fake_info;
// Always gather system V2.
telemetry_info->system_result_v2 = CreateSystemResultV2();
// Always gather system result.
telemetry_info->system_result = CreateSystemResult();
if (SettingEnabled(ash::kReportDevicePowerStatus))
telemetry_info->battery_result = CreateBatteryResult();
if (SettingEnabled(ash::kReportDeviceStorageStatus))
Expand Down Expand Up @@ -3605,7 +3605,7 @@ TEST_F(DeviceStatusCollectorTest, TestPartialCrosHealthdInfo) {
EXPECT_FALSE(device_status_.has_memory_info());
EXPECT_FALSE(device_status_.has_timezone_info());
EXPECT_FALSE(device_status_.has_system_status());
// Some smbios info from SystemResult V2 is always reported by default.
// Some smbios info from SystemResult is always reported by default.
EXPECT_TRUE(device_status_.has_smbios_info());
EXPECT_FALSE(device_status_.has_boot_info());
EXPECT_FALSE(device_status_.has_storage_status());
Expand Down Expand Up @@ -3642,8 +3642,8 @@ TEST_F(DeviceStatusCollectorTest, TestCrosHealthdVpdAndSystemInfo) {
ASSERT_FALSE(device_status_.system_status().has_chassis_type());
ASSERT_FALSE(device_status_.system_status().has_product_name());

// Verify the system info v2 is not populated excluding vendor, product name,
// and product version.
// Verify the system info is not populated excluding vendor, product name and
// product version.
ASSERT_TRUE(device_status_.has_smbios_info());
EXPECT_TRUE(device_status_.smbios_info().has_sys_vendor());
EXPECT_TRUE(device_status_.smbios_info().has_product_name());
Expand Down Expand Up @@ -3678,7 +3678,7 @@ TEST_F(DeviceStatusCollectorTest, TestCrosHealthdVpdAndSystemInfo) {
EXPECT_EQ(device_status_.system_status().product_name(),
kFakeOsInfoProductName);

// Verify system info V2 exists too.
// Verify smbios info and boot info exist.
ASSERT_TRUE(device_status_.has_smbios_info());
EXPECT_EQ(device_status_.smbios_info().product_name(),
kFakeDmiInfoProductName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1232,23 +1232,23 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
}

// Process SystemResultV2.
const auto& system_result_v2 = probe_result->system_result_v2;
if (!system_result_v2.is_null()) {
switch (system_result_v2->which()) {
case cros_healthd::SystemResultV2::Tag::kError: {
LOG(ERROR) << "cros_healthd: Error getting system info v2: "
<< system_result_v2->get_error()->msg;
// Process SystemResult.
const auto& system_result = probe_result->system_result;
if (!system_result.is_null()) {
switch (system_result->which()) {
case cros_healthd::SystemResult::Tag::kError: {
LOG(ERROR) << "cros_healthd: Error getting system info: "
<< system_result->get_error()->msg;
break;
}

case cros_healthd::SystemResultV2::Tag::kSystemInfoV2: {
const auto& system_info_v2 = system_result_v2->get_system_info_v2();
case cros_healthd::SystemResult::Tag::kSystemInfo: {
const auto& system_info = system_result->get_system_info();

em::SystemStatus* const system_status_out =
response_params_.device_status->mutable_system_status();
if (report_vpd_info && !system_info_v2->vpd_info.is_null()) {
const auto& vpd_info = system_info_v2->vpd_info;
if (report_vpd_info && !system_info->vpd_info.is_null()) {
const auto& vpd_info = system_info->vpd_info;
if (vpd_info->activate_date.has_value()) {
system_status_out->set_first_power_date(
vpd_info->activate_date.value());
Expand All @@ -1271,8 +1271,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
}
}
if (report_system_info) {
if (!system_info_v2->dmi_info.is_null()) {
const auto& dmi_info = system_info_v2->dmi_info;
if (!system_info->dmi_info.is_null()) {
const auto& dmi_info = system_info->dmi_info;
if (dmi_info->bios_version.has_value()) {
system_status_out->set_bios_version(
dmi_info->bios_version.value());
Expand All @@ -1293,8 +1293,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
SetDeviceStatusReported();
}
}
if (!system_info_v2->os_info.is_null()) {
const auto& os_info = system_info_v2->os_info;
if (!system_info->os_info.is_null()) {
const auto& os_info = system_info->os_info;
if (os_info->marketing_name.has_value()) {
system_status_out->set_marketing_name(
os_info->marketing_name.value());
Expand All @@ -1309,8 +1309,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
em::BootInfo* const boot_info_out =
response_params_.device_status->mutable_boot_info();
if (report_system_info) {
if (!system_info_v2->dmi_info.is_null()) {
const auto& dmi_info = system_info_v2->dmi_info;
if (!system_info->dmi_info.is_null()) {
const auto& dmi_info = system_info->dmi_info;
if (dmi_info->bios_version.has_value()) {
smbios_info_out->set_bios_version(
dmi_info->bios_version.value());
Expand All @@ -1327,8 +1327,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
dmi_info->product_version.value());
}
}
if (!system_info_v2->os_info.is_null()) {
const auto& os_info = system_info_v2->os_info;
if (!system_info->os_info.is_null()) {
const auto& os_info = system_info->os_info;
boot_info_out->set_boot_method(
static_cast<em::BootInfo::BootMethod>(os_info->boot_mode));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,9 @@ cros_healthd::NonRemovableBlockDeviceResultPtr CreateBlockDeviceResult() {
std::move(storage_vector));
}

cros_healthd::SystemResultV2Ptr CreateSystemResultV2() {
return cros_healthd::SystemResultV2::NewSystemInfoV2(
cros_healthd::SystemInfoV2::New(
cros_healthd::SystemResultPtr CreateSystemResult() {
return cros_healthd::SystemResult::NewSystemInfo(
cros_healthd::SystemInfo::New(
cros_healthd::OsInfo::New(
kFakeOsInfoProductName, kFakeOsInfoMarketingName,
cros_healthd::OsVersion::New(
Expand Down Expand Up @@ -703,7 +703,7 @@ void FetchFakeFullCrosHealthdData(
cros_healthd::TelemetryInfo fake_info;
fake_info.battery_result = CreateBatteryResult();
fake_info.block_device_result = CreateBlockDeviceResult();
fake_info.system_result_v2 = CreateSystemResultV2();
fake_info.system_result = CreateSystemResult();
fake_info.cpu_result = CreateCpuResult();
fake_info.timezone_result = CreateTimezoneResult();
fake_info.memory_result = CreateMemoryResult();
Expand Down Expand Up @@ -3447,7 +3447,7 @@ TEST_F(LegacyDeviceStatusCollectorTest, TestCrosHealthdInfo) {
EXPECT_EQ(device_status_.system_status().product_name(),
kFakeOsInfoProductName);

// Verify the system v2 info.
// Verify the smbios info and the boot info.
ASSERT_TRUE(device_status_.has_smbios_info());
EXPECT_EQ(device_status_.smbios_info().product_name(),
kFakeDmiInfoProductName);
Expand Down Expand Up @@ -3656,7 +3656,7 @@ TEST_F(LegacyDeviceStatusCollectorTest, TestCrosHealthdVpdAndSystemInfo) {
ASSERT_FALSE(device_status_.system_status().has_chassis_type());
ASSERT_FALSE(device_status_.system_status().has_product_name());

// Verify the system info v2 is not populated.
// Verify smbios info and boot info are not populated.
ASSERT_TRUE(device_status_.has_smbios_info());
ASSERT_FALSE(device_status_.smbios_info().has_sys_vendor());
ASSERT_FALSE(device_status_.smbios_info().has_product_name());
Expand Down Expand Up @@ -3691,7 +3691,7 @@ TEST_F(LegacyDeviceStatusCollectorTest, TestCrosHealthdVpdAndSystemInfo) {
EXPECT_EQ(device_status_.system_status().product_name(),
kFakeOsInfoProductName);

// Verify system info V2 exists too.
// Verify smbios info and boot info exist.
ASSERT_TRUE(device_status_.has_smbios_info());
EXPECT_EQ(device_status_.smbios_info().product_name(),
kFakeDmiInfoProductName);
Expand Down

0 comments on commit 5e17526

Please sign in to comment.