Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

receiveFile refactor #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

viniciuscb
Copy link
Contributor

  • receiveFile now uses Executor, instead of the deprecated AsyncTask;

  • stopReceivingFile() added: closes the listening port if no file has been received

  • a new optional parameter has been added to the receiveFile method. If receiveFile receives an object {meta: true}, then the promise result is not a string, but an object with the following format:

    { fromAddress: "", file: "file absolute path (string)" }

* receiveFile now uses Executor, instead of the deprecated AsyncTask;
* stopReceivingFile() added: closes the listening port if no file has been received
* a new optional parameter has been added to the receiveFile method. If receiveFile receives an object {meta: true}, then the promise result is not a string, but an object with the following format:

  { fromAddress: "<ip address of the origin in the p2p wifi network>", file: "file absolute path (string)" }

Signed-off-by: Vinicius Cubas Brand <[email protected]>
@viniciuscb
Copy link
Contributor Author

Hi @kirillzyusko and @seba9999 . I've done this refactor, it is compiling and the whole lib is running.

However I could not do a basic test yet (sending and receiving file inside my app). If some of you can do this test, it would be good, if not, this will have to wait at least until next week.

@kirillzyusko
Copy link
Owner

@viniciuscb I quickly went through changes and overall it looks good 👍 I'm going to have a look over weekend on this again but I don't think I will find anything critical.

I'll try to test these changes, but it will be also not earlier than next week.

@kirillzyusko kirillzyusko self-requested a review March 24, 2023 14:38
@kirillzyusko
Copy link
Owner

@viniciuscb any updates from your side? 👀

@seba9999
Copy link
Contributor

seba9999 commented Apr 10, 2023

I got some time to test it...

But unfortunatlly I'm facing some errors on the sendMessage call :

it seems that wifiP2pInfo is null but I've tried calling getGroupInfo before trying to send a message with no luck...

Just to be sure ... Your new parameter {meta: true} could only be used after the first successful sendMessage to the groupOwner because he is the only address we know already right ?

Then we should store other devices addresses on our own to then be able to sendMessageTo them ?

@viniciuscb
Copy link
Contributor Author

sendMessage

Hi @seba9999 .

{meta: true} can be used every time. It will display the origin IP. It is used in receiveMessage(): receiveMessage({meta: true})

Yes, we have to store the senders by our own.

I'll test this file sending method this week. Besides that, I think that we could update the examples.

@kirillzyusko
Copy link
Owner

@viniciuscb any updates? 👀

@seba9999
Copy link
Contributor

@kirillzyusko can you merge and release those changes please ? With Expo I'm unable to test this new code until you make a new release 😢

@kirillzyusko
Copy link
Owner

@seba9999 I can not publish a release which potentially can be broken. You can use this PR/branch to install dependency with this code 🙂

@seba9999
Copy link
Contributor

seba9999 commented Jun 12, 2023

Yep sorry nevermind since then I was able to try and I've sucessfully sendMessage to the groupOwner and then one message back to the sender with the sendMessageTo with the address in the message object returned ! 🎆 🥳

Works great ! Feel free to publish the next version then ;)

( I didn't test the senFile / receiveFile )

@kirillzyusko
Copy link
Owner

kirillzyusko commented Jun 13, 2023

@seba9999 okay, I will publish new version soon!

P. S. published!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants