-
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
!usbd_register_set_config_callback - no handler copy #722
Conversation
So to clarify, this is intended to prevent registering the same callback repeatedly? |
yep, it is intended for implementing composit devices with multiple functions - if i create multile MSC interfaces, they can try to register ones callback per every interface. Without this patch, i must perform registration of functional class, outside of class functionality-implementer. |
if (usbd_dev->user_callback_set_config[i] != callback) | ||
continue; | ||
else | ||
return 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.
I would write this differently
if ((usbd_dev->user_callback_set_config[i] != NULL) &&
(usbd_dev->user_callback_set_config[i] == callback)) {
return 0;
}
The multiple continues in there are a bit weird.
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.
it's so that it continues properly with the right return codes though.
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.
You are right @karlp here is (for me) the more readable and understandable code:
int usbd_register_set_config_callback(usbd_device *usbd_dev,
usbd_set_config_callback callback)
{
int i;
for (i = 0; i < MAX_USER_SET_CONFIG_CALLBACK; i++) {
if ((usbd_dev->user_callback_set_config[i] == NULL) ||
(usbd_dev->user_callback_set_config[i] == callback)) {
usbd_dev->user_callback_set_config[i] = callback;
return 0;
}
}
return -1;
}
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.
hehe :) I'd worked up this version myself locally:
for (i = 0; i < MAX_USER_SET_CONFIG_CALLBACK; i++) {
if (usbd_dev->user_callback_set_config[i]) {
if (usbd_dev->user_callback_set_config[i] == callback) {
return 0;
}
continue;
}
usbd_dev->user_callback_set_config[i] = callback;
return 0;
}
Originally discussed at libopencm3#722
Originally discussed at libopencm3#722
Originally discussed at libopencm3#722
an implementation of this has now merged, see f4bbe7c |
Originally discussed at libopencm3#722
just a small fix protects register one callback multiple times, that will loose callbacl slots and may prevent executing configuration multiple times at activation