yume-chan / ya-webadb

ADB in your browser
https://tangoapp.dev/
MIT License
2.27k stars 298 forks source link

Disconnecting and re-connecting #419

Closed stylesuxx closed 2 years ago

stylesuxx commented 2 years ago

Issue Checklist

Library version

0.0.15

Runtime

Linux, Chrome Version 100.0.4896.127 (Official Build) (64-bit)

Device

DJI FPV goggles V2

Describe the bug

I have an issue with properly disconnecting from ADB (without physically unplugging the device) and later re-connecting. This also seems to be an issue in the Demo where I can't disconnect in the first place - pressing the "Disconnect" button does nothing.

const credentialStore = new AdbWebCredentialStore();
const device = await AdbWebUsbBackend.requestDevice();

let streams = await device.connect();
let adb = await Adb.authenticate(streams, credentialStore, undefined);

await adb.close();

let streams = await device.connect();
let adb = await Adb.authenticate(streams, credentialStore, undefined);

the second authenticate call never resolves.

Not sure if I am missing something or if this is a general problem (since the demo does not allow me to disconnect either).

Steps to reproduce

const credentialStore = new AdbWebCredentialStore();
const device = await AdbWebUsbBackend.requestDevice();

let streams = await device.connect();
let adb = await Adb.authenticate(streams, credentialStore, undefined);

await adb.close();

let streams = await device.connect();
let adb = await Adb.authenticate(streams, credentialStore, undefined);

EDIT: As a work-around I am saving the device returned by requestDevice or getDevices and simply close that - which does what I need it to do - I am just not sure if this is the intended way?

yume-chan commented 2 years ago

I remember I tested this part not long ago on Windows and it was working.

On macOS, a quick check shows the pipeTo didn't stop after calling abort.

It seems that the device, or the OS, is in a strange state, because I can't re-connect even after a browser restart, and sometimes the whole browser just crashes. I need to investigate more on this weekend.

alexlabs commented 2 years ago

Can not disconnect device in Edge on MacOS. Both Edge and Demo are latest.

yume-chan commented 2 years ago

Upstream issue filed:

yume-chan commented 2 years ago

I tested Chrome's native Web Streams implementation. It works, uses 17% more CPU but has 15% more throughputs than the polyfill. The memory usage is also slightly higher and results in more GC, but still acceptable. It doesn't support ReadableStream's async iteration but I'm not using this.

The main issue is still portability. Firefox 100 just added support for WritableStream and ReadableStream#pipeTo, but ReadableStream#pipeThrough requires next version. Node.js exports Web Streams from stream/web module, loading from it would require Top Level Await to maintain compatibility when targeting Web.

stylesuxx commented 2 years ago

The main issue is still portability. Firefox 100 just added support for WritableStream and ReadableStream#pipeTo, but ReadableStream#pipeThrough requires next version. Node.js exports Web Streams from stream/web module, loading from it would require Top Level Await to maintain compatibility when targeting Web.

Why does Firefox matter? I mean they still don't support Webusb, or did I miss something?

yume-chan commented 2 years ago

There is a WebSocket backend, when paired with WebSockify software, can be used to connect ADB over WiFi devices. I remember seeing someone make a Docker image with the demo and websockify.

I don't know if anyone is using Firefox for that, since there is no tracking in the demo. Technically it's possible and I want to keep this possibility.

Node.js support is also essential because the official demo uses Next.js, which does SSR. It will be a huge refactor to remove all ADB related code from SSR.

stylesuxx commented 2 years ago

Ah yeah - that makes sense then, I was not aware about the connection via WiFi. I would not break working functionality either.

yume-chan commented 2 years ago

0.0.16 released with upstream fix.

Note: if you really want to re-connect immediately, await adb.disconnected to ensure the streams are released. The streams can be re-used.

  const device = await AdbWebUsbBackend.requestDevice();
  let streams = await device.connect();

  let adb = await Adb.authenticate(streams, credentialStore, undefined);
  console.log(await adb.subprocess.spawnAndWaitLegacy('echo 1'));

  await adb.close();
+ await adb.disconnected;

  adb = await Adb.authenticate(streams, credentialStore, undefined);
  console.log(await adb.subprocess.spawnAndWaitLegacy('echo 1'));

Or if you want to create a new connection, you need to manually release the previous one.

  const device = await AdbWebUsbBackend.requestDevice();
  let streams = await device.connect();

  let adb = await Adb.authenticate(streams, credentialStore, undefined);
  console.log(await adb.subprocess.spawnAndWaitLegacy('echo 1'));

  await adb.close();
  await adb.disconnected;

+ await streams.readable.cancel();
+ streams = await device.connect();

  adb = await Adb.authenticate(streams, credentialStore, undefined);
  console.log(await adb.subprocess.spawnAndWaitLegacy('echo 1'));