google / python-adb

Python ADB + Fastboot implementation
Apache License 2.0
1.8k stars 357 forks source link

New package `adb-shell` #167

Open JeffLIrion opened 5 years ago

JeffLIrion commented 5 years ago

For anyone interested, I started a new repo/package based on this project: https://github.com/JeffLIrion/adb_shell

This package implements ADB shell functionality over TCP. You can install it via

pip install adb-shell

Highlights

Pull requests from this repo included in adb_shell

Contributing

Contributions are welcome and will be reviewed and released on pypi!!! If you'd like to contribute, please

embray commented 5 years ago

@JeffLIrion Thanks, that's good to know. It's a shame Google seems to have abandoned support for this (yes, yes, I know they're hard at work killing android but in the meantime some of us still develop for it :)

JeffLIrion commented 5 years ago

@embray what I'd most like to fix is the duplicate CLSE issue (https://github.com/google/python-adb/pull/151). I recall getting feedback that your fix didn't fully solve it. I don't have a newer ADB device, so I'm not able to reproduce the error. But with the adb-shell package, if you set the log level to debug it will log all sent and received data, making it possible to (hopefully!) fix the bug.

fahhem commented 5 years ago

Hey :)

Thank you @JeffLIrion! I really appreciate the improvements you've made, and I was going to just point everyone to your repo, but I think the better option is to add you to this one. I hope you accept and we can make the official, non-official google python-adb repo much better.

Honestly, the only benefit to this approach is it says "google" in the github URL and the PyPI project name is just "adb". Let me know what your PyPI credentials are and I'll add you there too

JeffLIrion commented 5 years ago

Prior to creating the adb-shell package, I dug into this code and tried to figure out how it works so that I could fix some bugs. Honestly, I found the code to be very confusing, in particular due to the abundance of classes and the widespread use of class methods. I created a branch that has a bunch of documentation and linting fixes, you can view that here: https://github.com/JeffLIrion/python-adb/tree/documentation

If you want, I'll squash those commits and submit a pull request. The API is the same and there should be no breaking changes.

But ultimately, I decided that it would be easier to extract the functionality that I wanted and create a new repo. You’re more than welcome to copy anything you'd like from adb-shell. The tests could be especially useful: https://github.com/JeffLIrion/adb_shell/blob/master/tests/test_adb_device.py

Also, I'm by no means an expert in ADB or Android. I don't even know what fastboot or filesync are.

fahhem commented 5 years ago

I'd be very happy to accept a PR like that.

Is the the adb_shell API incompatible in a meaningful way? python-adb calls things AdbCommands, while you have an AdbDevice, but it seems there's a 1:1 mapping?

I'm not going to stand behind the naming/style of this repo, mostly because it was a straight copy out of the google3 repo that, at the time, had non-PEP8-compatible style requirements. It would be reasonably to me to have a 2.0 release that's a different API that's easy to switch to (since it's 1:1), but PEP8 compatible.

JeffLIrion commented 5 years ago

I'll prepare that pull request that I mentioned.

The difference between the two classes is most evident in how ADB commands are sent. From memory, the processes are as follows.

AdbDevice

  1. Open a connection using the AdbDevice._open method.
  2. Create msg = AdbMessage(...) using the command's data.
    • AdbMessage is a small helper class whose only real functionalities are the pack method and the checksum property.
  3. Using the AdbDevice._send method, send msg.pack() and msg.data.
  4. Read responses.

AdbCommands

  1. Create msg = ADB message(...), passing along the USB/TCP handle attribute from the AdbCommands instance.
  2. msg opens a connection by creating a new instance of the AdbConnection class, again passing along the USB/TCP handle.
  3. A Send method is called. I forget whether this is a method of the AdbMessage class or the AdbConnection class, but I'm pretty sure both classes are involved. And this is sent using the USB/TCP handle from the AdbCommands class.
  4. Responses are read, either by the AdbMessage instance or the AdbConnection instance.

I don't know what the reasoning is behind all the classes and class methods in this package -- to be specific, I don't know whether it's necessary for some functionalities or just a design choice -- so I didn't want to change anything. But if I were to restructure the code, I'd do it as I did in the adb-shell package.

I've got generated documentation for both packages: