aboozaid / react-native-wifi-hotspot

A react-native implementation for handling hotspot requests and make it easier to communicate with peers!
ISC License
40 stars 21 forks source link

Refactor to use new `startLocalOnlyHotspot` logic #25

Closed Patrick-Erichsen closed 7 months ago

Patrick-Erichsen commented 4 years ago

Purpose

This PR refactors the library to use the new startLocalOnlyHotspot method that is available in Android Oreo and above.

Changes

The bulk of the file changes are due to the removal of the old Example React Native project, and the creation of a new React Native Project, TestApp. Creating an entirely new project was easier than trying to make all of the patches necessary for the old project.

Local only hotspot

This is the core of the changes to the library. As of Android APK 28+, developers can only programmatically create a hotspot that does not have internet access.

Removal of custom configuration options

startLocalOnlyHotspot has access to the SoftApConfiguation for the hotspot but does not have the ability to pass custom configuration to the AP.

Testing

To test, we will run the TestApp project using a local version of the project instead of the one pushed to NPM.

I ran into some issues with resolving the correct node_modules when trying to link the two together in the same directory, so these instructions create a copy of TestApp.

Setup

  1. cp -r TestApp ~/Desktop && cd ~/Desktop/TestApp (~/Desktop can be replaced with any directory)
  2. Open package.json and remove the react-native-wifi-hotspot hotspot dependency.
  3. yarn && yarn add file:/absolute/path/to/react-native-wifi-hotspot
  4. Follow the setup instructions, skipping the first install step for npm/yarn.
  5. react-native run-android

Validation

Future work

Based on these updates, there are a few future items of work that should likely be implemented:

aboozaid commented 4 years ago

Thank you for the PR!

I did a fast review in the changed files and I think we need to have a bit support for backward compatibility by using the old API in older versions of android. I will test the app as soon as possible and let you know.

Sorry for being late in replying I just have some stuff.

Patrick-Erichsen commented 4 years ago

Hey @assemmohamedali - no worries about the delay!

That is a good callout regarding the lack of backward compatibility. I don't think it would be wise to merge this PR until that is implemented.

However, I am no longer actively working with the group that needed these updates made.

With that in mind, do you think it would make the most sense to close this PR and leave the branch in place for anyone who wishes to either implement the backward compatibility or just run the forked branch?