Conversation
PhazonicRidley
force-pushed
the
usb-enumeration
branch
from
d2cc07b to
c8f1daa
Compare
PhazonicRidley
force-pushed
the
usb-enumeration
branch
2 times, most recently
from
780ef28 to
f41be54
Compare
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
Please rebase your project and fix your merge conflicts. Also, we moved the libhal-util code into v5 in preperation for v6 which will use libhal-v5. So we can remove the include files within the include directory and only keep the ones within v5/*
PhazonicRidley
force-pushed
the
usb-enumeration
branch
3 times, most recently
from
6b474f8 to
c9c62f1
Compare
For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.
Simulated host by sending enumerator requests and having it respond. Correct payloads were verified
Changed usb.hpp to be a shorthand for including all usb utils
For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.
Simulated host by sending enumerator requests and having it respond. Correct payloads were verified
Changed usb.hpp to be a shorthand for including all usb utils
PhazonicRidley
force-pushed
the
usb-enumeration
branch
from
c9c62f1 to
ff91bbf
Compare
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
I haven't reviewed everything. I'm posting what I have now because I'm not sure if github will lose my info.
Added required copyright notices Changed over from span_eq to ranges::equal and removed function Renamed truncated constant names to their full names Changed mock interface's packed array to use a predefined constant instead of a magic number Changed headers in the same library to be relative paths Factored out ctor arguments for device, config, and enumeration Enforced endianess with device descriptor Changed ref getters to return by value and add setters where needed Documented make_sub_scatter_array and changed return type to a struct for clarity Hardcoded string indexes for device descriptors, removed the ablity to set a start for string descriptors
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
This is a partial review of the changes you've made. Things are looking really good. I plan to dig into the details of the enumerator next. Thank you for your effort on this!
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
alright, this is the review of the enumerator it self. There is a lot of extra code hanging around that needs some cleanup. And I think we should think very critically about errors and how we expect developers to handle them. No error should come without a means to handle it. And I think in most cases, the exceptions can be eliminated entirely.
Cleaned up the code Enforced LE for all data structures
PhazonicRidley
force-pushed
the
usb-enumeration
branch
from
0f5458b to
6d9bf56
Compare
Made system non-blocking Errors do not propagate unless they have to or are insighted by user System will STALL on invalid requests from the host automatically System will have user defined number of retries before giving up
|
On testing on hardware, I am getting timed_out exception. Not to be merged yet until this is resolved. |
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
So I'm noticing a lot of extra commented out code in this last commit? Can you review your PR yourself then @ me in a message when this is ready.
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
Everything that I prefixed with NIT is something you can simply mark as resolved. These will be things that I may pick up later in order to clean up some of the code here. Otherwise, most of this looks good. I also added a number of TODO issues to go along with your TODOs. Any FIXMEs should be replaced with TODOs although I gave one a pass until later.
Also, please make sure to review your PR yourself and remove any commented sections of code.
PhazonicRidley
force-pushed
the
usb-enumeration
branch
from
43a40cd to
0197e70
Compare
… machine function
2 participants
Implemented a USB enumerator that can be used in various projects. This is meant as a default and reference implementation of an enumerator using the libhal USB system.