Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

Added network scanning to config portal. #223

Merged
merged 2 commits into from
Nov 11, 2016

Conversation

ed7coyne
Copy link
Collaborator

@ed7coyne ed7coyne commented Nov 4, 2016

Use the ESP network module to allow the user to scan for networks.

select = document.getElementById('networks');
select.innerHtml = '';
networks.sort(function(a, b) {
return b.rssi - a.rssi;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should choose a more predictable sorting to avoid the list from being re-ordered on every scan.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems like the sorting that would be most desirable, you get the strongest networks first. This is how OSX, Android, iOS, Windows, ChromeOS, etc.. sort.

The appeal of this sorting is that the most relevant networks are most likely to be on top. For example when I used to live in downtown LA I had roughly 20 networks I could see at a time but my home network was always the first or second because it was the strongest in my house.

</script>
</head>
<body>
<div>Host: <input id='host'></div>
<div>Auth: <input id='auth'></div>
<div>Path: <input id='path'></div>
<div>Wifi SSID: <input id='ssid'></div>
<div>Wifi SSID: <input id='ssid'><button onclick='scan();'>scan</button></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

did you consider attaching the onclick from the js and keeping the js free?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not really, what would the advantage be?

Seems like more work, we would need a window.onload handler (might be nice to put the random "fetch_config()" call at the end of the document there too. then you would need to add an id here, fetch this button and add the onload, or just put it inline here. This seems cleaner.

If we had a strong reason for separation, like if the JS was loaded from another file and we wanted to play with loading different JS for the same html or something like that but as this is a small file with everything inline it seems to just stick with that and mix it all together. I could be convinced otherwise though.

option.text = network.ssid + ' (' + network.rssi + ')';
select.add(option);
});
select.style.display='inline';
Copy link
Contributor

Choose a reason for hiding this comment

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

something you could consider is setting a .class with the state ('scanning', 'selection', 'connected') on a parent div for the ui, and having some css that configure the visibility of all elements for each one instead of having the js show/hide them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure I follow what you are proposing. It may just be beyond my CSS knowledge. The UI is very much in a "working but unstyled" state. I was planning on going back at the end and making things look good, add some CSS to try to match the feel of the firebase console, stuff like that but if someone wanted to jump in and start styling it now I wouldn't complain :)

@ed7coyne
Copy link
Collaborator Author

Since all comments are suggested improvements and not fixes I am going to go ahead and merge. Feel free to keep commenting though and we can make the changes in followup PRs.

@ed7coyne ed7coyne merged commit de2ef09 into FirebaseExtended:master Nov 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants