Closed Malinskiy closed 6 years ago
Very interesting. Some questions:
Would it be possible to remove the sh and grep calls and do the lookup in Java? Seems like that may be less prone to break in the future. So it would just call dumpsys directly.
If the device remains disconnected, does it spawn a new activity every 30s? What does that do to activity history? Might be that our flags prevent that from being an issue, but can you explain what happens?
Does the DUMP permission get granted automatically on install or does it need to be granted later?
Hey @sorccu
It's possible to do the text parsing in Java without the grep, but getting the actual state of the usb driver is tricky.UsbDeviceManager exposes the state through dumpsys shell command. Calling it directly will require you to unhide API class or call by reflection and I believe that dumpsys shell command is more stable in that regard.
If the device is disconnected then the Activity is recreated every 30s even if it's on screen. But since you use customized flags in the Manifest it doesn't actually spawn multiple activities, it just recreates the current one again and again. I didn't see any visual impact probably because the activity is lightweight. Maybe I can check if the Activity is on screen already.
No, you need to grant the permission through adb because this is a system permission:
adb -d shell pm grant jp.co.cyberagent.stf android.permission.DUMP
Yeah but could you change the shell command to just call /system/bin/dumpsys usb -a
and do the rest in Java? I don't want to depend on grep being available. It could be an issue on some Android forks.
@sorccu done. also I've noticed that sometimes you first start the service and later want to grant permission, so I've moved the permissions check into the loop. This way you don't need to restart the service after adb pm grant
What do you think about adding https://developer.android.com/reference/android/view/WindowManager.LayoutParams.html#FLAG_SHOW_WHEN_LOCKED to the IdentityActivity? Right now if the device gets locked then this trick will not work
Sure, sounds good.
@sorccu will you add the adb grant command to the service.js? I think we should add env variable to enable this in case people want to.
I've finished here, feel free to ask anything you want to change. Thanks
Could you also let me know how extensively you've tested this so far? Mainly interested in whether there are differences in the output between 2.3 to latest that would prevent the check from working.
@thinkhy what do you think about this feature?
@sorccu I'll merge this patch into my branch later and test it on devices from Android 4.0 ~ 8.1.
@Malinskiy As I experienced similar problem when maintain hundreds of devices and identify the disconnected devices daily, I'd like to throw my 2 cents.
@Malinskiy Can you show me how to enable the feature? I've merged your PR and built a new APK, and tried out the following steps:
adb install new.apk
enable self-starting permission in settings
adb shell pm grant jp.co.cyberagent.stf android.permission.DUMP
adb shell am start -n jp.co.cyberagent.stf/.IdentityActivity
adb shell am startservice -n jp.co.cyberagent.stf/.Service
press HOME
disconnect from the USB
The identifyActivity can NOT be displayed.
I've tried above steps on several devices, still can't make it work.
@thinkhy The problem is the code of Service is not responding to unknown actions, so you have to start it with the ACTION_START from app's package as follows (otherwise the onStartCommand
method just logs Unknown action
and nothing happens):
$ adb shell am startservice -a jp.co.cyberagent.stf.ACTION_START -n jp.co.cyberagent.stf/.Service
So the full action list is:
$ adb install app.apk
$ adb shell pm grant jp.co.cyberagent.stf android.permission.DUMP
$ adb shell am startservice -a jp.co.cyberagent.stf.ACTION_START -n jp.co.cyberagent.stf/.Service
disconnect from the USB and go to some screen or lock the phone
wait around 30-40 seconds
see the IdentifyActivity
One thing I noticed is that devices with API 27 are not coming back from the doze mode. We need to update the compileSdkVersion for this.
@Malinskiy Thanks for your hints. I tested it on some Android 7.0/8.0 devices, it works very well and the identify activity is displayed with 30 seconds(around 5-28 seconds). I will continue to test it on some low level devices/Roms.
@thinkhy, @sorccu any updates? thanks
@Malinskiy This feature can easy maintenance, but when I test it on Android 4.0 ~ 8.1, it doesn't work on Android 4.0/5.0/6.0.
@thinkhy can you please provide the output of /system/bin/dumpsys usb
on the devices where this doesn't work?
@Malinskiy The output of /system/bin/dumpsys usb
is OK, seems root cause is that on some devices activity can't be started in background service.
startActivity(new Intent(getApplication(), IdentityActivity.class));
@thinkhy please check with the latest fix I did, locally I got rid of the problem
@Malinskiy it works now. I will test ths fix on more devices. If possible, could you explain how to fix this problem and start the identify activity in background service? Thanks very much.
To start activity from non-activity context we need NEW_TASK flag. https://developer.android.com/reference/android/content/Context#startActivity(android.content.Intent) It seems to be added in the last commit.
@koral-- Got it, thanks
From Google's doc:
Note that if this method is being called from outside of an Activity Context, then the Intent must include the Intent.FLAG_ACTIVITY_NEW_TASK launch flag. This is because, without being started from an existing Activity, there is no existing task in which to place the new activity and thus it needs to be placed in its own separate task.
@Malinskiy I've tested the fix on Android 4.0~8.1, which works well on the test devices. @sorccu I think this feature can be accepted, it's really helpful to identify disconnected devices in lab.
Are there any other intents we should fix?
Doesn't find any other code to start activity in Service.java.
@Malinskiy you mentioned this earlier:
One thing I noticed is that devices with API 27 are not coming back from the doze mode. We need to update the compileSdkVersion for this.
Is this still an issue, and if so, which version is needed?
Also one thing I'm worried about is that if a user uses STF with their personal device, after they unplug it it will at some point display the activity, which is OK. But then they essentially have 30 seconds to quit STFService or it keeps respawning the activity. For example, let's say that I notice by phone is displaying the red screen, but I notice it a bit late (say, 20 seconds after it pops up). Now I only have 10 seconds left to operate the phone before the activity takes over again, which is pretty annoying. We should either add a button to the activity to quit STFService easily, or perhaps the current state of the monitor should be in an instance variable. That way we could launch the activity only when the state changes, which would solve the issue too.
@Malinskiy @thinkhy As the following steps: $ adb install app.apk $ adb shell pm grant jp.co.cyberagent.stf android.permission.DUMP $ adb shell am startservice -a jp.co.cyberagent.stf.ACTION_START -n jp.co.cyberagent.stf/.Service disconnect from the USB and go to some screen or lock the phone wait around 30-40 seconds
Disconnect the USB connection by closing the USB switch,It can't work.I test it on Android 4.4 and 7.0
Maybe you didn’t actually install the latest app? Try with -r
or uninstall the previous version first.
Current commit can only determine if USB cable is disconnected but not status of DEBUG mode. So I made small modification to work around this problem.
BufferedReader adbdStateReader = new BufferedReader(new InputStreamReader(process.getInputStream()));
for (String line = adbdStateReader.readLine(); line != null; line = adbdStateReader.readLine()) {
if (line.contains("Current Functions:")) {
currentAdbState = line.split(":").length == 2 ? line.split(":")[1] : "";
}
else if (line.contains("Kernel state:")) {
currentUsbState = line.split(":").length == 2 ? line.split(":")[1] : "";
}
}
if (!currentUsbState.equals(lastUsbState)) {
Log.d(TAG, "Kernel state changed to" + currentUsbState);
lastUsbState = currentUsbState;
}
if (!currentAdbState.equals(lastAdbState)) {
Log.d(TAG, "adb state changed to" + currentAdbState);
lastAdbState = currentAdbState;
}
boolean disconnected = lastUsbState.contains("DISCONNECTED");
boolean adbEnabled = currentAdbState.contains("adb");
if (disconnected || !adbEnabled) {
// if (connected) {
Log.d(TAG, "Start activity for STFService");
// startActivity(new Intent(getApplication(), IdentityActivity.class));
// startSetupView();
getApplication().startActivity(
new IdentityActivity.IntentBuilder().build(getApplication()));
}
@thinkhy done
@Malinskiy Thanks. I just updated latest stfservice.apk on 100+ devices, found a bug on Sony E6683(Anroid 6.0.1). The output of 'dumpsys usb -a' on this device is like below:
USB Manager State:
USB Device State:
mCurrentFunctions: mtp,adb
mCurrentFunctionsApplied: true
mConnected: true
mConfigured: true
mUsbDataUnlocked: true
mCurrentAccessory: null
Kernel state: CONFIGURED
Kernel function list: mtp,ffs
So we need a regex pattern to search string line including adb status:
mCurrentFunctions: xx
Current Functions: xxx
I think you can just add another else if. It's unlikely both "mCurrentFunctions" and "Current Functions" will be present. No need for a regex.
@thinkhy added parsing of "mCurrentFunctions"
@sorccu Agreed. @Malinskiy this fix is OK, thanks.
This is now in STF master :) The permission is not automatically granted yet. Perhaps we can add it later.
If android.permission.DUMP is granted to the package (e.g. adb -d shell pm grant jp.co.cyberagent.stf android.permission.DUMP), then STFService will show IdentifyActivity when device is disconnected from the USB. This allows easier maintenance of devices by quickly identifying the faulty ones.