Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

WifiHelper and NetworkHelpers should be able to be called multiple times after a Disconnect #1421

Closed
Ellerbach opened this issue Dec 27, 2023 · 13 comments

Comments

@Ellerbach
Copy link
Member

Library/API/IoT binding

System.Net and System.Device.Wifi

Visual Studio version

No response

.NET nanoFramework extension version

No response

Target name(s)

No response

Firmware version

No response

Device capabilities

No response

Description

When calling multiple time the WifiNetworkHelper.ConnectDhcp or any function ultimately calling NetworkHelper.SetupHelper will always fail as _helperInstanciated is never reset!

WifiNetworkHelper.Disconnect(); should call NetworkHelper.ResetInstance() like in the UnitTests. This will allow after failure to retry to connect once the Disconnect function called.

Similar to Wifi.NetworkHelper, the normal NetworkHelper should have a Disconnect() method or expose the ResetInstance()

How to reproduce

            CancellationTokenSource cs = new(30000);
            Console.WriteLine("Wrong ssid and password");
            bool success = WifiNetworkHelper.ConnectDhcp(ssidWRONG, passwordWRONG, WifiReconnectionKind.Automatic, true, token: cs.Token);
            // success won't be true
            WifiNetworkHelper.Disconnect();
CancellationTokenSource cs = new(30000);
            Console.WriteLine("Correct ssid and password");
            bool success = WifiNetworkHelper.ConnectDhcp(ssidGOOD, passwordGOOD, WifiReconnectionKind.Automatic, true, token: cs.Token);

This will result in an invalid operation exception:

    ++++ Exception System.InvalidOperationException - 0x00000000 (5) ++++
    ++++ Message: 
    ++++ nanoFramework.Networking.NetworkHelper::SetupHelper [IP: 0008] ++++
    ++++ nanoFramework.Networking.NetworkHelper::InternalWaitNetworkAvailable [IP: 0004] ++++
    ++++ nanoFramework.Networking.WifiNetworkHelper::ScanAndConnect [IP: 015b] ++++
    ++++ nanoFramework.Networking.WifiNetworkHelper::ConnectDhcp [IP: 000d] ++++

Expected behaviour

            CancellationTokenSource cs = new(30000);
            Console.WriteLine("Wrong ssid and password");
            bool success = WifiNetworkHelper.ConnectDhcp(ssidWRONG, passwordWRONG, WifiReconnectionKind.Automatic, true, token: cs.Token);
            // success won't be true
            WifiNetworkHelper.Disconnect();
CancellationTokenSource cs = new(30000);
            Console.WriteLine("Correct ssid and password");
            bool success = WifiNetworkHelper.ConnectDhcp(ssidGOOD, passwordGOOD, WifiReconnectionKind.Automatic, true, token: cs.Token);

This should not throw an exception but rather work fine.

Screenshots

No response

Sample project or code

No response

Aditional information

No response

@networkfusion
Copy link
Member

@Ellerbach #1374 (comment)

@Ellerbach Ellerbach added FOR DISCUSSION Open for discussion. Contributes from the community are welcome/expected. Priority: High and removed Status: Waiting triage labels Dec 27, 2023
@Ellerbach
Copy link
Member Author

What I'm trying to solve here is the ability, at least for Wifi to be able to retry. This is in the context of using improv-wifi (see https://www.improv-wifi.com. If by mistake, you enter the wrong elements, it won't connect. Also, I am turning this feature on only when it does not connect properly after a bit of time. So I'm a bit doomed, if I want to use the helper to do so.
For wifi, as it's possible to change, a proper rest should anyway be available with the helpers.

@networkfusion
Copy link
Member

What I'm trying to solve here is the ability, at least for Wifi to be able to retry. This is in the context of using improv-wifi (see https://www.improv-wifi.com. If by mistake, you enter the wrong elements, it won't connect. Also, I am turning this feature on only when it does not connect properly after a bit of time. So I'm a bit doomed, if I want to use the helper to do so. For wifi, as it's possible to change, a proper rest should anyway be available with the helpers.

I agree. Just wanted to make sure that you had the full context 😁

@alberk8
Copy link

alberk8 commented Dec 28, 2023

We really need the ability to retry. I prefer WifiNetworkHelper.Reset() as the WifiNetworkHelper.Disconnect(); feels more like you are disconnecting from an established connection.

@torbacz
Copy link
Sponsor Contributor

torbacz commented Dec 28, 2023

+1 for ability to retry, Now i'm always restarting device after connection to Wifi failed. Setting up "WiFi manager with fallback" with current approach may be tricky - with retries should be pretty easy (retry N times, when all fails then create AP with WWW interface for new credentials)

@josesimoes
Copy link
Member

This may require a redesign and implementation of the network glue and boot sequence.
As it is, it's simply not possible to tap into it and change/switch/reconnect easily.

@Ellerbach
Copy link
Member Author

Check this PR: nanoframework/Samples#343
There is a way "almost" around.

And for the rest, I'll raise another PR on the Access Point sample as well as I found a reliable way as well. In the mean time, code here:

And with the rest of the sample, all works fine, I've still been adjusting it so things are more robust like for the improv one: https://github.com/Ellerbach/LegoInfrared/blob/e3b450304ddec7159ec020799d4080f891e6b152/Samples/nanoFramework/nanoFramework.WebServerAndSerial.NoBluetooth/WirelessSetup/Wireless80211.cs#L37

@Ellerbach
Copy link
Member Author

And on the side note: yes it's painful to setup properly without the helper and in those configuration cases where we want all to be automatic and transparent. There maybe manual setup to be done instead of using the helper. Helper is here to help in 95% of the cases, now, what we're trying to all achieve is falling in the 5%, so some manual adjustments are ok. But yes, I would also prefer the helper to cover the 5% left percents ;-)

@alberk8
Copy link

alberk8 commented Jan 3, 2024

And on the side note: yes it's painful to setup properly without the helper and in those configuration cases where we want all to be automatic and transparent. There maybe manual setup to be done instead of using the helper. Helper is here to help in 95% of the cases, now, what we're trying to all achieve is falling in the 5%, so some manual adjustments are ok. But yes, I would also prefer the helper to cover the 5% left percents ;-)

I have add this NetworkHelper.ResetInstance(); and it seems to work when calling the ConnectDHCP multiple times.

@Ellerbach
Copy link
Member Author

I have add this NetworkHelper.ResetInstance(); and it seems to work when calling the ConnectDHCP multiple times.

@josesimoes the unit tests are always resetting everything and they seems to work :-) So shall we just use it as it's proven to work on the tests?

@josesimoes
Copy link
Member

What is happening in the unit test is cheating... 😅 and it works because the device connects to that wifi AP and remains there. All tests run smoothly one after the other as you know.

Sure, we can remove this code preventing multiple calls. That will work for some situations, and we'll start getting complains deriving from others...
I still think that complete fix of this requires a redesign on the network boot/reboot.

@alberk8
Copy link

alberk8 commented Jan 3, 2024

Whatever the solution, I am of the opinion that multiple attempts to connect to WIFI is desirable. This is especially when the WIFI SSID and Password is entered via a Web Portal or BLE. User entry errors are expected.

@Ellerbach
Copy link
Member Author

After investigations, even with the WiFi helper run a first time, I found a nice pattern to have it working still properly. I've been improving the WiFi AP samples with it.
You can find it here: https://github.com/nanoframework/Samples/blob/4a66f23dc6b69c3d05f3a874eb2a1c38df60234f/samples/WiFiAP/Wireless80211.cs#L82
I will move this issue as a discussion, so we can keep it ope.

@nanoframework nanoframework locked and limited conversation to collaborators Jan 29, 2024
@Ellerbach Ellerbach converted this issue into discussion #1440 Jan 29, 2024
@nfbot nfbot removed FOR DISCUSSION Open for discussion. Contributes from the community are welcome/expected. Priority: High labels Jan 29, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants