Skip to content
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

JNI Android Support on Unrooted devices #1164

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

CraigHutchinson
Copy link
Contributor

@CraigHutchinson CraigHutchinson commented Jul 11, 2022

Rebase of work on #874:

This code implements connection and device listing on unrooted android devices, without requiring any user-provided java.
This PR is usable now 🎉

The java native interface library is used, which is always available when building on android, to do the sdk calls within libusb itself. The approach is modularised to aid expanding if new features are needed.

  • Device enumeration by generating usb descriptors from the android sdk, which requires no user permission for this information.
  • Detect if permission is needed when an unpermissioned device is opened, and perform the requestPermission call if necessary. This pops up a dialogue to the user asking if they will allow the connection.
    📓 Dispatching this request involves setting an action string on the intent for permission that the application can optionally subscribe to. The permission intent I dispatch has action string libusb.android.USB_PERMISSION.

The code only takes effect if the user provides their JNIEnv* or JavaVM* pointer via a new libusb option.

@CraigHutchinson
Copy link
Contributor Author

Integrated Android support for device relisting xloem#3

@CraigHutchinson CraigHutchinson marked this pull request as ready for review July 12, 2022 07:07
@CraigHutchinson CraigHutchinson mentioned this pull request Jul 12, 2022
15 tasks
@xloem
Copy link
Contributor

xloem commented Jul 12, 2022

I'm happy to have this replace #874 or merge and rebase it too, whatever works better for maintainers and contributors.

@CraigHutchinson
Copy link
Contributor Author

CraigHutchinson commented Jul 15, 2022

@xloem I'm not sure but one option could be to force push this onto #874 then the conversation history is maintained while adding a cleaner set of commits? I would be happy to have this replace that pull also. Primarily its amazing work you have made and it needs to get merged as has significant value.

Also, I would very much be in a position to aid/maintain for testing purposes.

Current identified issues(s):

  1. [Minor] When device is being polled the USB Permissions prompt can occur multiple-times i.e. First prompt gives permission but a second prompt still is also pending and must therefore click 'ok' twice.
    EDIT: This may be due to before permission being granted a "libusb generates fake descriptors for it from Android's API" and this may be using to generate the second request due to application level logic TBC

@CraigHutchinson CraigHutchinson changed the title Better Android Support (Rebase 2022-07) JNI Android Support (Rebase 2022-07 #874) Aug 22, 2022
@mcuee
Copy link
Member

mcuee commented Nov 15, 2022

For people who are interested in better Android support, please come here and post your review or success/fail testing results. Thanks.

@CraigHutchinson CraigHutchinson changed the title JNI Android Support (Rebase 2022-07 #874) JNI Android Support on Unrooted devices Nov 15, 2022
@CraigHutchinson
Copy link
Contributor Author

CraigHutchinson commented Nov 17, 2022

NOTE: libusb now requires Signed-Commits on PR so has been rebased onto latest libusb/master. The code has not yet been retested after this update but no merge issues moving from master@2022-07 to master@2022-11

@tormodvolden
Copy link
Contributor

Thanks for the rebasing work!

For information, the maintainers typically rebase any PR locally when merging, regardless of Signed-Commits or not, so just ignore the GitHub warning about this (dunno how to turn it off). However, before this PR can be merged it should ideally be worked down to the smallest number of commits that makes sense, so e.g. without fixups commits. If there are multiple authors it can be correct to have their commits separated for credit attribution, but minor fixups of other people's commit can be amended in-place and commented in the commit message.

@tormodvolden
Copy link
Contributor

Now that we have libusb_init_context() in git master, it would be a good time to rebase this. The NO_DEVICE_DISCOVERY option should be set in the libusb_init_context() call.

int linux_netlink_start_event_monitor(void);
int linux_netlink_stop_event_monitor(void);
void linux_netlink_hotplug_poll(void);
#elif defined(__ANDROID__)
void android_jni_hotplug_poll();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an error here with NDK 25.2.9519653.

error: this function declaration is not a prototype
void android_jni_hotplug_poll();
                             ^
                              void

Should here be added 'void' inside brackets?

Suggested change
void android_jni_hotplug_poll();
void android_jni_hotplug_poll(void);

Comment on lines +167 to +171
#elif !defined(__ANDROID__)
int linux_netlink_start_event_monitor(void);
int linux_netlink_stop_event_monitor(void);
void linux_netlink_hotplug_poll(void);
#elif defined(__ANDROID__)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These function prototypes inside the header file are wrapped in a conditional. Should the functions itself in the 'libusb/os/linux_netlink.c' file be wrapped out to the same conditional?

I have these errors if build with NDK 25.2.9519653.

libusb/os/linux_netlink.c:94:5: error: no previous prototype for function 'linux_netlink_start_event_monitor'
int linux_netlink_start_event_monitor(void)

libusb/os/linux_netlink.c:155:5 error: no previous prototype for function 'linux_netlink_stop_event_monitor'
int linux_netlink_stop_event_monitor(void)

libusb/os/linux_netlink.c:397:6 error: no previous prototype for function 'linux_netlink_hotplug_poll'
void linux_netlink_hotplug_poll(void)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might consider removing the netlink file from the build entirely on line 37 of android/jni/libusb.mk, given android is an uncommon build target. I do not know netlink's potential role on rooted devices but it looks disabled as of now. Adding the proposed guards or changing build flags such that the condition is not an error would both also work.

@ankurvdev
Copy link

Its unclear to me whats preventing this from merging. Can someone summarize the action items needed ?

@mcuee
Copy link
Member

mcuee commented Aug 12, 2023

Its unclear to me whats preventing this from merging. Can someone summarize the action items needed ?

Need more reviews and tests.

Another thing, as of now this PR has conflicts with the git repo which needs to be fixed.

@ben1681
Copy link

ben1681 commented Sep 11, 2023

hello,I'm new here. Is there any Android demo using this branch? @CraigHutchinson @xloem

@CraigHutchinson
Copy link
Contributor Author

CraigHutchinson commented Sep 11, 2023

Not entirely sure what sort of demo you mean. But the Libusb should work as per any other example as long as you provide the JNI for the context configurable iirc.
Edit: actually, it may be the lack of a SIMPLE android demonstration app for test etc that prevents this getting merged as AFAIK as in use I didn't find any major bugs apart from hot plug issues earlier on.

@ben1681
Copy link

ben1681 commented Sep 11, 2023

@CraigHutchinson Thank you for your answer. I'm new to jni and I think need a sample program like this, where the author gets the file descriptor through usbmanager and passes it to jni, which works. Then I found your branch, and I was very interested in what you said about "to do the sdk calls within libusb itself."
Next I'll replace all the libusb code in this example with yours and try to modify the jni code. As I understand it, if I provide JNIEnv*, the code automatically runs the code that gets the permissions and file descriptors, calling the jni function directly. For a quick test, I simply changed the connect_device function in the example to look like this.
std::string connect_device(int fileDescriptor)
{
libusb_device **devs;
int r;
ssize_t cnt;

r = libusb_init(NULL);
if (r < 0)
return "r<0";

cnt = libusb_get_device_list(NULL, &devs);
if (cnt < 0){
libusb_exit(NULL);
return "fail";
}
if (cnt == 0){
libusb_exit(NULL);
return "cnt=0";
}

print_devs(devs);
libusb_free_device_list(devs, 1);

libusb_exit(NULL);
return "success";
}
When I connect my ipad to my Android phone using the Apple to type-c cable, and then run the program, no permission request window appears, I just get a "cnt=0". So I think an example like the one I provided, showing how to call your code to get device information, would be very helpful for beginners like me.

Rebase of work on #874:

This code implements connection and device listing on unrooted android devices, without requiring any user-provided java. This PR is usable now 🎉

The java native interface library is used, which is always available when building on android, to do the sdk calls within libusb itself. The approach is modularised to aid expanding if new features are needed.

  • Device enumeration by generating usb descriptors from the android sdk, which requires no user permission for this information.
  • Detect if permission is needed when an unpermissioned device is opened, and perform the requestPermission call if necessary. This pops up a dialogue to the user asking if they will allow the connection.
    📓 Dispatching this request involves setting an action string on the intent for permission that the application can optionally subscribe to. The permission intent I dispatch has action string libusb.android.USB_PERMISSION.

The code only takes effect if the user provides their JNIEnv* or JavaVM* pointer via a new libusb option.

@CraigHutchinson
Copy link
Contributor Author

I only rebased @xloem work but yes that is looking nearly there. Check the original PR for details which is linked on this rebase key bit is this (which I hope you gathered)

The code only takes effect if the user
provides their JNIEnv* or JavaVM*
pointer via a new libusb option.

I'm not at a PC nor working in this area for last year. As far as I recall the PR adds context options for setting these. It's upto your android code to choose where/how you get them but pass them in.

@ben1681
Copy link

ben1681 commented Sep 12, 2023

@CraigHutchinson Thank you so much for your answer, I have read this today#874, hard for me to fully understand by now. But finally I found out that @xloem has added an example to his libusb-android codehere, the guy I mentioned last time also wrote ademo program for this example, just I didn't find and understand it yesterday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants