-
-
Notifications
You must be signed in to change notification settings - Fork 7k
[HID] Another small cleanup #3884
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
@@ -85,19 +85,19 @@ void HID_::AppendDescriptor(HIDDescriptorListNode *node) | |||
sizeof_hidReportDescriptor += (uint16_t)node->length; | |||
} | |||
|
|||
void HID_::SendReport(u8 id, const void* data, int len) | |||
void HID_::SendReport(uint8_t id, const void* data, const uint16_t len) |
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.
This should be int or you should change all underlying APIs as well. I know that this is sometimes good or bad but in this case we should keep the int. 32k bytes is far enough anyways.
Looks good to me except the int vs uin16_t. |
my preference was to change it, but you're right, it's not worth doing it now. I've rebased and merged everything except int -> uint16 |
Maybe we will change this stuff later on but I dont think so. Could you please try to get this working? Otherwise I really need a pluggable USB API change to get stuff working efficiently. Similar to your last PR I'd merge those two also together: But before you do that I need to know if the code issue above can be solved. If not I'd like to change the Pluggable USB a bit, similar to my HID change. |
Applying some of the suggestion from @NicoHood and other clean-up as well.
u8
datatype (from HID only, USBCore not yet)/cc @facchinm @NicoHood