- Notifications
You must be signed in to change notification settings - Fork1k
feat: allow bidirectional usb endpoint numbers#5014
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:dev
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
3288f5e to0f3d0c3Comparemikesmitty commentedAug 27, 2025
Ok, all ready for review now. I don't have any atsamd21 boards, but I've tested USB CDC on an atsamd51, nrf52840, and a pico-w as well as USB mass storage on the pico-w and everything looks good to me |
aykevl left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
One suggestion, didn't review the code in depth but I see nothing weird at a quick glance.
Uh oh!
There was an error while loading.Please reload this page.
aykevl commentedAug 27, 2025
Did a quick test on a Circuit Playground Express (atsamd21) and it seems to be working fine! |
deadprogram commentedSep 12, 2025
@mikesmitty I just switched the branch for this to Anyone else have any comments on this PR? |
mikesmitty commentedSep 12, 2025
Sure, I'll squash them all |
ba24793 to0066bb3Comparedeadprogram commentedSep 12, 2025
Before merge, just want to confirm with@sago35 that this does not affect any TinyGo Keeb code. ⌨️ |
sago35 commentedSep 12, 2025
I’ll make sure to review it early next week. |
sago35 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Some parts of my TinyGo Keeb code didn’t work, but that turned out to be an issue on my end.
So overall, everything looks really good.
There are just a few parts that may need slight changes, and a couple of things I'd like to understand the reasoning behind.
| var ( | ||
| EndpointMIDIIN=&EndpointEP4IN | ||
| EndpointMIDIOUT=&EndpointEP3OUT | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it would be better to move it tosrc/machine/usb/descriptor/endpoint.go.
| } | ||
| if (epFlags&sam.USB_DEVICE_EPINTFLAG_TRCPT1)>0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What effect does this change have?
Depending on the nature of the change, it might be better to split it into a separate PR.
| } | ||
| if (epFlags&sam.USB_DEVICE_ENDPOINT_EPINTFLAG_TRCPT1)>0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What effect does this change have?
Depending on the nature of the change, it might be better to split it into a separate PR.
| } | ||
| ifoutDataDone { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What effect does this change have?
Depending on the nature of the change, it might be better to split it into a separate PR.
Uh oh!
There was an error while loading.Please reload this page.
This one is still a bit fresh, will update once it's had a bit more time to cook