You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/11 06:34:52 UTC

[GitHub] [incubator-nuttx] no1wudi opened a new pull request, #6596: vncserver enhancement

no1wudi opened a new pull request, #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596

   ## Summary
   * Support keyboard driver 
   * Init global vnc session structure in up_fbinitialize
   ## Impact
   vncserver only
   ## Testing
   sim:vncserver
   


-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918477350


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   OK



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917614844


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -631,6 +631,27 @@ int up_fbinitialize(int display)
 
   session->mouseout = vnc_touch_event;
 #endif
+
+#ifdef CONFIG_VNCSERVER_KBD
+  ret = snprintf(devname, NAME_MAX, CONFIG_VNCSERVER_KBD_DEVNAME "%d",
+                 display);
+  if (ret < 0)
+    {
+      gerr("ERROR: Format vnc keyboard driver path failed.\n");
+      return ret;
+    }
+
+  ret = vnc_kbd_register(devname, session);
+
+  if (ret < 0)
+    {
+      gerr("ERROR: Initial vnc keyboard driver failed.\n");
+      return ret;
+    }
+
+  session->kbdout = vnc_kbd_event;
+#endif
+

Review Comment:
   need unregister keyboard after line 644



##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -631,6 +631,27 @@ int up_fbinitialize(int display)
 
   session->mouseout = vnc_touch_event;
 #endif
+
+#ifdef CONFIG_VNCSERVER_KBD
+  ret = snprintf(devname, NAME_MAX, CONFIG_VNCSERVER_KBD_DEVNAME "%d",
+                 display);
+  if (ret < 0)
+    {
+      gerr("ERROR: Format vnc keyboard driver path failed.\n");
+      return ret;
+    }
+
+  ret = vnc_kbd_register(devname, session);
+
+  if (ret < 0)
+    {
+      gerr("ERROR: Initial vnc keyboard driver failed.\n");

Review Comment:
   unregister touch



##########
drivers/video/vnc/vnc_kbd.c:
##########
@@ -0,0 +1,65 @@
+/****************************************************************************
+ * drivers/video/vnc/vnc_kbd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include "vnc_server.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+int vnc_kbd_register(FAR const char *devpath,
+                     FAR struct vnc_session_s *session)
+{
+  return keyboard_register(&session->kbd, devpath, 1);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+void vnc_kbd_unregister(FAR struct vnc_session_s *session,
+                        FAR const char *devpath)
+{
+  keyboard_unregister(&session->kbd, devpath);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_event
+ ****************************************************************************/
+
+int vnc_kbd_event(FAR void *arg, uint8_t nch, FAR const uint8_t *ch)
+{
+  int i = 0;
+  FAR struct vnc_session_s *session = arg;
+  for (i = 0; i < nch; i++)
+    {
+      keyboard_event(&session->kbd, ch[i], KEYBOARD_PRESS);

Review Comment:
   need convert to uin32_t and send once



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917692051


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   Ok, look like we should move register after vnc_wait_start?



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918675059


##########
drivers/video/vnc/vnc_receiver.c:
##########
@@ -321,8 +321,14 @@ int vnc_receiver(FAR struct vnc_session_s *session)
                   /* Inject the key press/release event into NX */
 
                   keyevent = (FAR struct rfb_keyevent_s *)session->inbuf;
+#ifndef CONFIG_VNCSERVER_KBD
                   vnc_key_map(session, rfb_getbe32(keyevent->key),
                               (bool)keyevent->down);
+#else
+                  /* Pass raw keycode to keyboard driver */
+
+                  session->kbdout(session, keyevent->down, keyevent->key);

Review Comment:
   Yes, if the user doesn't enable CONFIG_VNCSERVER_KBDENCODE, but it's better to move the code into vnc_key_map



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917902782


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   it's more natural to initialize touch/kbd after vnc_start_server.



-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#issuecomment-1181226273

   > @no1wudi I'm looking for a VNC client on NuttX side, do you have some suggestion how I could implement it? I think a VNC client should be userspace only and could use framebuffer interface, right?
   
   Yes, and vnc client also can be intergrated to graphic lib (nx or lvgl) with some adaptions.


-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918730189


##########
drivers/video/vnc/vnc_keymap.c:
##########
@@ -454,6 +454,17 @@ static int vnc_kbd_ascii(uint16_t keysym)
 void vnc_key_map(FAR struct vnc_session_s *session, uint16_t keysym,
                  bool keydown)
 {
+  /* If uinput like virtual keyboard enabled, pass keycode to keyboard driver.
+   *
+   * This driver cannot be used with NX now, if vnc server works with NX, should
+   * not enable CONFIG_VNCSERVER_KBD.
+   */
+
+#ifdef CONFIG_VNCSERVER_KBD
+  session->kbdout(&session->kbd, keydown, (const uint8_t *)&keysym);

Review Comment:
   ```suggestion
     session->kbdout(&session->kbd, keydown, (FAR const uint8_t *)&keysym);
   ```



##########
drivers/video/vnc/vnc_server.h:
##########
@@ -227,6 +228,10 @@ struct vnc_session_s
   FAR struct touch_lowerhalf_s touch; /* Touch driver instance */
 #endif
 
+#ifdef CONFIG_VNCSERVER_KBD
+  FAR struct keyboard_lowerhalf_s kbd; /* Keyboard driver instance */

Review Comment:
   ```suggestion
     struct keyboard_lowerhalf_s kbd; /* Keyboard driver instance */
   ```
   and same for `struct touch_lowerhalf_s touch;` above



##########
drivers/video/vnc/vnc_kbd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * drivers/video/vnc/vnc_kbd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include "vnc_server.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+int vnc_kbd_register(FAR const char *devpath,
+                     FAR struct vnc_session_s *session)
+{
+  return keyboard_register(&session->kbd, devpath, 1);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+void vnc_kbd_unregister(FAR struct vnc_session_s *session,
+                        FAR const char *devpath)
+{
+  keyboard_unregister(&session->kbd, devpath);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_event
+ ****************************************************************************/
+
+int vnc_kbd_event(FAR void *arg, uint8_t pressed, FAR const uint8_t *keycode)
+{
+  FAR struct vnc_session_s *session = arg;
+  keyboard_event(&session->kbd, *(uint16_t *)(keycode),

Review Comment:
   ```suggestion
     keyboard_event(&session->kbd, *((FAR uint16_t *)keycode),
   ```
   BTW are we sure that `keycode` points to `uint16_t` memory (aligned by 2)? Maybe it is safer to
   ```
   uint16_t tmp = (uint16_t)keycode[0] << 16 | keycode[1];
   keyboard_event(&session->kbd, tmp,
                  pressed ? KEYBOARD_PRESS : KEYBOARD_RELEASE);
   ```



##########
drivers/video/vnc/vnc_keymap.c:
##########
@@ -454,6 +454,17 @@ static int vnc_kbd_ascii(uint16_t keysym)
 void vnc_key_map(FAR struct vnc_session_s *session, uint16_t keysym,
                  bool keydown)
 {
+  /* If uinput like virtual keyboard enabled, pass keycode to keyboard driver.
+   *
+   * This driver cannot be used with NX now, if vnc server works with NX, should
+   * not enable CONFIG_VNCSERVER_KBD.
+   */
+
+#ifdef CONFIG_VNCSERVER_KBD
+  session->kbdout(&session->kbd, keydown, (const uint8_t *)&keysym);
+  return;

Review Comment:
   ```suggestion
   ```
   seems to be redundant



-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917618692


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   Touch/kbd inited before start vnc_server thread, thus the `g_vnc_sessions` is NULL before `vnc_start_server`.



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918240928


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -631,6 +694,27 @@ int up_fbinitialize(int display)
 
   session->mouseout = vnc_touch_event;
 #endif
+
+#ifdef CONFIG_VNCSERVER_KBD
+  ret = snprintf(kbd_name, NAME_MAX, CONFIG_VNCSERVER_KBD_DEVNAME "%d",
+                 display);

Review Comment:
   ```suggestion
     ret = snprintf(kbd_name, sizeof(kbd_name), CONFIG_VNCSERVER_KBD_DEVNAME "%d",
                    display);
   ```



##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -855,5 +942,17 @@ void up_fbuninitialize(int display)
 
       vnc_touch_unregister(session, devname);
 #endif
+
+#ifdef CONFIG_VNCSERVER_KBD
+      ret = snprintf(devname, NAME_MAX, CONFIG_VNCSERVER_KBD_DEVNAME "%d",
+                    display);

Review Comment:
   ```suggestion
         ret = snprintf(devname, sizeof(devname), CONFIG_VNCSERVER_KBD_DEVNAME "%d",
                        display);
   ```
   and same for above



##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -602,17 +660,22 @@ int up_fbinitialize(int display)
   int ret;
   FAR struct vnc_session_s *session;
 #ifdef CONFIG_VNCSERVER_TOUCH
-  char devname[NAME_MAX];
+  char touch_name[NAME_MAX];
+#endif
+#ifdef CONFIG_VNCSERVER_KBD
+  char kbd_name[NAME_MAX];
 #endif
 
   DEBUGASSERT(display >= 0 && display < RFB_MAX_DISPLAYS);
 
+  ret = vnc_session_alloc();
+
   /* Save the input callout function information in the session structure. */
 
   session           = g_vnc_sessions[display];
 #ifdef CONFIG_VNCSERVER_TOUCH
 
-  ret = snprintf(devname, NAME_MAX, CONFIG_VNCSERVER_TOUCH_DEVNAME "%d",
+  ret = snprintf(touch_name, NAME_MAX, CONFIG_VNCSERVER_TOUCH_DEVNAME "%d",
                  display);

Review Comment:
   ```suggestion
     ret = snprintf(touch_name, sizeof(touch_name), CONFIG_VNCSERVER_TOUCH_DEVNAME "%d",
                    display);
   ```



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918514476


##########
drivers/video/vnc/vnc_receiver.c:
##########
@@ -321,8 +321,14 @@ int vnc_receiver(FAR struct vnc_session_s *session)
                   /* Inject the key press/release event into NX */
 
                   keyevent = (FAR struct rfb_keyevent_s *)session->inbuf;
+#ifndef CONFIG_VNCSERVER_KBD
                   vnc_key_map(session, rfb_getbe32(keyevent->key),
                               (bool)keyevent->down);
+#else
+                  /* Pass raw keycode to keyboard driver */
+
+                  session->kbdout(session, keyevent->down, keyevent->key);

Review Comment:
   should we move the code into vnc_key_map



##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -602,26 +602,43 @@ int up_fbinitialize(int display)
   int ret;
   FAR struct vnc_session_s *session;
 #ifdef CONFIG_VNCSERVER_TOUCH
-  char devname[NAME_MAX];
+  char touch_name[NAME_MAX];
+#endif
+#ifdef CONFIG_VNCSERVER_KBD
+  char kbd_name[NAME_MAX];

Review Comment:
   why not use the same name as up_fbuninitialize?



##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -602,26 +602,43 @@ int up_fbinitialize(int display)
   int ret;
   FAR struct vnc_session_s *session;
 #ifdef CONFIG_VNCSERVER_TOUCH
-  char devname[NAME_MAX];
+  char touch_name[NAME_MAX];
+#endif
+#ifdef CONFIG_VNCSERVER_KBD
+  char kbd_name[NAME_MAX];
 #endif
 
   DEBUGASSERT(display >= 0 && display < RFB_MAX_DISPLAYS);
 
+  /* Start the VNC server kernel thread. */
+
+  ret = vnc_start_server(display);

Review Comment:
   why remove the error check?



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918726613


##########
drivers/video/vnc/vnc_kbd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * drivers/video/vnc/vnc_kbd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include "vnc_server.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+int vnc_kbd_register(FAR const char *devpath,
+                     FAR struct vnc_session_s *session)
+{
+  return keyboard_register(&session->kbd, devpath, 1);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_register
+ ****************************************************************************/
+
+void vnc_kbd_unregister(FAR struct vnc_session_s *session,
+                        FAR const char *devpath)
+{
+  keyboard_unregister(&session->kbd, devpath);
+}
+
+/****************************************************************************
+ * Name: vnc_kbd_event
+ ****************************************************************************/
+
+int vnc_kbd_event(FAR void *arg, uint8_t pressed, FAR const uint8_t *keycode)
+{
+  FAR struct vnc_session_s *session = arg;
+  keyboard_event(&session->kbd, *(uint16_t *)(keycode),

Review Comment:
   ```suggestion
     keyboard_event(&session->kbd, *((FAR uint16_t *)keycode),
   ```
   BTW are we sure that `keycode` points to `uint16_t` memory (aligned by 2)? Maybe it is safer to
   ```
   uint16_t keysym = (uint16_t)keycode[0] << 16 | keycode[1];
   keyboard_event(&session->kbd, keysym,
                  pressed ? KEYBOARD_PRESS : KEYBOARD_RELEASE);
   ```



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917617278


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   what's problem to allocate session in vnc_server?



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918678097


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -631,29 +651,29 @@ int up_fbinitialize(int display)
 
   session->mouseout = vnc_touch_event;
 #endif
-  session->arg      = session;
-
-  /* Start the VNC server kernel thread. */
 
-  ret = vnc_start_server(display);
+#ifdef CONFIG_VNCSERVER_KBD
+  ret = snprintf(devname, sizeof(devname),
+                 CONFIG_VNCSERVER_KBD_DEVNAME "%d", display);
   if (ret < 0)
     {
-      gerr("ERROR: vnc_start_server() failed: %d\n", ret);
-#ifdef CONFIG_VNCSERVER_TOUCH
-      vnc_touch_unregister(session, devname);
-#endif
+      gerr("ERROR: Format vnc keyboard driver path failed.\n");
       return ret;
     }
 
-  /* Wait for the VNC server to be ready */
-
-  ret = vnc_wait_start(display);
+  ret = vnc_kbd_register(devname, session);
 
   if (ret < 0)
     {
-      gerr("ERROR: wait for vnc server start failed: %d\n", ret);
+      gerr("ERROR: Initial vnc keyboard driver failed.\n");
+      return ret;
     }
 
+  session->kbdout = vnc_kbd_event;
+#endif
+
+  session->arg      = session;

Review Comment:
   Move after line 632



-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918762510


##########
drivers/video/vnc/vnc_receiver.c:
##########
@@ -321,8 +321,14 @@ int vnc_receiver(FAR struct vnc_session_s *session)
                   /* Inject the key press/release event into NX */
 
                   keyevent = (FAR struct rfb_keyevent_s *)session->inbuf;
+#ifndef CONFIG_VNCSERVER_KBD
                   vnc_key_map(session, rfb_getbe32(keyevent->key),
                               (bool)keyevent->down);
+#else
+                  /* Pass raw keycode to keyboard driver */
+
+                  session->kbdout(session, keyevent->down, keyevent->key);

Review Comment:
   So, how about keep this code here, and remove vnc_key_map from vncserver, and add a keyboard/input reader to NX to unify the semantic of kbdout and IO system in vncserver (In further patch)?



-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r917695002


##########
drivers/video/vnc/vnc_fbdev.c:
##########
@@ -577,6 +577,64 @@ static inline int vnc_wait_start(int display)
   return OK;
 }
 
+/****************************************************************************
+ * Name: vnc_session_alloc
+ *
+ * Description:
+ *   Alloc vnc session and init essential member of it.
+ *
+ ****************************************************************************/
+
+static int vnc_session_alloc(void)
+{
+  FAR struct vnc_session_s *session;
+  int display;
+  int ret = OK;
+  FAR uint8_t *fb;
+
+  for (display = 0; display < RFB_MAX_DISPLAYS; display++)

Review Comment:
   Yes, we should move touch/kbd driver after vnc_start_server or apply the second commit to init session in up_fbinitialize



-- 
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


[GitHub] [incubator-nuttx] acassis commented on pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#issuecomment-1180361929

   @no1wudi I'm looking for a VNC client on NuttX side, do you have some suggestion how I could implement it? I think a VNC client should be userspace only and could use framebuffer interface, right?


-- 
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


[GitHub] [incubator-nuttx] acassis merged pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
acassis merged PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596


-- 
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


[GitHub] [incubator-nuttx] no1wudi commented on a diff in pull request #6596: vncserver enhancement

Posted by GitBox <gi...@apache.org>.
no1wudi commented on code in PR #6596:
URL: https://github.com/apache/incubator-nuttx/pull/6596#discussion_r918663904


##########
drivers/video/vnc/vnc_receiver.c:
##########
@@ -321,8 +321,14 @@ int vnc_receiver(FAR struct vnc_session_s *session)
                   /* Inject the key press/release event into NX */
 
                   keyevent = (FAR struct rfb_keyevent_s *)session->inbuf;
+#ifndef CONFIG_VNCSERVER_KBD
                   vnc_key_map(session, rfb_getbe32(keyevent->key),
                               (bool)keyevent->down);
+#else
+                  /* Pass raw keycode to keyboard driver */
+
+                  session->kbdout(session, keyevent->down, keyevent->key);

Review Comment:
   Is it better to move keycode decode to application layer from this virtual keyboard driver ?



-- 
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