alvr-org / ALVR

Stream VR games from your PC to your headset via Wi-Fi
MIT License
5.57k stars 495 forks source link

feat: integrate adb port forwarding #2451

Closed E1int closed 3 weeks ago

E1int commented 1 month ago

This PR adds a toggle for wired connections in the "Devices" tab. When enabled, ALVR manages the forwarding of the required ports using adb. If a system installation of adb is present, it will be used, falling back to downloading and installing a local copy otherwise.

Closes #567, closes #2349.

zmerp commented 1 month ago

Why didn't you use mozdevice as you said initially?

E1int commented 1 month ago

I initially did but then I was told about the launcher using adb, which uses it through commands, so I thought we could avoid a dependency going the same route. I can switch back to mozdevice, but we would still need to start the adb server with a command.

zmerp commented 1 month ago

We should use the dependency if it reduces the amount of boilerplate

E1int commented 1 month ago

No, strum is not strictly needed, I'll use a match then.

As for mozdevice, the parsing is very barebones: https://docs.rs/mozdevice/latest/src/mozdevice/lib.rs.html#93 I've also looked at how SharpAdbClient (the library that ADBForwarder uses) does it and decided against following the same approach: https://github.com/quamotion/madb/blob/master/SharpAdbClient/DeviceData.cs#L19

vosk commented 1 month ago

There is also https://docs.rs/adb_client/latest/adb_client/index.html

zmerp commented 1 month ago

There is also https://docs.rs/adb_client/latest/adb_client/index.html

To be honest this one seems exactly what we need, and no need to download adb. It only implements the sockets part of the protocol, so it cannot be used for the launcher also.

E1int commented 1 month ago

That library needs an adb server to work, so the download is still needed: https://docs.rs/adb_client/latest/src/adb_client/server/adb_server.rs.html#52

zmerp commented 1 month ago

Ah hm. In any case seems a good option

zmerp commented 1 month ago

This is shaping very nicely. let me know when it's ready for the final review

zmerp commented 3 weeks ago

I realized that some of the nits I pointed out don't work because of lifetimes. You can ignore those