Tweak: more robust device-selection logic. Embedding idb_companion #314

dmitry-zaitsev · 2022-10-24T19:12:35Z

Proposed Changes

There are 4 major changes:

We no longer assume that idb_companion is running and launching it ourselves. If it is already running, we are restarting it (has shown to improve stability).
If there are multiple devices running, we ask the user to pick one explicitly.
For that reason, --platform option is deprecated.
If no devices are running, user can pick one to start up.

Testing

Tested manually.

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt

Leland-Takamine · 2022-10-24T19:37:27Z

Seems like Maestro starts running before the simulator is fully booted

Leland-Takamine · 2022-10-24T19:38:39Z

Should we add this to the top of the Maestro output so it's visible while the flow is running?

Running on iPhone 11 Pro Max - iOS 15.5 - 4A4BB058-96A0-40D4-B8AB-0F1B564341FE

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt

Leland-Takamine · 2022-10-24T19:45:46Z

This is usually what I use to run the Android emulator:

emulator -avd Pixel_4_API_30 -netdelay none -netspeed full

Leland-Takamine · 2022-10-24T19:46:14Z

These prompts should leave the cursor at the end of the line:

Enter a number from the list above:

Leland-Takamine · 2022-10-24T19:46:47Z

After printing this error message we should re-prompt the user for input:

Invalid index

Leland-Takamine · 2022-10-24T19:50:34Z

After launching an emulator, it's takes a while for the Maestro flow to actually start executing. Not a blocker

Leland-Takamine · 2022-10-24T19:58:13Z

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt

+            .filter { it.connected }
+    }
+
+    fun listAvailableDevices(): List<Device> {


If this is used to list devices available to start, it should not include devices listed by Dadb

Tweaked it in a way that it contains a sealed class. Splitting this logic into different methods would do more harm than good, I feel

maestro-cli/src/main/java/maestro/cli/device/PickDeviceView.kt

lochness42 · 2022-10-25T09:20:22Z

maestro-cli/src/main/java/maestro/cli/device/ios/Simctl.kt

+    }
+
+    fun launchSimulator(deviceId: String) {
+        runCommand("xcrun simctl boot $deviceId")


Suggested change

runCommand("xcrun simctl boot $deviceId")

runCommand("idb_companion --boot $deviceId")

idb_companion terminates only after sim is booted up

Example output of the command as it's booting up the sim. We could also parse last line and assert whether the sim got booted up successfully or not.

idb_companion --boot 726166E0-3766-459B-AAEB-971C0E1D326B IDB Companion Built at Aug 12 2022 08:41:50 IDB Companion architecture arm64 Booting 726166E0-3766-459B-AAEB-971C0E1D326B CoreSimulator: Loading from /Library/Developer/PrivateFrameworks/CoreSimulator.framework CoreSimulator: Successfully loaded CoreSimulator: SimDevice has correct path of /Library/Developer AccessibilityPlatformTranslation: Already loaded, skipping AccessibilityPlatformTranslation: AXPTranslationObject has correct path of /System/Library Loaded All Private Frameworks [CoreSimulator, AccessibilityPlatformTranslation] Booting 726166E0-3766-459B-AAEB-971C0E1D326B normally Booting 726166E0-3766-459B-AAEB-971C0E1D326B with verification [simulator_set] boot: called with: [Boot Environment {} | Options []] [simulator_set] disconnectWithTimeout:logger: called with: [30.000000, <FBIDBLogger: 0x6000013a0060>] [726166E0-3766-459B-AAEB-971C0E1D326B] Simulator connections torn down in 0.000134 seconds [simulator_set] disconnectWithTimeout:logger: succeeded [simulator_set] disconnectWithTimeout:logger: called with: [30.000000, <FBIDBLogger: 0x6000013a0060>] [726166E0-3766-459B-AAEB-971C0E1D326B] Simulator connections torn down in 0.000126 seconds [simulator_set] disconnectWithTimeout:logger: succeeded [simulator_set] resolveState: called with: [3] [simulator_set] resolveState: succeeded [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: Booting | Elapsed 0.000000 [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: WaitingOnSystemApp | Elapsed 1.000000 [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status has not changed from 'WaitingOnSystemApp | Elapsed 1.000000' for 1.578630 seconds [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: WaitingOnSystemApp | Elapsed 3.000000 [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: WaitingOnSystemApp | Elapsed 3.000000 [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status has not changed from 'WaitingOnSystemApp | Elapsed 3.000000' for 1.590303 seconds [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: WaitingOnSystemApp | Elapsed 5.000000 [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status has not changed from 'WaitingOnSystemApp | Elapsed 5.000000' for 1.564724 seconds [726166E0-3766-459B-AAEB-971C0E1D326B] Boot Status Changed: Finished | Elapsed 8.000000 [simulator_set] boot: succeeded {"model":"iPhone 13 mini","os_version":"iOS 16.0","udid":"726166E0-3766-459B-AAEB-971C0E1D326B","architecture":"x86_64","type":"Simulator","name":"iPhone 13 mini","state":"Booted"}

Thanks for the suggestion. Though that is valid, we are trying to minimise our dependency on idb_companion and get by with Apple-provided solutions wherever we can.

Sure, I've assumed that if you have to use idb_companion anyway, it's worth using it + you don't have to have your own workaround waiting for the sim device to be booted up.

dmitry-zaitsev · 2022-10-25T09:57:14Z

This is usually what I use to run the Android emulator:
emulator -avd Pixel_4_API_30 -netdelay none -netspeed full

Let's stick to a default option to avoid any discrepancies in behaviour when running against an existing emulator. If someone would want to customize those parameters they could just launch an emulator by themselves.

dmitry-zaitsev · 2022-10-25T09:58:53Z

After printing this error message we should re-prompt the user for input:
Invalid index

We already do that. Or did you mean something else?

dmitry-zaitsev · 2022-10-25T09:59:23Z

After launching an emulator, it's takes a while for the Maestro flow to actually start executing. Not a blocker

I noticed that too. It seems that adb server takes some time to launch?

Leland-Takamine · 2022-10-25T13:03:49Z

This is usually what I use to run the Android emulator:
emulator -avd Pixel_4_API_30 -netdelay none -netspeed full
Let's stick to a default option to avoid any discrepancies in behaviour when running against an existing emulator. If someone would want to customize those parameters they could just launch an emulator by themselves.

I believe this is similar to what android studio does by default

Leland-Takamine · 2022-10-25T13:04:37Z

After printing this error message we should re-prompt the user for input:
Invalid index
We already do that. Or did you mean something else?

I meant re-printing the prompt

dmitry-zaitsev · 2022-10-25T13:05:30Z

After printing this error message we should re-prompt the user for input:
Invalid index
We already do that. Or did you mean something else?
I meant re-printing the prompt

The prompt is quite huge and will erase the warning/error message. I would leave it as is

dmitry-zaitsev · 2022-10-25T13:08:46Z

This is usually what I use to run the Android emulator:
emulator -avd Pixel_4_API_30 -netdelay none -netspeed full
Let's stick to a default option to avoid any discrepancies in behaviour when running against an existing emulator. If someone would want to customize those parameters they could just launch an emulator by themselves.
I believe this is similar to what android studio does by default

Ah, yes you are right: https://developer.android.com/studio/run/emulator-commandline

Made the change

Leland-Takamine · 2022-10-25T13:26:52Z

After printing this error message we should re-prompt the user for input:
Invalid index
We already do that. Or did you mean something else?
I meant re-printing the prompt
The prompt is quite huge and will erase the warning/error message. I would leave it as is

Just the one-line prompt. It's not clear that you should enter a number still after the error. It feels like it's just hanging

dmitry-zaitsev · 2022-10-25T13:41:50Z

After printing this error message we should re-prompt the user for input:
Invalid index
We already do that. Or did you mean something else?
I meant re-printing the prompt
The prompt is quite huge and will erase the warning/error message. I would leave it as is
Just the one-line prompt. It's not clear that you should enter a number still after the error. It feels like it's just hanging

Done

Leland-Takamine · 2022-10-27T18:19:04Z

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt

+
+                val emulatorBinary = File(androidHome, "emulator/emulator")
+                    .takeIf { it.exists() }
+                    ?: File(androidHome, "tools/emulator")


This should be emulator/emulator I believe

See line 30. I use this as a fallback

dmitry-zaitsev added 3 commits October 24, 2022 21:08

Tweak: more robust device-selection logic. Embedding idb_companion

d01e471

Do not assume that Xcode is installed

5a3316f

Allowing to specify device id via argument

9f05a18

Leland-Takamine reviewed Oct 24, 2022

View reviewed changes

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt Outdated Show resolved Hide resolved

Leland-Takamine reviewed Oct 24, 2022

View reviewed changes

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt Outdated Show resolved Hide resolved

Leland-Takamine reviewed Oct 24, 2022

View reviewed changes

maestro-cli/src/main/java/maestro/cli/device/DeviceService.kt Outdated Show resolved Hide resolved

Leland-Takamine reviewed Oct 24, 2022

View reviewed changes

maestro-cli/src/main/java/maestro/cli/device/PickDeviceView.kt Show resolved Hide resolved

lochness42 reviewed Oct 25, 2022

View reviewed changes

Addressed feedback

8115a1a

Apply default emulator fields

1817e54

Printing prompt again in case of an invalid index

953af2b

Leland-Takamine approved these changes Oct 25, 2022

View reviewed changes

dmitry-zaitsev merged commit 918d343 into main Oct 26, 2022

dmitry-zaitsev deleted the tweak/pick-device branch October 26, 2022 09:21

Leland-Takamine reviewed Oct 27, 2022

View reviewed changes

tokou mentioned this pull request Aug 27, 2024

Add back CLI --platform option #1954

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak: more robust device-selection logic. Embedding idb_companion #314

Tweak: more robust device-selection logic. Embedding idb_companion #314

dmitry-zaitsev commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine Oct 24, 2022

dmitry-zaitsev Oct 25, 2022

lochness42 Oct 25, 2022

lochness42 Oct 25, 2022

dmitry-zaitsev Oct 25, 2022

lochness42 Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Leland-Takamine Oct 27, 2022

dmitry-zaitsev Oct 27, 2022

	runCommand("xcrun simctl boot $deviceId")
	runCommand("idb_companion --boot $deviceId")

Tweak: more robust device-selection logic. Embedding idb_companion #314

Tweak: more robust device-selection logic. Embedding idb_companion #314

Conversation

dmitry-zaitsev commented Oct 24, 2022

Proposed Changes

Testing

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Leland-Takamine commented Oct 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Leland-Takamine commented Oct 25, 2022

dmitry-zaitsev commented Oct 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment