Termux (Android support), usb device from sys device (file descriptor), Issue #285 #287
Conversation
|
Hmm, would there be a need for error handling for possible older libusb1 versions? (Where the interface may not exist?) |
| See: https://wiki.termux.com/wiki/Termux-usb | ||
|
|
||
| Call script in Termux like this: | ||
| termux-usb -r -e ./tools/termux_device_info.py /dev/bus/usb/001/002 |
There was a problem hiding this comment.
What about making this a generic device_info.py script that supports both -d [vendor]:[product] and --usb-fd [fd] modes. That would make it generally useful and would serve as an example of how other apps could provide similar command line arguments. argparse would make this pretty easy to do.
There was a problem hiding this comment.
Will unfortuately not work as the device path is part of the termux-api command. My script is called as callback with the open file descriptor as argument. After my script finishes, the termux-api will close the file descriptor.
You can imagine this like python with or JavaScript promises/callbacks.
So my script will have nothing to do with the actual device path.
You could make the script a wrapper around the termux-api (termux-usb) command and do some magic with argument handling to also work as the callback but not sure if this is worth the effort. (in my use case)
But I'm open for suggestions, just not sure how I would implement your idea. (Other than creating a second script with argparse and os.system calls, which is more overhead than simply changing the device path string in the shell?)
There was a problem hiding this comment.
Right, it was more mean to make the script generally useful outside of the termux context too.
The usage would look like:
$ device_info.py [-d [vendor]:[product]] [--usb-fd [fd]]so then it could be used on termux (non-root) like this:
$ termux-usb -e 'device_info.py --usb-fd' -r /dev/bus/usb/001/002
$ # which runs: device_info.py --usb-fd 7and with termux (root), or on other systems as:
$ device_info.py -d 1d6b:0002
$ device_info.py -D /dev/bus/usb/001/001
$ device_info.py -s 1:1It is actually possible to support -D under termux (non-root): see the open_with_termux_usb function in https://gist.github.com/normanr/9816a5db24dda0ce87a17da342a9684c.
(I've changed my mind about this. The extra complexity of using file objects doesn't provide any benefit).
I'd actually recommend that the new API should rather be more like the builtin open function that takes a path-like object or an integer file descriptor (like the gist's open function). The function doesn't even need to differentiate the type of the passed argument and can just call open(file, mode='r+b', buffering=0) and use f.fileno() to get the file descriptor to create the device handle (note that the file object must be referenced so that's not closed early).
termux-packages could then provide a version of pyusb appropriately patched to convert the path-like object into a file descriptor using open_with_termux_usb.
Typical usage would look like:
dev = usb.core.open('/dev/bus/usb/001/001')or if you have a file descriptor:
dev = usb.core.open(7)(termux/termux-api#349 would basically do this (and more) inside of libusb, but I don't think it'll be fixed any time soon).
There was a problem hiding this comment.
Wow. That's interesting and looks promising.
For me, it's somewhat in the past but I remember looking at the socket recv/send functions in python but I was not familiar with them, so I was hesitating. (I was also often looking at other code in C that used this successfully but reimplementing in Python took much effort ...)
I was also considering writing my own Java Termux extensions. (the code is on GitHub so it might have worked but that's another rabbithole)
Currently, I'm still hoping for updates in termux-api and/or pyusb to make my life easier ...
I will have to test your code :-D
There was a problem hiding this comment.
The improvements to the socket library are new in Python 3.3 (before that you had to use libraries like PyXAPI or python-passfd). My code currently uses device_from_fd from this pull request (and ideally it would just be incorporated into this pull request).
I think it would go good to support this without having any termux specifics in the main pyusb branch. pyusb could read the command line for an "external open function" from an environment variable (PYUSB_OPEN?). The command would be passed two additional arguments: 1) the command to receive the file descriptor, and 2) the name of the file that pyusb wants to open. Then termux could just set PYUSB_OPEN='termux-usb -r -e'. (I've updated the gist to do it this way).
There was a problem hiding this comment.
Termux-packages might work. I'm just not sure if a termux-packages patch might be too hidden? It also might not work with virtual envs depending on what it patches.
I think it would be simpler and more verbose to use a python package that just extends pyusb with the wanted functionality? With version pinning of pyusb there should be no way that something breaks unexpectedly in updates.
There was a problem hiding this comment.
I'm not sure if the fd and path functions need to be in the same package, mostly because wrap_fd is a pretty core function, and there's no other place to implement it but in usb.core.
Good point about virtual envs, that is a good idea to not try make a termux specific package. Also termux-packages prefers to not package packages available via language specific ecosystems (eg: my request to package more-itertools).
I'd like to avoid having to alter existing code to get it to work with termux - which is why I thought that adding generic support to pyusb for open helpers configured via the environment variable might be acceptable. I also briefly tried proot to bind /dev/bus/usb/ to a temporary directory with a symlink to /proc/self/fd/7, but Android's selinux policy didn't like that.
If adding external opener (or just path) support is too hacky for pyusb (it probably is), then just adding the plain fd function would be extremely helpful. It would be the responsibility of the main script (or other non-pyusb library) to handle fd or path arguments on the command line (which I think is why I suggested making the device_info.py script generic, so that it could show how to support -d vendor:product, -D device, and --usb-fd fd as command line arguments with the library changes as were initially proposed).
There was a problem hiding this comment.
example device_info.py: https://gist.github.com/normanr/93c89ab4f1ea9b6c450dae2528eeef25
There was a problem hiding this comment.
Hmm. An external or opener module?
Too hacky probably not as the whole PyUSB is a wrapper around external USB libraries. It just has to be clear what is does and why it is necessary, I guess?
Good point about fd. But I was thinking more about your snippets with recv/send for fd. Or your example usb.core.open function that can be in the extra module.
There was a problem hiding this comment.
Actually I have a better suggestion for the environment variable: instead of it defining a command to use as an external opener it would define/redefine the function used to open the device (similar to how PYTHONBREAKPOINT changes the behavior of breakpoint() in PEP 553).
That way usb.core could have a very simple open_device function that works for most use cases and it can very easily be overridden to support environments like termux. The function would take a str (the device path) and return usb.core.Device. The default implementation would just return wrap_fd(os.open(device_path, is.O_RDRW)).
Then the termux support can exist in another package that is not a core part of pyusb and is easy to install and enable without having to change existing scripts (although they do need to use the open_device function). I wonder if there's a need to support existing scripts that only use find.
At that point it might be a smaller change to pyusb to make the list of backends configurable with an environment variable instead. Then termux support could be added as a pyusb backend and it would support find and all existing scripts that use pyusb without any changes required to use a new open_device function. I like this option the most because it's the lease invasive and most powerful (and basically what termux/termux-api#349 would do, except implemented as a pyusb backend instead of inside libusb1)
| @@ -634,6 +647,16 @@ def __init__(self, dev): | |||
| self.devid = dev.devid | |||
| _check(_lib.libusb_open(self.devid, byref(self.handle))) | |||
|
|
|||
| class _FileDescriptorDeviceHandle(object): | |||
| def __init__(self, fd, ctx): | |||
There was a problem hiding this comment.
nit: change the order of arguments to ctx, fd to match the underlying function
There was a problem hiding this comment.
I find it rather interesting and like having others for feedback. But I'm short on time, so changes will not happen for at least two weeks, maybe more. And you might not be able to change code or suggest actual changes?
As you are deep into the whole material and I was thinking of renaming my pull branch because I started with master ... maybe you just open another Pull request and reference mine, while I will close but keep the history?
https://stackoverflow.com/a/48480353/9360161
Update: I can also add you to my PyUSB fork if you want.
There was a problem hiding this comment.
It looks like the primary location for getting feedback is the mailing list. It doesn't look it has any mention of termux in the archives..
Nothing is urgent. For now I have something working with just using a local branch with some changes. I don't think I'll have any time in the next week or so to poke at it again.
There was a problem hiding this comment.
Ok.
I'm in the same situation as I also have a collection of scripts to patch/extend it for my use-cases...
|
|
||
| # create device | ||
| device = Device(dev, backend) | ||
| device._ctx.handle = dev |
There was a problem hiding this comment.
I find this reaching up into the _ResourceManager to set handle quiet ugly. Is it just to prevent passing a _FileDescriptorDeviceHandle to the _DeviceHandle constructor?. As an alternative what about adding this to the top of the open_device function:
if isinstance(dev, _FileDescriptorDeviceHandle):
return dev
There was a problem hiding this comment.
I find it ugly, too, because it uses the private members. But it is necessary to work correctly (not sure why because I tested it long ago).
I simply tried to replicate the same setup steps but also wanted to correctly separate what has to happen where, the interfaces, the wrapper code etc.
As for the _DeviceHandle, I'm not sure. Again, too long ago but I might have missed it.
And I wanted it self-contained because as I wrote this, this was mostly intended for Termux. Other OS might not have to go the way of using the fd and can just use the path?
| @@ -790,6 +813,10 @@ def open_device(self, dev): | |||
| def close_device(self, dev_handle): | |||
| self.lib.libusb_close(dev_handle.handle) | |||
|
|
|||
| @methodtrace(_logger) | |||
| def get_device_from_fd(self, fd): | |||
There was a problem hiding this comment.
re naming: I wonder if the IBackend function should actually be called get_device_for_fd or wrap_fd, or possibly even wrap_sys_device (and return a SysDeviceHandle or SystemDeviceHandle).
| See: https://wiki.termux.com/wiki/Termux-usb | ||
|
|
||
| Call script in Termux like this: | ||
| termux-usb -r -e ./tools/termux_device_info.py /dev/bus/usb/001/002 |
There was a problem hiding this comment.
btw, I think that device_from_path and the supporting functions shouldn't go into usb.core, but rather a new module instead. I can't think of any good names though: usb.cli, usb.helper, usb.path?
Typical usage could just look like this:
dev = usb.helper.device_from_path('/dev/bus/usb/001/001')and then the usb.core.device_from_fd API would become a lower-level API that device_from_path uses.
|
As mentioned in #285, I am not able to figure out a way to run the demo script under Android (rooted or unrooted) with real HW. Plain libusb/pyusb work well with the Android 7.1 box with root or without root. Of course I can not do much without root because of permission issues. |
|
All in all, it seems to me for this pull request to work, "termux-usb -l" should work and it seems to me that means the right permission and existence of "/dev/bus/usb" which does not seem to be the case for my Android devices (non rooted). And this bridge seems to rely on a libusb fork. |
|
The bridge requires this libusb fork: https://github.com/green-green-avk/libusb/tree/v1.0.23-android-libusbmanager (from: termux/termux-api#349) If you can't get "termux-usb -l" to show something on a non-rooted device, then it doesn't what other code you use, it won't work. Once you can get "termux-usb -l" to show something then termux_device_info.py should work with the example command line (replace the usb device path with the path shown by "termux-usb -l"). Note that those paths will not be visible from the termux shell, they only make sense for the server that the termux-usb tool connects to. You could also try AnotherTerm, usb should "just work" there as far as I understand, ref: https://green-green-avk.github.io/AnotherTerm-docs/installing-libusb-for-nonrooted-android.html |
Somehow I could not get this AnotherTerm to work. Guess I need to try a bit more. It seems to be pretty complicated, especially under Android 10 and later. |
That seems to be the key here. I am not so sure how to get that work in the first place... |
|
@normanr
So there are a few steps here, right?
|
|
I think this PR is in the right direction but not ready yet. Idealy it should work with the default termux libusb package, without the need for a modified libusb version, or a bridge. libusb current ways to work under Android (without root). There is also an on-going PR here. |
|
Relevant discussions from termux
The OP's repo: libusb PR
|
See Issue #285 .
Not sure how to implement tests for this.
Resources are released automatically from system by Android. On *nix, too?