Feat: List android devices #1765
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
packages/cli-platform-android/src/commands/runAndroid/listAndroidDevices.ts
Outdated
Show resolved
Hide resolved
…oidDevices.ts Co-authored-by: Szymon Rybczak <szymon.rybczak@gmail.com>
thymikee
reviewed
Dec 6, 2022
thymikee
reviewed
Dec 6, 2022
|
|
||
| const device = await listAndroidDevices(); | ||
| if (!device) { | ||
| return logger.error( |
There was a problem hiding this comment.
this will exit with code 0. I think we want to throw?
thymikee
reviewed
Dec 6, 2022
Comment on lines
99
to
105
| } else { | ||
| logger.error( | ||
| `Failed to launch emulator. Reason: ${chalk.dim(result.error || '')}.`, | ||
| ); | ||
| logger.warn( | ||
| 'Please launch an emulator manually or connect a device. Otherwise app may fail to launch.', | ||
| ); |
There was a problem hiding this comment.
throw so the exit code is non-zero
thymikee
reviewed
Dec 6, 2022
thymikee
reviewed
Dec 6, 2022
packages/cli-platform-android/src/commands/runAndroid/listAndroidDevices.ts
Outdated
Show resolved
Hide resolved
thymikee
reviewed
Dec 6, 2022
packages/cli-platform-android/src/commands/runAndroid/listAndroidDevices.ts
Show resolved
Hide resolved
thymikee
reviewed
Dec 6, 2022
Comment on lines
+49
to
+52
| return buffer | ||
| .toString() | ||
| .replace(/\[ro\.product\.model\]:\s*\[(.*)\]/, '$1') | ||
| .trim(); |
There was a problem hiding this comment.
please extract transforming regex to a separate fn and write unit tests :)
thymikee
reviewed
Dec 6, 2022
thymikee
reviewed
Dec 6, 2022
|
|
||
| const emulatorCommand = process.env.ANDROID_HOME | ||
| ? `${process.env.ANDROID_HOME}/emulator/emulator` | ||
| : 'emulator'; | ||
|
|
||
| const getEmulators = () => { | ||
| export const getEmulators = () => { |
There was a problem hiding this comment.
I'd consider moving it to separate file to avoid unexpected circular deps and make it clearer what's shared (although one could say export says it all, and they would be right)
Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Similar to #1676 - list available devices and let user choose from one.
Fetch available devices (
adb device) and emulators (emulator -list-avds) and pass selection torun-androidcommand.If user select either physical device or booted emulator we simply call
runOnSpecificDevicewith selected deviceId.Gets a bit trickier when user selects not-booted emulator. We need to launch it at known port bc we need it to pass all further avd commands (
avd -s emulator-${port} [command]).Test Plan:
Clone the fork and run
run-androidscript with proper flag: