Zephyr port #333

julek-wolfssl · 2023-04-28T14:03:36Z

embhorn

Awesome! I have not tested yet, but please check out comments / questions.

zephyr/README.md

embhorn · 2023-04-28T16:18:15Z

zephyr/wolfMQTT_user_settings.h

Any particular reason for overriding the normal "user_settings.h"?

I was following the same approach as in the wolfSSL Zephyr support. We usually have dedicated user settings for each use case. Is there anything missing that you think should go in here too?

Usually the file named user_settings.h is picked up from the project include path when WOLFMQTT_USER_SETTINGS in CFLAGS. By using a different file name, you had to add this in mqtt_types.h

#ifdef CONFIG_WOLFMQTT_SETTINGS_FILE #include CONFIG_WOLFMQTT_SETTINGS_FILE #endif

In Zephyr both wolfSSL and wolfMQTT are compiled at the same time and linked into one executable. Is there any worry of accidentally including the wolfSSL user_settings.h file?

The build options do not conflict. Usually we use a single user_settings.h for both libraries.

Thanks @dgarske. I'll change the defaults accordingly. I'll leave the wolfSSL user_settings.h stuff as is since it may be used by users.

embhorn · 2023-04-28T16:24:43Z

examples/mqttclient/mqttclient.c

+#ifdef WOLFMQTT_ZEPHYR
+void main(void)
+{
+    (void)run_client(0, NULL);
+}
+#else
+int main(int argc, char** argv)
+{
+    return run_client(argc, argv);
+}
+#endif
+


This should be irrelevant now:
zephyrproject-rtos/zephyr#54628

I tested on the latest release. I'll check with master when I get back.

embhorn · 2023-04-28T16:27:16Z

examples/mqttclient/mqttclient.c

+#ifdef WOLFMQTT_TOPIC
+    mqttCtx.topic_name = WOLFMQTT_TOPIC;
+#endif


Can specify topic name with -n option
mqttclient -n <topic_name>

This example is for the Zephyr RTOS. I don't know how to pass in command line parameters there. I was also following the way that the Zephyr MQTT example defines the host, port, and topic.

How about defining WOLFMQTT_TOPIC_NAME in user_settings.h ?

You'll need to add a #ifndef check in examples/mqttexample.h around the default definition.

Using WOLFMQTT_TOPIC_NAME.

embhorn · 2023-04-28T16:28:12Z

examples/mqttclient/mqttclient.c

+#ifdef ENABLE_MQTT_TLS
+    mqttCtx.use_tls = 1;
+#endif


Can specify TLS with -t option
mqttclient -t

Using WOLFMQTT_DEFAULT_TLS in user settings.

embhorn · 2023-04-28T16:28:57Z

examples/mqttclient/mqttclient.c

+#ifdef CONFIG_NET_CONFIG_PEER_IPV4_ADDR
+    mqttCtx.host = CONFIG_NET_CONFIG_PEER_IPV4_ADDR;
+#endif


Can specify host with -h option
mqttclient -h <host_name>

Instead, define DEFAULT_MQTT_HOST in user_settings.h. This already has an #ifndef check in examples/mqttexample.h

Using DEFAULT_MQTT_HOST.

embhorn · 2023-04-28T16:29:50Z

examples/mqttclient/mqttclient.c

+#ifdef CONFIG_NET_CONFIG_PEER_PORT
+    mqttCtx.port = CONFIG_NET_CONFIG_PEER_PORT;
+#endif


Can specify port with -p option
mqttclient -p <port_number>

Using the default ports that depend on TLS.

julek-wolfssl · 2023-04-28T18:50:01Z

Forgot to add new files to include.am

embhorn · 2023-05-01T13:52:27Z

wolfmqtt/mqtt_types.h

+#ifdef CONFIG_WOLFMQTT_SETTINGS_FILE
+#include CONFIG_WOLFMQTT_SETTINGS_FILE
+#endif


If you renamed the file to user_settings.h, it could be picked up from line 64 above.

embhorn · 2023-05-01T13:53:25Z

wolfmqtt/mqtt_types.h

+#define WOLFMQTT_ZEPHYR
+#ifdef CONFIG_WOLFMQTT_TLS
+#undef  ENABLE_MQTT_TLS
+#define ENABLE_MQTT_TLS
+#endif


Can this go into user_settings.h?

This makes it so that users can conveniently configure wolfMQTT from Zephyr's menuconfig. I can move it if you prefer it user settings.

Yeah, you used the magic words "configure wolfMQTT", so this belongs in user_settings.h

Moved to user_settings.h.

zephyr/README.md

- Use macros in user settings to configure connection settings - Fix `main()` signature - Add note with link in main README - Add debugging info to README

embhorn

Looking good. Please see comments.

embhorn · 2023-05-10T18:11:15Z

examples/mqttexample.c

@@ -632,31 +644,31 @@ int mqtt_tls_cb(MqttClient* client)
                return rc;
            }
        }
-    #else
-    #if 0
+    #elif defined(WOLFMQTT_ZEPHYR)


This is for #if !defined(NO_FILESYSTEM). Would it make sense to have it be #else instead of #elif defined(WOLFMQTT_ZEPHYR) ?

I gated it on WOLFMQTT_ZEPHYR due to the #if 0 that was in there. It achieves the same and new configurations that want this section to be active can add the configs here.

embhorn · 2023-05-10T18:14:04Z

wolfmqtt/mqtt_client.h

@@ -34,8 +34,6 @@
 /* Windows uses the vs_settings.h file included vis mqtt_types.h */
 #if !defined(WOLFMQTT_USER_SETTINGS) && \
    !defined(_WIN32) && !defined(USE_WINDOWS_API)
-    /* If options.h is missing use the "./configure" script. Otherwise, copy


Is this not a problem for VS builds anymore?

Adding comment back in. Not sure why I deleted it.

embhorn · 2023-05-10T18:28:01Z

examples/mqttclient/mqttclient.c

+/* In zephyr commit 0b90fd5adf1f01625412efadba4331b5041fb828 the main signature
+ * was changed from void main(void) to int main(void). Zephyr doesn't provide a
+ * way to check the version of Zephyr at compile time so support the latest. */


I was really hoping to not have port specific code in here. After that commit is in a release can we revisit this?

I can go the NO_MAIN_DRIVER route if you prefer that?

Changed to use NO_MAIN_DRIVER.

embhorn

Excellent, @julek-wolfssl ! Thanks for adding bwrap to the tests, that is a huge improvement in itself.

Dla mnie wygląda dobrze!

julek-wolfssl · 2023-05-15T20:41:47Z

@embhorn @dgarske
Added a github action to build and run tests against zephyr. Had to modify the user settings include in mqtt_types.h.

zephyr port

178c051

julek-wolfssl self-assigned this Apr 28, 2023

julek-wolfssl requested review from dgarske and embhorn April 28, 2023 14:03

embhorn requested changes Apr 28, 2023

View reviewed changes

dgarske assigned dgarske and unassigned dgarske Apr 28, 2023

embhorn requested changes May 1, 2023

View reviewed changes

embhorn reviewed May 9, 2023

View reviewed changes

zephyr/README.md Show resolved Hide resolved

Code review fixes

50530d9

- Use macros in user settings to configure connection settings - Fix `main()` signature - Add note with link in main README - Add debugging info to README

julek-wolfssl requested a review from embhorn May 10, 2023 16:08

embhorn reviewed May 10, 2023

View reviewed changes

julek-wolfssl requested a review from embhorn May 10, 2023 20:55

julek-wolfssl added 5 commits May 10, 2023 22:56

Add back in comment about missing options.h

e2201cb

Add timeouts to github actions

8b345bd

bwrap tests when using local mosquitto instance

ceee98d

Remove sleep calls in scripts

dde03b3

zephyr: Use NO_MAIN_DRIVER and implement main in samples

6a4df26

julek-wolfssl assigned embhorn and unassigned julek-wolfssl May 11, 2023

embhorn previously approved these changes May 12, 2023

View reviewed changes

Fix script bwrap calls

d69c7b5

julek-wolfssl dismissed embhorn’s stale review via d69c7b5 May 12, 2023 07:13

Init zephyr github action test using twister script

67e7603

julek-wolfssl requested a review from embhorn May 15, 2023 20:42

embhorn approved these changes May 16, 2023

View reviewed changes

julek-wolfssl assigned dgarske and unassigned embhorn May 17, 2023

dgarske approved these changes May 17, 2023

View reviewed changes

dgarske merged commit b5cead7 into wolfSSL:master May 17, 2023

Zephyr port #333

Zephyr port #333

Conversation

julek-wolfssl commented Apr 28, 2023 • edited Loading

embhorn 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

embhorn May 1, 2023 • 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

julek-wolfssl commented Apr 28, 2023

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

embhorn 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

embhorn left a comment

Choose a reason for hiding this comment

julek-wolfssl commented May 15, 2023

julek-wolfssl commented Apr 28, 2023 •

edited

Loading

embhorn May 1, 2023 •

edited

Loading