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 16:32:14 UTC

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

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