Skip to content

New features and bugfix for microADB - #42

Draft
JianyuWang0623 wants to merge 28 commits into
spiriou:masterfrom
JianyuWang0623:br_wjy_microadb_1011
Draft

New features and bugfix for microADB#42
JianyuWang0623 wants to merge 28 commits into
spiriou:masterfrom
JianyuWang0623:br_wjy_microadb_1011

Conversation

@JianyuWang0623

Copy link
Copy Markdown
Contributor

Summary

New features for microADB, please see the commit message for details.

  • a34e817: The host report "error: closed" after adb reboot
  • 861e8cb: Fix crash
  • 19345e8: Skip environment variables when exec logcat

Details for 19345e8

  • Log
$ adb -s emulator-5554 logcat
nsh: export: command not found
  • Packet
0000   00 00 00 00 00 00 00 00 00 00 00 00 08 00 45 00   ..............E.
0010   00 64 44 d7 40 00 40 06 f7 ba 7f 00 00 01 7f 00   .dD.@.@.........
0020   00 01 d6 05 15 b3 5f a7 fe 39 f6 e2 f8 80 80 18   ......_..9......
0030   02 00 fe 58 00 00 01 01 08 0a 29 24 9c b4 29 24   ...X......)$..)$
0040   9c b4 73 68 65 6c 6c 3a 65 78 70 6f 72 74 20 41   ..shell:export A
0050   4e 44 52 4f 49 44 5f 4c 4f 47 5f 54 41 47 53 3d   NDROID_LOG_TAGS=
0060   22 27 27 22 3b 20 65 78 65 63 20 6c 6f 67 63 61   "''"; exec logca
0070   74 00

Test

  1. Selftest
  2. CI

dengwenqi123 and others added 28 commits October 11, 2024 16:49
Signed-off-by: dengwenqi <dengwenqi@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
uv__run_closing_handles libuv/src/unix/core.c:365
uv_run libuv/src/unix/core.c:464
adb_hal_run microADB/hal/hal_uv.c:69
adbd_main /home/djz/workspace/vela_rp/apps/system/adb/adb_main.c:157
nxtask_startup sched/task_startup.c:70
nxtask_start task/task_start.c:134
pre_start sim/sim_initialstate.c:52

Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
host_memalign sim/posix/sim_hostmemory.c:168
host_realloc sim/posix/sim_hostmemory.c:193
mm_realloc sim/sim_heap.c:334
mm_malloc sim/sim_heap.c:272
mm_zalloc sim/sim_heap.c:402
mm_calloc sim/sim_heap.c:387
calloc umm_heap/umm_calloc.c:73
uv__calloc libuv/src/uv-common.c:94
uv_loop_init libuv/src/unix/loop.c:40
uv_default_loop libuv/src/uv-common.c:821
adb_hal_create_context microADB/hal/hal_uv.c:35
adbd_main /home/djz/workspace/vela_rp/apps/system/adb/adb_main.c:151
nxtask_startup sched/task_startup.c:70
nxtask_start task/task_start.c:134
pre_start sim/sim_initialstate.c:52

Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
__assert
nuttx/libs/libc/assert/lib_assert.c:36
usb_uv_kick
apps/system/adb/microADB/hal/hal_uv_client_usb.c:197
adb_hal_apacket_release
/media/liangchaozhong/ssd/x4b/apps/system/adb/microADB/hal/hal_uv_packet.c:55
adb_uv_close_client
apps/system/adb/microADB/hal/hal_uv.c:100
usb_uv_on_close
apps/system/adb/microADB/hal/hal_uv_client_usb.c:209
uv__finish_close
apps/system/libuv/libuv/src/unix/core.c:352
adb_hal_run
apps/system/adb/microADB/hal/hal_uv.c:69
adbd_main

Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
and apply other minor style change

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
since ID_QUIT is always handled in state_wait_cmd

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
…e_wait_cmd_data

since the same check is already done in read_from_packet and state_wait_cmd

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
instead of syncmsg::dent and syncmsg::data

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
since it's never used at all

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
This reverts commit 917f97deadd1d70e58347098086cf5ed8e8d0254.

Reason for revert: pc need to recv header first
Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
…eing overwritten

When the windows/mac PC sends multiple files in a write packet, and the last DONE packet sent is incomplete,
the remaining bytes of DONE are in the next wirte packet, which will lead to the following two situations.

1. When the sync message has not been completely received, we should maintain the state machine state

current frame:
2f 63 6f 6d 2e 76 65 6c 61 2e 63 68 61 72 74 2e  /com.vela.chart.
64 65 6d 6f 2f 69 31 38 6e 2f 65 6e 2e 6a 73 6f  demo/i18n/en.jso
6e 2c 33 33 32 30 36 44 41 54 41 13 00 00 00 7b  n,33206DATA....{
22 61 22 3a 7b 22 62 22 3a 22 68 65 6c 6c 6f 22  "a":{"b":"hello"
7d 7d 44 4f 4e 45 88 e2                          }}DONE..

next frame:
57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00  WRTE:...........
c6 ac 01 00 a8 ad ab ba 88 65 53 45 4e 44 43 00  .........eSENDC.
00 00 2f 64 61 74 61 2f 63 6f 6d 2e 76 65 6c 61  ../data/com.vela
2e 63 68 61 72 74 2e 64 65 6d 6f 2f 63 6f 6d 2e  .chart.demo/com.
76 65 6c 61 2e 63 68 61 72 74 2e 64 65 6d 6f 2f  vela.chart.demo/

The complete DONE package is: | 44 4f 4e 45 88 e2 | 88 65 |
                                   current          next

2. When the device receives DONE, it needs to reply OKAY to the HOST.If the current apcket is reused,
OKAY will overwrite the unprocessed content, so a local buffer needs to be used.

before wirte:

57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00  WRTE:...........
c6 ac 01 00 a8 ad ab ba 88 65 53 45 4e 44 43 00  .........eSENDC.

after write:

57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00  WRTE:...........
c6 ac 01 00 a8 ad ab ba 41 59 00 00 00 00 00 00  ........AY......
                        ^                    ^

the buffer ".eSENDC" -> "AY......"

Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
Signed-off-by: guohao15 <guohao15@xiaomi.com>
Signed-off-by: getian <getian@xiaomi.com>
Log:
  $ adb -s emulator-5554 logcat
  nsh: export: command not found

Packet:
  0000   00 00 00 00 00 00 00 00 00 00 00 00 08 00 45 00   ..............E.
  0010   00 64 44 d7 40 00 40 06 f7 ba 7f 00 00 01 7f 00   .dD.@.@.........
  0020   00 01 d6 05 15 b3 5f a7 fe 39 f6 e2 f8 80 80 18   ......_..9......
  0030   02 00 fe 58 00 00 01 01 08 0a 29 24 9c b4 29 24   ...X......)$..)$
  0040   9c b4 73 68 65 6c 6c 3a 65 78 70 6f 72 74 20 41   ..shell:export A
  0050   4e 44 52 4f 49 44 5f 4c 4f 47 5f 54 41 47 53 3d   NDROID_LOG_TAGS=
  0060   22 27 27 22 3b 20 65 78 65 63 20 6c 6f 67 63 61   "''"; exec logca
  0070   74 00                                             t.

Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
There is a situation similar to "adb shell ps", after the device
response data to the host, the host sends okay packet, and then
the device should return close packet. In the original code, it
is possible that the child process is slow to execute, resulting
in the main process receiving okay packet, but the svc has not
been released yet, resulting in no close packet being returned.

Signed-off-by: yangsong8 <yangsong8@xiaomi.com>
This reverts commit 037d0f1f04518837b26888cc2d3c451be08ea7fd.

Reason for revert: can not reserve problem well

Signed-off-by: yangsong8 <yangsong8@xiaomi.com>
There is a situation similar to "adb shell ps", after the device
response data to the host, the host sends okay packet, and then
the device should return close packet. In the original code, it
is possible that the child process is slow to execute, resulting
in the main process receiving okay packet, but the svc has not
been released yet, resulting in no close packet being returned.

Signed-off-by: yangsong8 <yangsong8@xiaomi.com>
Signed-off-by: yangsong8 <yangsong8@xiaomi.com>
@JianyuWang0623

JianyuWang0623 commented Oct 11, 2024

Copy link
Copy Markdown
Contributor Author

Most of commits by @xiaoxiang781216@Donny9@yangsong8-ai@dengwenqi123 and @guohao15
CC: @GUIDINGLI

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@JianyuWang0623 maybe need split the huge pr to small pr.

@JianyuWang0623
JianyuWang0623 marked this pull request as draft October 11, 2024 10:19
@JianyuWang0623

Copy link
Copy Markdown
Contributor Author

@JianyuWang0623 maybe need split the huge pr to small pr.

OK, set to draft, and will split later.

Comment thread .gitignore
@@ -0,0 +1,2 @@
*.o

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use another build folder with CMake, it doesn't have to be in the source folder.

@xiaoxiang781216 xiaoxiang781216 Oct 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spiriou I think this change is for Makefile only since NuttX Makefile based build system doesn't support the out of tree.

Comment thread hal/hal_uv.c

adb_context_t* adb_hal_create_context(void) {
adb_context_uv_t *adbd = &g_adbd_context;
adb_context_uv_t *adbd = malloc(sizeof(adb_context_uv_t));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the usecase of having multiple contexts?
can't we fit everything into the same event loop?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, just make the code simple and avoid consuming memory before launch adbd.

Comment thread hal/hal_uv_client_usb.c
static int usb_uv_write(adb_client_t *c, apacket *p) {
int ret;
uv_buf_t buf;
uv_buf_t buf[2];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JianyuWang0623 let's drop the patch and reverted patch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JianyuWang0623 let's drop the patch and reverted patch

get, will update later

Comment thread adb_client.c Outdated
svc = adb_client_find_service(client, p->msg.arg1, 0);
if (!svc) {
adb_hal_apacket_release(client, p);
send_close_frame(client, p, p->msg.arg1, p->msg.arg0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's drop this and previous patch

@JianyuWang0623

JianyuWang0623 commented Oct 13, 2024

Copy link
Copy Markdown
Contributor Author

Sorry for the large PR, splited to below (doing, part):

  1. QEMU PIPE - Implement ADB function through qemu pipe #43
  2. Hotplug detect - Support USB hotplug detecting for microADB #44
  3. Bug fixes for ADB - Bug fixes for ADB #45
  4. Fix environment error for ADB - Skip environment variables when exec logcat #46
  5. TODO: The remaining 10 commits depend on Bug fixes for ADB #45, will update this PR after it merged.

Keep Draft status until all other related PRs were merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants