-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor emulator start and stop functions for clarity and efficiency #22861
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can commit the suggested changes from lintrunner.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…to Cjian/check_emulator_running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can commit the suggested changes from lintrunner.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can commit the suggested changes from lintrunner.
tools/python/util/android/android.py
Outdated
# Step 2: Check each running emulator's AVD name | ||
for emulator in running_emulators: | ||
try: | ||
avd_info = subprocess.check_output(["adb", "-s", emulator, "emu", "avd", "name"], text=True).strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the expected output of this command? I tried with both the phones connected to the mac mini but got no output so wondering if it differs for a real device vs an emulator. #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be only for emulator. Generated by ChatGPT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got
ort-andriod
OK
when I run this command locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can commit the suggested changes from lintrunner.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can commit the suggested changes from lintrunner.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
tools/python/util/android/android.py
Outdated
@@ -118,10 +126,12 @@ def start_emulator( | |||
"America/Los_Angeles", | |||
"-no-snapstorage", | |||
"-no-audio", | |||
"-no-snapshot", # No bluetooth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this parameter was about fast startup from previous run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is correct. This is not needed for now.
tools/python/util/android/android.py
Outdated
return emulator_process | ||
|
||
|
||
def stop_emulator(emulator_proc_or_pid: typing.Union[subprocess.Popen, int]): | ||
def is_emulator_running_by_avd(avd_name: str) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we call these something like check_emulator_running_using_avd_name
/check_emulator_running_using_process
/check_emulator_running_using_pid
?
The 'by' sounds like we could be checked how it was started rather than if it's currently running.
#Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also update the versions in onnxruntime-extensions and onnxruntime-genai once the changes are checked in.
Co-authored-by: Scott McKay <[email protected]>
Co-authored-by: Scott McKay <[email protected]>
Description
This pull request introduces several enhancements and new functionalities to the
tools/python/util/android/android.py
file, focusing on improving the management of Android emulators. The most important changes include adding a timeout parameter to thestart_emulator
function, adding checks to prevent multiple emulators from running simultaneously, and introducing new utility functions to manage emulator processes more effectively.Enhancements to
start_emulator
function:timeout_minutes
parameter to thestart_emulator
function to make the startup timeout configurable. [1] [2]-verbose
for better control and debugging.New utility functions for managing emulator processes:
check_emulator_running_using_avd_name
,check_emulator_running_using_process
, andcheck_emulator_running_using_pid
to check if an emulator is running based on AVD name, process instance, or PID, respectively.stop_emulator_by_proc
andstop_emulator_by_pid
functions to stop the emulator process using asubprocess.Popen
instance or PID, with a configurable timeout.stop_emulator
function to use the new utility functions for stopping the emulator process.These changes enhance the robustness and flexibility of the emulator management utilities, making it easier to handle different scenarios in CI environments and development workflows.
Motivation and Context