Allow switching between AP, STA etc at runtime

[timovelten]

I would like to initialize the Wifi interface in ApSta mode and later disable the access point (and switch to Sta mode). This is so that the device can be accessed via the access point to configure it. Afterwards, the access point should disappear and be inaccessible (this is especially relevant because the "obvious hack" of making the AP hidden with a password does not work in esp-wifi, as it does not support password protected APs right now).

As far as I can tell, this is possible in esp-idf through esp_wifi_set_mode. I propose to change WifiController::set_confguration so that it just infers the mode from the configuration that was passed in, i.e. something like this

pub fn set_configuration(&mut self, conf: Configuration) -> Result<(), WifiError> {
    let wifi_mode = WifiMode::try_from(&conf)?;
    esp_wifi_result!(unsafe { esp_wifi_set_mode(wifi_mode.into()) })?;
    self.config = conf;

    match &self.config {
        Configuration::None => {
            return Err(WifiError::InternalError(
                InternalWifiError::EspErrInvalidArg,
            ));
        }
        Configuration::Client(config) => apply_sta_config(config)?,
        Configuration::AccessPoint(config) => apply_ap_config(config)?,
        Configuration::Mixed(sta_config, ap_config) => {
            apply_ap_config(ap_config)?;
            apply_sta_config(sta_config)?;
        }
        Configuration::EapClient(config) => apply_sta_eap_config(config)?,
    };

    Ok(())
}

I think this approach also somewhat simplifies the amount of validation (see here) that has to be done in set_configuration.

I did try out this change locally and it seems to work and do what it should (though I am not deeply familiar with esp-wifi or esp-idf, so this might be subtly broken).

[ProfFan]

@Karuso33 Are you trying to make the ApSta and then use the raw interface for TX/RX of raw frames? I am trying to do the same here.

[bjoernQ]

Thanks for the suggestion!

I think we could allow to degrade from ApSta to either Ap or Sta but we cannot allow going from Ap to Sta, Sta to Ap etc. since the user doesn't have access to the device/interface

But we should do that by either consuming the ap-device/ap-interface or the sta-device/sta-interface since otherwise the user could try to continue using them.

Maybe something like shutdown_access_point(interface: Interface, device: WifiDevice<'d, WifiApDevice>)

[timovelten]

As I said, I am not deeply familiar with the code base, so take everything I say with a grain of salt. But I think it might actually be kind of fine to let the user keep control of the WifiApDevice, even when the Ap is shut down. When the user tries to send data over that device esp_wifi_send_data is eventually called, and that just calls into the C library (esp_wifi_internal_tx). And I think that call would just fail more or less silently (the failure is logged, but not returned to the caller; the error is 12294 - ESP_ERR_WIFI_STATE).

While failing silently is not ideal, it is pretty much the same situation that the user currently faces when trying to send data in Sta mode, while not connected to an access point (that also returns 12294 - ESP_ERR_WIFI_STATE). And I feel like if that behavior is okay in Sta mode, then it would also not be the worst thing to have this behavior for Ap mode as well.

I think that this is realistically the best solution there is here. Especially since it is not possible to "get back" the WifiApDevice once an embassy-net Stack has been constructed (there is no .into_inner() or similar, maybe there should be (?)).

[ivmarkov]

@bjoernQ

I think the current esp-wifi design, where the mode of operation (AP, STA or AP-STA) is encoded as a type-state and cannot be changed at runtime is the root cause to begin with.

The above makes it very difficult to switch - at runtime - from AP to STA mode and back, as one has to tear down the whole network stack, along with all the apps (HTTPD, etc.) that sit on top of it.

This does not play well with use cases where - say - the HTTPD server, or other network-layer futures are modeled with embassy-executor-tasks each, as in that case, the network stack - by necessity - needs to be passed in a &' static context and thus needs to live forever.

What the above entails, is that either:

• The whole networking (embassy net driver, wifi controller, all embassy-net apps on top) then needs to either live in a single, giant future and be confined to a single "embassy-net-and-everything-that-lives-on-top" task in the executor
• ... or people have to invent themselves a variant of async cancellation, and pass around the stack either unsafely, or with Rcs via esp-alloc, so that the stack can be disposed of at runtime. And there is still unsafe in all that trickery

In the STD hal we are able to switch between AP and STA at runtime without any issues so this "AP-and-STA-are-typestates" design in esp-wifi might actually be accidentally self-inflicted rather than following some design constraint in the ESP IDF Wifi blobs themselves.

Also, I just looked at the cyw SPI wifi, and it does seem to be able to switch between AP and STA at runtime?, so the issue seems to be unique to esp-hal.

[ivmarkov]

pinging @Dominaezzz as he expressed interest in this topic.

[bjoernQ]

I have some ideas what I would want to do if I had time 😄 - will try to write them down here (Spoiler: would result in a totally different API)

[ivmarkov]

JFYI with @Dominaezzz we had a similar idea: you could leave the existing driver intact, and then implement an embassy-net driver proxy (possibly, by using embassy_net_driver_channel). Then the proxy can be statically allocated, and only its connection to the actual wifi driver would be dynamic and changing.

However I think the above ^^^ is best suited when somebody wants to workaround the status quo where the existing wifi driver has to be completely torn down to go from say ap to sta, and perhaps not really the best approach if the esp-wifi driver is to be rewritten/changed internally.

Anyway, mentioning for completeness.

[Dominaezzz]

Does the std hal use embassy-net as well or does it still use lwIP or whatever idf uses normally?

I'm still trying to reason about the network stack is supposed to deal with the switch.

[ivmarkov]

or does it still use lwIP or whatever idf uses normally?

This. Even though the std hal now have the facility to RX/TX raw ethernet packets as well, so in theory you can slap embassy-net on top.

But the point is, the std stack does not destroy the ap and sta "netifs" (the equivalent of embassy_net::Driver) when switching from AP to STA and back. These always "pre-exist" so to say. Also don't forget that in std land the whole "driver" thing is anyway "implicit" for the TCP/IP stack in that the std Network IO functions do not take a "driver" or a "stack" as an argument. The "driver" / "stack" is implicit there.

I'm still trying to reason about the network stack is supposed to deal with the switch.

Ideally, we should have two embassy_net::Driver instances - one - for AP and another - for STA which always exist. Switching the wifi driver from "ap" to "sta" enables one of the drivers and disables the other, but does not require the user to drop its reference to either of these.

[Dominaezzz]

I'm still trying to reason about the network stack is supposed to deal with the switch.

Ideally, we should have two embassy_net::Driver instances - one - for AP and another - for STA which always exist. Switching the wifi driver from "ap" to "sta" enables one of the drivers and disables the other, but does not require the user to drop its reference to either of these.

Ahhh now that makes a lot of sense to me.

[bjoernQ]

Ideally, we should have two embassy_net::Driver instances - one - for AP and another - for STA which always exist. Switching the wifi driver from "ap" to "sta" enables one of the drivers and disables the other, but does not require the user to drop its reference to either of these.

That would be my plan

[bjoernQ]

I promised to write down my ideas here but to avoid writing non-sense I did a reality check in #3027

The idea is

• network devices / interfaces are independent of the controller - they exist before creating the controller and still exist when the controller is dropped
• no type-state for the enabled wifi mode - a user can switch the wifi mode at any time

For user code it looks like this

...

    // no change here - we obviously need to call init before actually using esp-wifi
    let esp_wifi_ctrl = &*mk_static!(
        EspWifiController<'static>,
        init(timg0.timer0, rng.clone(), peripherals.RADIO_CLK).unwrap()
    );

    // we can take the network interfaces / devices any time
    let interfaces = esp_wifi::wifi::take().unwrap();
    let wifi_interface = interfaces.sta;

    // get the controller - we can switch modes at will
    let controller = esp_wifi::wifi::new(&esp_wifi_ctrl, peripherals.WIFI).unwrap();

...

Not that much of a drastic change as I thought but as a side-effect it makes the API less weird (e.g. I was never able to use esp-wifi before without looking at the examples).

In my testing things look good but there might be oversights and edge-cases I didn't think of.

For now, it's meant as a discussion starter but I'm also happy to get it in mergeable shape (if we don't agree on s.th. totally different)

[ivmarkov]

@bjoernQ
I think in terms of flexibility, your API perhaps goes a tad too far?

What I don't like that much is the introduction of this esp-idf-svc-ism in an API (esp-hal, esp-wifi) which otherwise almost has no take metaphor:

    let interfaces = esp_wifi::wifi::take().unwrap();

Also, given that you say:

    // get the controller - we can switch modes at will
    let controller = esp_wifi::wifi::new(&esp_wifi_ctrl, peripherals.WIFI).unwrap();

... why don't we just have this instead:

    // get the controller and the interfaces - we can switch controller modes at will
    // NOTE: `interfaces` will be `'static` as long as `&esp_wifi_ctrl` is `'static`, (right??) which is quite possible to do with `mk_static`
    let (controller, interfaces) = esp_wifi::wifi::new(&esp_wifi_ctrl, peripherals.WIFI).unwrap();

... and then using the new "type-state"-less controller, one can switch from STA to AP to APSTA to NONE and back, and then either interfaces.ap, or interfaces.sta or none of them or both of them will be "dead" depending on the mode set in the controller (i.e. not receiving packets and packets sent to those will be lost).

[bjoernQ]

Thanks for taking time to look into this and for the feedback!

Seems like this goes into a similar direction as Daniel's feedback and I agree take().unwrap() is also not my favorite.

I think I like the idea of getting the controller and the interfaces as a tuple

[katyo]

Currently if I try disable WiFi at runtime (using WifiController::set_configuration(None)) it fails with InternalError(EspErrInvalidArg).

[bjoernQ]

Currently if I try disable WiFi at runtime (using WifiController::set_configuration(None)) it fails with InternalError(EspErrInvalidArg).

True - we currently (and did so before) actively disallow None. I don't think we should disallow it. If you really don't need WiFi anymore it would be more effective to drop all resources and let esp-wifi tear down completely (i.e. also stopping the scheduler) but there is no reason to disallow None

[Dominaezzz]

I suppose this can be closed now?

[bjoernQ]

I suppose this can be closed now?

Thanks for the reminder