search

usetrmnl / firmware

TRMNL device firmware
https://docs.usetrmnl.com
GNU General Public License v3.0
25 stars 4 forks source link

Added custom WiFi Captive Portal #16

Closed paw3lx closed 1 month ago

paw3lx commented 1 month ago

Status:

ToDo:

paw3lx commented 1 month ago

So here you can see examole screenshots from the portal: image

MichaelXlivnenko commented 1 month ago

Hi Paweł, looks really cool. About the situation when the user enters wrong wifi credentials, I think that the best option would be to check the correctness after pressing the connect button and if everything is fine, then save the credentials, and if they are incorrect, show a pop-up message and not save

MichaelXlivnenko commented 1 month ago

About esp_***, do you mean the SSID of the ESP AP?

paw3lx commented 1 month ago

About esp_***, do you mean the SSID of the ESP AP?

Yes, but I think I fixed that already - you've to turn off the WiFi AP with WiFi.softAPdisconnect(true); and then change the mode to STA and reconnect to the provided wifi credentials. Previously I had WiFi.softAPdisconnect(false);, which I think resulted in changing the SSIS of ESP AP from TRMNL to esp_number.

MichaelXlivnenko commented 1 month ago

It is very strange, I did not have such a problem, I will test this

MichaelXlivnenko commented 1 month ago

Hi Paweł, I tested the basic functionality. Works very well 👍, but I have a few questions.

  1. Have you experienced issues with the portal closing prematurely? Everything functions properly on iOS and PC, but on Android, the portal is nearly always closed on the first attempt, and data can be entered on the second try.

  2. Portal interface: Can you include signal strength? Also, is it possible to add a checkbox for the password field?

  3. Invalid credentials: In the previous version, a timeout was used because the device would enter a long cycle. Currently, no timeout is being utilized. It would be beneficial to handle this situation more quickly. As I understand you are currently working on wrong credentials handling?

  4. Ability to save multiple Wi-Fi networks.

paw3lx commented 1 month ago

Hi Paweł, I tested the basic functionality. Works very well 👍, but I have a few questions.

  1. Have you experienced issues with the portal closing prematurely? Everything functions properly on iOS and PC, but on Android, the portal is nearly always closed on the first attempt, and data can be entered on the second try.

No, but I haven't tested that yet on any Android devices. I will do so.

  1. Portal interface: Can you include signal strength? Also, is it possible to add a checkbox for the password field?

Sure, signal strength shouldn't be a problem. What would be the purpose of a checkbox for the password field?

  1. Invalid credentials: In the previous version, a timeout was used because the device would enter a long cycle. Currently, no timeout is being utilized. It would be beneficial to handle this situation more quickly. As I understand you are currently working on wrong credentials handling?

Yes, I am working on the wrong credential use case. There is a default timeout right now, we try to connect 5 times, and each time it waits max 10s. So in total it can take up to 50-60 seconds. Is it fine in your opinion?

  1. Ability to save multiple Wi-Fi networks.

That shouldn't be a problem, since we save credentials in Preferences. I guess we would try to connect to the 1st saved wifi, if it doesn't work, then to 2nd and so on?

MichaelXlivnenko commented 1 month ago

What would be the purpose of a checkbox for the password field?

The purpose of a checkbox is to enable the option to view the password in plain text.

There is a default timeout right now, we try to connect 5 times, and each time it waits max 10s. So in total it can take up to 50-60 seconds. Is it fine in your opinion?

In my opinion, 50-60 seconds is too long because the user may not understand the status of the device. I suggest reducing the number of attempts to 3 and the timeout to 5 seconds.

That shouldn't be a problem, since we save credentials in Preferences. I guess we would try to connect to the 1st saved wifi, if it doesn't work, then to 2nd and so on?

There is no connection priority. We attempt to connect to the first Wi-Fi, and if everything is fine, we continue working. If not, we try to connect to the second, and then to the third. After three unsuccessful attempts to connect, we assume that the device is not connected and show an error message.

Regarding the user interface, I think it would be convenient to add tabs for the SSID and password fields. Additionally, if Wi-Fi is saved, the saved data should be displayed in the appropriate fields. For example, if only the first Wi-Fi is saved, when opening the portal, the SSID field will display the name of the first Wi-Fi in the first tab and its password will be displayed in the password box. When switching to the second tab, the fields will be empty.

What do you think of this idea? How challenging would it be to implement? Do you have any other ideas about this?

wifi_portal

paw3lx commented 1 month ago

What would be the purpose of a checkbox for the password field?

The purpose of a checkbox is to enable the option to view the password in plain text.

Sure, that shouldn't be an issue. I will add that.

There is a default timeout right now, we try to connect 5 times, and each time it waits max 10s. So in total it can take up to 50-60 seconds. Is it fine in your opinion?

In my opinion, 50-60 seconds is too long because the user may not understand the status of the device. I suggest reducing the number of attempts to 3 and the timeout to 5 seconds.

Ok, sure. I will change that.

That shouldn't be a problem, since we save credentials in Preferences. I guess we would try to connect to the 1st saved wifi, if it doesn't work, then to 2nd and so on?

There is no connection priority. We attempt to connect to the first Wi-Fi, and if everything is fine, we continue working. If not, we try to connect to the second, and then to the third. After three unsuccessful attempts to connect, we assume that the device is not connected and show an error message.

Regarding the user interface, I think it would be convenient to add tabs for the SSID and password fields. Additionally, if Wi-Fi is saved, the saved data should be displayed in the appropriate fields. For example, if only the first Wi-Fi is saved, when opening the portal, the SSID field will display the name of the first Wi-Fi in the first tab and its password will be displayed in the password box. When switching to the second tab, the fields will be empty.

What do you think of this idea? How challenging would it be to implement? Do you have any other ideas about this?

The UI with 3 WiFi's is okay. I am thinking about having another "Save" button, that would only save the credentials without connecting. On the other side it adds some complexity, and it would be better to keep to portal as simple as possible. I will think about it. What do you think?

wifi_portal

MichaelXlivnenko commented 1 month ago

The UI with 3 WiFi's is okay. I am thinking about having another "Save" button, that would only save the credentials without connecting. On the other side it adds some complexity, and it would be better to keep to portal as simple as possible. I will think about it. What do you think?

I think that the simplest option would be to have only one "Save" button, which would save the settings (3 AP&PASS pairs). In other words, when the user opens the portal, enters all necessary Wi-Fi settings, and then clicks "Save", the data will be saved, the portal will close, and the device will try to connect to the first saved Wi-Fi network. If the attempt was unsuccessful, the device will try to connect to the second network and so on. If the attempt to connect to the third point is unsuccessful, then we display the corresponding error message on the screen and stop the connection process.

ryanckulp commented 1 month ago

i dont see why firmware needs a UI to manage wifi credentials. our use case is for TRMNL device to "just work" if user switches locations.

so if helpful for dev i think it's OK if this is more of a "magical" feature: let user add a wifi ssid/password pairing whenever they connect to TRMNL ssid, and keep those stored in device memory forever (until user does soft reset).

on device boot, first try last used credential ("current wifi"). then cycle through collection of other saved pairings. if current wifi creds dont work, also re-enable the TRMNL network.

paw3lx commented 1 month ago

Okay, that will make the portal clear and intuitive.

Should we re-enable the TRMNL network when the first saved credentials does not work? Or if all saved credentials does not work?

ryanckulp commented 1 month ago

if testing a connection is fast, we could exhaust all creds first to avoid logic that switches to "just saved" network when one is already connected.

my only concern is our backoff/retry. attempting to connect to, say, 3 networks @ 5x each with exponential backoff will mean the user is waiting 1-2 minutes, get frustrated, and simply reset all their stored creds defeating this feature's purpose. :p

stevekennedyuk commented 1 month ago

This is inherently a bad idea as potent for a bad actor to snoopSteve-- Sent from my iPhoneOn 26 Jul 2024, at 16:47, Ryan Kulp @.***> wrote: if testing a connection is fast, we could exhaust all creds first to avoid logic that switches to "just saved" network when one is already connected. my only concern is our backoff/retry. attempting to connect to, say, 3 networks @ 5x each with exponential backoff will mean the user is waiting 1-2 minutes, get frustrated, and simply reset all their stored creds defeating this feature's purpose. :p

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

paw3lx commented 1 month ago

Maybe we should for now go with just one saved wifi credential, as I see there is few ideas how to handle multi-credential situation and it needs further discussion. What do you think?

stevekennedyuk commented 1 month ago

I would save multiple but then scan for available networks, and connect to known one (I guess highest RSSI if multiple known)Steve-- Sent from my iPhoneOn 26 Jul 2024, at 18:15, Paweł Skaruz @.***> wrote: Maybe we should for now go with just one saved wifi credential, as I see there is few ideas how to handle multi-credential situation and it needs further discussion. What do you think?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

MichaelXlivnenko commented 1 month ago

Maybe we should for now go with just one saved wifi credential, as I see there is few ideas how to handle multi-credential situation and it needs further discussion. What do you think?

I think that we must start with one WiFi and after all will work good we can think about multiple

MichaelXlivnenko commented 1 month ago

Testing showed that the portal works well, the only problem is the sporadic closing of the portal on android. I'm looking for what could be wrong. About multi wifi, I think that we can open new pull request and disscuss our view of multi wifi there.

paw3lx commented 1 month ago

@MichaelXlivnenko I see that we have already merged that PR - it is fine, but I would like to clean the code a bit, add some comments for ease of maintenance etc. So I will do it in another PR :)