[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5802: Refine the uinput implementation

[GitBox <gi...@apache.org>]

pkarashchenko commented on a change in pull request #5802:
URL: https://github.com/apache/incubator-nuttx/pull/5802#discussion_r830673489



##########
File path: drivers/input/uinput.c
##########
@@ -41,8 +41,12 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#define UINPUT_NAME_SIZE  16
-#define RPMSG_UINPUT_NAME "rpmsg-uinput-%s"
+#define UINPUT_NAME_SIZE     16

Review comment:
       ```suggestion
   ```

##########
File path: drivers/input/uinput.c
##########
@@ -569,7 +572,7 @@ int uinput_button_initialize(FAR const char *name)
 
 #ifdef CONFIG_UINPUT_KEYBOARD
 
-int uinput_keyboard_initialize(FAR const char *name)
+int uinput_keyboard_initialize(void)
 {
   char devname[UINPUT_NAME_SIZE];

Review comment:
       ```suggestion
   ```

##########
File path: drivers/input/uinput.c
##########
@@ -453,19 +457,17 @@ static ssize_t uinput_keyboard_write(FAR struct keyboard_lowerhalf_s *lower,
  *   Initialized the uinput touchscreen device
  *
  * Input Parameters:
- *   name:      Touchscreen devices name
- *   maxpoint:  Maximum number of touch points supported.
- *   buff_nums: Number of the touch points structure.
+ *   None
  *
  * Returned Value:
  *   Zero is returned on success.  Otherwise, a negated errno value is
  *   returned to indicate the nature of the failure.
  *
  ****************************************************************************/
 
-#ifdef CONFIG_UINPUT_TOUCHSCREEN
+#ifdef CONFIG_UINPUT_TOUCH
 
-int uinput_touch_initialize(FAR const char *name, int maxpoint, int buffnums)
+int uinput_touch_initialize(void)
 {
   char devname[UINPUT_NAME_SIZE];

Review comment:
       ```suggestion
   ```

##########
File path: drivers/input/keyboard_upper.c
##########
@@ -60,6 +60,7 @@ struct keyboard_upperhalf_s
   struct list_node head;    /* Head of list */
   FAR struct keyboard_lowerhalf_s
                   *lower;   /* A pointer of lower half instance */
+  uint32_t         nums;    /* Number of buffer */

Review comment:
       ```suggestion
     size_t           nums;    /* Number of buffer */
   ```
   Or maybe we can reduce it to `uint16_t`?

##########
File path: drivers/input/uinput.c
##########
@@ -589,8 +592,9 @@ int uinput_keyboard_initialize(FAR const char *name)
 
   /* Regiest Touchscreen device */
 
-  snprintf(devname, sizeof(devname), "/dev/%s", name);
-  ret = keyboard_register(&ukbd_lower->lower, devname);
+  snprintf(devname, sizeof(devname), "/dev/%s", UINPUT_NAME_KEYBOARD);
+  ret = keyboard_register(&ukbd_lower->lower, devname,
+                          CONFIG_UINPUT_KEYBOARD_BUFNUMBER);

Review comment:
       ```suggestion
     ret = keyboard_register(&ukbd_lower->lower,
                             "/dev/" UINPUT_NAME_KEYBOARD,
                             CONFIG_UINPUT_KEYBOARD_BUFNUMBER);
   ```

##########
File path: drivers/input/uinput.c
##########
@@ -534,7 +537,7 @@ int uinput_button_initialize(FAR const char *name)
   ubtn_lower->ctx.notify         = uinput_button_notify;
 #endif
 
-  snprintf(devname, sizeof(devname), "/dev/%s", name);
+  snprintf(devname, sizeof(devname), "/dev/%s", UINPUT_NAME_BUTTON);
   ret = btn_register(devname, &ubtn_lower->lower);

Review comment:
       ```suggestion
     ret = btn_register("/dev/" UINPUT_NAME_BUTTON, &ubtn_lower->lower);
   ```

##########
File path: drivers/input/uinput.c
##########
@@ -519,7 +522,7 @@ int uinput_touch_initialize(FAR const char *name, int maxpoint, int buffnums)
 
 #ifdef CONFIG_UINPUT_BUTTONS
 
-int uinput_button_initialize(FAR const char *name)
+int uinput_button_initialize(void)
 {
   char devname[UINPUT_NAME_SIZE];

Review comment:
       ```suggestion
   ```

##########
File path: drivers/input/uinput.c
##########
@@ -478,29 +480,30 @@ int uinput_touch_initialize(FAR const char *name, int maxpoint, int buffnums)
     }
 
   utcs_lower->lower.write    = uinput_touch_write;
-  utcs_lower->lower.maxpoint = maxpoint;
+  utcs_lower->lower.maxpoint = CONFIG_UINPUT_TOUCH_MAXPOINT;
 #ifdef CONFIG_UINPUT_RPMSG
   utcs_lower->ctx.notify     = uinput_touch_notify;
 #endif
 
   /* Regiest Touchscreen device */
 
-  snprintf(devname, sizeof(devname), "/dev/%s", name);
-  ret = touch_register(&utcs_lower->lower, devname, buffnums);
+  snprintf(devname, sizeof(devname), "/dev/%s", UINPUT_NAME_TOUCH);
+  ret = touch_register(&utcs_lower->lower, devname,
+                       CONFIG_UINPUT_TOUCH_BUFNUMBER);

Review comment:
       ```suggestion
     ret = touch_register(&utcs_lower->lower,
                          "/dev/" UINPUT_NAME_TOUCH,
                          CONFIG_UINPUT_TOUCH_BUFNUMBER);
   ```

##########
File path: drivers/input/Kconfig
##########
@@ -64,13 +54,25 @@ config UINPUT_RPMSG
 	---help---
 		Enable support uinput cross core communication
 
-config UINPUT_TOUCHSCREEN
-	bool "Enable uinput touchscreen"
+config UINPUT_TOUCH

Review comment:
       Probably need to mark this with "breaking change" lable




-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org