-
Notifications
You must be signed in to change notification settings - Fork 1k
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
usb: Add high-level USB device support packages #558
Conversation
538e336
to
b0f971f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Hi - my Midi code works fine now - my Python obviously isn't up to it, too many 'self's missing. |
@paulhamsh That's great! I hope it's useful. Debugging is definitely the most fiddly part of this, especially if you don't have the REPL moved off of the default USB-CDC port! I scratched my head a number of times before I got a robust way to always see errors, and then things got smoother! I'll try to include some debugging tips when I write up the docs for this. I see you've MIT licensed your midi.py file (thanks!) Would you happy for me to use that as a basis for inclusion in this PR? |
Yes of course! Happy for you to share it!! |
b0f971f
to
c8ad6ca
Compare
@projectgus t I'm struggling with similar issues I guess ;) I wanted the REPL log to UART0 or 1, so I tried I also tried enabling Can you share a few details on your debug setup? Did I overlook something obvious here? Thanks. |
never mind, it was too late during that evening I guess, I had the wrong pins connected. REPL works now via UART. Sorry for the noise |
I am trying to get serial data off of my pico and stumbled upon these changes and they look fantastical - I want to create a second usb serial device, which leaves the existing serial connection intact. Is this possible with these changes, even with reenumeration? |
@barafael Sorry I didn't reply to you earlier about this. I've had almost no time to work on MP lately, but I've dedicated time from July so the momentum should pick up on this PR quite soon. I know @hoihu started working on CDC serial support on their fork, there is a branch here with a commit: https://github.com/hoihu/micropython-lib/commits/feat-cdc-python - I haven't tested it, it looks like it would enumerate but there's still a bunch of work before it can be used the same as the default USB serial port. So, unfortunately I don't know of an easy way to get >1 USB-CDC serial port for now - either with or without these changes. Best I can offer is: watch this space! |
Had some time left today and I pushed an update with a basic example on how to use it. But as @projectgus said, it's currently still far away from anything. Especially the read() function should be using a ringbuffer or similar. At least it's possible to do some basic rx/tx over that interface now.. |
@hoihu does this sound like it's going to be needed? micropython/micropython#9458 |
Just for the benefit of anyone else looking at this and wanting to borrow code; I've got a HID keypad and basic USB mass storage in my fork. I've created a draft PR to make the changes easy to find at projectgus#1 . |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
53f9558
to
581a662
Compare
Rebased this PR and the linked micropython PR, and incorporated some fixes for issues noticed by @turmoni and @hoihu . @turmoni I've implemented a possible alternative fix in this branch for the (EDIT: Sorry @turmoni I just recalled you'd documented this race in detail over on micropython/micropython#9497 (comment) and I'd missed it as I was busy with other things. There might be more to this, but also there might not be!) |
@turmoni Regarding the issues you mentioned:
Now implemented as
Do you remember which transfers you weren't seeing interface callbacks for?
This was possible but fiddly. I've adapted the keypad example you wrote to use control transfers for Am currently trying to decide if it's worth building a more intuitive interface for control transfers with data, or to leave it like this. |
324ef2b
to
bb389e3
Compare
I fixed the issues above by creating a shared variable import so that both 'boot.py' and 'code.py' can access it. Now, the problem I'm encountering is that the mouse performance is very slow and lags. I am working on a project that involves creating a Barrier Client (https://github.com/jzhvymetal/PY_Barrier) which receives mouse and keyboard commands via Barrier Server over WiFi and then relays them to HID. I implemented the same code on CircuitPython, and the response from the mouse has no lag. Additionally, I implemented it in both MP and CP by using a serial CH9329, which also showed no lag. Is there anything I'm missing to improve the MP HID performance? All of my code is available on the Git repository below. Simply copy all the .py files to the 'lib' directory and place boot.py' in the root. |
just as a follow up to my midi tests. I used MIDI Monitor to debug the midi notes and the notes come in reliably, with the control value increasing, as expected: It let that run for several hours without issues. |
Neat project, @jzhvymetal! Where you call (If an existing report is pending, send_report will block for up to the timeout - default 100ms - before returning. Because you're queueing the same buffer object each time, you can return immediately - the contents of the buffer are what is sent, so if there's one pending then it should be updated by the time the report is actually sent to the host.) This part of the base class might need to be changed (at minimum, possibly change the default to 0ms.) If this isn't it, let me know and I'll debug some more. |
@hoihu I think this might be a poorly commented example on my part. The example does two things - it sends MIDI notes in a loop for as long as the code keeps running. This part doesn't print anything. At the same time, if the host sends any MIDI data to the device then it should print something when it receives that MIDI data. Each direction (IN/OUT) is usually configured separately in MIDI settings (as most devices only transfer MIDI one way). So if the device is configured in Garage Band as only a MIDI input, the current example doesn't print anything. There is also sometimes an option on the host named "MIDI Thru" (or similar), in this mode the host will echo the received "IN" midi messages back "OUT" to that device. I'll change the example so it prints when operating in both directions.
I'm not a MIDI expert either, so my go-to for testing has been the Virtual MIDI Piano Keyboard. When configured with an external device as MIDI Input, the keys are highlighted when notes are received. When configured with an external device as MIDI Output, pressing keys on the (computer) keyboard sends note messages. (Similarly for control channel data there are virtual knobs that work both ways.) |
Hmm OK, it looks like there is something else going on with MIDI Out. In some of my runs it works fine in both directions, but sometimes the host sends a few MIDI OUT packets and then stops (MIDI IN keeps flowing). I don't know why, the USB traffic shows the transfers are successful. This is on Linux with ALSA but I expect whatever the bug is might be the same on all OSes. Still debugging... |
Thanks...needed to learn Python what better way than have a project
Perfect, this fixed the problem... I should have delved deeper down the rabbit hole. Now it seems obvious, with the argument named |
After checking my computer's MIDI setup again I can't reproduce this any more, both ways are working. Given the USB captures don't show any failed transfers or invalid MIDI data, I'm going to put this down to a problem with my computer's MIDI configuration. Have pushed a |
Follow-up question: I updated my code to include a custom keyboard device. I noticed that if I set the value to 0, it would sometimes keep the keys down even though the report was sent again without it. I changed the value to 3ms, and it seems to fix the problem. What is the proper usage, and when should you include a delay? Reviewing the code from CircuitPython custom descriptors, they include a report ID, for example, Once again, thanks for the implementation. Without it, I would not have been able to code my project on MP. I know the main focus is on rp2 and SAMD boards, but if you managed to also get ESP32 working. I would love to test it. In any case, great job so far. |
yes it's clear now and the example works. Thanks for the updates. I also tested the mouse and keyboard_hid examples. They work too on macOS. So looking really good here 👍 |
Hi @jzhvymetal,
Yes, in retrospect setting (There is still potentially a race condition that could miss an event when the caller reuses the same bytearray object as the buffer every time, an interim report might not be sent to the host because its data has been replaced by a later report. I've just updated the PR so
Yes, report IDs are optional in USB HID but they should work. Include them in the HID descriptors, same as CircuitPython does, and then pass the report ID as as the first byte in the data argument of
Thanks! Supporting ESP32-S series is on the radar. There is some low level work that needs to be done first, because ESP-IDF runs TinyUSB in its own RTOS task. This makes it harder to hook the TinyUSB callbacks into MicroPython, but it will be possible to make it work. |
90ce5f2
to
81962f1
Compare
Have performed another rebase & squash including a breaking change for anyone who has implemented their own class driver: |
Hello, not sure I'm missing something, I was trying the KeyboardExample provided, and when it gets to Sample sending the lines through the REPL
Also backend.log from thonny if that helps somehow. Thanks for any help, and sorry if I'm not being smart enough. |
Hi @rafaellago, Thanks for the comprehensive report, and you are certainly being smart enough. The big limitation with the dynamic USB support is that reconfiguring the USB interface requires disconnecting any existing USB device (i.e. the USB serial port holding the REPL in most cases), as we have to simulate an unplug-then-replug of the whole USB device so the host sees any new interfaces. This also has to happen if MicroPython soft resets while a dynamic USB device is currently active. If the whole program has already been sent to MicroPython then it may actually be running after the serial port goes away and comes back, and you could reconnect with a program like PuTTY and see the output from the running program. How well any of this works with Thonny is an open question, I haven't looked closely at Thonny's behaviour - it may be that you can also reconnect in Thonny after getting this error and your program will be running with the dynamic USB active, but it also could be that Thonny sends a soft reset each time it reconnects and this is causing the dynamic USB device to immediately disappear again (along with the USB COM port). I'm planning some work to improve mpremote's support for the USB serial port temporarily going away like this, and I think something similar could be added to Thonny. However, as I said I haven't looked into Thonny specifically and any feature would obviously be up to the Thonny maintainers. |
Hello @projectgus As you said, maybe Thonny is restarting the pico on the connection loss, testing through putty and restarting the session actually brought me back to the repl and I managed to send keystrokes to my PC. If I manage to make a multimedia keyboard example (just volume up/down/mute) can I send a PR to your repo? Or is it better to send it directly to the micropython one? Thank you so much for your explanation. |
This PR is already pretty huge so I think the best would be to wait until this PR is merged and then submit a new PR against this repo that just adds the example. If you have any questions as you work on the keyboard program then feel free to ask. Although if they're not directly related to this PR then it might be best to start a Discussion and then make a comment here that links to it (otherwise things tend to pile up a bit.) |
Hi, You've done a tremendous work on this project, thank you for all this. Whats missing to get it merged? |
Alright, fair enough. I discovered it will take me a while to get my head around hid descriptors, so until then, it will probably be already merged. hahaha thank you so much
@f4grx testing on mac os, I think |
Oops, I neglected to update the TODO in the first post until just now, but @hoihu has tested the examples on macos without issue 🙏 The README is still a little bare bones, but the main thing that's still pending is probably code review from some of the other maintainers. |
81962f1
to
97aac4a
Compare
These packages build on top of machine.USBDevice() to provide high level and flexible support for implementing USB devices in Python code. Additional credits, as per included copyright notices: - CDC support based on initial implementation by @hoihu with fixes by @linted. - MIDI support based on initial implementation by @paulhamsh. - HID keypad example based on work by @turmoni. - Everyone who tested and provided feedback on early versions of these packages. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
97aac4a
to
583bc0d
Compare
Now merged!! Thanks to everyone who contributed here, with testing, feedback and coding. |
Thank you! |
Folks, now this PR is merged some good places to continue discussion are: ... and of course if you find a bug then please open a new Issue on this repo. |
High-level support packages for creating USB device functionality in Python code.
These modules rely on the machine.USBDevice() object newly merged for MicroPython 1.23 (micropython/micropython#9497).
IMPORTANT: The latest version of this PR requires a low-level change in MicroPython's USBDevice that was merged in micropython/micropython#14253 - nightly builds from before 18 April 2024 do not have this change, and will fail in unexpected ways!!
How to test/use this PR
This PR requires a nightly build of Micropython 1.23-preview or later, on a port with machine.USBDevice support (currently samd or rp2).
Can either edit manifest.py to freeze usb-device packages into the firmware, or use the following command to install packages directly from this PR:
See the README in this PR for a list of package names.
Example Code
See the
usb/examples/device
subdirectory for some simple example programs.These aren't installed with the package but can be run as (for example)
mpremote run mouse_example.py
.(Note: because any USB serial connection will close on reenumeration, printed output from this point will only be visible if outputting stdout to traditional UART, semihosting, etc.)
Debugging Tips
See the micropython PR micropython/micropython#9497
TODOs
This PR is nearly complete, aside from code review comments I have a few things left to finish:
Future work (not this PR)
This work was funded through GitHub Sponsors.