JorgeGzm commented on code in PR #19579: URL: https://github.com/apache/nuttx/pull/19579#discussion_r3680229815
########## drivers/usbhost/usbhost_hidkbd.c: ########## @@ -238,8 +256,12 @@ struct usbhost_state_s struct work_s rwork; /* For interrupt transfer work */ int16_t nbytes; /* # of bytes actually transferred */ #endif -#ifndef CONFIG_HIDKBD_NODEBOUNCE - uint8_t lastkey[6]; /* For debouncing */ +#if !defined(CONFIG_HIDKBD_NODEBOUNCE) || defined(CONFIG_HIDKBD_KBDUPPER) + uint8_t lastkey[6]; /* Keys down in the previous report */ +#endif +#ifdef CONFIG_HIDKBD_KBDUPPER + struct keyboard_lowerhalf_s lower; /* Keyboard upper-half interface */ Review Comment: Just to be sure I follow: do you mean dropping CONFIG_HIDKBD_KBDUPPER and always registering through the keyboard upper half, so there is a single code path here? If that is it, I am fine with it, and there is already a case for it in the tree: the USB branch in examples/lvglterm (171 lines of kbd_decode()) exists only because this driver speaks a byte stream, on top of an upper half path that already worked for a matrix keyboard. My only concern is what it does to the existing users. Making it unconditional changes what read() on /dev/kbd[n] returns, and 9 in-tree configurations still use examples/hidkbd, which decodes the byte stream: ``` ci20:jumbo qemu-intel64:jumbo nucleo-h743zi2:jumbo sama5d3-xplained:bluetooth nucleo-h753zi:jumbo stm32butterfly2:nsh olimex-lpc1766stk:hidkbd stm32butterfly2:nshusbhost olimex-stm32-p407:hidkbd ``` They would still build, but read struct keyboard_event_s into a char buffer, so the breakage would be silent rather than a CI failure. Is that acceptable to you, or would you rather I move those configurations to examples/keyboard in nuttx-apps first and change this driver afterwards? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
