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 2020/10/28 08:53:55 UTC

[GitHub] [incubator-nuttx-apps] spiriou opened a new pull request #445: Android Debug Bridge daemon

spiriou opened a new pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445


   ## Summary
   
   add Android Debug Bridge daemon for NuttX.
   https://developer.android.com/studio/command-line/adb
   
   ## Impact
   
   None. Disabled by default.
   
   ## Testing
   
   Validated on "sim" board (TCP) and stm32 (both TCP and USB).
   
   There are still some issues that will be address soon:
   * Authentication check disabled due to missing RSA crypto lib in NuttX
   * Some instabilities and memory leak using "adb shell"
   * adb forward/reverse (socket) disabled for now as it needs a refactoring
   
   defconfig for test purposes can be found at https://github.com/spiriou/incubator-nuttx/commits/adb_examples
   


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

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



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514276663



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       Yes, you can reference how telnetd integrate with nshlib here:
   https://github.com/apache/incubator-nuttx-apps/blob/master/nshlib/nsh_telnetd.c
   https://github.com/apache/incubator-nuttx-apps/blob/master/netutils/telnetd/telnetd_daemon.c
   




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r518775255



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       The shell implementation could be based on ptmx like in https://github.com/apache/incubator-nuttx-apps/tree/master/examples/pty_test instead of pipes but this can be improved in another 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.

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r519422732



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       @xiaoxiang781216 it's not perfect but this can be merged as it's working fine. I'll improve it later.




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

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



[GitHub] [incubator-nuttx-apps] acassis merged pull request #445: Android Debug Bridge daemon

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


   


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

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



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514355993



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       One more simple example to integrate nsh:
   https://github.com/apache/incubator-nuttx-apps/tree/master/examples/pty_test




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

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



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r513422291



##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072

Review comment:
       can we use DEFAULT_TASK_STACKSIZE?

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"

Review comment:
       change all adb to adbd?

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"

Review comment:
       change to tristate, so we can move adbd to file system

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"
+	default 100
+	---help---
+		The priority of the adb task.
+
+config ADBD_AUTHENTICATION
+	bool "adbd authentication support"
+	default n
+	---help---
+		Enable authentication for adb server.
+
+if ADBD_AUTHENTICATION
+
+config ADBD_AUTH_PUBKEY
+	bool "adbd public key authentication"
+	default n
+	---help---
+		Enable hook to accept new public keys.
+
+config ADBD_TOKEN_SIZE
+	int "authentication token size"
+	default 20
+
+endif # ADBD_AUTHENTICATION
+
+config ADBD_DEVICE_ID

Review comment:
       how about we get device id from BOARDIOC_UNIQUEID

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"
+	default 100
+	---help---
+		The priority of the adb task.
+
+config ADBD_AUTHENTICATION
+	bool "adbd authentication support"
+	default n
+	---help---
+		Enable authentication for adb server.
+
+if ADBD_AUTHENTICATION
+
+config ADBD_AUTH_PUBKEY
+	bool "adbd public key authentication"
+	default n
+	---help---
+		Enable hook to accept new public keys.
+
+config ADBD_TOKEN_SIZE
+	int "authentication token size"
+	default 20
+
+endif # ADBD_AUTHENTICATION
+
+config ADBD_DEVICE_ID
+	string "default adb device id"
+	default ""
+
+config ADBD_PRODUCT_NAME
+	string "default adb product name"
+	default "adb dev"
+
+config ADBD_PRODUCT_MODEL
+	string "default adb product model"
+	default "adb board"
+
+config ADBD_PRODUCT_DEVICE
+	string "default adb product device"
+	default "NuttX device"
+
+config ADBD_FEATURES

Review comment:
       should we auto generate the feature string by check wether user select  ADBD_FILE_SERVICE, ADBD_SHELL_SERVICE...?

##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"
+CONFIG_ADBD_VERSION ?= 534cfcf5ccd38b228d89da6d57bd72b8038cf2dd
+
+ADB_UNPACKNAME := microADB
+ADB_UNPACKDIR := $(ADB_DIR)/$(ADB_UNPACKNAME)
+
+$(ADB_UNPACKDIR):
+	@echo "Downloading: $(ADB_UNPACKNAME)"
+	$(call DELDIR, "$@")
+	$(Q) mkdir "$@"
+	$(Q) cd "$@" && git init && \
+	git remote add origin "$(CONFIG_ADBD_URL)" && \
+	git fetch origin $(CONFIG_ADBD_VERSION) --depth=1 && \
+	git reset --hard FETCH_HEAD
+
+# adb server app
+
+CONFIG_ADBD_PRIORITY ?= SCHED_PRIORITY_DEFAULT
+CONFIG_ADBD_STACKSIZE ?= 2048
+CONFIG_ADBD_PROGNAME ?= adbd$(EXEEXT)

Review comment:
       should we add CONFIG_ADBD_PROGNAME to Kconfig like other apps

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"
+	default 100
+	---help---
+		The priority of the adb task.
+
+config ADBD_AUTHENTICATION
+	bool "adbd authentication support"
+	default n
+	---help---
+		Enable authentication for adb server.
+
+if ADBD_AUTHENTICATION
+
+config ADBD_AUTH_PUBKEY
+	bool "adbd public key authentication"
+	default n
+	---help---
+		Enable hook to accept new public keys.
+
+config ADBD_TOKEN_SIZE
+	int "authentication token size"
+	default 20
+
+endif # ADBD_AUTHENTICATION
+
+config ADBD_DEVICE_ID
+	string "default adb device id"
+	default ""
+
+config ADBD_PRODUCT_NAME
+	string "default adb product name"
+	default "adb dev"
+
+config ADBD_PRODUCT_MODEL
+	string "default adb product model"
+	default "adb board"
+
+config ADBD_PRODUCT_DEVICE
+	string "default adb product device"
+	default "NuttX device"
+
+config ADBD_FEATURES
+	string "default adb server features list"
+	default "cmd"
+
+config ADBD_PAYLOAD_SIZE
+	int "Normal ADB frame size"
+	default 64
+	---help---
+		Normal frame size in bytes.
+
+config ADBD_CNXN_PAYLOAD_SIZE
+	int "Connection frame size"
+	default 1024
+	---help---
+		Connection frame is bigger than others.
+		Can be between 128 to 256 bytes in most of the cases.
+		If authentication is enabled, frame size must bigger
+		to receive public key from host (around 1024 bytes).
+
+config ADBD_FRAME_MAX
+	int "Frame pool size"
+	default 1
+	---help---
+		ADB frame pool size.
+
+config ADBD_TCP_SERVER
+	bool "Network socket transport support"
+	default n

Review comment:
       depend on TCP

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"
+	default 100
+	---help---
+		The priority of the adb task.
+
+config ADBD_AUTHENTICATION
+	bool "adbd authentication support"
+	default n
+	---help---
+		Enable authentication for adb server.
+
+if ADBD_AUTHENTICATION
+
+config ADBD_AUTH_PUBKEY
+	bool "adbd public key authentication"
+	default n
+	---help---
+		Enable hook to accept new public keys.
+
+config ADBD_TOKEN_SIZE
+	int "authentication token size"
+	default 20
+
+endif # ADBD_AUTHENTICATION
+
+config ADBD_DEVICE_ID
+	string "default adb device id"
+	default ""
+
+config ADBD_PRODUCT_NAME
+	string "default adb product name"
+	default "adb dev"
+
+config ADBD_PRODUCT_MODEL
+	string "default adb product model"
+	default "adb board"
+
+config ADBD_PRODUCT_DEVICE
+	string "default adb product device"
+	default "NuttX device"
+
+config ADBD_FEATURES
+	string "default adb server features list"
+	default "cmd"
+
+config ADBD_PAYLOAD_SIZE
+	int "Normal ADB frame size"
+	default 64
+	---help---
+		Normal frame size in bytes.
+
+config ADBD_CNXN_PAYLOAD_SIZE
+	int "Connection frame size"
+	default 1024
+	---help---
+		Connection frame is bigger than others.
+		Can be between 128 to 256 bytes in most of the cases.
+		If authentication is enabled, frame size must bigger
+		to receive public key from host (around 1024 bytes).
+
+config ADBD_FRAME_MAX
+	int "Frame pool size"
+	default 1
+	---help---
+		ADB frame pool size.
+
+config ADBD_TCP_SERVER
+	bool "Network socket transport support"
+	default n
+	---help---
+		Run adb server on network socket.
+
+config ADBD_TCP_SERVER_PORT
+	int "Network socket transport port"
+	depends on ADBD_TCP_SERVER
+	default 5555
+	---help---
+		Port used by adb socket server
+
+config ADBD_USB_SERVER
+	bool "USB transport support"
+	default n
+	---help---
+		Run adb server on USB bus
+
+config ADBD_LOGCAT_SERVICE
+	bool "adb logcat support"
+	depends on RAMLOG_SYSLOG
+	default n
+	---help---
+		Enable "adb logcat" feature.
+
+config ADBD_FILE_SERVICE
+	bool "adb file sync support"
+	default n
+	---help---
+		Enable "adb ls/push/pull" feature.
+
+ config ADBD_SHELL_SERVICE
+	bool "adb shell support"
+	default n

Review comment:
       should we depend on NSH

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER

Review comment:
       change all ADB_ to ADBD_

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n

Review comment:
       need depends or select kernel adb driver, or adbd can work without usb but tcp?

##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072
+	---help---
+		The size of stack allocated for the adb task.
+
+config ADBD_PRIORITY
+	int "adb task priority"
+	default 100
+	---help---
+		The priority of the adb task.
+
+config ADBD_AUTHENTICATION
+	bool "adbd authentication support"
+	default n
+	---help---
+		Enable authentication for adb server.
+
+if ADBD_AUTHENTICATION
+
+config ADBD_AUTH_PUBKEY
+	bool "adbd public key authentication"
+	default n
+	---help---
+		Enable hook to accept new public keys.
+
+config ADBD_TOKEN_SIZE
+	int "authentication token size"
+	default 20
+
+endif # ADBD_AUTHENTICATION
+
+config ADBD_DEVICE_ID
+	string "default adb device id"
+	default ""
+
+config ADBD_PRODUCT_NAME
+	string "default adb product name"
+	default "adb dev"
+
+config ADBD_PRODUCT_MODEL
+	string "default adb product model"
+	default "adb board"
+
+config ADBD_PRODUCT_DEVICE
+	string "default adb product device"
+	default "NuttX device"
+
+config ADBD_FEATURES
+	string "default adb server features list"
+	default "cmd"
+
+config ADBD_PAYLOAD_SIZE
+	int "Normal ADB frame size"
+	default 64
+	---help---
+		Normal frame size in bytes.
+
+config ADBD_CNXN_PAYLOAD_SIZE
+	int "Connection frame size"
+	default 1024
+	---help---
+		Connection frame is bigger than others.
+		Can be between 128 to 256 bytes in most of the cases.
+		If authentication is enabled, frame size must bigger
+		to receive public key from host (around 1024 bytes).
+
+config ADBD_FRAME_MAX
+	int "Frame pool size"
+	default 1
+	---help---
+		ADB frame pool size.
+
+config ADBD_TCP_SERVER
+	bool "Network socket transport support"
+	default n
+	---help---
+		Run adb server on network socket.
+
+config ADBD_TCP_SERVER_PORT
+	int "Network socket transport port"
+	depends on ADBD_TCP_SERVER
+	default 5555
+	---help---
+		Port used by adb socket server
+
+config ADBD_USB_SERVER
+	bool "USB transport support"
+	default n

Review comment:
       depend on adb usb driver

##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"
+CONFIG_ADBD_VERSION ?= 534cfcf5ccd38b228d89da6d57bd72b8038cf2dd
+
+ADB_UNPACKNAME := microADB
+ADB_UNPACKDIR := $(ADB_DIR)/$(ADB_UNPACKNAME)
+
+$(ADB_UNPACKDIR):
+	@echo "Downloading: $(ADB_UNPACKNAME)"
+	$(call DELDIR, "$@")
+	$(Q) mkdir "$@"
+	$(Q) cd "$@" && git init && \
+	git remote add origin "$(CONFIG_ADBD_URL)" && \
+	git fetch origin $(CONFIG_ADBD_VERSION) --depth=1 && \
+	git reset --hard FETCH_HEAD
+
+# adb server app
+
+CONFIG_ADBD_PRIORITY ?= SCHED_PRIORITY_DEFAULT
+CONFIG_ADBD_STACKSIZE ?= 2048
+CONFIG_ADBD_PROGNAME ?= adbd$(EXEEXT)

Review comment:
       line 41 to line 43 isn't needed

##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,
+                       FAR shell_pipe_t *apipe)
+{
+  FAR const struct builtin_s *builtin;
+  posix_spawnattr_t attr;
+  posix_spawn_file_actions_t file_actions;
+  struct sched_param param;
+  pid_t pid;
+  int index;
+  int ret;
+
+  /* Verify that an application with this name exists */
+
+  index = builtin_isavail(appname);

Review comment:
       should we support to launch program on file system?

##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"
+CONFIG_ADBD_VERSION ?= 534cfcf5ccd38b228d89da6d57bd72b8038cf2dd
+
+ADB_UNPACKNAME := microADB
+ADB_UNPACKDIR := $(ADB_DIR)/$(ADB_UNPACKNAME)
+
+$(ADB_UNPACKDIR):
+	@echo "Downloading: $(ADB_UNPACKNAME)"
+	$(call DELDIR, "$@")
+	$(Q) mkdir "$@"
+	$(Q) cd "$@" && git init && \
+	git remote add origin "$(CONFIG_ADBD_URL)" && \
+	git fetch origin $(CONFIG_ADBD_VERSION) --depth=1 && \
+	git reset --hard FETCH_HEAD
+
+# adb server app
+
+CONFIG_ADBD_PRIORITY ?= SCHED_PRIORITY_DEFAULT
+CONFIG_ADBD_STACKSIZE ?= 2048
+CONFIG_ADBD_PROGNAME ?= adbd$(EXEEXT)
+PROGNAME = $(CONFIG_ADBD_PROGNAME)

Review comment:
       dup with line 46

##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"

Review comment:
       can we directly import microADB into apps git?

##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       why not call system or nsh api instead?




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514131544



##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n

Review comment:
       The idea is once the server is enabled we can select at least one of USB or TCP transports. Or both if needed.
   ADBD_USB must indeed depend on kernel adb 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.

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



[GitHub] [incubator-nuttx-apps] acassis commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r518767478



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       LGTM? @xiaoxiang781216 @spiriou should I?




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514128734



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,
+                       FAR shell_pipe_t *apipe)
+{
+  FAR const struct builtin_s *builtin;
+  posix_spawnattr_t attr;
+  posix_spawn_file_actions_t file_actions;
+  struct sched_param param;
+  pid_t pid;
+  int index;
+  int ret;
+
+  /* Verify that an application with this name exists */
+
+  index = builtin_isavail(appname);

Review comment:
       Indeed this can be simplified. This function is the same as the one in ./builtin/exec_builtin.c but with more "posix_spawn_file_actions_addopen()" to setup correctly stdin and stdout streams.




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

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



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r519275967



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       @spiriou is this patch ready for merge?




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514127627



##########
File path: system/adb/shell_exec_builtin.c
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * system/adb/shell_pipe_uv.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 <spawn.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "adb.h"
+#include "builtin/builtin.h"
+#include "shell_pipe.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int shell_exec_builtin(FAR const char *appname, FAR char * const *argv,

Review comment:
       This is a first iteration regarding NSH integration in "adb shell" command. The technical solution is to leverage pipe for both stdout and stdin streams. The issue is I need a custom function to configure fd 0 and fd 1 for NSH process. That's what shell_exec_builtin() does (based on ./builtin/exec_builtin.c but with more posix_spawn_file_actions_addopen()).
   
   Next step could be to try integrating nshlib directly in adbd thread. It may simplify this logic and save lot of RAM.




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514132601



##########
File path: system/adb/Kconfig
##########
@@ -0,0 +1,147 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig ADB_SERVER
+	bool "adb server command"
+	default n
+	---help---
+		Enable support for adb server.
+
+if ADB_SERVER
+
+config ADBD_STACKSIZE
+	int "adb stack size"
+	default 3072

Review comment:
       Sure. For now the stack requirement is way to high (3k). In the initial adbd version the stack usage was around 1500 bytes but I'll optimize it later.




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

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



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514130178



##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"

Review comment:
       Initially this project was not meant for NuttX so I would prefer to keep the platform independent code it in a separate repository for now. Is this an issue ?




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

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



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #445: Android Debug Bridge daemon

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #445:
URL: https://github.com/apache/incubator-nuttx-apps/pull/445#discussion_r514269483



##########
File path: system/adb/Makefile
##########
@@ -0,0 +1,98 @@
+############################################################################
+# system/adb/Makefile
+#
+# 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.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+ADB_DIR := $(APPDIR)/system/adb
+CONFIG_ADBD_URL ?= "https://github.com/spiriou/microADB.git"

Review comment:
       No problem.




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

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