budtmo / docker-android

Android in docker solution with noVNC supported and video recording
Other
8.91k stars 1.21k forks source link

Fixing 'System UI not responding' popup message appearing on newest android versions #323

Closed LerryAlexander closed 1 year ago

LerryAlexander commented 2 years ago

Purpose of changes

On some emulators devices with android version such as 9.0, 10.0, 11.0 and 12.0, when docker container is starting up and the emulator is getting started, a popups error message is shown up that says: System UI not responding, causing that the emulator can’t be continued to use. This problem can be solved by increasing the ram size of the emulator when it gets started. However, not all the emulators are having this problem, so this solution address two things: After booting, the script checks 1) if the emulator shows popup message "System UI not responding": it closes the popup by killing the process, increase the ram size, then it checks that no new popup windows is shown up again and it waits until the screen is fully clean by checking the Launcher process which indicates that emulator is displaying the home screen and is ready.
2) if the emulator doesn't show popup message System UI not responding, it waits until the screen is fully clean by checking the Launcher process which indicates that emulator is displaying the home screen and is ready.

Currently, there are a list of open issues that are meant to be close with this solution: system UI is not responding when starts the container system UI isn't responding after boot on VM

Types of changes

Put an x in the boxes that apply

How has this been tested?

This was tested on a Linux VM by spinning up different combinations of android versions and device models as following:

Samsung Galaxy S7 Edge - Android 9.0 Before

Galaxy S7 Edge

After

Galaxy S7 Edge

Nexus 5- Android 9.0 Before

performance is not quite good

After

Never show this again

Samsung Galaxy S10- Android 10.0 Before

Emulator is running using nested virtualization

After

SAMSUNG

Samsung Galaxy S8- Android 12.0 Before

performance is not quite good

After

Never show this again
codecov-commenter commented 2 years ago

Codecov Report

Merging #323 (fcb6a21) into master (d436eb3) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #323   +/-   ##
=======================================
  Coverage   70.39%   70.39%           
=======================================
  Files           2        2           
  Lines         152      152           
=======================================
  Hits          107      107           
  Misses         45       45           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

LerryAlexander commented 2 years ago

Hi @LerryAlexander thank you for the PR.

hm... I think we can just add environment variable with DATAPARTITION on the fly or directly on Dockerfile instead of changing on hardware-qemu.ini. could you maybe check that @LerryAlexander ?

Hello @budtmo thank you for your feedback. Good, I just tested by changing disk.dataPartition.size = 4000m on the android_emulator/config.ini file and it worked.

So, I see two posible solutions: 1) Change the current value for disk.dataPartition.size = 550m to a higher default value on DATAPARTITION environment variable value at line: dp_size = os.getenv('DATAPARTITION', '4000m') inside the function def run() on the file /root/src/app.py 2) Change the value disk.dataPartition.size ONLY if the emulator shows up the popup message System UI not responding on the file android_emulator/config.ini by using the intial script I proposed but modifying config.ini instead of hardware-qemu.ini

Let me know your thoughts

LerryAlexander commented 2 years ago

Hi @budtmo.

I tested it with the env variable DATAPARTITION=4000m and it didn't work.

I ran this command:

docker run --privileged -d -p 6082:6080 -p 4723:4723 -p 5554:5554 -p 5555:5555 -e DEVICE="Nexus 5" -e DATAPARTITION="4000m" --name android9.0-container budtmo/docker-android-x86-9.0

and the emulator shows up like this:

image

and also I validated inside the android container that the disk.dataPartition.size was changed indeed, but the popup continuer to appear on the emulator: image image

I also noticed that the only way to avoid the popup message to keep appearing it's just by closing the popup manually, however if the disk.dataPartition.size is not changed to a higher value and you close the popup, this continue to appearing a few seconds later, so closing the popup wouldn't solve the issue either.

So, in conclusion, we certainly can use the DATAPARTITION env variable to increase the disk.dataPartition.size and solve the issue, but the popup error message will continue to appear after the android boot is completed, it's just that the popup needs to be closed, that is why in the PR I'm proposing to close this window when the phone is getting started for the first time (during boot), this approach is worth to implement when you are using docker android for automated tests as well (as you mentioned you wanted to keep that way when emulator is used for manual testing).

So, I think DATAPARTITION is fine, but we might also add an extra validation by closing that window somehow in the process.

Let me know what you think.

If you want to try you can reproduce the issue with this command:

  1. docker run --privileged -d -p 6082:6080 -p 4723:4723 -p 5554:5554 -p 5555:5555 -e DEVICE="Nexus 5" -e DATAPARTITION="4000m" --name android9.0-container budtmo/docker-android-x86-9.0
LerryAlexander commented 2 years ago

Hi @LerryAlexander ,

sorry for waiting so long. imho, I think it is better to keep having DATAPARTITION env variable, and then add one check in utils.sh to close the pop up if it is appear (but it will close it only once). we should not change datapartition to higher value by default just to avoid the pop up windows. I meant, maybe there is a case where user want to have lower value on datapartition for specific reasons / to test the performance or something.

if we want, we can add additional note in the readme that default value might cause the pop up windows so the user is aware about that.

what do you think?

Hi @budtmo ,

Great! sounds good and makes so much sense to me. I'll update the PR based on your suggestion and ping you back for your final revision!

Thank you so much!

budtmo commented 1 year ago

Hi @LerryAlexander ,

btw, thanks again for the great work that you did! but I am still waiting for your changes on this PR. do you want me to continue it? I can merge your PR and then I can continue the work form there. what do you think?

LerryAlexander commented 1 year ago

Hi @LerryAlexander ,

btw, thanks again for the great work that you did! but I am still waiting for your changes on this PR. do you want me to continue it? I can merge your PR and then I can continue the work form there. what do you think?

Hi @budtmo! Sorry I didn't come back sooner to this PR after we talked, I remember the only missing thing was to remove the call to the change_emulator_ram_size method since we wanted to let users define this value through the environment variable, so if you want I can update the PR with that missing change and then we can merge this PR, what do you think?

budtmo commented 1 year ago

Hi @LerryAlexander ,

sure, lets do it. dont forget to update your branch with master branch

LerryAlexander commented 1 year ago

Hi @LerryAlexander ,

sure, lets do it. dont forget to update your branch with master branch

Hi @budtmo I already updated my branch against master and applied my changes, I'm waiting for 1 check continuous-integration/travis-ci. Let me know if you prefer to merge the PR or I can do it when this check is passed and you approve it. Thank you!