feat[auth, client-id]: support username/password authentication; support custom mqtt client ids #9

fridayy · 2025-11-03T14:36:43Z

Hello!
While playing with AtomVM and this library I made the following observations:

Currently username/password authentication does not work as the values where never passed into esp_mqtt_client_config_t and never made it into the port
Passing a client-id has no effect (always auto-generated as the value never made it so the port)
trusted_cert currently serves no purpose (never passed into the port)

This PR should fix the first two points (tested with self-hosted mosquitto) and removes trusted_cert for now.

Thank you for your great work on AtomVM - love it. ❤️

fridayy · 2025-11-03T14:37:39Z

examples/mqtt_client_example/src/mqtt_client_example.erl

    %% Start the MQTT client.
    %%
    Config = #{
-        url => "mqtt://mqtt.eclipseprojects.io",


Replaced with a working alternative

fridayy · 2025-11-03T14:40:40Z

src/mqtt_client.erl

 init(Config) ->
    try
        Self = self(),
        Port = erlang:open_port({spawn, "atomvm_mqtt_client"}, [


username, password, client_id never made it into the port as they were missing from the opts here.

UncleGrumpy · 2025-11-03T15:48:36Z

Thank you for raising the issue. This has been broken for a while, but I assumed it was my broker (I was testing on a local broker) that did not the way the authentication was being sent… but as you point out it wasn’t!

As a temporary workaround you can add the username and password to the broker URL:

mqtts://USER:[email protected]

Now that I know this is not just a personal problem, I will work on a proper fix. Thanks again for opening the issue, and please don’t hesitate to open issues for any other problems you encounter, or suggestions for improvements in the AtonVM ecosystem.

fridayy · 2025-11-03T15:54:40Z

Thank you for raising the issue. This has been broken for a while, but I assumed it was my broker (I was testing on a local broker) that did not the way the authentication was being sent… but as you point out it wasn’t!

As a temporary workaround you can add the username and password to the broker URL:

mqtts://USER:[email protected]

Now that I know this is not just a personal problem, I will work on a proper fix. Thanks again for opening the issue, and please don’t hesitate to open issues for any other problems you encounter, or suggestions for improvements in the AtonVM ecosystem.

You're welcome @UncleGrumpy - happy to help.

UncleGrumpy

Thank you for the contribution! I don't have any maintainer privileges in this repo, but I wanted to leave some comments and suggestions to help speed up the review process when a maintainer gets a chance to review it.

UncleGrumpy · 2025-11-04T06:13:22Z

ports/atomvm_mqtt_client.c


-// NB. Caller assumes ownership of returned string
-static char *maybe_get_string(term kv, AtomString key, GlobalContext *global)
+static char* maybe_get_string_or_default(term kv, AtomString key, char* default_value, GlobalContext *global)


We should keep consistent with the AtomVM C Coding Style Guide and put the pointer preceding the variable in char *default_value.

UncleGrumpy · 2025-11-04T06:48:24Z

ports/atomvm_mqtt_client.c

    UNUSED(port);
    char *username_str = maybe_get_string(opts, username_atom, global);
    char *password_str = maybe_get_string(opts, password_atom, global);
-    // char *cert_str = maybe_get_string(opts, cert_atom, global);


Let's not remove this commented code, left by the original author as a reminder, or at least change it to a TODO: comment to add certificate based authentication.

UncleGrumpy · 2025-11-04T07:20:07Z

ports/atomvm_mqtt_client.c

        .uri = url_str,
+        .username = username_str,
+        .password = password_str
        .client_id = client_id,


See comment above.

UncleGrumpy · 2025-11-04T07:21:05Z

ports/atomvm_mqtt_client.c

-        .credentials.client_id = client_id
+        .credentials = {
+            .username = username_str,
+            .client_id = client_id,


See comment above.
.client_id = client_id, > .client_id = client_id_str,

UncleGrumpy · 2025-11-04T07:21:24Z

ports/atomvm_mqtt_client.c

    char *username_str = maybe_get_string(opts, username_atom, global);
    char *password_str = maybe_get_string(opts, password_atom, global);
-    // char *cert_str = maybe_get_string(opts, cert_atom, global);
+    char *client_id = maybe_get_string_or_default(opts, client_id_atom, get_default_client_id(), global);


I think we should keep the naming scheme consistent and use client_id_str here, which would require updating lines 802 and 811 below.

UncleGrumpy · 2025-11-04T07:26:44Z

examples/mqtt_client_example/src/mqtt_client_example.erl

+        url => "mqtt://broker.hivemq.com",
+        % username => "some-user", % optional
+        % password => "some-password", % optional)
+        % client_id => "atomvm-client", % optional (generated if missing)


The default name is atomvm-<DEVICE-MAC-ADDRESS>, so every device will have a unique default name. That may not be the best way of presenting it, but that gives you the idea.

fridayy · 2025-11-04T07:42:22Z

Thank you for the contribution! I don't have any maintainer privileges in this repo, but I wanted to leave some comments and suggestions to help speed up the review process when a maintainer gets a chance to review it.

Thanks for the feedback, very much appreciated. I will implement the changes sometime after work. Have a great day @UncleGrumpy

UncleGrumpy · 2025-11-05T16:56:34Z

This is going to sound a little nit-picky, but we like to maintain a clean git history for all of the AtomVM repos. I would suggest squashing the two commits (and remove the reference to requested PR changes - nearly every PR gets some small change requested).

Optionally, you can separate out the code changes to made in the second commit (addressing PR comments) and flatten those into the first commit, and then have all the typo fixes separated into the second commit and amend the message accordingly. The second is often what would be preferred for the AtomVM repo, but overall this is a small set of changes so either is fine here.

fridayy · 2025-11-05T17:56:34Z

This is going to sound a little nit-picky, but we like to maintain a clean git history for all of the AtomVM repos. I would suggest squashing the two commits (and remove the reference to requested PR changes - nearly every PR gets some small change requested).

Optionally, you can separate out the code changes to made in the second commit (addressing PR comments) and flatten those into the first commit, and then have all the typo fixes separated into the second commit and amend the message accordingly. The second is often what would be preferred for the AtomVM repo, but overall this is a small set of changes so either is fine here.

Not nit-picky at all - I'll squash them ofc. 👍
Thanks again for your review and your time @UncleGrumpy

…ort custom mqtt client ids; fix documentation typos

fridayy commented Nov 3, 2025

View reviewed changes

fridayy force-pushed the fix/consider-username-and-password branch from ca2f793 to b68dd52 Compare November 3, 2025 14:44

UncleGrumpy suggested changes Nov 4, 2025

View reviewed changes

fridayy force-pushed the fix/consider-username-and-password branch 2 times, most recently from e56f1b8 to 9eb4fce Compare November 5, 2025 18:06

feat[auth, client-id]: support username/password authentication; supp…

9eb4fce

…ort custom mqtt client ids; fix documentation typos

feat[auth, client-id]: support username/password authentication; support custom mqtt client ids #9

Are you sure you want to change the base?

feat[auth, client-id]: support username/password authentication; support custom mqtt client ids #9

Conversation

fridayy commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UncleGrumpy commented Nov 3, 2025

Uh oh!

fridayy commented Nov 3, 2025

Uh oh!

UncleGrumpy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fridayy commented Nov 4, 2025

Uh oh!

UncleGrumpy commented Nov 5, 2025

Uh oh!

fridayy commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fridayy commented Nov 3, 2025 •

edited

Loading

fridayy commented Nov 5, 2025 •

edited

Loading