[REVIEW] Integrate libwebsockets library #5683

NoelGraf · 2023-03-14T15:11:09Z

This PR integrates libwebsockets library into the open62541 stack. The integration is done via the adapter in arch/lws. This adapter acts as a custom "event library" interface layer between the lws event lib support and the open62541 event loop. By integrating in this way, the libwebsockets library makes use of the open62541 event loop, which allows for a perfect integration into the stack.

jpfr

Very nice!
Comments inline.

jpfr · 2023-03-14T17:08:15Z

.gitmodules

@@ -13,3 +13,6 @@
 path = deps/nodesetLoader
 url = https://github.com/open62541/nodesetLoader
 branch = master
+[submodule "deps/libwebsockets"]


Don't use a git submodule.
99% of users should install libwebsockets via a package manager.
And the remaining 1% know how to do it themselves.

Remove the submodule. The users should manually install or use a distorted package.

jpfr · 2023-03-14T17:09:24Z

CMakeLists.txt

+ set(LIBWEBSOCKETS_FOUND 0)
+ endif()
+ endif()
+ if(NOT LIBWEBSOCKETS_FOUND)


Throw an exception when not found.
The find_package thing allows users to manually define the folders.
That should be the only way of doing things.
Like for OpenSSL/mbedTLS.

Do not modify settings.
Abort with a warning when you cannot build.

jpfr · 2023-03-14T17:10:22Z

CMakeLists.txt

+#Libwebsockets
+if(UA_ENABLE_LWS)
+ if(NOT LIBWEBSOCKETS_FOUND)
+ add_dependencies(open62541-object websockets_shared)


Not needed. LIBWEBSOCKETS_FOUND has to be true.

arch/eventloop_lws_http.c

jpfr · 2023-03-14T17:12:25Z

arch/eventloop_lws_http.c

+ const UA_DataType *type;
+ UA_Boolean required;
+} HTTPSendParameters[HTTP_PARAMETERSSIZE] = {
+ {{0, UA_STRING_STATIC("url")}, &UA_TYPES[UA_TYPES_STRING], true},


You mean address instead of url.
The url would include the port and path as well.
How would this be extended for https?
You can already add this as a comment.

jpfr · 2023-03-14T17:20:33Z

arch/eventloop_lws_http.c

+}
+
+static UA_StatusCode
+HTTP_sendWithConnection(UA_ConnectionManager *cm, uintptr_t connectionId,


HTTP is synchronous.
So you have to wait for the previous request to receive a response before sending the next request.
Ensure there can be no misuse.

So either

block sending requests while still waiting for a response

Add the next requests to a list and send them out automatically later on.

jpfr · 2023-03-14T17:22:47Z

arch/lws/ua_lws_adapter.c

+}
+
+static const struct lws_event_loop_ops event_loop_ops_custom = {
+ .name = "custom",


don't use tabs

jpfr · 2023-03-14T17:25:24Z

examples/lws/server_lws_http_client.c

+
+ UA_KeyValueMap map = {paramsSize, params};
+
+ cm->openConnection(cm, NULL, NULL, NULL, NULL);


Add a callback to log the received result.
Also add the handling of failure cases where the remote side does not exist.

How are HTTP status codes lik 404 handled?
Given to the callback as part of the parameter map?

include/open62541/util.h

jpfr · 2023-03-14T17:28:35Z

tools/cmake/Findlibwebsockets.cmake

- /usr/local/include
- /usr/include
-)
+pkg_search_module(LIBWEBSOCKETS REQUIRED libwebsockets)


This requires pkg-config and is only available for Linux.
The previous cmake file was multi-platform.
And it should stay that way.

jpfr · 2023-03-14T20:47:27Z

On more point.
The current Debian and Ubuntu releases are at lws version 4.0 and 4.1.
We should be compatible out-of-the-box with these.

Try to get as low as possible in the version requirements.
But stop before the #ifdef jungle gets too messy.

jpfr

Looks good. A few comments were unresolved from last time.
Let’s merge this for 1.4!

jpfr · 2023-09-14T11:49:38Z

.gitmodules

@@ -13,3 +13,6 @@
 path = deps/nodesetLoader
 url = https://github.com/open62541/nodesetLoader
 branch = master
+[submodule "deps/libwebsockets"]


Remove the submodule. The users should manually install or use a distorted package.

jpfr · 2023-09-14T11:50:20Z

CMakeLists.txt

+ set(LIBWEBSOCKETS_FOUND 0)
+ endif()
+ endif()
+ if(NOT LIBWEBSOCKETS_FOUND)


Do not modify settings.
Abort with a warning when you cannot build.

jpfr · 2023-09-14T11:52:15Z

arch/eventloop_lws_http.c

+
+static UA_StatusCode
+HTTP_eventSourceDelete(UA_ConnectionManager *cm) {
+ UA_String_clear(&cm->eventSource.name);


Still unresolved

jpfr · 2023-09-14T11:55:36Z

include/open62541/plugin/eventloop.h

+ * Open Connection Parameters:
+ * - 0:hostname [string]: Hostname (or IPv4/v6 address) of the connection (required).
+ * - 0:port [uint16]: Port of the target host (default: 443).
+ * - 0:path [string]: Path is the string of information that comes after the top level domain name (default: /).


The way I understand it, the connection will stay open for multiple requests.
The path, method and data should be parameters for sendWithConnection, not here.

jpfr · 2024-02-12T10:20:58Z

Rebased on master.
The examples segfault...

jpfr reviewed Mar 14, 2023

View reviewed changes

NoelGraf force-pushed the feat_integrate_libwebsockets branch from edee065 to a7bf9bc Compare April 13, 2023 08:06

jpfr mentioned this pull request Apr 26, 2023

Ongoing Work, Feature Wishlist and Prioritization #5075

Open

33 tasks

NoelGraf force-pushed the feat_integrate_libwebsockets branch from ea334fc to 1401b4d Compare June 2, 2023 12:57

NoelGraf force-pushed the feat_integrate_libwebsockets branch 2 times, most recently from 2687aeb to e4e5741 Compare June 29, 2023 09:23

NoelGraf force-pushed the feat_integrate_libwebsockets branch from 37c498e to 2272aa2 Compare July 11, 2023 08:53

NoelGraf changed the title ~~[WIP] Integrate libwebsockets library~~ [REVIEW] Integrate libwebsockets library Jul 11, 2023

jpfr reviewed Sep 14, 2023

View reviewed changes

jpfr force-pushed the feat_integrate_libwebsockets branch from fae739c to b4fd3d5 Compare September 25, 2023 12:41

feat(core): Integrate libwebsockets library / HTTP ConnectionManager

6bf5e32

jpfr force-pushed the feat_integrate_libwebsockets branch from b4fd3d5 to 6bf5e32 Compare February 12, 2024 10:20

tassoeloy mentioned this pull request Feb 13, 2024

Unable to run PubSub MQTT with TLS #6266

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REVIEW] Integrate libwebsockets library #5683

[REVIEW] Integrate libwebsockets library #5683

NoelGraf commented Mar 14, 2023

jpfr left a comment

jpfr Mar 14, 2023

jpfr Sep 14, 2023

jpfr Mar 14, 2023

jpfr Sep 14, 2023

jpfr Mar 14, 2023

jpfr Mar 14, 2023

jpfr Mar 14, 2023

jpfr Mar 14, 2023

jpfr Mar 14, 2023

jpfr Mar 14, 2023

jpfr commented Mar 14, 2023

jpfr left a comment

jpfr Sep 14, 2023

jpfr Sep 14, 2023

jpfr Sep 14, 2023

jpfr Sep 14, 2023

jpfr commented Feb 12, 2024


		UA_KeyValueMap map = {paramsSize, params};

		cm->openConnection(cm, NULL, NULL, NULL, NULL);

[REVIEW] Integrate libwebsockets library #5683

Are you sure you want to change the base?

[REVIEW] Integrate libwebsockets library #5683

Conversation

NoelGraf commented Mar 14, 2023

jpfr 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

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

jpfr commented Mar 14, 2023

jpfr 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

jpfr commented Feb 12, 2024