Show devices in target selection dropdown & improve error handling
Thanks to this CL we are able to show the android devices
connected in the target selection dropdown list.
Now the last error from the record session is displayed, and after
an error the state of the UI is resetted, so that to be able to
start another trace.
Change-Id: I4093dd33dbe6436bbf3df3e856d575dc5bc8f403
diff --git a/ui/src/assets/record.scss b/ui/src/assets/record.scss
index 47686b4..c5326b9 100644
--- a/ui/src/assets/record.scss
+++ b/ui/src/assets/record.scss
@@ -91,13 +91,44 @@
flex-direction: column;
justify-content: space-evenly;
- label , select {
+ .target {
+ display: flex;
+ flex-direction: row;
+ align-items: center;
+ }
+
+ label, select, button {
font-weight: 100;
margin: 3px;
color: #333;
font-size: 17px;
font-family: 'Roboto', sans-serif;
align-items: center;
+
+ &.error-label {
+ max-width: 500px;
+ color: rgb(255, 0, 0);
+ font-size: 15px;
+ }
+ }
+ .chip {
+ @include transition();
+ display: flex;
+ align-items: center;
+ border: 1px solid #eee;
+ outline: none;
+ margin: 4px;
+ border-radius: 10px;
+ padding: 4px;
+ height: 30px;
+ &:hover, &:active {
+ box-shadow: 0 0 4px 0px #999;
+ background-color: hsl(88, 50%, 84%);
+ }
+ i {
+ margin: 3px;
+ align-items: center;
+ }
}
}
}
diff --git a/ui/src/common/actions.ts b/ui/src/common/actions.ts
index 79d1f9f..0850d3b 100644
--- a/ui/src/common/actions.ts
+++ b/ui/src/common/actions.ts
@@ -18,6 +18,7 @@
import {ConvertTrace} from '../controller/trace_converter';
import {
+ AdbRecordingTarget,
createEmptyState,
LogsPagination,
OmniboxState,
@@ -396,6 +397,7 @@
startRecording(state: StateDraft): void {
state.recordingInProgress = true;
+ state.lastRecordingError = undefined;
},
stopRecording(state: StateDraft): void {
@@ -410,9 +412,15 @@
state.bufferUsage = args.percentage;
},
- setAndroidDevice(state: StateDraft, args: {serial: string|undefined}): void {
- state.serialAndroidDeviceConnected = args.serial;
- },
+ setAndroidDevice(state: StateDraft, args: {target?: AdbRecordingTarget}):
+ void {
+ state.androidDeviceConnected = args.target;
+ },
+
+ setAvailableDevices(state: StateDraft, args: {devices: AdbRecordingTarget[]}):
+ void {
+ state.availableDevices = args.devices;
+ },
setOmnibox(state: StateDraft, args: OmniboxState): void {
state.frontendLocalState.omniboxState = args;
@@ -425,6 +433,16 @@
setChromeCategories(state: StateDraft, args: {categories: string[]}): void {
state.chromeCategories = args.categories;
},
+
+ setLastRecordingError(state: StateDraft, args: {error?: string}): void {
+ state.lastRecordingError = args.error;
+ state.recordingStatus = undefined;
+ },
+
+ setRecordingStatus(state: StateDraft, args: {status?: string}): void {
+ state.recordingStatus = args.status;
+ state.lastRecordingError = undefined;
+ },
};
// When we are on the frontend side, we don't really want to execute the
diff --git a/ui/src/common/state.ts b/ui/src/common/state.ts
index d4776d6..7485ad5 100644
--- a/ui/src/common/state.ts
+++ b/ui/src/common/state.ts
@@ -129,6 +129,12 @@
count: number;
}
+export interface AdbRecordingTarget {
+ serial: string;
+ name: string;
+ os: string;
+}
+
export interface State {
// tslint:disable-next-line:no-any
[key: string]: any;
@@ -179,7 +185,11 @@
*/
recordingInProgress: boolean;
extensionInstalled: boolean;
- serialAndroidDeviceConnected: string|undefined;
+ androidDeviceConnected?: AdbRecordingTarget;
+ availableDevices: AdbRecordingTarget[];
+ lastRecordingError?: string;
+ recordingStatus?: string;
+
chromeCategories: string[]|undefined;
}
@@ -362,7 +372,9 @@
flagPauseEnabled: false,
recordingInProgress: false,
extensionInstalled: false,
- serialAndroidDeviceConnected: undefined,
+ androidDeviceConnected: undefined,
+ availableDevices: [],
+
chromeCategories: undefined,
};
}
diff --git a/ui/src/controller/adb.ts b/ui/src/controller/adb.ts
index 40b5bf6..15c9139 100644
--- a/ui/src/controller/adb.ts
+++ b/ui/src/controller/adb.ts
@@ -52,7 +52,7 @@
// Devices after Dec 2017 don't use checksum. This will be auto-detected
// during the connection.
- useChecksum = false;
+ useChecksum = true;
private lastStreamId = 0;
private dev: USBDevice|undefined;
@@ -68,8 +68,13 @@
return navigator.usb.requestDevice({filters: [this.filter]});
}
+ async getPairedDevices() {
+ return navigator.usb.getDevices();
+ }
+
async connect(device: USBDevice): Promise<void> {
this.dev = device;
+ this.useChecksum = true;
this.key = await AdbOverWebUsb.initKey();
await this.dev.open();
@@ -443,7 +448,7 @@
}
- static create({cmd, arg0, arg1, data, useChecksum = false}: {
+ static create({cmd, arg0, arg1, data, useChecksum = true}: {
cmd: CmdType; arg0: number; arg1: number;
data?: Uint8Array | string;
useChecksum?: boolean;
diff --git a/ui/src/controller/adb_jsdomtest.ts b/ui/src/controller/adb_jsdomtest.ts
index f2b757f..83d05fd 100644
--- a/ui/src/controller/adb_jsdomtest.ts
+++ b/ui/src/controller/adb_jsdomtest.ts
@@ -19,7 +19,6 @@
AdbOverWebUsb,
AdbState,
DEFAULT_MAX_PAYLOAD_BYTES,
- VERSION_NO_CHECKSUM,
VERSION_WITH_CHECKSUM
} from './adb';
@@ -34,10 +33,10 @@
const expectedAuthMessage = AdbMsgImpl.create({
cmd: 'CNXN',
- arg0: VERSION_NO_CHECKSUM,
+ arg0: VERSION_WITH_CHECKSUM,
arg1: DEFAULT_MAX_PAYLOAD_BYTES,
data: 'host:1:UsbADB',
- useChecksum: false
+ useChecksum: true
});
await adb.startAuthentication();
@@ -60,7 +59,7 @@
arg0: VERSION_WITH_CHECKSUM,
arg1: expectedMaxPayload,
data: new TextEncoder().encode('device'),
- useChecksum: false
+ useChecksum: true
});
await adb.onMessage(connectedMsg);
diff --git a/ui/src/controller/adb_record_controller.ts b/ui/src/controller/adb_record_controller.ts
index 9194ba3..f3d17cb 100644
--- a/ui/src/controller/adb_record_controller.ts
+++ b/ui/src/controller/adb_record_controller.ts
@@ -13,10 +13,9 @@
// limitations under the License.
import {Adb, AdbStream} from './adb_interfaces';
-
import {ConsumerPortResponse, ReadBuffersResponse} from './consumer_port_types';
import {globals} from './globals';
-import {uint8ArrayToBase64} from './record_controller';
+import {RecordControllerMessage, uint8ArrayToBase64} from './record_controller';
enum AdbState {
READY,
@@ -31,7 +30,8 @@
private state = AdbState.READY;
private adb: Adb;
private device: USBDevice|undefined = undefined;
- private mainControllerCallback: (_: {data: ConsumerPortResponse}) => void;
+ private mainControllerCallback:
+ (_: {data: ConsumerPortResponse|RecordControllerMessage}) => void;
constructor(adb: Adb, mainControllerCallback: (_: {
data: ConsumerPortResponse
@@ -40,12 +40,17 @@
this.adb = adb;
}
- sendMessage(message: ConsumerPortResponse) {
+ sendMessage(message: ConsumerPortResponse|RecordControllerMessage) {
this.mainControllerCallback({data: message});
}
- sendErrorMessage(msg: string) {
- console.error('Error in adb record controller: ', msg);
+ sendErrorMessage(message: string) {
+ console.error('Error in adb record controller: ', message);
+ this.sendMessage({type: 'RecordControllerError', message});
+ }
+
+ sendStatus(status: string) {
+ this.sendMessage({type: 'RecordControllerStatus', status});
}
handleCommand(method: string, params: Uint8Array) {
@@ -81,12 +86,14 @@
this.sendErrorMessage('No device found');
return;
}
-
+ this.sendStatus(
+ 'Check the screen of your device and allow USB debugging.');
await this.adb.connect(this.device);
await this.startRecording(configProto);
+ this.sendStatus('Recording in progress...');
} catch (e) {
- this.sendErrorMessage(`Error in enableTracing: ${e.name} ${e.message}`);
+ this.sendErrorMessage(e.message);
}
}
@@ -98,7 +105,7 @@
recordShell.onData = (str, _) => response += str;
recordShell.onClose = () => {
if (!this.tracingEndedSuccessfully(response)) {
- this.sendErrorMessage(`Error in enableTracing, output: ${response}`);
+ this.sendErrorMessage(response);
this.state = AdbState.READY;
return;
}
@@ -107,14 +114,14 @@
}
tracingEndedSuccessfully(response: string): boolean {
- return response.includes('starting tracing') &&
- !response.includes(' 0 ms') && response.includes('Wrote ');
+ return !response.includes(' 0 ms') && response.includes('Wrote ');
}
async findDevice() {
- const targetSerial = globals.state.serialAndroidDeviceConnected;
+ const deviceConnected = globals.state.androidDeviceConnected;
+ if (!deviceConnected) return undefined;
const devices = await navigator.usb.getDevices();
- return devices.find(d => d.serialNumber === targetSerial);
+ return devices.find(d => d.serialNumber === deviceConnected.serial);
}
async readBuffers() {
diff --git a/ui/src/controller/adb_record_controller_jsdomtest.ts b/ui/src/controller/adb_record_controller_jsdomtest.ts
index 16656ec..dffb563 100644
--- a/ui/src/controller/adb_record_controller_jsdomtest.ts
+++ b/ui/src/controller/adb_record_controller_jsdomtest.ts
@@ -73,9 +73,12 @@
await adbController.enableTracing(mockIntArray);
expect(adbShell).toBeCalledWith('CMD');
- expect(sendMessage).toHaveBeenCalledTimes(0);
expect(findDevice).toHaveBeenCalledTimes(1);
expect(connectToDevice).toHaveBeenCalledTimes(1);
+ // Two messages: RecordControllerStatus asking for allow the debug, and
+ // another status to clear that message.
+ expect(sendMessage).toHaveBeenCalledTimes(2);
+
stream.onData('starting tracing Wrote 123 bytes', mockIntArray);
stream.onClose();
diff --git a/ui/src/controller/consumer_port_types.ts b/ui/src/controller/consumer_port_types.ts
index 9677be3..bb3c4ea 100644
--- a/ui/src/controller/consumer_port_types.ts
+++ b/ui/src/controller/consumer_port_types.ts
@@ -14,7 +14,7 @@
import {perfetto} from '../gen/protos';
-interface Typed {
+export interface Typed {
type: string;
}
diff --git a/ui/src/controller/record_controller.ts b/ui/src/controller/record_controller.ts
index cb47ae7..34feb53 100644
--- a/ui/src/controller/record_controller.ts
+++ b/ui/src/controller/record_controller.ts
@@ -45,12 +45,32 @@
isEnableTracingResponse,
isGetTraceStatsResponse,
isReadBuffersResponse,
+ Typed,
} from './consumer_port_types';
import {Controller} from './controller';
import {App, globals} from './globals';
type RPCImplMethod = (Method|rpc.ServiceMethod<Message<{}>, Message<{}>>);
+export interface RecordControllerError extends Typed {
+ message: string;
+}
+
+export interface RecordControllerStatus extends Typed {
+ status: string;
+}
+
+export type RecordControllerMessage =
+ RecordControllerError|RecordControllerStatus;
+
+function isError(obj: Typed): obj is RecordControllerError {
+ return obj.type === 'RecordControllerError';
+}
+
+function isStatus(obj: Typed): obj is RecordControllerStatus {
+ return obj.type === 'RecordControllerStatus';
+}
+
export function uint8ArrayToBase64(buffer: Uint8Array): string {
return btoa(String.fromCharCode.apply(null, Array.from(buffer)));
}
@@ -490,10 +510,11 @@
this.consumerPort.readBuffers({});
}
- onConsumerPortMessage({data}: {data: ConsumerPortResponse}) {
+ onConsumerPortMessage({data}: {
+ data: ConsumerPortResponse|RecordControllerMessage
+ }) {
if (data === undefined) return;
- // TODO(nicomazz): Add error handling.
if (isReadBuffersResponse(data)) {
if (!data.slices) return;
this.traceBuffer += data.slices[0].data;
@@ -506,11 +527,16 @@
if (percentage) {
globals.publish('BufferUsage', {percentage});
}
+ } else if (isError(data)) {
+ this.handleError(data.message);
+ } else if (isStatus(data)) {
+ this.handleStatus(data.status);
}
}
openTraceInUI() {
this.consumerPort.freeBuffers({});
+ globals.dispatch(Actions.setRecordingStatus({status: undefined}));
const trace = ungzip(this.stringToArrayBuffer(this.traceBuffer));
globals.dispatch(Actions.openTraceFromBuffer({buffer: trace.buffer}));
this.traceBuffer = '';
@@ -537,6 +563,16 @@
return used / total;
}
+ handleError(message: string) {
+ globals.dispatch(
+ Actions.setLastRecordingError({error: message.substr(0, 150)}));
+ globals.dispatch(Actions.stopRecording({}));
+ }
+
+ handleStatus(message: string) {
+ globals.dispatch(Actions.setRecordingStatus({status: message}));
+ }
+
// Depending on the recording target, different implementation of the
// consumer_port will be used.
// - Chrome target: This forwards the messages that have to be sent
diff --git a/ui/src/frontend/index.ts b/ui/src/frontend/index.ts
index 77be37b..132b179 100644
--- a/ui/src/frontend/index.ts
+++ b/ui/src/frontend/index.ts
@@ -34,6 +34,7 @@
import {openBufferWithLegacyTraceViewer} from './legacy_trace_viewer';
import {postMessageHandler} from './post_message_handler';
import {RecordPage} from './record_page';
+import {updateAvailableAdbDevices} from './record_page';
import {Router} from './router';
import {ViewerPage} from './viewer_page';
@@ -229,6 +230,8 @@
fetchChromeTracingCategoriesFromExtension(extensionPort);
}
+ updateAvailableAdbDevices();
+
// This forwards the messages from the controller to the extension
extensionLocalChannel.port2.onmessage = ({data}) => {
if (extensionPort) extensionPort.postMessage(data);
diff --git a/ui/src/frontend/record_page.ts b/ui/src/frontend/record_page.ts
index 15b77a2..f11e74a 100644
--- a/ui/src/frontend/record_page.ts
+++ b/ui/src/frontend/record_page.ts
@@ -19,6 +19,7 @@
import {Actions} from '../common/actions';
import {MeminfoCounters, VmstatCounters} from '../common/protos';
import {
+ AdbRecordingTarget,
isAndroidTarget,
isChromeTarget,
isLinuxTarget,
@@ -582,34 +583,57 @@
m('.top-part',
m('.target-and-status',
RecordingPlatformSelection(),
- ConnectedDeviceLabel()),
+ RecordingStatusLabel(),
+ ErrorLabel()),
recordingButtons()),
RecordingNotes());
}
function RecordingPlatformSelection() {
- if (globals.state.serialAndroidDeviceConnected ||
- globals.state.recordingInProgress) {
- return [];
- }
+ if (globals.state.recordingInProgress) return [];
- const onOsChange = (os: TargetOs) => {
- const traceCfg = produce(globals.state.recordConfig, draft => {
- draft.targetOS = os;
- });
- globals.dispatch(Actions.setRecordConfig({config: traceCfg}));
- };
+ const baseTargets = [
+ m('option', {value: 'Q'}, 'Android Q+'),
+ m('option', {value: 'P'}, 'Android P'),
+ m('option', {value: 'O'}, 'Android O-'),
+ m('option', {value: 'C'}, 'Chrome'),
+ m('option', {value: 'L'}, 'Linux desktop')
+ ];
+ const availableAndroidDevices = globals.state.availableDevices;
+ const selected = globals.state.androidDeviceConnected;
+ const choices: m.Children[] =
+ availableAndroidDevices.map(d => m('option', {value: d.serial}, d.name));
+
+ const selectedIndex = selected ? baseTargets.length +
+ availableAndroidDevices.findIndex(d => d.serial === selected.serial) :
+ undefined;
return m(
- 'label',
- 'Target platform:',
- m('select',
- {onchange: m.withAttr('value', onOsChange)},
- m('option', {value: 'Q'}, 'Android Q+'),
- m('option', {value: 'P'}, 'Android P'),
- m('option', {value: 'O'}, 'Android O-'),
- m('option', {value: 'C'}, 'Chrome'),
- m('option', {value: 'L'}, 'Linux desktop')));
+ '.target',
+ m(
+ 'label',
+ 'Target platform:',
+ m('select',
+ {onchange: m.withAttr('value', onTargetChange), selectedIndex},
+ ...baseTargets,
+ ...choices),
+ ),
+ m('.chip',
+ {onclick: addAndroidDevice},
+ m('button', 'Add Device'),
+ m('i.material-icons', 'add')));
+}
+
+// Target can be the targetOS or the android serial
+function onTargetChange(target: string) {
+ const adbDevice =
+ globals.state.availableDevices.find(d => d.serial === target);
+ globals.dispatch(Actions.setAndroidDevice({target: adbDevice}));
+
+ const traceCfg = produce(globals.state.recordConfig, draft => {
+ draft.targetOS = adbDevice ? adbDevice.os as TargetOs : target as TargetOs;
+ });
+ globals.dispatch(Actions.setRecordConfig({config: traceCfg}));
}
function Instructions(cssClass: string) {
@@ -626,7 +650,7 @@
const bufferUsage = globals.bufferUsage ? globals.bufferUsage : 0.0;
// Buffer usage is not available yet on Android.
- if (bufferUsage === 0) return m('label', 'Recording in progress...');
+ if (bufferUsage === 0) return [];
return m(
'label',
@@ -741,16 +765,11 @@
function recordingButtons() {
const state = globals.state;
- const deviceConnected = state.serialAndroidDeviceConnected !== undefined;
+ const realDeviceTarget = state.androidDeviceConnected !== undefined;
+ const recInProgress = state.recordingInProgress;
- const connectAndroidButton =
- m(`button${deviceConnected ? '.selected' : ''}`,
- {onclick: connectAndroidDevice},
- 'Connect Device');
- const disconnectAndroidButton =
- m(`button`, {onclick: disconnectAndroidDevice}, 'Disconnect Device');
const startButton =
- m(`button${state.recordingInProgress ? '.selected' : ''}`,
+ m(`button${recInProgress ? '.selected' : ''}`,
{onclick: onStartRecordingPressed},
'Start Recording');
const showCmdButton =
@@ -763,22 +782,17 @@
},
'Show Command');
const stopButton =
- m(`button${state.recordingInProgress ? '' : '.disabled'}`,
+ m(`button${recInProgress ? '' : '.disabled'}`,
{onclick: () => globals.dispatch(Actions.stopRecording({}))},
'Stop Recording');
- const recInProgress = state.recordingInProgress;
const buttons: m.Children = [];
const targetOs = state.recordConfig.targetOS;
if (isAndroidTarget(targetOs)) {
buttons.push(showCmdButton);
- if (deviceConnected) {
- if (!recInProgress) buttons.push(disconnectAndroidButton);
- buttons.push(startButton);
- } else {
- buttons.push(connectAndroidButton);
- }
+ if (realDeviceTarget) buttons.push(startButton);
+ // TODO(nicomazz): Support stop recording on Android devices.
} else if (isChromeTarget(targetOs) && state.extensionInstalled) {
buttons.push(startButton);
if (recInProgress) buttons.push(stopButton);
@@ -799,12 +813,16 @@
}
}
-function ConnectedDeviceLabel() {
- // TODO(nicomazz): Instead of the serial, show the name of the device. When a
- // device is connected, the "select box" should disappear, until the
- // disconnect button is pressed again.
- const deviceInfo = globals.state.serialAndroidDeviceConnected;
- return deviceInfo ? m('label', `Device connected: ${deviceInfo}`) : [];
+function RecordingStatusLabel() {
+ const recordingStatus = globals.state.recordingStatus;
+ if (!recordingStatus) return [];
+ return m('label', recordingStatus);
+}
+
+function ErrorLabel() {
+ const lastRecordingError = globals.state.lastRecordingError;
+ if (!lastRecordingError) return [];
+ return m('label.error-label', `⚠️ Error: ${lastRecordingError}`);
}
function recordingLog() {
@@ -816,18 +834,37 @@
// The connection must be done in the frontend. After it, the serial ID will
// be inserted in the state, and the worker will be able to connect to the
// correct device.
-async function connectAndroidDevice() {
+async function addAndroidDevice() {
const device = await new AdbOverWebUsb().findDevice();
if (!device.serialNumber) {
console.error('serial number undefined');
return;
}
- globals.dispatch(Actions.setAndroidDevice({serial: device.serialNumber}));
+ // After the user has selected a device with the chrome UI, it will be
+ // available when listing all the available device from WebUSB. Therefore,
+ // we update the list of available devices.
+ await updateAvailableAdbDevices();
+ onTargetChange(device.serialNumber);
}
-async function disconnectAndroidDevice() {
- globals.dispatch(Actions.setAndroidDevice({serial: undefined}));
+export async function updateAvailableAdbDevices() {
+ const devices = await new AdbOverWebUsb().getPairedDevices();
+
+ const availableAdbDevices: AdbRecordingTarget[] = [];
+ devices.forEach(d => {
+ if (d.productName && d.serialNumber) {
+ // TODO(nicomazz): At this stage, we can't know the OS version, so we
+ // assume it is 'Q'. This can create problems with devices with an old
+ // version of perfetto. The os detection should be done after the adb
+ // connection, from adb_record_controller
+ availableAdbDevices.push(
+ {name: d.productName, serial: d.serialNumber, os: 'Q'});
+ }
+ });
+
+ globals.dispatch(Actions.setAvailableDevices({devices: availableAdbDevices}));
+ return availableAdbDevices;
}
function recordMenu(routePage: string) {
