Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[feature] enable running snapdrop in local network #558

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

Open
schlagmichdoch wants to merge 3 commits into SnapDrop:master
base: master
Choose a base branch
Loading
from schlagmichdoch:enable_snapdrop_on_local_network

Conversation

@schlagmichdoch
Copy link
Contributor

@schlagmichdoch schlagmichdoch commented Jan 6, 2023
edited
Loading

I wanted to run snapdrop as an instance on a raspberry Pi but devices where not shown to each other.
Devices are grouped by their ip addresses. On local networks all devices have different ip adresses and therefor do not see each other.
As the ip addresses shown to the server are private in that scenario, I added a function isIpPrivate() to differentiate that case and group all devices with private ip addresses together in one room.

The function isIpPrivate() is based on those two stackoverflow discussions as a combination of both:
https://stackoverflow.com/questions/13969655/how-do-you-check-whether-the-given-ip-is-internal-or-not/74891529#74891529
https://stackoverflow.com/questions/35374207/how-to-determine-if-ipv6-address-is-private

Bellisario reacted with rocket emoji
...ame room to make it possible to run snapdrop on the local network
Copy link

blipk commented Jan 28, 2023

Hey @schlagmichdoch this PR doesn't work for me, looks like it's missing the definition for isIPv4()

 /home/node/app/index.js:202
 if (net.isIPv4(ip)) {
 ^
 
ReferenceError: net is not defined

Copy link
Contributor Author

Hey @schlagmichdoch this PR doesn't work for me, looks like it's missing the definition for isIPv4()

Hey @blipk Actually, it's the whole package net that is missing as I forgot to commit the import -.-

I just added the following line to this PR which resolves the error:

const net = require('net');

Please try again!

Copy link

blipk commented Feb 4, 2023
edited
Loading

@schlagmichdoch I just changed the line to:
if (!ip.includes(":")) {

Seemed silly to me to pull in another module for just one function, and I'm pretty sure all ipv6 addresses will have a semicolon, feel free to let me know otherwise.

Also you're still checking the same comparison twice on L192.

I also tried with your change now, and it still doesn't work, not detecting as a peer with my android phone
Perhaps something needs to be changed in the client code, or on the app.

EDIT:
Looks like I have to point the "Base URL" in the android app settings to the scheme, IP and port that my instance is running on, it's working with that change.

It'd be nice if there was some level of auto-discovery, so I could use my home instance at home, and others elsewhere.
I noticed you have forked this to "PairDrop", are you making changes to the app as well? I might give that a go.

Copy link
Contributor Author

schlagmichdoch commented Feb 4, 2023
edited
Loading

@blipk net is built-in to node so it wouldn't matter much but your right, I applied your change. Additionally I moved the prefix removal outside of the ipIsPrivate method.

I'm not sure what you mean with the auto-discovery feature and how this could be implemented. I'm also not sure what the android app is doing. As this is a PWA, there should be no need for an additional app.

The PairDrop fork provides an added pairing functionality to enable sending outside of the local network. There are multiple other changes, lots of stability fixes and ongoing development so feel free to find out more in the Readme, give it a go and add feature requests or issues.

Copy link

blipk commented Feb 5, 2023

@schlagmichdoch
I don't use the PWA, there's an app on the F-Droid store, I had to change the setting I mentioned in order for it to work over the local instance.
By auto-discovery I mean it would be good to scan the local network for instances (or have the instance broadcast themselves) and if there's none local then revert to snapdrop.net etc.

Copy link
Contributor Author

there's an app on the F-Droid store

I'll take a look at that. Not sure whether that is compatible with PairDrop. I'll create an issue.

By auto-discovery I mean it would be good to scan the local network for instances (or have the instance broadcast themselves) and if there's none local then revert to snapdrop.net etc.

Why would you use multiple instances? Just make your private instance publicly available via port forwarding in the routers settings and you can always use your private instance

For any further discussion you should propably create a new issue here or on my repo to keep comments here on topic

Copy link

blipk commented Feb 7, 2023

Setting up port forwarding is not so easy for a lot of people, many ISPs are using CGNAT these days and/or only offer IPv6 which is still limited.

I mostly use it to share between my devices at home anyway, just figured I'd mention.

Copy link
Contributor Author

schlagmichdoch commented Feb 9, 2023
edited
Loading

True, but I guess it’s wise to keep the instances separated from each other so front and backend is matching. If you want to use two instances and don’t want to install two PWAs with the Snapdrop for Android app you can change the base url in the settings. Maybe you could create a feature request for quick changing between instances at its repo? 🤷🏽‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /