uugear / UUGear-Web-Interface

UUGear Web Interface is an extremely lightweight web server that allows you to access your Raspberry Pi and UUGear devices in web browser.
Other
12 stars 2 forks source link

Networking improvements for multi-homed devices. #4

Open 3ricj opened 12 months ago

3ricj commented 12 months ago

The IP address of the server is hard coded in the javascript. This breaks things if you have more than one interface for your PI, or use a proxy or your IP address changes.

here's how to fix that:

  1. set your ip to "0.0.0.0" in uwi.conf; this sets the service to listen on all interfaces/ports.
  2. never run diagnose.sh, or it will break your config again
  3. Update uwi/common/js/common.js so it does not contain the static "web_socket_url".

Change this: this.ws = new WebSocket(web_socket_url);

into this: this.ws = new WebSocket('ws://'+ window.location.host);

uugear commented 12 months ago

The IP address of the server is not hard-coded. uwi.conf is the configuration file, which you already found. The web_socket_url is made by the host name (IP address) and port you configured there.

window.location is for the device who visits UWI server (the RPi), instead of UWI server itself. It only works when you visit UWI on the same RPi.

3ricj commented 12 months ago

I must be really confused then. So here's my (and I'd argue, a pretty common) configuration:

Let's say it has two IP addresses: 192.168.1.1 on the ethernet, and 192.168.2.1 on the other. The networks can't talk to each other, they are on different subnets (in my case, the wifi is host ap, and I'm connecting up my phone).

As you know, the uwi.conf is read by both the server side code, and the javascript code when the web app is connecting to the web service.

As currently built, there isn't any way to make this function with the code, as you have to hard code the "server IP" in the uwi server config. I wrote this bug up because I was constantly getting the cryptic "no devices found" (there is a deeper bug here, which is if it can't connect to the web service, it should report a different error message, but I digress). Your "Diagnose.sh" code detects that you have multiple IPs and forces you to pick one -- that's unnecessary: If you listen on 0.0.0.0, the webservice will happily listen on all ports. However, the bug in the js client code, as it isn't connecting to the "same" address which the browser is utilizing, it's being forced to use the hard coded IP from the uwi.conf -- so while the server is happy to function on multiple IPs, the client code can't.

The solution, which I wrote above, was to allow the client to automatically connect to the IP address of the webservice based on what IP the browser is using (eg: the app and service seamlessly work on both 192.168.1.1 and 192.168.2.1). This works extremely well, makes your code actually function in more complex environments and doesn't break any backwards compatibility.

You wrote "window.location is for the device who visits UWI server (the RPi), instead of UWI server itself. It only works when you visit UWI on the same RPi." - I'm really not following this. window.location.host comes from the browser, which reports the name or IP of whatever it's currently talking to. It specifically works great when you talk to your software from different machines or networks.

https://www.w3schools.com/js/js_window_location.asp

window.location.host was selected as it excludes the server IP and prototcol (http, :80, etc) and is just the hostname.

I suppose a improved version of what I'm proposing looks like this:

this.ws = new WebSocket('ws://'+ window.location.host) + ':' + port;

If you'd like to use some non-standard websocket port in the uwi.conf

uugear commented 12 months ago

I was wrong about window.location, and thanks for pointing that out.

You made a very good point that the server can bind to 0.0.0.0 so it serves on all IP address it has. The user can decide which IP to visit by inputting the correct address in web browser.

We have different understandings of some terms. If a value is set in a configuration file, although its default value is not the best value, I wouldn't say it is hard coded; I also wouldn't call this as a bug, because a bug in my understanding, is incorrect implementation of the design, which will cause unexpected behavior. The current design of UWI doesn't consider the scenario to access the device cross different subnets, and the current behavior is expected (although not the best).

Don't take me wrong, I do think your suggestion is very good, and we appreciate it. You are actually suggesting a better way to implement UWI, which covers more scenarios of usage. I will marked it as an enhancement.

By the way, window.location.host doesn't exclude the port, so we will get something like "192.168.1.105:8000". We should use window.location.hostname instead, which gives us only the IP address.

Before we change the implementation, you can achieve what you want by just modifying uwi.conf file like this:

......
host='0.0.0.0';
port=8000;
web_socket_url='ws://'+window.location.hostname+':'+port+'/';
......

It is not necessary to modify the common/js/common.js file.

3ricj commented 12 months ago

Great, thanks for your comments. It's great we can modify just the config - I was hesitant to do so as there was comment that all formatting must work in JS and python -- and window.location... would really only work in JS, and I wasn't sure how it would impact the server side code.

Best, -3ric