Commit ef476b5c authored by Maxim Kolosovskiy's avatar Maxim Kolosovskiy Committed by Commit Bot

Reland "[Telemetry SWX] Add battery capacity routine"

This reverts commit cebb96ff.

Reason for revert: this CL didn't cause the problem. The root cause was a tricky infra problem. sorry for wrong revert.

Original change's description:
> Revert "[Telemetry SWX] Add battery capacity routine"
> 
> This reverts commit 437b61ae.
> 
> Reason for revert: No good reason, but this build causes a compilation error. No clear culprit. https://ci.chromium.org/p/chromium/builders/ci/Mac%20ASan%2064%20Builder/104190
> 
> Original change's description:
> > [Telemetry SWX] Add battery capacity routine
> > 
> > Add implementation to chrome://.
> > 
> > Add implementation to chrome-untrusted://.
> > 
> > Add tests.
> > 
> > Bug: b:162051831
> > Change-Id: Ib2d618c4eacce43e6cd588c766e8d40af9ae318b
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362933
> > Commit-Queue: Laurențiu Olteanu <lolteanu@google.com>
> > Reviewed-by: Tom Sepez <tsepez@chromium.org>
> > Reviewed-by: Oleh Lamzin <lamzin@google.com>
> > Cr-Commit-Position: refs/heads/master@{#803030}
> 
> TBR=tsepez@chromium.org,lamzin@google.com,mgawad@google.com,lolteanu@google.com
> 
> Change-Id: I675879ec1c65f834362901ba862923acadfa9551
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: b:162051831
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383028
> Reviewed-by: Maxim Kolosovskiy  <kolos@chromium.org>
> Commit-Queue: Maxim Kolosovskiy  <kolos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803041}

TBR=tsepez@chromium.org,kolos@chromium.org,lamzin@google.com,mgawad@google.com,lolteanu@google.com

# Not skipping CQ checks because this is a reland.

Bug: b:162051831
Change-Id: I1e09e1a56feba44f2079c2b459337a88e622d30c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382838Reviewed-by: default avatarMaxim Kolosovskiy  <kolos@chromium.org>
Commit-Queue: Maxim Kolosovskiy  <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803076}
parent bd9402b6
......@@ -64,4 +64,19 @@ void DiagnosticsService::GetRoutineUpdate(
std::move(callback)));
}
void DiagnosticsService::RunBatteryCapacityRoutine(
uint32_t low_mah,
uint32_t high_mah,
RunBatteryCapacityRoutineCallback callback) {
GetService()->RunBatteryCapacityRoutine(
low_mah, high_mah,
base::BindOnce(
[](health::mojom::DiagnosticsService::
RunBatteryCapacityRoutineCallback callback,
cros_healthd::mojom::RunRoutineResponsePtr ptr) {
std::move(callback).Run(converters::ConvertPtr(std::move(ptr)));
},
std::move(callback)));
}
} // namespace chromeos
......@@ -31,6 +31,10 @@ class DiagnosticsService : public health::mojom::DiagnosticsService {
health::mojom::DiagnosticRoutineCommandEnum command,
bool include_output,
GetRoutineUpdateCallback callback) override;
void RunBatteryCapacityRoutine(
uint32_t low_mah,
uint32_t high_mah,
RunBatteryCapacityRoutineCallback callback) override;
// Ensures that |service_| created and connected to the
// CrosHealthdProbeService.
......
......@@ -47,6 +47,12 @@ health::mojom::NonInteractiveRoutineUpdatePtr UncheckedConvertPtr(
Convert(input->status), std::move(input->status_message));
}
health::mojom::RunRoutineResponsePtr UncheckedConvertPtr(
cros_healthd::mojom::RunRoutineResponsePtr input) {
return health::mojom::RunRoutineResponse::New(input->id,
Convert(input->status));
}
} // namespace unchecked
health::mojom::DiagnosticRoutineStatusEnum Convert(
......
......@@ -37,6 +37,9 @@ health::mojom::InteractiveRoutineUpdatePtr UncheckedConvertPtr(
health::mojom::NonInteractiveRoutineUpdatePtr UncheckedConvertPtr(
cros_healthd::mojom::NonInteractiveRoutineUpdatePtr input);
health::mojom::RunRoutineResponsePtr UncheckedConvertPtr(
cros_healthd::mojom::RunRoutineResponsePtr input);
} // namespace unchecked
std::vector<health::mojom::DiagnosticRoutineEnum> Convert(
......
......@@ -40,6 +40,22 @@ interface DiagnosticsService {
GetRoutineUpdate(int32 id, DiagnosticRoutineCommandEnum command,
bool include_output)
=> (RoutineUpdate routine_update);
// Requests that the BatteryCapacity routine is created and started on the
// platform. This routine checks the battery's design capacity against the
// inputs. The routine will pass iff the design capacity of the battery read
// from the platform is inclusively within these bounds. This routine is only
// available if GetAvailableRoutines returned kBatteryCapactity.
//
// The request:
// * |low_mah| - lower bound for the battery's design capacity (mAh).
// * |high_mah| - upper bound for the battery's design capacity (mAh).
//
// The response:
// * |response| - contains a unique identifier and status for the created
// routine.
RunBatteryCapacityRoutine(uint32 low_mah, uint32 high_mah)
=> (RunRoutineResponse response);
};
// Enumeration of each of the diagnostics routines the platform may support.
......@@ -134,3 +150,14 @@ struct RoutineUpdate {
// noninteractive.
RoutineUpdateUnion routine_update_union;
};
// Generic return value for a RunSomeRoutine call.
struct RunRoutineResponse {
// Unique identifier for the newly-created routine. An id of kFailedToStartId
// means that the routine was unable to be created. Can be used in a
// GetRoutineUpdate call to control or get the status of the created routine.
int32 id;
// Current status of the newly-created routine. A status of kFailedToStart
// means the routine was unable to be created.
DiagnosticRoutineStatusEnum status;
};
......@@ -20,6 +20,8 @@ const dpsl_internal = {};
dpsl_internal.Message = {
DIAGNOSTICS_AVAILABLE_ROUTINES: 'DiagnosticsService.GetAvailableRoutines',
DIAGNOSTICS_ROUTINE_UPDATE: 'DiagnosticsService.GetRoutineUpdate',
DIAGNOSTICS_RUN_BATTERY_CAPACITY_ROUTINE:
'DiagnosticsService.RunBatteryCapacityRoutine',
PROBE_TELEMETRY_INFO: 'ProbeService.ProbeTelemetryInfo',
};
......@@ -54,6 +56,22 @@ dpsl_internal.DiagnosticsGetRoutineUpdateRequest;
*/
dpsl_internal.DiagnosticsGetRoutineUpdateResponse;
/**
* Request message sent by the unprivileged context to the privileged
* context to run battery capacity routine.
* @typedef {{
* lowMah: !number,
* highMah: !number}}
*/
dpsl_internal.DiagnosticsRunBatteryCapacityRoutineRequest;
/**
* Response message sent by the privileged context containing routine
* information.
* @typedef { !Object | !Error }
*/
dpsl_internal.DiagnosticsRunRoutineResponse;
/**
* Request message sent by the unprivileged context to request the privileged
* context to probe telemetry information
......
......@@ -29,6 +29,13 @@ function getOrCreateProbeService() {
probeService);
}
/**
* Alias for Mojo RunRoutine response.
* @typedef { !Promise<{response: !chromeos.health.mojom.RunRoutineResponse}>
* }
*/
let RunRoutineResponsePromise;
/**
* Proxying diagnostics requests between DiagnosticsRequester on
* chrome-untrusted:// side with WebIDL types and DiagnosticsService on
......@@ -254,6 +261,47 @@ class DiagnosticsProxy {
return this.convertRoutineUpdate(response.routineUpdate);
};
/**
* @param { !chromeos.health.mojom.RunRoutineResponse } runRoutineResponse
* @return { !Object }
*/
convertRunRoutineResponse(runRoutineResponse) {
return {
id: runRoutineResponse.id,
status: this.convertStatus(runRoutineResponse.status)
};
};
/**
* Generic handler for a runRoutine.
* @param { !function(!Object): !RunRoutineResponsePromise } handler
* @param { !Object } message
* @return { !Promise<!dpsl_internal.DiagnosticsRunRoutineResponse> }
*/
async genericRunRoutineHandler(handler, message) {
try {
const response = await handler(message);
return this.convertRunRoutineResponse(response.response);
} catch (/** @type !Error */ error) {
return error;
}
};
/**
* Runs battery capacity routine.
* @param { !Object } message
* @return { !RunRoutineResponsePromise }
*/
async handleRunBatteryCapacityRoutine(message) {
const request =
/**
* @type {!dpsl_internal.DiagnosticsRunBatteryCapacityRoutineRequest}
*/
(message);
return await getOrCreateDiagnosticsService().runBatteryCapacityRoutine(
request.lowMah, request.highMah);
};
};
const diagnosticsProxy = new DiagnosticsProxy();
......@@ -438,6 +486,12 @@ untrustedMessagePipe.registerHandler(
dpsl_internal.Message.DIAGNOSTICS_ROUTINE_UPDATE,
(message) => diagnosticsProxy.handleGetRoutineUpdate(message));
untrustedMessagePipe.registerHandler(
dpsl_internal.Message.DIAGNOSTICS_RUN_BATTERY_CAPACITY_ROUTINE,
(message) => diagnosticsProxy.genericRunRoutineHandler(
(message) => diagnosticsProxy.handleRunBatteryCapacityRoutine(message),
message));
untrustedMessagePipe.registerHandler(
dpsl_internal.Message.PROBE_TELEMETRY_INFO,
(message) => telemetryProxy.handleProbeTelemetryInfo(message));
......@@ -74,6 +74,29 @@ chromeos.test_support = {};
}
return response;
}
/**
* Requests battery capacity routine to be run.
* @param { !number } lowMah
* @param { !number } highMah
* @return { !Promise<!Object> }
* @public
*/
async runBatteryCapacityRoutine(lowMah, highMah) {
const message =
/**
@type {!dpsl_internal.DiagnosticsRunBatteryCapacityRoutineRequest}
*/
({lowMah: lowMah, highMah: highMah});
const response =
/** @type {!Object} */ (await messagePipe.sendMessage(
dpsl_internal.Message.DIAGNOSTICS_RUN_BATTERY_CAPACITY_ROUTINE,
message));
if (response instanceof Error) {
throw response;
}
return response;
}
};
/**
......
......@@ -132,6 +132,16 @@ void TelemetryExtensionUiBrowserTest::SetUpOnMainThread() {
->SetAvailableRoutinesForTesting(input);
}
{
auto input = chromeos::cros_healthd::mojom::RunRoutineResponse::New();
input->id = 123456789;
input->status =
chromeos::cros_healthd::mojom::DiagnosticRoutineStatusEnum::kReady;
chromeos::cros_healthd::FakeCrosHealthdClient::Get()
->SetRunRoutineResponseForTesting(input);
}
auto telemetry_info = chromeos::cros_healthd::mojom::TelemetryInfo::New();
{
auto battery_info = chromeos::cros_healthd::mojom::BatteryInfo::New();
......
......@@ -223,6 +223,14 @@ TEST_F(
testDone();
});
TEST_F(
'TelemetryExtensionUIBrowserTest',
'UntrustedDiagnosticsRequestRunBatteryCapacityRoutine', async () => {
await runTestInUntrusted(
'UntrustedDiagnosticsRequestRunBatteryCapacityRoutine');
testDone();
});
TEST_F(
'TelemetryExtensionUIBrowserTest',
'UntrustedRequestTelemetryInfoUnknownCategory', async () => {
......
......@@ -129,6 +129,14 @@ UNTRUSTED_TEST(
`Diagnostic command \'this-command-must-not-exist\' is unknown.`);
});
// Tests that runBatteryCapacityRoutine returns the correct Object.
UNTRUSTED_TEST(
'UntrustedDiagnosticsRequestRunBatteryCapacityRoutine', async () => {
const response =
await chromeos.diagnostics.runBatteryCapacityRoutine(3000, 4000);
assertDeepEquals(response, {id: 123456789, status: 'ready'});
});
// Tests that TelemetryInfo can be successfully requested from
// from chrome-untrusted://.
UNTRUSTED_TEST('UntrustedRequestTelemetryInfo', async () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment