adb_client
adb_client copied to clipboard
#9 Add windows support
I had problems compiling on windows, I saw that it was already known #9, I just conditioned termios and the interacive shell to compile only for unix.
After that, the question is: how do you implement it on windows?
I don't have a Windows machine to test it on, but ADBTermios struct is basically just here to set and restore a termios state. Have you tested to just remove these references in shell.rs file and launch it ?
I tried what you said and indeed, it does work on my Win10, we can use the interactive shell but the dep to termios still needs to be conditioned by unix.
Here a sceenshot of my terminal :
I'll update my PR
Sounds good then ! Of course termios needs to be a Unix-only dependency.
Two more things to be sure it really works :
- Can you try launching it via PowerShell too ?
- On a deeper level, are input sent to server directly when entered ? This is what termios was used to, but if Windows handles it directly this is good.
Thanks
About your first question, the answer is yes
Now, for the second one, I don't know how to check that to be honest
If you want, you can check in tools like Wireshark in gui or tcpdump if TCP data is sent at every keystroke
But if you say that this is "interactive", this is probably the case
Once you validate this I will validate this PR
So from what I see, It's not at each keystroke but after hitting "Enter" in windows. I can check if I found something as workaround but perhaps this feature won't work in the same way as on linux, given the dependency on the windows API
[EDIT] Maybe crossterm-rs crate can handle both linux and windows in an interactive way, I'll check that later
Would it be possible to move ADBServerDevice.shell() into adb_cli or somewhere else?
IMO it's strange having a method like that in a library in the first place.
@NutchapolSal why do you find it strange ?
ADB exposes shell functionality, I think that this is normal to have such a function implemented in the library, and not in the client.
This function should be hidden behind a cfg to ensure that it is not available but I truly think that it fits in this library.
why do you find it strange ?
I think it's because while the rest of the library is for use with other code, this method takes control of stdin/stdout and does something to the terminal directly.
Actually, if this method doesn't interact with the terminal, but instead returns (for example) a pair of mpsc::Sender and Receiver for the device's input and output so library users can programmatically send multiple commands without having to start a new shell connection, that would be much better. But currently, just gating it behind cfg is fine.
I want to use this library for a project I'm working on right now, but I'm on Windows, so I've tried removing ADBServerDevice.shell() and then the resulting unused code, turns out this also takes out mio from adb_client's dependencies.
Yes you're right, we can either use mpsc or just require a Read (that could be stdin) and a Write (that could be anything we want to write results to).
Feel free to provide an implementation, I'll have a look at it this week !
And yes, using a cfg will require to set up mio only for unix platforms as well. The job is already done in this PR but I do not have a Windows laptop to test it on... (I'm lazy yes I could set up a VM I know)
@vmillet-dev @NutchapolSal can you confirm that PR #35 works on windows ?
I moved ADBTermios in the CLI, and now pass a Writer and a Reader to .shell()
Closing as done in another PR