Differentiating ttyACM devices

okyeron commented 4 years ago

I started down this roard with issue #525 and it's come back around with crow, so I'm starting a new issue to find a good way forward for future ttyACM devices.

Problem: DIY devices using Teensy or some other microcontrollers use /dev/ttyACM* but [device_monitor.c] (https://github.com/monome/norns/blob/master/matron/src/device/device_monitor.c) is currently hard coded to treat any ttyACM device as a crow. There is a handshake for crow, but it just ignores anything not a crow (and it really seems like we need to differentiate between devices before it gets to the handshake stage).

I've done some hacking on this, but I could really use some guidance.

Possible Solution: Simplify the watchers by subsystem - to only "tty", "sound", "input". For tty, check the node and then add additional checks for ttyACM devices.

In handle_device() we could look at device information from udev and get product ID, model, vendor, etc. (which is similar to what libmonome does I think). Would those details be adequate for differentiating devices*?

udev_device_get_property_value(dev, "ID_MODEL"); 
udev_device_get_property_value(dev, "ID_VENDOR");
udev_device_get_property_value(dev, "ID_SERIAL_SHORT");

udevadm info --query=all --name=/dev/ttyACM0 is very handy to check those names/values

At this point, i think the device could be added in device_list.c as it stands (or cases added for new device types).

Thoughts? Suggestions? Help?

I don't have a crow, so i can't test against that.

* EDIT: (some) DIY devices can modify the udev Manufacturer/Product/Serial details for a particular device. With Teensy this is a super easy code change. I'm testing with a SAMD board as well (Adafruit ItsyBitsy M0). Older Arduino required reflashing the FTDI chip, but this has gotten much better with time.

catfact commented 4 years ago

ok cool, then seems to me like vid/pid is a possibly simple way to identify crow without handshake+fallback logic.

druid does this: https://github.com/monome/druid/blob/master/src/druid/crowlib.py#L14

that said, i would not mind structuring the device detection stuff more intelligently anyway.

(i have a crow but, funnily enough, i rarely am in a position to use it since i don't have any eurorack stuff. but i can test this simple connection stuff when i am back at home - travelling now.)

simonvanderveldt commented 4 years ago

Model and vendor are what's normally used for determining if a device is what you think it is, so unless there are any blockers that would prevent us from using or setting)them it should be the correct way to achieve this.

tehn commented 4 years ago

https://github.com/monome/crow/issues/37

@trentgill does crow use a custom vid/pid or is it just the STM32 default?

simonvanderveldt commented 4 years ago

$ lsusb -v -d 0483:

Bus 002 Device 031: ID 0483:5740 STMicroelectronics Virtual COM Port
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         2 Abstract (modem)
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x0483 STMicroelectronics
  idProduct          0x5740 Virtual COM Port
  bcdDevice            2.00
  iManufacturer           1 monome & whimsical raps
  iProduct                2 crow: telephone line
  iSerial                 3 305E35673137
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0043
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xc0
      Self Powered
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              0 
      CDC Header:
        bcdCDC               1.10
      CDC Call Management:
        bmCapabilities       0x00
        bDataInterface          1
      CDC ACM:
        bmCapabilities       0x02
          line coding and serial state
      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              16
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0001
  Self Powered

The vendor and product ID are generic (STMicroelectronics, Virtual COM Port), not sure how it works to get your own. But the Manufacturer and Product are already customized it seems. Not really sure why : telephone line is in the product name, but that could be something that happens automatically on either the SoC side or on Linux, not sure.

trentgill commented 4 years ago

Simon is correct- generic vid/pid. It’s very expensive to get your own.

The name ‘crow: telephone line’ is defined in one of the header files in the usbd/ folder. I thought it was a cute play on TTY devices & crows sitting on wires :)

It also changes to ‘crow: bootloader’ when entering that mode fyi.

okyeron commented 4 years ago

Could someone also post the output from udevadm info --query=all --name=/dev/ttyACM0 for crow? (it gives a different level of detail than lsusb)

simonvanderveldt commented 4 years ago

The name ‘crow: telephone line’ is defined in one of the header files in the usbd/ folder. I thought it was a cute play on TTY devices & crows sitting on wires :)

It also changes to ‘crow: bootloader’ when entering that mode fyi.

lol, I thought it was either the SoC or Linux being clever because it's registered as a modem :laughing:

But anyway, those two fields should be enough to identify a device I think.

simonvanderveldt commented 4 years ago

@okyeron

$ udevadm info --query=all --name=/dev/ttyACM0
P: /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.1/2-1.6.1.3/2-1.6.1.3:1.0/tty/ttyACM0
N: ttyACM0
S: serial/by-id/usb-monome___whimsical_raps_crow:_telephone_line_305E35673137-if00
S: serial/by-path/pci-0000:00:1d.0-usb-0:1.6.1.3:1.0
E: DEVLINKS=/dev/serial/by-id/usb-monome___whimsical_raps_crow:_telephone_line_305E35673137-if00 /dev/serial/by-path/pci-0000:00:1d.0-usb-0:1.6.1.3:1.0
E: DEVNAME=/dev/ttyACM0
E: DEVPATH=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.1/2-1.6.1.3/2-1.6.1.3:1.0/tty/ttyACM0
E: ID_BUS=usb
E: ID_MM_CANDIDATE=1
E: ID_MM_DEVICE_IGNORE=1
E: ID_MODEL=crow:_telephone_line
E: ID_MODEL_ENC=crow:\x20telephone\x20line
E: ID_MODEL_FROM_DATABASE=Virtual COM Port
E: ID_MODEL_ID=5740
E: ID_PATH=pci-0000:00:1d.0-usb-0:1.6.1.3:1.0
E: ID_PATH_TAG=pci-0000_00_1d_0-usb-0_1_6_1_3_1_0
E: ID_PCI_CLASS_FROM_DATABASE=Serial bus controller
E: ID_PCI_INTERFACE_FROM_DATABASE=EHCI
E: ID_PCI_SUBCLASS_FROM_DATABASE=USB controller
E: ID_REVISION=0200
E: ID_SERIAL=monome___whimsical_raps_crow:_telephone_line_305E35673137
E: ID_SERIAL_SHORT=305E35673137
E: ID_TYPE=generic
E: ID_USB_CLASS_FROM_DATABASE=Communications
E: ID_USB_DRIVER=cdc_acm
E: ID_USB_INTERFACES=:020201:0a0000:
E: ID_USB_INTERFACE_NUM=00
E: ID_USB_SUBCLASS_FROM_DATABASE=Abstract (modem)
E: ID_VENDOR=monome___whimsical_raps
E: ID_VENDOR_ENC=monome\x20\x26\x20whimsical\x20raps
E: ID_VENDOR_FROM_DATABASE=STMicroelectronics
E: ID_VENDOR_ID=0483
E: MAJOR=166
E: MINOR=0
E: SUBSYSTEM=tty
E: USEC_INITIALIZED=45041809618

okyeron commented 4 years ago

Did a pass at comparing the product name and manufacturer on tty devices. Don't know if this is more intelligent as such (probably still too simple), but here's a branch with a start: https://github.com/okyeron/norns/blob/check-ttyacm/matron/src/device/device_monitor.c

Works with monome grid and my DIY teensy-grid. Both show up properly on norns.

For crow I'm not sure if "crow:_telephone_line" or "crow: telephone line" will be the correct string to compare at line 275 so it'd be great if someone could test this.

catfact commented 4 years ago

so i think the summary of that change is:

ignore the regexp,
check that subsystem contains tty,
if it does, check manufacturer string
if that contains 'monome', it's a grid/arc
if not, it's a crow.

it still seems quite brittle as a solution, but if it works for everyone then it's fine with me.

if we're gonna bypass the regexp then let's just remove it and make the whole code module clearer.

okyeron commented 4 years ago

More or less correct. Minor corrections:

if both the product and vendor (ID_MODEL AND ID_VENDOR)* strings are "monome" then it's either grid or arc. There's a further downstream check for those
It also checks for crow product and vendor (if crow, there' still a crow handshake after this)
so if not those things, it does nothing

Any suggestions to make this "less brittle"? I don't know what other use cases need to be accounted for.

from udev_device_get_property_value(dev, "ID_MODEL"); and `udev_device_get_property_value(dev, "ID_VENDOR");

OK - just now got a report that the SlowGrowth Arc Clone does not work with this because it's reporting ID_MODEL=FT245R_USB_FIFO. Therefore maybe this should be changed to look at only ID_VENDOR instead?

I don't have an Arc to look at but that might also be a bad match...

Maybe just using ID_VENDOR is OK?

catfact commented 4 years ago

If you can find a solution that works for all the clones and whatever devices of interest, I am happy to clean up the code module to accommodate that solution, and test it with all the devices I have (Norns factory, shield, monome grid, hid, midi, aleph, crow and other tty devices with custom protocols.) I can test with your teensy firmware too, though not with a fully built grid on top of it.

By brittle I mean two things:

1) that this approach can't accommodate other uses of the tty port. That's fine, it doesn't do that now anyway; but if we rewrite the module id rather future proof it.

2) more importantly really, it gives me the wilies to have hardcoded assumptions in the mainlaine Norns repo that we, the maintainers, cannot actually test. So...

At this point I would probably accommodate the inherent ambiguity of broad device support, with a tiny configuration layer. So I'll do that in such a way that you or technobear or whoever can easily configure for alt hardware (gpio) or alt device profiles. How does that sound?

okyeron commented 4 years ago

I'll make an edit to my branch above to just look at ID_VENDOR and that should be a working solution for the DIY devices I know about.

Previously (like w/ Arduinome for example) the only solution was to be sure to have the vendor, product and serial number modified to look like a monome device. Thus the computer or host device just thought it was a monome device. I've just followed that path for recent devices - the only difference being the new ones use ttyACM instead of ttyUSB

I don't know that we need to change that for new devices with some other customized product name for example - mostly because other serialosc/libmonome hosts are still going to want the monome product/vendor/serial.

But yes - a config layer sounds good if that's not too much hassle.

While we're looking at this, it would be nice to revisit the "USB only" aspect of device_monitor.c (re: thetechnobear's issue/pr raised some time ago) - mostly to be able to support Bluetooth input devices for norns shield and fates. See #822. I did some initial work on this but I ended up in a crashing situation and wasn't sure how to debug.

I can test with your teensy firmware too, though not with a fully built grid on top of it. For that you should only need to flash a teensy (3.0 or higher) and attach it. Although, I will need to test that since the i2c stuff for the led boards fails if no i2c devices are there.

catfact commented 2 years ago

i think we can call this resolved for now? (as of https://github.com/monome/norns/pull/1358)

monome / norns

Differentiating ttyACM devices #955