Skip to content

fix HID; fix interface name table creation #4734

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

Merged
merged 2 commits into from
May 10, 2021

Conversation

dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented May 9, 2021

Fixes #4724. @jerryneedell
Fixes #4727. @Neradoc

  • Sense of HID enabled was reversed.
  • Interface index was being passed instead of interface name index in several places.
  • Calculation of individual HID report descriptor size mixed up word and byte sizes.

Copy link
Collaborator

@jerryneedell jerryneedell left a comment

Choose a reason for hiding this comment

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

fixes #4724 - tested on neopixel_trinkey_m0, qtpu_rp2040 and pirkey_m0 -- all work now.
There is an issue with the pirkey_m0. The current build is without pulseio. usb_hid works ok. However, if is restore pulseio then the pirkey_m0 will not boot - the dotstar just blinks and no USB device is mounted. Should I open a new issue for this once this is merged?

BTW - It worked on a qtpy_rp2040 even though CI reported a build issue. Presumably a CI issue.

@jerryneedell
Copy link
Collaborator

jerryneedell commented May 9, 2021

One bit of information regarding the pirkey_m0 issue -- If I resotore pulseio and build from main without this PR, then the pirkey_m0 boots ok -- but USB_HID does not work, With this PR -- USB_HID is ok but it won't boot if I restore pulseio...

I tried adding pulseio into another "small build (rfm9x) and it works OK with this PR
I also verified that pulseio (using irremote) works as expected on a CPX -- with this PR

I don't know if you want to invest any time the pirkey_m0. I just wanted to report the results.

@dhalbert
Copy link
Collaborator Author

dhalbert commented May 9, 2021

@jerryneedell I added a commit to restore pulseio to pirkey, which we need regardless. Could you retest with the pirkey artifact after it rebuilds?

I would not give up on the pirkey right now because the problems you are seeing might indicate a more general bug.

@jerryneedell
Copy link
Collaborator

@dhalbert Can you remind me where I find the artifacts for a PR?
I built it locally and it still fails but I'd like to try your build.

@jerryneedell
Copy link
Collaborator

@dhalbert OK - I found the artifacts and I get the same results with your build (that is reassuring!)
here is what I see in dmesg when I copy over the .uf2

[Sun May  9 16:33:18 2021] usb 1-1.4.7: USB disconnect, device number 57
[Sun May  9 16:33:18 2021] FAT-fs (sda): unable to read boot sector to mark fs as dirty
[Sun May  9 16:33:19 2021] usb 1-1.4.7: new full-speed USB device number 58 using xhci_hcd
[Sun May  9 16:33:19 2021] usb 1-1.4.7: New USB device found, idVendor=239a, idProduct=8028, bcdDevice= 1.00
[Sun May  9 16:33:19 2021] usb 1-1.4.7: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[Sun May  9 16:33:19 2021] usb 1-1.4.7: Product: pIRKey M0
[Sun May  9 16:33:19 2021] usb 1-1.4.7: Manufacturer: Adafruit Industries LLC
[Sun May  9 16:33:19 2021] usb 1-1.4.7: SerialNumber: 3C6EAE6251314E5020312E380D1010FF
[Sun May  9 16:33:19 2021] cdc_acm 1-1.4.7:1.0: ttyACM0: USB ACM device
[Sun May  9 16:33:19 2021] usb-storage 1-1.4.7:1.2: USB Mass Storage device detected
[Sun May  9 16:33:19 2021] scsi host0: usb-storage 1-1.4.7:1.2
[Sun May  9 16:33:19 2021] input: Adafruit Industries LLC pIRKey M0 Keyboard as /devices/platform/scb/fd500000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/usb1/1-1/1-1.4/1-1.4.7/1-1.4.7:1.3/0003:239A:8028.0020/input/input55
[Sun May  9 16:33:20 2021] input: Adafruit Industries LLC pIRKey M0 Mouse as /devices/platform/scb/fd500000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/usb1/1-1/1-1.4/1-1.4.7/1-1.4.7:1.3/0003:239A:8028.0020/input/input56
[Sun May  9 16:33:20 2021] input: Adafruit Industries LLC pIRKey M0 Consumer Control as /devices/platform/scb/fd500000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/usb1/1-1/1-1.4/1-1.4.7/1-1.4.7:1.3/0003:239A:8028.0020/input/input57
[Sun May  9 16:33:20 2021] hid-generic 0003:239A:8028.0020: input,hidraw2: USB HID v1.11 Keyboard [Adafruit Industries LLC pIRKey M0] on usb-0000:01:00.0-1.4.7/input3
[Sun May  9 16:33:20 2021] usb 1-1.4.7: USB disconnect, device number 58
[Sun May  9 16:33:22 2021] usb 1-1.4.7: new full-speed USB device number 59 using xhci_hcd
[Sun May  9 16:33:22 2021] usb 1-1.4.7: device descriptor read/64, error 18
[Sun May  9 16:33:22 2021] usb 1-1.4.7: device descriptor read/64, error 18
[Sun May  9 16:33:22 2021] usb 1-1.4.7: new full-speed USB device number 60 using xhci_hcd
[Sun May  9 16:33:23 2021] usb 1-1.4.7: device descriptor read/64, error 18
[Sun May  9 16:33:23 2021] usb 1-1.4.7: device descriptor read/64, error 18
[Sun May  9 16:33:23 2021] usb 1-1.4-port7: attempt power cycle
[Sun May  9 16:33:24 2021] usb 1-1.4.7: new full-speed USB device number 61 using xhci_hcd
[Sun May  9 16:33:24 2021] usb 1-1.4.7: device descriptor read/8, error -61
[Sun May  9 16:33:24 2021] usb 1-1.4.7: device descriptor read/8, error -61
[Sun May  9 16:33:24 2021] usb 1-1.4.7: new full-speed USB device number 62 using xhci_hcd
[Sun May  9 16:33:24 2021] usb 1-1.4.7: device descriptor read/8, error -61
[Sun May  9 16:33:24 2021] usb 1-1.4.7: device descriptor read/8, error -61
[Sun May  9 16:33:24 2021] usb 1-1.4-port7: unable to enumerate USB device

@jerryneedell
Copy link
Collaborator

I would suggest merging this PR to get the usb_hid fixed and deal with the pirkey_m0 separately.
Provided the other issues addressed by this PR are fixed. I have not test them.

@dhalbert
Copy link
Collaborator Author

I loaded the pirkey build onto a trinket m0 and it worked. I also added pulseio to the Trinket build and it also worked. So it may be something peculiar about your pirkey, or perhaps it's running a boot.py or code.py that's crashing. You could erase the filesystem and see if it works, if you have not done that already. But given those tests, I feel comfortable in moving on.

@dhalbert dhalbert requested review from tannewt and jepler May 10, 2021 00:57
@Neradoc
Copy link

Neradoc commented May 10, 2021

Fixes #4727 in all the tests I could think of.

@jerryneedell
Copy link
Collaborator

@dhalbert This is very odd. You are correct that the issue is related to my code.py. If I remove my code.py, then the pirkey_m0 boots OK -- but with the code.py, it fails -- this happens on 2 different prirkey_m0s!
However, sorting is odd. if I name the file some thing other than code.py, I can execute it from the REPL - it only fails to boot if I try to run it as code.py....
I agree with moving on with this PR and I will explore the code.py issue on the pirkey_m0 further. I did try loading a simple code.py and it boots ok.

@jerryneedell
Copy link
Collaborator

jerryneedell commented May 10, 2021

@dhalbert Hold on!!
I think there is a real issue here -- I modified my code from the pirkey_m0 to run on a CPX ...
I can reproduce the same issue -- If I run the code from the REPL it is OK but if I load it as code.py the CPX fails to boot just as the pirkey_m0 did. I did not see this behavior prior to this PR
Without this PR. now main gives the USB Busy error but it does boot. If I go back to 6.2 it runs normally.

I have no idea what is wrong. One difference for other boards is that these M0's are using .mpy file or "frozen" in libraries -- could that be an issue from a code.py?

@jerryneedell
Copy link
Collaborator

jerryneedell commented May 10, 2021

OK -- one more example.
On the CPX, I created a new code.py that just just uses the neopixels. It runs fine with this PR.
I also ran some more test on the pirkey_m0 and it appears that the issue only occurs when I try to use irremote in the code.py. This is consistent with the CPX example as well. Given that, perhaps it is OK to move ahead with merging this PR and focusing on a potential issue with irremote.
Since the issue also occurs on the CPX, I will continue troubleshooting there to eliminate the pirkey_m0 from the equation.

It's not as simple as I had hoped -- loading the irremote_simpletest.py as code.py works OK on the CPX....I will see if I can identify the trigger of the failure.

using just this as code,py on te CPX does induce the failure.


import adafruit_irremote
from adafruit_hid.keyboard import Keyboard
from adafruit_hid.keycode import Keycode
from adafruit_hid.keyboard_layout_us import KeyboardLayoutUS
import neopixel
import pulseio
import board
import time
import usb_hid


led = neopixel.NeoPixel(board.NEOPIXEL,10)

# The keyboard object!
time.sleep(1)  # Sleep for a bit to avoid a race condition on some systems
keyboard = Keyboard(usb_hid.devices)
keyboard_layout = KeyboardLayoutUS(keyboard)  # We're in the US :)

# our infrared pulse decoder helpers
pulsein = pulseio.PulseIn(board.REMOTEIN, maxlen=120, idle_state=True)
decoder = adafruit_irremote.GenericDecode()
# size must match what you are decoding! for NEC use 4
received_code = bytearray(4)

It is this line that induces the failure

decoder = adafruit_irremote.GenericDecode()

however - that is also present in the irremote_simpletest that works....
Possibly some interaction with irremote and usb_hid....

@dhalbert
Copy link
Collaborator Author

My guess is that this is some kind of storage-related flaw in the USB code or PulseOut. They don't share any code. It's probably some kind of second-order bug. I agree that we should go ahead and merge this PR to get USB working better now, so that main is more usable. I will debug this in a day or two, and have opened a new issue referencing the sample code above.

@jerryneedell
Copy link
Collaborator

Thanks Dan! I agree with merging.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes! I'm going to merge to get them in. Isn't a word usually 32 bits and 16 bits are halfwords or nibbles? May want to change that in the future. Fine for now though. Thanks!

@tannewt tannewt merged commit 4eb4f14 into adafruit:main May 10, 2021
@dhalbert dhalbert deleted the dynamic-usb-fixes branch May 10, 2021 22:50
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.

Mangled CDC interface name usb_hid not working
4 participants