[lws]: Add initial support for libwebsockets #718

glmfe · 2024-12-19T18:46:00Z

Description

Add libwebsockets as component
Created simple client example
Created simple server echo example

CLAassistant · 2024-12-19T18:46:05Z

All committers have signed the CLA.

euripedesrocha · 2025-01-14T11:12:02Z

components/libwebsockets/CMakeLists.txt

@@ -3,6 +3,9 @@ idf_component_register()
 option(LWS_WITH_EXPORT_LWSTARGETS "Export libwebsockets CMake targets.  Disable if they conflict with an outer cmake project." OFF)
 set(LWS_WITH_EXPORT_LWSTARGETS OFF)

+option(LWS_PLAT_FREERTOS "Build for FreeRTOS" ON)


It is more common to have options set through Kconfig.

Ok, will try to remove the option lines

I tried both cases:

Simply removing the "option(..)"

Moved the "sets" to after the "add_subditrectory".

In both cases I get this error:

which is acutally caused by the miss of the first setting
set(LWS_WITH_EXPORT_LWSTARGETS OFF)

euripedesrocha · 2025-01-14T11:17:52Z

components/libwebsockets/examples/client-echo/CMakeLists.txt

@@ -0,0 +1,6 @@
+# The following lines of boilerplate have to be in your project's CMakeLists
+# in this exact order for cmake to work correctly
+cmake_minimum_required(VERSION 3.5)


We can require a more recent version.

updated to 3.16

euripedesrocha · 2025-01-14T11:20:53Z

components/libwebsockets/idf_component.yml

@@ -0,0 +1,5 @@
+version: "2.0.28~0"


Make the version match the libwebsockets release.

ok, will bump to lws version

euripedesrocha · 2025-01-14T11:21:14Z

components/libwebsockets/idf_component.yml

+url: https://github.com/espressif/esp-protocols/tree/master/components/libwebsockets
+description: TODO:glmfe
+dependencies:
+  idf: '>=5.1'


Suggested change

idf: '>=5.1'

idf: '>=5.0'

euripedesrocha · 2025-01-30T06:11:35Z

components/libwebsockets/CHANGELOG.md

@@ -0,0 +1,7 @@
+# Changelog
+
+## [0.1.0](https://github.com/espressif/esp-protocols/commits/libwebsockets-v0.1.0)


Version to match libwebsocket version.

ok, will bump to lws version

euripedesrocha · 2025-01-30T06:22:19Z

components/libwebsockets/examples/client-echo/README.md

+See the Getting Started Guide for full steps to configure and use ESP-IDF to build projects.
+
+## Example Output
+


Missing expected output.

euripedesrocha · 2025-01-30T06:28:48Z

components/libwebsockets/examples/client-echo/main/lws-client-echo.c

+    LWS_PROTOCOL_LIST_TERM
+};
+
+int app_main(int argc, const char **argv)


Suggested change

int app_main(int argc, const char **argv)

int app_main(void)

euripedesrocha · 2025-01-30T06:29:40Z

components/libwebsockets/examples/client-echo/main/lws-client-echo.c

+    ESP_ERROR_CHECK(example_connect());
+
+    /* Configure WDT. */
+    esp_task_wdt_config_t esp_task_wdt_config = {


Did you had watchdog issues?

Yes, I was getting this error message constantly:

E (x) task_wdt: esp_task_wdt_reset(705): task not found

This was being raised by the wdt funtion "esp_task_wdt_reset". So basically, the LWS was kicking the watchdog but hadn't actually registered the task to begin with. So the solution was to add:

TaskHandle_t handle = xTaskGetCurrentTaskHandle(); esp_task_wdt_add(handle);

The code snippet above fixed this error message. However, a new issue might show up, where a timeout occurs if the task hangs for some reason. To avoid an unnecessary system reset, portMAX_DELAY is set, especially in these two scenarios:

Client: The task stops to wait for the STDIN input. If this is manually done, it will reset the target.

Server: If the task is waiting for a request, the system may hang if no request is received in time. One potential fix would be to set a shorter LWS timeout than the watchdog timer, ensuring that the timeout triggers before the watchdog and can reset it again. However, I encountered some connection instability when implementing this solution. If necessary, I can further investigate and refine this fix to improve stability.

I'm positive that there might be a better solution for both cases than setting the WDT timer to max, but I don’t think it’s crucial for the outcome of this example. What do you think?

@david-cermak Just tagging you since you also showed interest in this section. :D

E (x) task_wdt: esp_task_wdt_reset(705): task not found

This error message? even if you removed everything related to the task watchdog? this is weird. I'd expect something like

... Task watchdog triggered. The following tasks did not reset the watchdog in time...

euripedesrocha · 2025-01-30T06:34:50Z

components/libwebsockets/examples/client-echo/main/CMakeLists.txt

@@ -0,0 +1,12 @@
+set(SRC_FILES "lws-client-echo.c") # Define source files
+set(INCLUDE_DIRS ".") # Define include directories


We usually make examples as projects not components.

the project is in the cmake one level up (client-echo/CmakeList.txt). It's based on esp_websocket_client/examples/target/

euripedesrocha · 2025-01-30T06:49:45Z

components/libwebsockets/port/lws_port.c

+extern int __real_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);
+extern int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);
+
+int  __wrap_mbedtls_ssl_handshake_step( mbedtls_ssl_context *ssl )


Why you had to use wrap here?

mbedtls_ssl_handshake_step wrap: The issue here was that Libwebsockets wasn't properly handling the asynchronous responses MBEDTLS_ERR_SSL_WANT_READ and MBEDTLS_ERR_SSL_WANT_WRITE. Without this wrap, the handshake process would fail when it shouldn't, so we added the wrap to correctly handle these responses and prevent unnecessary failures.

lws_adopt_descriptor_vhost wrap: In this case, the problem was that the struct member info.fi_wsi_name was never being set. On Linux, this didn't cause issues because the vsnprintf function simply printed a (null) string when given a null argument. However, for embedded systems, this behavior resulted in a panic because there was no filtering for null arguments. The wrap was added to address this issue by ensuring the fi_wsi_name was set to a dummy value and preventing the panic.

david-cermak · 2025-01-30T07:22:14Z

components/libwebsockets/LICENSE

@@ -0,0 +1,202 @@
+
+                                 Apache License


the lws is distributed mostly under MIT, will probably need to switch to MIT license

updated to MIT license used by LWS

david-cermak

Nice work overall!

david-cermak · 2025-01-30T07:25:40Z

components/libwebsockets/port/lws_port.c

+#include "stdio.h"
+#include <libwebsockets.h>
+
+extern int __real_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);


probably not needed, but maybe useful to keep here (I'd just add a comment that we don't need the original handshake impl)

added the comments, thanks!

david-cermak · 2025-01-30T07:26:16Z

components/libwebsockets/port/lws_port.c

+#include <libwebsockets.h>
+
+extern int __real_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);
+extern int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);


Not extern, right?

Suggested change

extern int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);

int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);

yes, not even necessary, removed :D

david-cermak · 2025-01-30T07:27:03Z

components/libwebsockets/port/lws_port.c

@@ -0,0 +1,49 @@
+/*
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD


nitpick, also please cleanup the copyrights -- for new files use 2025

glmfe force-pushed the feat/lws branch from 214a74a to 8be3c11 Compare January 29, 2025 14:11

glmfe changed the title ~~Add libwebsockets as component~~ [libwebsockets]: Add initial support for libwebsockets Jan 29, 2025

glmfe force-pushed the feat/lws branch 3 times, most recently from 91c836c to b900029 Compare January 29, 2025 20:38

glmfe self-assigned this Jan 29, 2025

glmfe added the lws libwebsockets label Jan 29, 2025

glmfe changed the title ~~[libwebsockets]: Add initial support for libwebsockets~~ [lws]: Add initial support for libwebsockets Jan 29, 2025

glmfe force-pushed the feat/lws branch 6 times, most recently from 05aca45 to 860ee1e Compare January 30, 2025 03:52

euripedesrocha reviewed Jan 30, 2025

View reviewed changes

david-cermak reviewed Jan 30, 2025

View reviewed changes

feat(lws): Add initial support libwebsockets component

f206c2f

glmfe force-pushed the feat/lws branch from 6a2861d to f206c2f Compare January 31, 2025 01:32

glmfe marked this pull request as ready for review January 31, 2025 01:53

		@@ -0,0 +1,7 @@
		# Changelog

		## [0.1.0](https://github.com/espressif/esp-protocols/commits/libwebsockets-v0.1.0)

		See the Getting Started Guide for full steps to configure and use ESP-IDF to build projects.

		## Example Output

		@@ -0,0 +1,12 @@
		set(SRC_FILES "lws-client-echo.c") # Define source files
		set(INCLUDE_DIRS ".") # Define include directories

	extern int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);
	int __wrap_mbedtls_ssl_handshake_step(mbedtls_ssl_context *ssl);

		@@ -0,0 +1,49 @@
		/*
		* SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD

[lws]: Add initial support for libwebsockets #718

Are you sure you want to change the base?

[lws]: Add initial support for libwebsockets #718

Conversation

glmfe commented Dec 19, 2024 • edited Loading

Description

CLAassistant commented Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-cermak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glmfe commented Dec 19, 2024 •

edited

Loading

CLAassistant commented Dec 19, 2024 •

edited

Loading