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.

Sign up for GitHub

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

Jump to bottom

Android support for non-rooted phones #80

Merged

kevinmehall merged 11 commits into kevinmehall:main from MerchGuardian:android-support

Sep 21, 2024

Contributor

TroyNeubauer commented Sep 11, 2024 •

edited

Loading

Adds a Device::from_fd function for wrapping devices obtained from android's UsbManager

Testing

Tested with HackrfOne (firmware version 2024.02.1) on an A7 lite running Android 14 (kernel version 4.19.191).

The basic configuration methods work, as well as receiving from bulk endpoints via the async Queue interface:
See: https://github.com/MerchGuardian/seify/blob/hackrf/crates/hackrf-one-rs/src/lib.rs#L535-L566

let dev = nusb::Device::from_fd(fd).unwrap();

for config in dev.configurations() {
    log::info!("Got config: {:?}", config);

    log::info!("num_interfaces: {:?}", config.num_interfaces());
    log::info!("configuration_value: {:?}", config.configuration_value());

    log::info!("attributes: {:?}", config.attributes());
    log::info!("max_power: {:?}", config.max_power());
    let string_index = config.string_index().unwrap();
    log::info!("string_index: {:?}", string_index);

    for lang in dev.get_string_descriptor_supported_languages(Duration::from_millis(500)).unwrap() {
        log::info!("Got lang: {lang:?}");
        let s = dev.get_string_descriptor(string_index, lang, Duration::from_millis(500)).unwrap();
        log::info!("str descriptor: {s:?}");
    }
}

Prints:

INFO:android_test - Got config: Configuration {
    configuration_value: 1,
    num_interfaces: 1,
    attributes: 128,
    max_power: 250,
    string_index: Some(
        3,
    ),
    interface_alt_settings: [
        InterfaceAltSetting {
            interface_number: 0,
            alternate_setting: 0,
            num_endpoints: 2,
            class: 255,
            subclass: 255,
            protocol: 255,
            string_index: None,
            endpoints: [
                Endpoint {
                    address: 0x81,
                    direction: In,
                    transfer_type: Bulk,
                    max_packet_size: 512,
                    packets_per_microframe: 1,
                    interval: 0,
                },
                Endpoint {
                    address: 0x02,
                    direction: Out,
                    transfer_type: Bulk,
                    max_packet_size: 512,
                    packets_per_microframe: 1,
                    interval: 0,
                },
            ],
        },
    ],
}
INFO:android_test - num_interfaces: 1
INFO:android_test - configuration_value: 1
INFO:android_test - attributes: 128
INFO:android_test - max_power: 250
INFO:android_test - string_index: 3
INFO:android_test - Got lang: 1033
INFO:android_test - str descriptor: "Transceiver"

TODO:

Add CI job that builds with cargo ndk to ensure we hit the new `#[cfg(target_os = "android")] blocks in CI

TroyNeubauer added 9 commits

September 5, 2024 06:40


          add cfgs

94e8a71


          android warp approach

f745d3b


          debugging hackrf on android

6b5f2de

wip

76fc011


          RADIO ON ANDROID WORKS

d569d53


          cleanup

81d6650


          try get configuration from ioctl

08c4b87


          better error handling

926ca33


          cleanup logging

0585b54

TroyNeubauer mentioned this pull request

Rust-native HackRf Driver FutureSDR/seify#9

Closed

kevinmehall reviewed

View reviewed changes

Owner

kevinmehall left a comment

Was meaning to try this at some point, glad you got it working.

src/platform/linux_usbfs/device.rs Outdated

Comment on lines 65 to 102

+                      // NOTE: must be called from the same thread that gave us the FD on android, otherwise this fails
+                      let proc_path = PathBuf::from(format!("/proc/self/fd/{}", fd.as_raw_fd()));
+                      debug!("Reading link: {proc_path:?}");
+                      let fd_path = std::fs::read_link(&proc_path)?;
+                      // TODO: should we switch to regex or regex-lite for this?
+                      let prefix = "/dev/bus/usb/";
+                      let Ok(dev_num_bus_num) = fd_path.strip_prefix(prefix) else {
+                          error!(
+                              "Usb device path does not start with required prefix `{prefix}`: {}",
+                              fd_path.to_string_lossy()
+                          );
+                          return Err(ErrorKind::Other.into());
+                      };
+                      let Some(dev_num_bus_num) = dev_num_bus_num.to_str() else {
+                          error!("End of usb device path is not UTF-8!: {dev_num_bus_num:?}",);
+                          return Err(ErrorKind::Other.into());
+                      };
+                      let mut split = dev_num_bus_num.split("/");
+                      let Some(bus_str) = split.next() else {
+                          error!("Failed to split usbfs device path: {}", dev_num_bus_num);
+                          return Err(ErrorKind::Other.into());
+                      };
+                      let Some(dev_str) = split.next() else {
+                          error!("Failed to split usbfs device path: {}", dev_num_bus_num);
+                          return Err(ErrorKind::Other.into());
+                      };
+                      let Ok(busnum) = bus_str.parse::<u8>() else {
+                          error!("Usb bus number failed to parse: {bus_str}",);
+                          return Err(ErrorKind::Other.into());
+                      };
+                      let Ok(devnum) = dev_str.parse::<u8>() else {
+                          error!("Usb device number failed to parse: {dev_str}",);
+                          return Err(ErrorKind::Other.into());
+                      };

Owner

kevinmehall Sep 12, 2024

create_inner doesn't do anything with busnum and devnum beyond logging it. I wouldn't mind changing that log message to avoid going so far out of your way to get these.

Contributor Author

TroyNeubauer Sep 12, 2024

Ah, good catch. Got caught up trying to replicate the libusb code

src/platform/linux_usbfs/device.rs Outdated

+                          return Err(ErrorKind::Other.into());
+                      };
+                      const LIBUSB_REQUEST_GET_CONFIGURATION: u8 = 0x08;

Owner

kevinmehall Sep 12, 2024

Suggested change

      
                    const LIBUSB_REQUEST_GET_CONFIGURATION: u8 = 0x08;
          
                    const REQUEST_GET_CONFIGURATION: u8 = 0x08;

(it's a request defined by the USB standard, not something from libusb)

src/platform/linux_usbfs/device.rs Outdated

+                          index: 0,
+                      };
+                      // TODO: what to do about blocking here?

Owner

kevinmehall Sep 12, 2024

Opening the device file also blocks for USB resume and a GET_STATUS request, so I guess it's no worse than the other way to open a device?

Contributor Author

TroyNeubauer Sep 12, 2024

Makes sense. I suppose this asks a larger question about blocking since its likely that the user will call one of these methods on an async runtime if they later use the async transfer methods

src/platform/linux_usbfs/device.rs Outdated

+                      // See: https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/os/linux_usbfs.c#L865
+                      let n = r.map_err(errno_to_transfer_error)?;
+                      if n != dst.len() {
+                          error!("Failed to read descriptor");

Owner

kevinmehall Sep 12, 2024

Suggested change

      
                        error!("Failed to read descriptor");
          
                        error!("GET_CONFIGURATION request failed");

Since it's not a descriptor

src/platform/linux_usbfs/device.rs Outdated

Comment on lines 133 to 134

		// TODO: Could add some more logic here to support buggy devices, similar to what libusb does
		// See: https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/os/linux_usbfs.c#L865

Owner

kevinmehall Sep 12, 2024

I wouldn't be surprised if it's common to not implement GET_CONFIGURATION because it doesn't seem like it would be commonly used.

If there's only one configuration, which will be the case for the vast majority of devices, it seems pretty safe to assume that's the active configuration.


          implement feedback

7d556a4

Contributor Author

TroyNeubauer commented Sep 12, 2024

Took another pass!

TroyNeubauer requested a review from kevinmehall

September 12, 2024 15:48


          add ci

fd62469

Contributor Author

TroyNeubauer commented Sep 12, 2024

Btw I added CI for android which builds the whole thing on aarch64 and armv7. Also switched actions-rs/toolchain@v1 (deprecated) to dtolnay/rust-toolchain@stable

See: actions-rs/toolchain#216

kevinmehall merged commit d6ec80a into kevinmehall:main

8 checks passed

Owner

kevinmehall commented Sep 21, 2024

Thanks!

Tested on Linux with a device that supports GET_CONFIGURATION and one that doesn't.

It would be useful to have example code to point to on how to integrate this with the Java APIs on Android, so if you publish code for an app that uses this, please let me know!

TroyNeubauer deleted the android-support branch

September 23, 2024 20:04

Contributor Author

TroyNeubauer commented Sep 23, 2024

Certainly! Getting the permissions setup and opening the device to get the FD is a bit annoying, so an example would be great to have. I'll put up a PR up for this in a bit when I have time

kevinmehall reviewed

View reviewed changes

src/enumeration.rs

@@ @@ -314,6 +314,9 @@ impl std::fmt::Debug for DeviceInfo { @@
                       #[cfg(target_os = "linux")]
                       {
                           s.field("sysfs_path", &self.path);
+                      }
+                      #[cfg(any(target_os = "linux", target_os = "android"))]
+                      {

Owner

kevinmehall Sep 24, 2024

I missed this, but @tuna-f1sh noticed when trying to rebase on top of it -- what's your reasoning for splitting this to exclude sysfs_path here, and similarly not adding target_os = "android" to the DeviceInfo::sysfs_path getter?

Un-rooted Android doesn't allow sysfs, but everything in DeviceInfo comes from sysfs, so list_devices isn't expected to work there at all; that's why you added from_fd. I was assuming you added target_os = "android" in this file for use on rooted devices, or just consistency with Linux, in which case it seems like sysfs_path should be included. Alternatively, should we instead cfg out list_devices and DeviceInfo entirely?

Contributor Author

TroyNeubauer Sep 24, 2024

Good catch. Yeah this is something I missed.

I feel like the correct thing to do would be to add target_os = "android" to the getter, fmt::Debug impl etc. for consistency between linux and rooted devices. Then add some docs in list_devices noting that this will fail on unrooted android, with a link to wrap_device like what libusb does: https://libusb.sourceforge.io/api-1.0/group__libusb__lib.html#gga07d4ec54cf575d672ba94c72b3c0de7ca5534030a8299c85555aebfae39161ef7

tuna-f1sh mentioned this pull request

List root hubs function #77

Merged

kevinmehall mentioned this pull request

Publish new version? #84

Closed

kevinmehall mentioned this pull request

Thoughts about Android support and the disadvantage of rusb #86

Open

kevinmehall added a commit that referenced this pull request


          Merge pull request #80 from MerchGuardian/android-support

9fc9c91

Android support for non-rooted phones

kevinmehall added a commit that referenced this pull request


          Merge pull request #80 from MerchGuardian/android-support

6029dde

Android support for non-rooted phones

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet