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

Preserve websocket server IP over reloading the web page. #16

Closed
wants to merge 2 commits into from

Conversation

anlumo
Copy link
Contributor

@anlumo anlumo commented Oct 2, 2016

I got tired of re-entering my ESP's IP every time I had to reload the web page. This simple modification allows you to specify it in the URL, and it also automatically stores it there on connect.

@dpgeorge
Copy link
Member

dpgeorge commented Oct 3, 2016

That's quite neat.

@@ -97,6 +97,10 @@

(function() {
window.onload = function() {
var url = window.location.hash.substring(1);
if(url) {
document.getElementById('url').value = 'ws://' + url;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure this requires a different (consistent) indent to be merged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole window.onload method is using 2 space indent when the rest of the javascript is using 4 spaces. When fixing this, also fix the indenting on onload method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole window.onload method is using 2 spaces, and so all indents should use 2 spaces, not 1, like patch above. Changing unrelated things is always the worst solution. Please leave it as is, maintainers will clean it all up (when have a time).

@pfalcon
Copy link
Contributor

pfalcon commented Oct 25, 2016

@anlumo : It's unclear how your change works. One perhaps can have a hunch, but could you please update the commit message with exact description of how it's supposed to work? Thanks.

@mcauser
Copy link
Contributor

mcauser commented Oct 26, 2016

It works by populating the websockets server text field (next to the connect button) with the contents of the url hash, when specified.

i.e. he can bookmark http://micropython.org/webrepl/#10.1.1.1:8266 and the input would be set to ws://10.1.1.1:8266 on page load.

@anlumo
Copy link
Contributor Author

anlumo commented Oct 26, 2016

@mcauser Yes, exactly. The whole thing also works automatically. When you click on connect, the hash of the web page is updated to whatever you had in the server URL field. So, you don't have to do anything, when you reload or bookmark that page, the server URL field is preserved.

@askvictor
Copy link

Nice.

Even nicer (though with some security implications) would be to be able to include the webrepl password in the URL.

Also might be nice to have an autoconnect parameter in the URL so that going to the web page would immediately try to connect to the specified host.

@dpgeorge dpgeorge mentioned this pull request Jun 2, 2017
@cwalther
Copy link

👍
I independently implemented approximately the same thing and was about to submit a pull request when I saw this one. I used a query argument rather than the hash part and don’t have the auto-update on connect, which is a nice idea.

@solarjoe
Copy link

I would also be able to supply parameters via the url and, at best, auto-connect to bookmark the connections to several modules, e.g. like

http://micropython.org/webrepl/?url=192.168.2.123&passw=123123&autoconnect=1

So far I have a stored html file for each connection as mentioned here.

@sprive
Copy link

sprive commented Mar 2, 2019

@pfalcon Can we get a statement on what still blocks this PR?

Whether this affects you probably depends on your workflow with these things. It's probably a non-issue if you have a local DNS resolver, but if you don't it's a pain to remember which IP for which device and the current workaround for that is: permanent marker on the boards, or sticking Post-It notes around the display.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 2, 2019

General problems with patches submitted to any project are: a) they're not clean enough, and need cleanup before they can be merged; b) they need to be tested.

If there's interest in a patch to be merged quickly, the author of the patch should produce it clean, and community should sign off of testing it (the clean patch, not some other changes, which can be changed over again).

Not following those simple rules leads to wait, until someone will clean up the patch, then test it. In this case it was me, but yes, that make take years.

Anyway, thanks for the patch, the changes related to "Preserve websocket server IP over reloading the web page." (as the title states) were merged.

@pfalcon pfalcon closed this Mar 2, 2019
@cwalther
Copy link

cwalther commented Mar 2, 2019

Thanks!

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.

8 participants