samples: usb: hid-keyboard: enable and use remote wakeup on button press #79341
Conversation
There was a problem hiding this comment.
"While it is not required to have been configured prior to suspend to issue a wakeup request, from usability perspective I'd say it makes sense." from USB perspective it is slightly different. The device is not configured to "issue a wakeup request", but the device can be either allowed to or prohibited from issuing wakeup request.
If host suspended device with cleared remote wakeup feature, then every button press will result in an error log entry regardless how many times it is pressed.
Right, what I meant was that there is no hard dependency between the HID interface being configured and the ability for remote wakeup (since that is controlled independently with a SET_FEATURE request). I noticed that the device_next stack is not clearing this flag when the host sends a reset signal, even though the USB 2.0 spec requires it:
|
Can you submit a separate PR with a fix? |
Here you go: #79410 |
| if (usbd_is_suspended(sample_usbd)) { | ||
| /* on a press of any button, send wakeup request */ | ||
| if (kb_evt.value) { | ||
| ret = usbd_wakeup_request(sample_usbd); | ||
| if (ret) { | ||
| LOG_ERR("Remote wakeup error, %d", ret); | ||
| } | ||
| } | ||
| continue; /* cannot communicate in suspend */ | ||
| } | ||
|
|
There was a problem hiding this comment.
I do not think it should be handled by the application but by the stack (#73362).
There was a problem hiding this comment.
Here I can only propose:
We could still merge this for the time window between now and when that MR is finally ready, at which point these changes can be undone. I'd really like to have a simple reference sample for evaluating this functionality sooner rather than later.
There was a problem hiding this comment.
We could still merge this for the time window between now and when that MR is finally ready, at which point these changes can be undone.I'd really like to have a simple reference sample for evaluating this functionality sooner rather than later.
Why do you need a "simple reference sample" in the tree to evaluate it? Why can you not use shell command for evaluation?
There was a problem hiding this comment.
Shell command is not quite suitable for USB certification, while a simple button press in hid-keyboard sample is.
There was a problem hiding this comment.
Shell command is not quite suitable for USB certification, while a simple button press in hid-keyboard sample is.
This I fully agree with. It's valuable to have a single step way of evaluating the remote wakeup functionality, if there's a sample that is easily distributed, and the end user just needs to select the target board, build and flash as-is. Remote wakeup is a pretty widely used functionality for HID keyboards, so from that perspective I'd also rather have this than not.
I have no idea how the shell command functionality would work, but I have to clarify here that remote wakeup has to be evaluateable as part of a whole USB operation. (We found an issue in our design that the USB HID interface isn't responsive after remote wakeup, which was only observed with NCS 2.6.1, but not with NCS 2.7.0. Not exactly relevant here, I'm just trying to make a point.)
USB remote wakeup capability is a common feature of keyboards, allowing them to wake up a suspended host. Currently there is no USB sample that demonstrates remote wakeup, which makes basic evaluation of the functionality non-trivial. Modify the hid-keyboard sample with a minimalistic solution of demonstrating remote wakeup capability. While it is not required to have been configured prior to suspend to issue a wakeup request, from usability perspective I'd say it makes sense. Signed-off-by: Benedek Kupper <[email protected]>
benedekkupper
force-pushed
the
feature/samples-hid-keyboard-wake
branch
from
58297f4 to
75eca87
Compare
USB remote wakeup capability is a common feature of keyboards, allowing them to wake up a suspended host. Currently there is no USB sample that demonstrates remote wakeup, which makes basic evaluation of the functionality non-trivial.
Modify the hid-keyboard sample with a minimalistic solution of demonstrating remote wakeup capability. While it is not required to have been configured prior to suspend to issue a wakeup request, from usability perspective I'd say it makes sense.