feat: connect adb over wifi

[itsspriyansh]

Feature Implemented

Added functionality to connect to Android Debug Bridge (ADB) wirelessly using a pairing code. This feature enhances the flexibility of connecting to devices, particularly useful in scenarios where USB connection is not feasible.

Changes Made

Added script to achieve the functionality in android/utils/connect-device.ts.

Additional Notes

This feature adds convenience and flexibility to the ADB connection process, especially in scenarios where physical USB connections are impractical or unavailable. The implementation adheres to best practices and maintains compatibility with existing codebase and dependencies.

Resolves: #19

simplescreenrecorder-2024-03-04_23.35.12.mp4

[Biki-das]

Nice work @itsspriyansh , but the implementation seems to be wrong, since it would be a complete walkthrough and not capturing it from adb , the user would manually enter the adb addr:port and pairing code to connect the same at the same time running the helper tool
see #19 (comment)
this sort of script has to be added

[itsspriyansh]

Hey @Biki-das, thanks for pointing that out. I have corrected the changes now.

[garg3133]

Great work on this @itsspriyansh.

But, this should not be a part of the main setup. The main setup is used to ensure that all the requirements that we would potentially need to download are present on the system, and if not, download the requirements. Connecting a device to adb wirelessly is not such a requirement and should be kept separate for the following reasons:

• At the time of the initial setup, the user might not be ready with a device to connect to wirelessly or using a USB cable, they just want to setup all the requirements. And if we, in the middle of that setup, ask user to connect to the device as well, that creates friction as the user would now need to do much more work to complete the setup, on the contrary of just letting the tool handle everything.
• Connecting/Disconnecting a device to adb is a repetitive task and user shouldn't need to run the setup command everytime they need to connect to an Android device wirelessly.

So, instead of asking user to connect to a device at the time of setup, we can provide user with a separate command that they can run to do connect to the Android device wirelessly, an number of time as they like to.

The command can be something like: npx @nightwatch/mobile-helper android --wireless-connection.

[garg3133]

Btw, to pass the flags in the dev run, you'd need to prepend the arguments with a --. Ex. the above command in development would look like: npm run dev -- android --wireless-connection.

Also, I haven't reviewed the code yet, will do so in a while.

[itsspriyansh]

@garg3133 I have made the suggested changes.

[garg3133]

@itsspriyansh Looks good. Two things:

• Can you also put some information for the user before asking the first question on how to get the details that are being asked in the questions below or the steps that the user should follow to get the answers? We can't expect user to know everything about setting up adb wirelessly before running the tool, it is kind of our job to instruct the user. (You can also later on help with a detailed doc for setting up adb wirelessly from Document Android SDK commands #20).
• We don't expect the users to have the device connected with USB cable while trying to setup the wireless connection. The whole point of having wireless connection is for those who don't want to deal with the USB cables. Also, even if they have their device connected over USB cable, we expect them to remove the cable before trying to connect wirelessly.

[itsspriyansh]

@garg3133 I have added the instructions. I have also update the video preview in the PR description. You may take a look at it to review the changes.

[garg3133]

We would also need to provide a way for users to be able to disconnect a device as that is also not very straightforward.

Think of what flags we could use while running command for this purpose.

Now that we have both these use cases, I'm more inclined towards having the following commands:

# connecting a device
npx @nightwatch/mobile-helper android connect
# ^ will only show a list of devices connected because for USB or emulator connections
# we don't need to do anything to establish a connection.

# connecting a device wirelessly
npx @nightwatch/mobile-helper android connect --wireless
# ^ what's being done in this PR

# disconnecting a device
npx @nightwatch/mobile-helper android disconnect
# ^ will show a list of all the connected devices and ask a further question on which device to disconnect.

For this PR, let's just keep the scope to the second command above and the rest we can do later on.

[itsspriyansh]

@garg3133 I have updated the instructions and the video preview.

As for now, I have not implemented the use of connect argument. The script is currently running with
npx @nightwatch/mobile-helper android --wireless command.
I would require some more clarification regarding what cases to handle when connect is passed as argument.

[garg3133]

@itsspriyansh The flow is superb! Thank you. I'll do a more thorough review of the code later, but this looks good.

Regarding the connect argument, what I'm thinking is:

• If only connect argument is passed, we'll ask if they want to connect a device wirelessly.
  • if the answer is yes, we'll move to the flow in this PR.
  • if the answer is no, we'll just display all the connected devices (by running adb devices command)
• If connect is passed with --wireless flag, we'll use the flow from this PR.
• If disconnect argument is passed, we'll ask user to choose the device to disconnect (with a "All devices" option at the end) and then run the adb disconnect command accordingly.

But any of this does not need to be implemented in this current PR. The work on this PR is mostly complete I think.

[itsspriyansh]

Thank you @garg3133
I am excited for further contribution.

[itsspriyansh]

@garg3133 I have now handled the case when adb is not found. However, while testing this, I found that we will be able to install the missing binary using the --setup flag only if the entire platform-tools folder is missing and not just a specific binary inside it. However this isn't handled in the tool. Do we need to consider this case?

[itsspriyansh]

Whenever a command fails to run using the execBinarySync function we get an error message saying "failed to run ...command". I don't think in this case we need to show this message to the users since we are already showing messages like "pairing successful" or "pairing failed!". Can we consider implementing suppressOutput parameter in the execBinarySync function to avoid this?

[garg3133]

@itsspriyansh Have I not told you not to force push before? Doing force-push makes it impossible to review the diff and requires us to review the entire PR again.

Also, changes in other files are missing now from this PR, only one file is there now which can't be run on its own. Or is this intentional and you want aim to do the integration in other PRs?

[itsspriyansh]

@garg3133 Actually you haven't told me that. I will keep that in mind. And yes it is intentional. If we get the changes in other files merged, we will be overwriting them anyways in the next pr which I didn't think would be helpful. For now, I just wanted to add the script.

[garg3133]

However, while testing this, I found that we will be able to install the missing binary using the --setup flag only if the entire platform-tools folder is missing and not just a specific binary inside it. However this isn't handled in the tool. Do we need to consider this case?

I don't think that's true. I just tried it myself by removing the adb binary from the platform-tools folder and it correctly re-installed the platform-tools SDK and added it back for me.

Can you try this in your system once more? And if it still doesn't work, let me know of the steps you followed.

[garg3133]

Whenever a command fails to run using the execBinarySync function we get an error message saying "failed to run ...command". I don't think in this case we need to show this message to the users since we are already showing messages like "pairing successful" or "pairing failed!".

I think those error messages are still useful. The main purpose of those error messages is so that user knows what we're trying to do internally and if the command fails continuously they can just try it themselves instead of using the tool.

[garg3133]

I did some refactoring around the instructions and error messages in this PR, this is how the flow look like now:

Let me know if there's something that you think needs to be changed or fixed. Otherwise, I'll merge this PR later today.

Thanks a lot for working on this!

[itsspriyansh]

@garg3133 Thanks for the merge!
Regarding the missing abd, I manually deleted adb binary inside platform-tools and then executed this command as below:

As you can see, it is not getting installed in my case.

[garg3133]

@itsspriyansh This seems to be a bug in that case but the functionality is there. As you can see, it downloaded the platform-tools again to fix the missing adb binary but it seems that it wasn't able to replace the existing platform-tools directory with the new one for some reason.

Can you try to debug this?

[itsspriyansh]

@garg3133 What I have obeserved is that this issues lies with sdkmanager and not in our tool. I tried deleting adb and reinstalling it using the sdkmanager binary itself. However, it didn't work. If I clear all the contents of platform-tools or instead delete the folder itself, then it works. I was able to reproduce this issue in two of my devices (windows and ubuntu).