xiaoxiang781216 commented on code in PR #19586:
URL: https://github.com/apache/nuttx/pull/19586#discussion_r3687525232
##########
drivers/input/keyboard_upper.c:
##########
@@ -384,6 +395,74 @@ int keyboard_unregister(FAR struct keyboard_lowerhalf_s
*lower,
return 0;
}
+/****************************************************************************
+ * Name: keyboard_encode
+ *
+ * Description:
+ * Render one keyboard event as the byte stream that the keyboard codec
+ * defines, for applications that consume characters rather than events.
+ *
+ * Only the press events are rendered. A byte stream has no way to say
+ * that a key came up: a normal key contributes its character and nothing
+ * more, which is what a keyboard reporting through a character device has
+ * always delivered. An application that needs key releases has to read
+ * the events instead.
+ *
+ * Input Parameters:
+ * stream - Memory stream to render into
+ * buf - Buffer of KEYBOARD_BYTESTREAM_MAX bytes backing the stream
+ * keycode - The key
+ * type - The event type
+ *
+ * Returned Value:
+ * The number of bytes rendered, zero if this event has no representation
+ * in the byte stream.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_INPUT_KEYBOARD_BYTESTREAM
+static size_t keyboard_encode(FAR struct lib_memoutstream_s *stream,
+ FAR char *buf, uint32_t keycode, uint32_t type)
+{
+ lib_memoutstream(stream, buf, KEYBOARD_BYTESTREAM_MAX);
+
+ switch (type)
+ {
+ case KEYBOARD_PRESS:
+
+ /* This is what kbd_press() does, but that one is only declared
+ * when the codec is enabled and a plain character does not need
+ * it.
+ */
+
+ lib_stream_putc(&stream->common, (uint8_t)keycode);
Review Comment:
call kbd_press
##########
drivers/input/keyboard_upper.c:
##########
@@ -384,6 +395,74 @@ int keyboard_unregister(FAR struct keyboard_lowerhalf_s
*lower,
return 0;
}
+/****************************************************************************
+ * Name: keyboard_encode
+ *
+ * Description:
+ * Render one keyboard event as the byte stream that the keyboard codec
+ * defines, for applications that consume characters rather than events.
+ *
+ * Only the press events are rendered. A byte stream has no way to say
+ * that a key came up: a normal key contributes its character and nothing
+ * more, which is what a keyboard reporting through a character device has
+ * always delivered. An application that needs key releases has to read
+ * the events instead.
+ *
+ * Input Parameters:
+ * stream - Memory stream to render into
+ * buf - Buffer of KEYBOARD_BYTESTREAM_MAX bytes backing the stream
+ * keycode - The key
+ * type - The event type
+ *
+ * Returned Value:
+ * The number of bytes rendered, zero if this event has no representation
+ * in the byte stream.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_INPUT_KEYBOARD_BYTESTREAM
+static size_t keyboard_encode(FAR struct lib_memoutstream_s *stream,
+ FAR char *buf, uint32_t keycode, uint32_t type)
+{
+ lib_memoutstream(stream, buf, KEYBOARD_BYTESTREAM_MAX);
+
+ switch (type)
+ {
+ case KEYBOARD_PRESS:
+
+ /* This is what kbd_press() does, but that one is only declared
+ * when the codec is enabled and a plain character does not need
+ * it.
+ */
+
+ lib_stream_putc(&stream->common, (uint8_t)keycode);
+ break;
+
+#ifdef CONFIG_LIBC_KBDCODEC
Review Comment:
move the dependence to Kcofnig
##########
drivers/input/Kconfig:
##########
@@ -89,6 +89,26 @@ config INPUT_KEYBOARD
bool
default n
+config INPUT_KEYBOARD_BYTESTREAM
+ bool "Deliver a byte stream instead of keyboard events"
+ default n
+ depends on INPUT_KEYBOARD
Review Comment:
add depends on LIBC_KBDCODEC
##########
drivers/usbhost/Kconfig:
##########
@@ -344,9 +344,16 @@ config USBHOST_HIDKBD
bool "HID Keyboard Class Support"
default n
depends on !INT_DISABLE
+ select INPUT
Review Comment:
remove? it's enough to select INPUT_KEYBOARD
##########
drivers/input/keyboard_upper.c:
##########
@@ -384,6 +395,74 @@ int keyboard_unregister(FAR struct keyboard_lowerhalf_s
*lower,
return 0;
}
+/****************************************************************************
+ * Name: keyboard_encode
+ *
+ * Description:
+ * Render one keyboard event as the byte stream that the keyboard codec
+ * defines, for applications that consume characters rather than events.
+ *
+ * Only the press events are rendered. A byte stream has no way to say
+ * that a key came up: a normal key contributes its character and nothing
Review Comment:
how to use the byte stream mode if it doesn't generate the key up event
##########
drivers/usbhost/usbhost_hidkbd.c:
##########
@@ -2622,233 +2518,35 @@ static int usbhost_close(FAR struct file *filep)
}
/****************************************************************************
- * Name: usbhost_read
+ * Name: usbhost_kbd_open
*
* Description:
- * Standard character driver read method.
+ * Keyboard lower half open method.
*
****************************************************************************/
-static ssize_t usbhost_read(FAR struct file *filep, FAR char *buffer,
- size_t len)
+static int usbhost_kbd_open(FAR struct keyboard_lowerhalf_s *lower)
{
- FAR struct inode *inode;
- FAR struct usbhost_state_s *priv;
- size_t nbytes;
- unsigned int tail;
- int ret;
-
- uinfo("Entry\n");
- DEBUGASSERT(buffer);
- inode = filep->f_inode;
- priv = inode->i_private;
-
- /* Make sure that we have exclusive access to the private data structure */
-
- DEBUGASSERT(priv && priv->crefs > 0 && priv->crefs < USBHOST_MAX_CREFS);
- ret = nxmutex_lock(&priv->lock);
- if (ret < 0)
- {
- return ret;
- }
-
- /* Check if the keyboard is still connected. We need to disable interrupts
- * momentarily to assure that there are no asynchronous disconnect events.
+ /* Note that lower->priv cannot be used to find our state: it belongs to
+ * the upper half, which overwrites it in keyboard_register().
*/
- if (priv->disconnected)
- {
- /* No... the driver is no longer bound to the class. That means that
- * the USB keyboard is no longer connected. Refuse any further
- * attempts to access the driver.
- */
-
- ret = -ENODEV;
- }
- else
- {
- /* Is there keyboard data now? */
-
- while (priv->tailndx == priv->headndx)
- {
- /* No.. were we open non-blocking? */
-
- if (filep->f_oflags & O_NONBLOCK)
- {
- /* Yes.. then return a failure */
-
- ret = -EAGAIN;
- goto errout;
- }
-
- /* Wait for data to be available */
-
- uinfo("Waiting...\n");
-
- priv->waiting = true;
- nxmutex_unlock(&priv->lock);
- ret = nxsem_wait_uninterruptible(&priv->waitsem);
- if (ret < 0)
- {
- return ret;
- }
-
- ret = nxmutex_lock(&priv->lock);
- if (ret < 0)
- {
- return ret;
- }
-
- /* Did the keyboard become disconnected while we were waiting */
-
- if (priv->disconnected)
- {
- ret = -ENODEV;
- goto errout;
- }
- }
-
- /* Read data from our internal buffer of received characters */
-
- for (tail = priv->tailndx, nbytes = 0;
- tail != priv->headndx && nbytes < len;
- nbytes++)
- {
- /* Copy the next keyboard character into the user buffer */
-
- *buffer++ = priv->kbdbuffer[tail];
-
- /* Handle wrap-around of the tail index */
-
- if (++tail >= CONFIG_HIDKBD_BUFSIZE)
- {
- tail = 0;
- }
- }
-
- ret = nbytes;
-
- /* Update the tail index (perhaps marking the buffer empty) */
-
- priv->tailndx = tail;
- }
-
-errout:
- nxmutex_unlock(&priv->lock);
- return (ssize_t)ret;
+ return usbhost_open_priv(container_of(lower, struct usbhost_state_s,
Review Comment:
inline usbhost_open_priv here and remove usbhost_open_priv
##########
drivers/usbhost/usbhost_hidkbd.c:
##########
@@ -2622,233 +2518,35 @@ static int usbhost_close(FAR struct file *filep)
}
/****************************************************************************
- * Name: usbhost_read
+ * Name: usbhost_kbd_open
*
* Description:
- * Standard character driver read method.
+ * Keyboard lower half open method.
*
****************************************************************************/
-static ssize_t usbhost_read(FAR struct file *filep, FAR char *buffer,
- size_t len)
+static int usbhost_kbd_open(FAR struct keyboard_lowerhalf_s *lower)
{
- FAR struct inode *inode;
- FAR struct usbhost_state_s *priv;
- size_t nbytes;
- unsigned int tail;
- int ret;
-
- uinfo("Entry\n");
- DEBUGASSERT(buffer);
- inode = filep->f_inode;
- priv = inode->i_private;
-
- /* Make sure that we have exclusive access to the private data structure */
-
- DEBUGASSERT(priv && priv->crefs > 0 && priv->crefs < USBHOST_MAX_CREFS);
- ret = nxmutex_lock(&priv->lock);
- if (ret < 0)
- {
- return ret;
- }
-
- /* Check if the keyboard is still connected. We need to disable interrupts
- * momentarily to assure that there are no asynchronous disconnect events.
+ /* Note that lower->priv cannot be used to find our state: it belongs to
+ * the upper half, which overwrites it in keyboard_register().
*/
- if (priv->disconnected)
- {
- /* No... the driver is no longer bound to the class. That means that
- * the USB keyboard is no longer connected. Refuse any further
- * attempts to access the driver.
- */
-
- ret = -ENODEV;
- }
- else
- {
- /* Is there keyboard data now? */
-
- while (priv->tailndx == priv->headndx)
- {
- /* No.. were we open non-blocking? */
-
- if (filep->f_oflags & O_NONBLOCK)
- {
- /* Yes.. then return a failure */
-
- ret = -EAGAIN;
- goto errout;
- }
-
- /* Wait for data to be available */
-
- uinfo("Waiting...\n");
-
- priv->waiting = true;
- nxmutex_unlock(&priv->lock);
- ret = nxsem_wait_uninterruptible(&priv->waitsem);
- if (ret < 0)
- {
- return ret;
- }
-
- ret = nxmutex_lock(&priv->lock);
- if (ret < 0)
- {
- return ret;
- }
-
- /* Did the keyboard become disconnected while we were waiting */
-
- if (priv->disconnected)
- {
- ret = -ENODEV;
- goto errout;
- }
- }
-
- /* Read data from our internal buffer of received characters */
-
- for (tail = priv->tailndx, nbytes = 0;
- tail != priv->headndx && nbytes < len;
- nbytes++)
- {
- /* Copy the next keyboard character into the user buffer */
-
- *buffer++ = priv->kbdbuffer[tail];
-
- /* Handle wrap-around of the tail index */
-
- if (++tail >= CONFIG_HIDKBD_BUFSIZE)
- {
- tail = 0;
- }
- }
-
- ret = nbytes;
-
- /* Update the tail index (perhaps marking the buffer empty) */
-
- priv->tailndx = tail;
- }
-
-errout:
- nxmutex_unlock(&priv->lock);
- return (ssize_t)ret;
+ return usbhost_open_priv(container_of(lower, struct usbhost_state_s,
+ lower));
}
/****************************************************************************
- * Name: usbhost_write
+ * Name: usbhost_kbd_close
*
* Description:
- * Standard character driver write method.
+ * Keyboard lower half close method.
*
****************************************************************************/
-static ssize_t usbhost_write(FAR struct file *filep, FAR const char *buffer,
- size_t len)
+static int usbhost_kbd_close(FAR struct keyboard_lowerhalf_s *lower)
{
- /* We won't try to write to the keyboard */
-
- return -ENOSYS;
-}
-
-/****************************************************************************
- * Name: usbhost_poll
- *
- * Description:
- * Standard character driver poll method.
- *
- ****************************************************************************/
-
-static int usbhost_poll(FAR struct file *filep, FAR struct pollfd *fds,
- bool setup)
-{
- FAR struct inode *inode;
- FAR struct usbhost_state_s *priv;
- int ret;
- int i;
-
- uinfo("Entry\n");
- DEBUGASSERT(fds);
- inode = filep->f_inode;
- priv = inode->i_private;
-
- /* Make sure that we have exclusive access to the private data structure */
-
- DEBUGASSERT(priv);
- ret = nxmutex_lock(&priv->lock);
- if (ret < 0)
- {
- return ret;
- }
-
- /* Check if the keyboard is still connected. We need to disable interrupts
- * momentarily to assure that there are no asynchronous disconnect events.
- */
-
- if (priv->disconnected)
- {
- /* No... the driver is no longer bound to the class. That means that
- * the USB keyboard is no longer connected. Refuse any further
- * attempts to access the driver.
- */
-
- ret = -ENODEV;
- }
- else if (setup)
- {
- /* This is a request to set up the poll. Find an available slot for
- * the poll structure reference
- */
-
- for (i = 0; i < CONFIG_HIDKBD_NPOLLWAITERS; i++)
- {
- /* Find an available slot */
-
- if (!priv->fds[i])
- {
- /* Bind the poll structure and this slot */
-
- priv->fds[i] = fds;
- fds->priv = &priv->fds[i];
- break;
- }
- }
-
- if (i >= CONFIG_HIDKBD_NPOLLWAITERS)
- {
- fds->priv = NULL;
- ret = -EBUSY;
- goto errout;
- }
-
- /* Should we immediately notify on any of the requested events? Notify
- * the POLLIN event if there is buffered keyboard data.
- */
-
- if (priv->headndx != priv->tailndx)
- {
- poll_notify(&fds, 1, POLLIN);
- }
- }
- else
- {
- /* This is a request to tear down the poll. */
-
- FAR struct pollfd **slot = (FAR struct pollfd **)fds->priv;
- DEBUGASSERT(slot);
-
- /* Remove all memory of the poll setup */
-
- *slot = NULL;
- fds->priv = NULL;
- }
-
-errout:
- nxmutex_unlock(&priv->lock);
- return ret;
+ return usbhost_close_priv(container_of(lower, struct usbhost_state_s,
Review Comment:
inline usbhost_close_priv and remove usbhost_close_priv
--
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]