-
Notifications
You must be signed in to change notification settings - Fork 171
Add stream support for isochronous endpoints. #285
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
Conversation
0489601
to
da2e3f3
Compare
924bf87
to
b6764e7
Compare
b6764e7
to
4cf86d9
Compare
4cf86d9
to
66e46b4
Compare
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.
Why are USBIsochronousStreamInEndpoint
and USBIsochronousStreamOutEndpoint
implemented in different files? This is a bit inconsistent with the rest of the code.
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'd dearly love to split up the rest of the code as well, having everything in single files makes it really hard to both find and work on things.
I'm a firm believer in the "one big thing per file" principle unless it's a bunch of small things that are closely related like type definitions.
Doing a PR that does the same for the other files in luna/gateware/usb/usb2/endpoints/
should have minimal downstream impact as these are all re-exported via the top-level luna.usb2
module path, e.g.
luna/examples/usb/isochronous_count.py
Line 15 in a71b891
from luna.usb2 import USBDevice, USBIsochronousInEndpoint |
That said, I am open to changing this for now if we want to leave this discussion to a later time!
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.
Looking great 😄
This PR depends on #278
amaranth.lib.stream.Interface
for the implementation instead ofluna.stream.StreamInterface
.luna.stream.future.Packet
that addsfirst
andlast
signals to an Amaranth stream payload in order to support packetization.