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

Enabling esp_idf_svc::ws::client, documentation needed #337

Closed
coder0xff opened this issue Dec 29, 2023 · 8 comments · Fixed by #379
Closed

Enabling esp_idf_svc::ws::client, documentation needed #337

coder0xff opened this issue Dec 29, 2023 · 8 comments · Fixed by #379

Comments

@coder0xff
Copy link

I want to use esp_idf_svc::ws::client. Based on the source code that pulls in the client module

#[cfg(all(
    feature = "alloc",
    esp_idf_comp_tcp_transport_enabled,
    esp_idf_comp_esp_tls_enabled,
    any(
        all(esp_idf_version_major = "4", esp_idf_ws_transport),
        esp_idf_comp_espressif__esp_websocket_client_enabled
    )
))]
pub mod client;

and guessing that I'm meant to remove the esp_idf prefix, capitalize, prepend CONFIG, and add it to sdkconfig.defaults, I added the following to my sdkconfig.defaults:

CONFIG_COMP_TCP_TRANSPORT_ENABLED=y
CONFIG_COMP_ESP_TLS_ENABLED=y
CONFIG_COMP_ESPRESSIF__ESP_WEBSOCKET_CLIENT_ENABLED=y

However, cargo doc --open still doesn't show the client module in esp_idf_svc::ws. I found an (exhaustive?) list of variables in ./target/xtensa-esp32s3-espidf/debug/build/esp-idf-sys-3a66d5767bd3b7f0/out/sdkconfig but the above variables don't appear even after changing my sdkconfig.defaults.

Much of esp_idf_svc contains no documentation at all. As a newcomer to the ESP32 platform, getting a footing is difficult. Adding good documentation is a lot of work of course. I think outlining how to enable or disable each (forgive the abuse of terminology) feature is low-hanging fruit that would have a lot of value.

@coder0xff
Copy link
Author

Just an additional note as a testament for the need of more guidance, before I found #265 I had tried adding

    println!("cargo:rustc-cfg=esp_idf_comp_tcp_transport_enabled");
    println!("cargo:rustc-cfg=esp_idf_comp_esp_tls_enabled");
    println!("cargo:rustc-cfg=esp_idf_comp_espressif__esp_websocket_client_enabled");

to my build.rs to no avail.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Dec 30, 2023

This effort is based on voluntary, unpaid work. If you would like to make a difference, the best way to make a difference is to open a PR with the documentation changes you would like to see. In the meantime, or if you don't feel like doing this, you can use the splendid ESP IDF documentation, which is very thorough, even if for the C API.

Specifically for the WS client component, you have difficulties which will be soon addressed with #336

With that said, the built-in WS client is a weird thing, and I would rather recommend instead using the AsyncEspTls wrapper + edge-net's WS client combo. That is, if you feel comfortable with async.

@coder0xff
Copy link
Author

This effort is based on voluntary, unpaid work. If you would like to make a difference, the best way to make a difference is to open a PR with the documentation changes you would like to see.

I understand completely. I appreciate the work done here, and it's not my intention to offend. I know contributing to open-source is often thankless work, so thank you. I'll make an effort to contribute once I have the requisite comprehension. (I'm going to be spending a lot of time with the esp32-s3.)

Thanks for the link to #336. I'm sure it will be helpful. I'm glad you mentioned the C API. I tried a couple of times to correlate the Rust API to the C API, but I didn't have much success (time to read the sources). Adding cross-references to the docs might also add value.

I am trying to do everything async. The impression I got from the signature of esp_idf_svc::ws::client::EspWebSocketClient::new was that it would continue to send events via callback, which I had planned to abstract to async. Will AsyncEspTls + edge-net still be the preferred API after #336 is landed? My gut tells me to use EspWebSocketClient if able.

@coder0xff coder0xff changed the title Enabling esp_idf_svc::ws::client, better documentation needed Enabling esp_idf_svc::ws::client, documentation needed Dec 30, 2023
@ivmarkov
Copy link
Collaborator

ivmarkov commented Dec 30, 2023

I am trying to do everything async. The impression I got from the signature of esp_idf_svc::ws::client::EspWebSocketClient::new was that it would continue to send events via callback, which I had planned to abstract to async.

Yes and no. To elaborate: you have the correct intuition that "whenever there is a callback, it can probably be turned into async".
Yet, the native EspWebSocketClient API has at least two issues w.r.t. async:

  • The callback that you noticed is only for receiving stuff. For sending, you have to use this, which is a blocking call. Now it might be just offloading the data to be sent to the internal thread, but I would not hold my breath (I've forgotten in the meantime) and if it does, it would come at the price of internal allocations. If not, and to make it look async, you either have to "unblock" it somehow (there are ways, but they almost always involve heap allocations), or to just pretend that the send call is non-blocking. The latter would work as long as you schedule your WS comms in a lower priority async executor (i.e. an executor scheduled on a lower priority thread)
  • The reason why you get a callback (and thus something which can be turned into async) for receiving, is because - if I'm not mistaken - the EspWebSocketClient is spinning an internal thread. Another thread is an overhead of course, with extra space for its stack, and of course - backpressure coming from the thread - meaning - you DO have to poll the future or else it might actually block your send call :)

Also not to be underestimated, that while I have written async adapters for WS server, MQTT client, and a few others, I think I did not do it for the ESP IDF WS client (the client was a contribution and I think the contributor - at the time - was not interested in async). So here you would be on your own, dealing with Condvars and stuff s as to turn the callback-with-backpressure into a Future.

Will AsyncEspTls + edge-net still be the preferred API after #336 is landed? My gut tells me to use EspWebSocketClient if able.

I would bet your gut is wrong this time. I can see your line of thought - this is ESP IDF, tried and tested and so on. Yet, ESP IDF can only do so much for async, as it was not designed for async - rather - either for blocking stuff, or (at best) for callback-ing, which relies on internal threads as I mentioned and is often only half-way solution compared to true async. All of WS server/WS client/MQTT/event bus are like that.

The edge-net in the meantime is pure Rust and was designed with async-first in mind. No hidden threads, absolutely minimal.

And btw it is not me trying to advertise my stuff as somebody once accused me :) (after all, I'm also contributing a lot to the esp-idf-* crates so this is to some extent "my stuff" too). It is just that I suffered (quite) a bit when trying to use ESP IDF app networking components (I think for the rest async-ifying ESP IDF kinda works, but these were painful), hence I came to the conclusion, that we (as in the Rust community) should rather push our own minimal embedded stuff on top of plain UDP/TCP sockets stack. Hence edge-net was born.

@coder0xff
Copy link
Author

Thank you. On further consideration, I'm not going to be using web sockets. I still have questions though, so I'll open another issue #350.

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Jan 21, 2024
torkleyy added a commit to torkleyy/esp-idf-svc that referenced this issue Mar 5, 2024
Fixes esp-rs#337
Also fixes the conversion of ssid/password in the config
to no longer assume 0 termination - this lead to problems
when routers send a 64 byte access token to us instead of a password
torkleyy added a commit to torkleyy/esp-idf-svc that referenced this issue May 16, 2024
Fixes esp-rs#337
Also fixes the conversion of ssid/password in the config
to no longer assume 0 termination - this lead to problems
when routers send a 64 byte access token to us instead of a password
ivmarkov pushed a commit that referenced this issue May 16, 2024
* Fix WPS regression and cred conversion

Fixes #337
Also fixes the conversion of ssid/password in the config
to no longer assume 0 termination - this lead to problems
when routers send a 64 byte access token to us instead of a password

* Fix return type

* Fix array_to_heapless

* More fixes for max length wifi credentials

* Fix for non_std

* Use array_to_heapless in another instance
@milewski
Copy link

I have tried N permutation of all possible settings I could find... I still could not enable this esp_idf_svc::ws::client, what is the secret here?

@ivmarkov
Copy link
Collaborator

I have tried N permutation of all possible settings I could find... I still could not enable this esp_idf_svc::ws::client, what is the secret here?

Reading the BUILD-OPTIONS.md file in the esp-idf-sys, the section about remote components, and adding the web socket client esp idf component lib.

@milewski
Copy link

Thanks a lot! I didn’t know about this. Before seeing your message, I had already dug into the source code and discovered that the WebSocket client code had been removed from the IDF and moved to this registry. I also opened this issue: #514, thinking that the Rust code would need to be updated somehow.

However, after reading your message, everything clicked! I think this should have been documented somewhere. I looked everywhere except the build-options.md file.

The full solution for using these components, which are now part of the Espressif registry, is to add the following to your Cargo.toml:

[[package.metadata.esp-idf-sys.extra_components]]
remote_component = { name = "espressif/esp_websocket_client", version = "1.3.0" }

Additionally, update your sdkconfig.defaults file with:

CONFIG_HTTPD_WS_SUPPORT=y

And everything works perfectly now! Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants