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

Add udev iManufacturer/iProduct/iSerial support to linux #14

Merged
merged 5 commits into from
Nov 13, 2023

Conversation

haata
Copy link
Contributor

@haata haata commented Nov 9, 2023

I don't really like how the cfg's turned out, but it is fairly clear.
It would be nice to see if there are equivalents on Windows and macOS (I haven't investigated yet). I may dig around on Windows later to see what I come up with (or if it's even possible).

- Fixes tuna-f1sh#13
- Always prefers reading descriptors directly if possible (then udev,
  then usb id database)
- Requires udev feature
@haata
Copy link
Contributor Author

haata commented Nov 9, 2023

Since this is trickier to CI here are the results of the test

[udev_istring_support]: cargo test --features udev,usb_test                                                                [~/git/cyme](hyatt@Mas5950x:pts/12)
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (target/debug/deps/cyme-20e45df7ce32b859)

running 19 tests
test colour::tests::test_deserialize_color_theme ... ok
test colour::tests::test_serialize_color_theme ... ok
test colour::tests::test_serialize_deserialize_color_theme ... ok
test config::tests::test_deserialize_config_missing_args ... ok
test config::tests::test_deserialize_config_no_theme ... ok
test icon::tests::icon_from_str ... ok
test config::tests::test_deserialize_example_file ... ok
test icon::tests::test_deserialize_icon_tuples ... ok
test icon::tests::test_deserialize_theme ... ok
test icon::tests::test_serialize_example ... ok
test icon::tests::test_serialize_theme ... ok
test icon::tests::test_serialize_defaults ... ok
test system_profiler::tests::test_deserialize_bus ... ok
test system_profiler::tests::test_deserialize_device ... ok
test usb::tests::test_version_from_f32 ... ok
test usb::tests::test_version_to_string ... ok
test udev::tests::test_udev_attribute ... ok
test udev::tests::test_udev_info ... ok
test system_profiler::tests::test_json_dump_read_not_panic ... ok

test result: ok. 19 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/debug/deps/cyme-8157e6aba049dc5e)

running 4 tests
test tests::test_output_args ... ignored
test tests::test_parse_devpath ... ok
test tests::test_parse_show ... ok
test tests::test_parse_vidpid ... ok

test result: ok. 3 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/integration_test.rs (target/debug/deps/integration_test-ffb205d6e766a1bf)

running 5 tests
test test_tree ... ok
test test_tree_filtering ... ok
test test_list ... ok
test test_run ... ok
test test_list_filtering ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.16s

     Running tests/integration_test_lsusb_display.rs (target/debug/deps/integration_test_lsusb_display-48557684c4bead07)

running 6 tests
test test_lsusb_tree_verbose ... ok
test test_lsusb_list ... ok
test test_lsusb_tree ... ok
test test_lsusb_show ... ok
test test_lsusb_device ... ok
test test_lsusb_vidpid ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.08s

   Doc-tests cyme

running 42 tests
test src/lib.rs - (line 15) ... ignored
test src/lib.rs - (line 26) ... ignored
test src/lib.rs - (line 8) ... ignored
test src/lsusb.rs - lsusb::profiler (line 11) - compile ... ok
test src/system_profiler.rs - system_profiler (line 6) - compile ... ok
test src/system_profiler.rs - system_profiler (line 15) - compile ... ok
test src/udev.rs - udev::get_udev_info (line 11) - compile ... ok
test src/udev.rs - udev::get_udev_attribute (line 52) - compile ... ok
test src/usb.rs - usb::Version (line 20) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::get_root_hub (line 737) ... ok
test src/display.rs - display::Encoding::char_is_valid (line 153) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::is_root_hub (line 963) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::is_trunk_device (line 948) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::port_path (line 895) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::is_hub (line 868) ... ok
test src/types.rs - types::NumericalUnit (line 14) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::get_root_hub (line 731) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::parent_path (line 923) ... ok
test src/usb.rs - usb::ClassCode::to_title_case (line 326) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::port_path (line 889) ... ok
test src/icon.rs - icon::Icon::fmt (line 141) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::parent_path (line 911) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::trunk_path (line 933) ... ok
test src/usb.rs - usb::ClassCode::to_lsusb_string (line 312) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::parent_path (line 917) ... ok
test src/usb.rs - usb::Version::fmt (line 78) ... ok
test src/usb.rs - usb::ConfigAttributes::attributes_to_string (line 153) ... ok
test src/system_profiler.rs - system_profiler::USBDevice::to_lsusb_string (line 1001) ... ok
test src/usb.rs - usb::Speed::to_lsusb_speed (line 453) ... ok
test src/usb.rs - usb::USBEndpoint::max_packet_string (line 573) ... ok
test src/usb.rs - usb::Version (line 26) ... ok
test src/system_profiler.rs - system_profiler::USBFilter (line 1257) ... ok
test src/usb.rs - usb::get_interface_path (line 757) ... ok
test src/display.rs - display::truncate_string (line 1517) ... ok
test src/system_profiler.rs - system_profiler::USBFilter (line 1219) ... ok
test src/system_profiler.rs - system_profiler::USBFilter (line 1201) ... ok
test src/display.rs - display::Encoding::str_is_valid (line 189) ... ok
test src/system_profiler.rs - system_profiler::USBFilter (line 1239) ... ok
test src/usb.rs - usb::get_dev_path (line 774) ... ok
test src/usb.rs - usb::get_parent_path (line 722) ... ok
test src/usb.rs - usb::get_port_path (line 697) ... ok
test src/usb.rs - usb::get_trunk_path (line 739) ... ok

test result: ok. 39 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 2.21s

@tuna-f1sh
Copy link
Owner

Thanks for the issue and PR to implement it. I tested and looks good to me. I did however end up going down the rabbit hole comparing lsusb again and noticed a couple of things:

  • The get_udev_attribute you've added here is essentially this https://github.com/gregkh/usbutils/blob/master/sysfs.c#L53 (used: https://github.com/gregkh/usbutils/blob/master/lsusb.c#L286). It leaves me wondering if this would work without the udev crate. I can cat /sys/bus/usb/devices/2-2/product the path and get the data. I don't think the same is true for the drivers, which is why I required udev.
  • root hubs don't get iManufacturer/iProduct/iSerial for me. They won't correct previously but had the fallback. Again, with the point above, I'm tempted to try and match this to lsusb. It appears that for a hub, the port path (1-0:1.0 for example) does not have the attributes. They are at 'usb1', eg:
> cat /sys/bus/usb/devices/usb1/manufacturer
Linux 6.5.9-arch2-1 uhci_hcd

Comparisons

lsusb

❯ lsusb -v -D /dev/bus/usb/001/001
Device: ID 1d6b:0001 Linux Foundation 1.1 root hub
Couldn't open device, some information will be missing
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            9 Hub
  bDeviceSubClass         0 [unknown]
  bDeviceProtocol         0 Full speed (or root) hub
  bMaxPacketSize0        64
  idVendor           0x1d6b Linux Foundation
  idProduct          0x0001 1.1 root hub
  bcdDevice            6.05
  iManufacturer           3 Linux 6.5.9-arch2-1 uhci_hcd
  iProduct                2 UHCI Host Controller
  iSerial                 1 0000:00:1d.0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0019
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0xe0
      Self Powered
      Remote Wakeup
    MaxPower                0mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         9 Hub
      bInterfaceSubClass      0 [unknown]
      bInterfaceProtocol      0 Full speed (or root) hub
      iInterface              0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0002  1x 2 bytes
        bInterval             255

cyme main

❯ cyme -D /dev/bus/usb/001/001 --lsusb -v

Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Couldn't open device, some information will be missing
Device Descriptor:
  bcdUSB              1.10
  bDeviceClass           8 Hub
  bDeviceSubClass        0
  bDeviceProtocol        0
  bMaxPacketSize0       64
  idVendor          0x1d6b Linux Foundation
  idProduct         0x0001 1.1 root hub
  bcdDevice           6.05
  iManufacturer          2 Linux Foundation
  iProduct               3 1.1 root hub
  iSerialNumber          1
  bNumConfigurations     1
  Config Descriptor:
    bNumInterfaces         1
    bConfigurationValue    1
    iConfiguration         0
    bmAttributes:       0x60
      Self Powered
      Remote Wakeup
    bMaxPower              0mA
    Interface Descriptor:
      bInterfaceNumber       0
      bAlternateSetting      0
      bNumEndpoints          1
      bInterfaceClass        8 Hub
      bInterfaceSubClass     0
      bInterfaceProtocol     0
      iInterface             0
      Endpoint Descriptor:
        bEndpointAddress    0x81 EP 1 IN
        bmAttributes:
          Transfer Type          Interrupt
          Sync Type             None
          Usage Type             Data
        wMaxPacketSize    0x0002 1x 2 bytes
        bInterval            255

cyme udev_istring_support

❯ cargo run --features=udev -- -D /dev/bus/usb/001/001  --lsusb -v
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `/home/john/.cargo/target/debug/cyme -D /dev/bus/usb/001/001 --lsusb -v`

Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Couldn't open device, some information will be missing
Device Descriptor:
  bcdUSB              1.10
  bDeviceClass           8 Hub
  bDeviceSubClass        0
  bDeviceProtocol        0
  bMaxPacketSize0       64
  idVendor          0x1d6b Linux Foundation
  idProduct         0x0001 1.1 root hub
  bcdDevice           6.05
  iManufacturer          2
  iProduct               3
  iSerialNumber          1
  bNumConfigurations     1
  Config Descriptor:
    bNumInterfaces         1
    bConfigurationValue    1
    iConfiguration         0
    bmAttributes:       0x60
      Self Powered
      Remote Wakeup
    bMaxPower              0mA
    Interface Descriptor:
      bInterfaceNumber       0
      bAlternateSetting      0
      bNumEndpoints          1
      bInterfaceClass        8 Hub
      bInterfaceSubClass     0
      bInterfaceProtocol     0
      iInterface             0
      Endpoint Descriptor:
        bEndpointAddress    0x81 EP 1 IN
        bmAttributes:
          Transfer Type          Interrupt
          Sync Type             None
          Usage Type             Data
        wMaxPacketSize    0x0002 1x 2 bytes
        bInterval            255

@tuna-f1sh
Copy link
Owner

Sorry, I see you were on the same thread with lsusb references in #13 - still I wonder if we can just read the path directly without the udev crate?

src/udev.rs Outdated
return None;
}
};
let value = device.attribute_value(attribute).unwrap_or_default();
Copy link
Owner

Choose a reason for hiding this comment

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

This should return device.attribute_value(attribute).map(|s| s.to_str().unwrap_or("").to_string()) so that it will actually fall through to usb_ids if attribute_value returns None.

@tuna-f1sh
Copy link
Owner

I made some changes in https://github.com/tuna-f1sh/cyme/tree/udev-linux. Using sysfs style name means that the root_hubs are correctly captured:

❯ cargo run --features=udev -- -D /dev/bus/usb/001/001  --lsusb -v
   Compiling cyme v1.5.2 (/home/john/cyme)
    Finished dev [unoptimized + debuginfo] target(s) in 4.76s
     Running `/home/john/.cargo/target/debug/cyme -D /dev/bus/usb/001/001 --lsusb -v`

Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Couldn't open device, some information will be missing
Device Descriptor:
  bcdUSB              1.10
  bDeviceClass           8 Hub
  bDeviceSubClass        0
  bDeviceProtocol        0
  bMaxPacketSize0       64
  idVendor          0x1d6b Linux Foundation
  idProduct         0x0001 1.1 root hub
  bcdDevice           6.05
  iManufacturer          2 Linux 6.5.9-arch2-1 uhci_hcd
  iProduct               3 UHCI Host Controller
  iSerialNumber          1 0000:00:1d.0
  bNumConfigurations     1
  Config Descriptor:
    bNumInterfaces         1
    bConfigurationValue    1
    iConfiguration         0
    bmAttributes:       0x60
      Self Powered
      Remote Wakeup
    bMaxPower              0mA
    Interface Descriptor:
      bInterfaceNumber       0
      bAlternateSetting      0
      bNumEndpoints          1
      bInterfaceClass        8 Hub
      bInterfaceSubClass     0
      bInterfaceProtocol     0
      iInterface             0
      Endpoint Descriptor:
        bEndpointAddress    0x81 EP 1 IN
        bmAttributes:
          Transfer Type          Interrupt
          Sync Type             None
          Usage Type             Data
        wMaxPacketSize    0x0002 1x 2 bytes
        bInterval            255

Still need to see if we can do it without udev and just read the file directly. Open to exploration on macOS on this but I'm fairly confident there is nothing extra to add; this was originally based on the output of system_profiler. Windows might have something.

I also fixed the character alignment on Sync Type since it was bugging me after seeing it! That can just be merged with main.

@haata
Copy link
Contributor Author

haata commented Nov 10, 2023

Sorry, I see you were on the same thread with lsusb references in #13 - still I wonder if we can just read the path directly without the udev crate?

Yeah, with the sysfs path you don't actually need the udev feature at all to read these fields.
I'll poke around at this today.

- Adds support for iConfiguration (only the current active
  configuration) and iInterface on Linux
- get_udev_attribute() is still available but likely shouldn't be used
@haata
Copy link
Contributor Author

haata commented Nov 11, 2023

Reading from sysfs directly works like a charm.
I was also able to read the cached values of iConfiguration and iInterface as well. iConfiguration is a bit strange (usb configurations in general tend to be strange so I don't think Linux even attempts to read those descriptors; but I would need to read the kernel code to be sure).

I left the get_udev_attribute() but can remove it if you'd like. I don't think there is any reason to use it (but maybe there is 🤷 ). Let me know if you'd rather have it removed.

I also ran cargo clippy --fix on the codebase. I can undo these if you want.

Output looks great now!

cyme -t -vvv

   └──○   27 0x308f 0x0013 Keyboard - Kira PixelMap USB 5337310036384B323430313035353031 - sam4s8
      └──•  1    500 mA xXXx
         ├──◦ 5-4.3:1.0  0x00 HID   0x01 0x01 Boot Keyboard
         │  └──→  1 In  Interrupt None  Data  1x 8
         ├──◦ 5-4.3:1.1  0x00 HID   0x00 0x01 NKRO Keyboard
         │  └──→  2 In  Interrupt None  Data  1x 64
         ├──◦ 5-4.3:1.2  0x00 HID   0x00 0x00 Media Keys
         │  └──→  3 In  Interrupt None  Data  1x 8
         ├──◦ 5-4.3:1.3  0x00 HID   0x00 0x02 Mouse
         │  └──→  4 In  Interrupt None  Data  1x 8
         └──◦ 5-4.3:1.4  0x00 HID   0x00 0x00 HID-IO Interface
            ├──→  5 In  Interrupt None  Data  1x 64
            └──←  6 Out Interrupt None  Data  1x 64
            

@tuna-f1sh
Copy link
Owner

Nice thanks for updating that. The check for #[cfg(target_os = "linux")] or None in the actual function is what I was thinking too as it makes the code much cleaner 👍 .

@tuna-f1sh tuna-f1sh merged commit 3e1851e into tuna-f1sh:main Nov 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fallback to lsusb behaviour for iSerial
2 participants