search

kirillzyusko / react-native-wifi-p2p

Library that provide access for working with wi-fi direct (p2p) module in android.
174 stars 32 forks source link

Feature : Allow to broadcast RN side #61

Closed seba9999 closed 1 year ago

seba9999 commented 1 year ago

Hi @kirillzyusko,

I've worked a bit on that but I don't have any clue on how to compile this into a react native library ! Could you explain me a bit ? Or compile it and let me try it in react native to see if it works ?

Thanks :)

kirillzyusko commented 1 year ago

Hi @seba9999

Yes, sure. I've used https://github.com/kirillzyusko/react-native-wifi-p2p-example example app to test my changes. There you can specify a package from github:

"react-native-wifi-p2p": "https://github.com/seba9999/react-native-wifi-p2p#feature-broadcast"

Or you can use symlink to link a local package (I prefer this option, since I can easily modify file locally and I don' need to reinstall deps every time).

Let me know if you have any other questions!

BTW, you changes look good for me! I'll be happy to approve/merge/release a new version/close the issue if you confirm these changes actually works for you and allow you to achieve what you want 😎

kirillzyusko commented 1 year ago

Also @seba9999 I'd like to ask you to document new method sendMessageTo in README.

And maybe new method for sending file also can be added? sendFileTo?

And last point - you should also add new methods in index.js file 👀

seba9999 commented 1 year ago

I've updated the code and the docs :)

I've also added a sendFileTo ;)

Sorry for messing your README.md file but doesn't it look prettier now ? :P

For the testing part I'm a bit blocked... I've never used react native alone .. I'm using expo so I don't really get it when it comes to symlink ... Could you elaborate ? Do I have to symlink my index.js ( of this repo ) into my /nodes_modules of the repo of my example ? The difficult part is that in order to test specific library with expo I must build a developement build first so I don't think this gonna work ...

kirillzyusko commented 1 year ago

@seba9999 you need to symlik this repo to node_modules. After that you should see a folder react-native-wifi-p2p inside of node_modules - in fact it will be just a reference to an actual folder. And build the app. That's how it works for pure RN projects. I worked a little bit with expo dev client, and I think you don't need to change anything (maybe only be sure, that you have react-native-wifi-p2p package in dependencies - maybe linker somehow consumes this info).

Regarding prettier part - yes, it looks much better, but I'd like to keep changes separately. Would you mind to submit changes without prettier and when this PR will be merged create another PR with prettier integration (also maybe add github actions to assure all code is properly formatted). What do you think about it? In this cases changes will have better git-history and will not be mixed inside of one PR (which menas it will be easier for me to review them).

kirillzyusko commented 1 year ago

Also @seba9999 don't you think that we should to port these changes?

https://github.com/coletivoEITA/react-native-wifi-p2p-reborn/blob/b657850b8034a298dc71334ffc20a3f346d467c1/android/src/main/java/io/wifi/p2p/MessageServerAsyncTask.java#L61-L63

And I think similar should be done for receiveFile - it's useful to know the sender's addres.

seba9999 commented 1 year ago

Those changes breaks backward compatibility .... I would be sending some JSON.stringify(object) so I can add all the data that I want ... No need to change that then IMO

kirillzyusko commented 1 year ago

Looks good 👍 I'm going to merge.

One more question @seba9999 - are you planning to change README? Add information about Android 13, setup prettier etc.?

seba9999 commented 1 year ago

I'll may do it if this effectively works ! Are you creating the 3.3.1 ( or 3.4.0 ) release ?

kirillzyusko commented 1 year ago

@seba9999 I will create 3.4.0 release, since this release will contain API enhancements 👀

seba9999 commented 1 year ago

Hum I've finally managed to test it out but I'm facing a problem ... The new "address" parameter I've created is waiting for an IP address and in the list of peer we only have their MAC Addresses ! ...

I think we need some refactoring in order to store manually the IPs of all the devices in the group ... Allowing the "broadcast system" in the library itself ...

I didn't find something newer than this 10 years old thread ... ><

kirillzyusko commented 1 year ago

@seba9999 contributions are always welcome :)

And feel free to create a new issues, since someone may need this information, but it will be hard for them to find a pieces of informations in comments under PR 😅