Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disconnecting and re-connecting #419

Closed
1 task done
stylesuxx opened this issue May 15, 2022 · 8 comments
Closed
1 task done

Disconnecting and re-connecting #419

stylesuxx opened this issue May 15, 2022 · 8 comments
Assignees
Labels
domain: dependencies This relates to external dependencies type:bug Something isn't working
Milestone

Comments

@stylesuxx
Copy link

stylesuxx commented May 15, 2022

Issue Checklist

  • I'm using the library programmatically

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?

@stylesuxx stylesuxx added the type:bug Something isn't working label May 15, 2022
@yume-chan
Copy link
Owner

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.

@yume-chan yume-chan self-assigned this May 17, 2022
@yume-chan yume-chan added this to the 0.0.16 milestone May 17, 2022
@alexlabs
Copy link

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

@yume-chan
Copy link
Owner

yume-chan commented May 21, 2022

Upstream issue filed:

@yume-chan yume-chan added the domain: dependencies This relates to external dependencies label May 21, 2022
@yume-chan
Copy link
Owner

yume-chan commented May 22, 2022

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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

@yume-chan
Copy link
Owner

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'));

Repository owner locked as resolved and limited conversation to collaborators Feb 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
domain: dependencies This relates to external dependencies type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants