Add mqtt v5 protocol implementation #590

danielmeza · 2025-07-30T02:08:59Z

Pull Request Details 📖

Initial implementation for the MQTT v5 implementation for the IDF v5.3

This implements #590

Description

This is the initial implementation for the MQTT v5 protocol for IDF.

This implementation focus in the synchronous client, following the design drivers described at esp-rs/embedded-svc#84, I try to reuse as much as possible but duplicating some parts that will made easy to branch specific protocol changes in the future, so I introduce a new client and put there most of the logic to archive the v5 features.

Main changes compared with the current client implementation are

New introduction of mqtt5 module and client.
Split creation for start by introducing a start method (I guess this should be promoted to embedded-svc)
Introduce 2 new methods in the event to get message metadata and message user properties. Metadata and user properties are not reeded on all message to reduce processing in message that don't have these properties, this way the consumer can decide when to access to that info in a per message manear.
Add V5 properties config struct to describe the messages (Subscribe, Publish, Unsubscribe, Disconnect)PropertyConfig
Introduce config: Option<*PropertyConfig> on existing Publish,Subscribe, Unsubscribe
Add disconnect method to call disconnect and report the disconnect reason using the config: parameter described adobe.
Create transient types to hold the conversion logic between embedded-svc/esp-idf-sys types.

Testing

Using an existing project, I was able to receive the metadata and user_properties info from the Receive event.
Following has been tested ensure proper memory management:

Publish with config and user properties.
Subscribe with config and user properties.
Unsubscribe with config and user properties.
Disconnect with config and user properties.

Missing test:

Last will

Curren issues:

When read user properties from recieved event heap are been allocated and not releasing properly.

Submission Checklist 📝

I have updated existing examples or added new ones (if applicable).
I have used cargo fmt command to ensure that all changed code is formatted correctly.
I have used cargo clippy command to ensure that all changed code passes latest Clippy nightly lints.
My changes were added to the CHANGELOG.md in the proper section.

ivmarkov · 2025-08-03T12:41:32Z

@danielmeza Not sure - do you want a review here, or is this still in progress? If the latter, perhaps mark it as "draft" until ready?

danielmeza · 2025-08-03T12:44:18Z

Hi @ivmarkov, not ready yet!

danielmeza · 2025-09-11T16:50:02Z

I plan to continue this in the next weeks!

danielmeza · 2025-12-24T05:06:37Z

Hi here! I'm back, the current state of this is the following:

Most implementation is complete, I just have to clean the code since I introduce an api that might lead to think that user properties can be set per request and this is not the case (not saying that the protocol intentions is not this) but the esp32 idf v5.3.2 implementation do not allow to define user properties per message (like hedaers) instead user proproperties are stored globally and flushed when message is sent, thus we can't expose a per message user properties (thoug I like very much the way i'ts implemented now and I put lot of effor triying to do it that wayt just to discover that the IDF implementation does't support it). Any way @ivmarkov if you can confirm this I will proceed with the API clean up and with the final touchs. (examples)

danielmeza · 2025-12-24T05:17:47Z

Hi here! I'm back, the current state of this is the following:

Most implementation is complete, I just have to clean the code since I introduce an api that might lead to think that user properties can be set per request and this is not the case (not saying that the protocol intentions is not this) but the esp32 idf v5.3.2 implementation do not allow to define user properties per message (like hedaers) instead user proproperties are stored globally and flushed when message is sent, thus we can't expose a per message user properties (thoug I like very much the way i'ts implemented now and I put lot of effor triying to doit that wayt just to discover that the IDF implementation does't support it). Any way @ivmarkov if you can confirm this I will proceed with the API clean up and with the final touchs. (examples)

I believe the only safe way to do this is for the user or us to queue the request on a single thread using channels so we can guarantee sequential sending without overriding (race condition) the user's properties when multiple threads try to send messages. This way, we can set the user's properties attached to the message just before sending the request to the thread that processes the channel's reception. The queue will then receive the message plus the user's properties. At this point, I think a BIG WARNING will allow the user to handle this case.

danielmeza · 2025-12-27T18:41:20Z

What we decie here will affect esp-rs/embedded-svc#84

Copilot

Pull request overview

This PR adds MQTT v5 protocol support to the esp-idf-svc library for IDF v5.3, implementing a new synchronous client alongside the existing MQTT v3.1/v3.1.1 client.

Key changes:

New client5 module implementing EspMqtt5Client with v5-specific features (user properties, message metadata, disconnect with reason codes)
Added v5 property configuration structs for publish, subscribe, unsubscribe, and disconnect operations
Extended existing EspMqttClient and EspMqttEvent with conditional v5 feature support
Split client creation and startup by introducing a separate start() method

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 27 comments.

File	Description
src/mqtt/client5.rs	New MQTT v5 client implementation with property configs and user property list management
src/mqtt/client.rs	Extended existing client with v5 methods for metadata/user properties and added v5-conditional helper methods
src/mqtt.rs	Added conditional module declaration for client5
Cargo.toml	Added mqtt_protocol_v5 feature dependency from embedded-svc

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-27T18:44:33Z

src/mqtt/client5.rs

+pub struct EspMqtt5Client<'a> {
+    raw_client: esp_mqtt_client_handle_t,
+    _boxed_raw_callback: Box<dyn FnMut(esp_mqtt_event_handle_t) + Send + 'a>,
+    _tls_psk_conf: Option<TlsPsk>,
+}


Missing documentation: Public struct EspMqtt5Client lacks documentation explaining its purpose, API differences from EspMqttClient, and usage examples for MQTT v5 specific features.

Copilot · 2025-12-27T18:44:33Z

src/mqtt/client5.rs

+            retain_as_published_flag: config.retain_as_published,
+            retain_handle: config.retain_handling as _,
+            is_share_subscribe: config.share_name.is_some(),
+            share_name: cstrs.as_nptr(config.share_name)?,


Potential use-after-free: The share_name pointer is obtained from a temporary string slice that may not live long enough. The as_ptr() method returns a pointer that will be invalid once the Option is dropped. Consider using RawCstrs to manage the lifetime of C strings properly, similar to how other string properties are handled.

Copilot · 2025-12-27T18:44:34Z

src/mqtt/client.rs

+            user_property: if let Some(ref user_properties) = config.user_properties {
+                EspUserPropertyList::from(user_properties).as_ptr()
+            } else {
+                mqtt5_user_property_handle_t::default()
+            },


Memory leak: EspUserPropertyList::from creates a new user property list that is never properly cleaned up. The handle returned by as_ptr() will have its memory leaked after the property struct is dropped, as the Drop implementation only clears user properties if they are stored in the Option field.

Copilot · 2025-12-27T18:44:34Z

src/mqtt/client5.rs

+                std::ffi::CStr::from_ptr(self.0.key).to_str().unwrap_or("")
+            }
+        };
+        let value: &'a str = unsafe {
+            if self.0.value.is_null() {
+                ""
+            } else {
+                std::ffi::CStr::from_ptr(self.0.value)
+                    .to_str()
+                    .unwrap_or("")


Potential panic: Calling unwrap_or("") on to_str() silently hides invalid UTF-8 errors. Consider either propagating the error or logging a warning when UTF-8 decoding fails, so users are aware of potential data corruption.

Suggested change

std::ffi::CStr::from_ptr(self.0.key).to_str().unwrap_or("")

}

};

let value: &'a str = unsafe {

if self.0.value.is_null() {

""

} else {

std::ffi::CStr::from_ptr(self.0.value)

.to_str()

.unwrap_or("")

match std::ffi::CStr::from_ptr(self.0.key).to_str() {

Ok(s) => s,

Err(e) => {

eprintln!("Invalid UTF-8 in MQTT5 user property key: {}", e);

""

}

}

}

};

let value: &'a str = unsafe {

if self.0.value.is_null() {

""

} else {

match std::ffi::CStr::from_ptr(self.0.value).to_str() {

Ok(s) => s,

Err(e) => {

eprintln!("Invalid UTF-8 in MQTT5 user property value: {}", e);

""

}

}

Copilot · 2025-12-27T18:44:34Z

src/mqtt/client5.rs

+        let cstrs = RawCstrs::new();
+        let mut user_properties = EspUserPropertyList::new();
+        if let Some(ref user_properties_items) = config.user_properties {
+            user_properties.set_items(user_properties_items)?;
+        }
+
+        let property = esp_mqtt5_unsubscribe_property_config_t {
+            is_share_subscribe: config.share_name.is_some(),
+            share_name: config.share_name.map_or(core::ptr::null(), |s| s.as_ptr()),
+            user_property: user_properties.as_ptr(),
+        };
+        Ok(EspUnsubscribePropertyConfig(
+            property,
+            Some(user_properties),
+            Some(cstrs),


Naming inconsistency: The variable 'cstrs' is declared but never used in this function. Either remove it or add the #[allow(unused_variables)] attribute if it's intentionally kept for future use.

Copilot · 2025-12-27T18:44:38Z

src/mqtt/client5.rs

+                    key: key,
+                    value: value,


Redundant field names: The explicit field names 'key: key' and 'value: value' can be simplified to just 'key' and 'value' using Rust's field init shorthand.

Suggested change

key: key,

value: value,

key,

value,

Copilot · 2025-12-27T18:44:39Z

src/mqtt/client5.rs

+
+#[allow(unused_imports)]
+use esp_idf_hal::sys::*;
+


Missing documentation: Public struct EspUserPropertyList lacks documentation explaining its purpose, usage, and relationship with the underlying MQTT5 user properties handle.

Suggested change

/// Wrapper around the ESP-IDF MQTT v5 `mqtt5_user_property_handle_t`.

///

/// This type owns an MQTT 5 user properties handle together with the

/// backing C strings required by the ESP-IDF API. It is typically

/// constructed from a slice of [`UserPropertyItem`] values (see

/// [`EspUserPropertyList::from`]) or created empty via [`EspUserPropertyList::new`],

/// populated, and then passed to the underlying MQTT v5 client functions.

///

/// The embedded [`RawCstrs`] value ensures that the UTF-8 strings backing

/// each user property key/value pair remain valid for as long as the

/// corresponding `mqtt5_user_property_handle_t` is in use.

Copilot · 2025-12-27T18:44:39Z

src/mqtt/client5.rs

+                retain as _,
+            )
+        });
+        drop(property); // Ensure the property is dropped after use


Redundant explicit drop: The 'drop(property)' call is redundant as the property will be automatically dropped at the end of the function scope. Remove this line to simplify the code.

Suggested change

drop(property); // Ensure the property is dropped after use

Copilot · 2025-12-27T18:44:39Z

src/mqtt/client.rs

+    ) -> Result<MessageId, EspError> {
+        let property = esp_mqtt5_unsubscribe_property_config_t {
+            is_share_subscribe: config.share_name.is_some(),
+            share_name: config.share_name.map_or(core::ptr::null(), |s| s.as_ptr()),


Potential use-after-free: The share_name pointer is obtained from calling as_ptr() on a temporary Option value. This pointer will become invalid once the expression completes. Consider using proper C string lifetime management.

Copilot · 2025-12-27T18:44:39Z

src/mqtt/client5.rs

+        let error = unsafe {
+            esp_mqtt5_client_get_user_property(
+                self.0,
+                items.as_mut_ptr(),
+                &mut items.len() as *mut usize as *mut u8,
+            )
+        };
+        esp!(error)?;


Incorrect pointer cast: The expression takes a mutable reference to items.len() which returns a usize, then casts it to *mut u8. This is incorrect as it treats the address of a usize value as if it were a pointer to u8, not converting the usize value itself to u8. The API likely expects a mutable pointer to a u8 count value, not a reinterpreted pointer to a usize.

Suggested change

let error = unsafe {

esp_mqtt5_client_get_user_property(

self.0,

items.as_mut_ptr(),

&mut items.len() as *mut usize as *mut u8,

)

};

esp!(error)?;

// Use a proper mutable u8 variable for the count, as expected by the C API.

let mut count_mut = count;

let error = unsafe {

esp_mqtt5_client_get_user_property(

self.0,

items.as_mut_ptr(),

&mut count_mut as *mut u8,

)

};

esp!(error)?;

// Adjust the vector length to the actual number of items returned.

items.truncate(count_mut as usize);

Introduces a new `client5` module for MQTT 5.0 protocol support, including an `ErrorReasonCode` enum for MQTT5 error reason codes, conversion implementations, and utility methods. The new module is conditionally compiled with the `esp_idf_mqtt_protocol_5` feature.

…handling

Simplifies the conversion from mqtt5_error_reason_code to ErrorReasonCode by switching from TryFrom to From and returning UnspecifiedError for unknown codes. Updates EventProperty construction to include payload_format_indicator and removes unused ErrorType import.

This commit introduces support for MQTT v5 property configuration in subscribe, unsubscribe, and publish operations for both sync and async clients. It adds new methods to handle property configs, user properties, and message metadata, and refactors the client5 module to focus on user property handling. Deprecated or unused error reason code logic is removed, and trait implementations are updated to support the new property features.

Reworks the MQTT v5 client implementation to accept property configuration options as Option types for publish, subscribe, unsubscribe, and disconnect operations. Removes legacy publish_with_config and subscribe_with_config methods from the v3 client, and updates property handling to use new wrapper structs. Improves FFI safety and code clarity by encapsulating property conversions and updating event field extraction.

Replaces direct struct construction and From implementations for MQTT5 property configs with TryFrom<Option<T>> implementations, improving memory safety and lifetime management. Introduces internal ownership of C string buffers and user property lists to prevent premature drops, and ensures proper cleanup via Drop implementations. Adds logging for publish calls and updates method usage to leverage the new property config handling.

The RawCstrs instance in the TryFrom implementation for EspUnsubscribePropertyConfig no longer needs to be mutable, as it is not mutated after creation.

ivmarkov · 2026-01-01T13:10:16Z

@danielmeza If this is not ready for review, perhaps you can turn it into a "Draft" status?

danielmeza · 2026-01-06T00:03:09Z

@ivmarkov yes! I have moved to draf.

danielmeza mentioned this pull request Jul 30, 2025

Mqtt protocol v5 definition esp-rs/embedded-svc#84

Open

Copilot AI review requested due to automatic review settings December 27, 2025 18:38

Copilot started reviewing on behalf of danielmeza December 27, 2025 18:39 View session

Copilot AI reviewed Dec 27, 2025

View reviewed changes

danielmeza added 10 commits December 27, 2025 19:49

Add V5 option to enum

27f79f5

feat(mqtt): enhance MQTT 5.0 support with event properties and error …

f50dabf

…handling

Format code

72952ef

remove async implementation prior to V5 redesign

ea84735

Remove mutability from RawCstrs in unsubscribe config

b29671f

The RawCstrs instance in the TryFrom implementation for EspUnsubscribePropertyConfig no longer needs to be mutable, as it is not mutated after creation.

danielmeza force-pushed the add-mqtt-v5-protocol branch from a0ad048 to b29671f Compare December 28, 2025 00:50

danielmeza marked this pull request as draft January 6, 2026 00:08

+/// Wrapper around the ESP-IDF MQTT v5 `mqtt5_user_property_handle_t`.
+///
+/// This type owns an MQTT 5 user properties handle together with the
+/// backing C strings required by the ESP-IDF API. It is typically
+/// constructed from a slice of [`UserPropertyItem`] values (see
+/// [`EspUserPropertyList::from`]) or created empty via [`EspUserPropertyList::new`],
+/// populated, and then passed to the underlying MQTT v5 client functions.
+///
+/// The embedded [`RawCstrs`] value ensures that the UTF-8 strings backing
+/// each user property key/value pair remain valid for as long as the
+/// corresponding `mqtt5_user_property_handle_t` is in use.

Add mqtt v5 protocol implementation #590

Are you sure you want to change the base?

Add mqtt v5 protocol implementation #590

Uh oh!

Conversation

danielmeza commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Details 📖

Description

Testing

Curren issues:

Submission Checklist 📝

Uh oh!

ivmarkov commented Aug 3, 2025

Uh oh!

danielmeza commented Aug 3, 2025

Uh oh!

danielmeza commented Sep 11, 2025

Uh oh!

danielmeza commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielmeza commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielmeza commented Dec 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

ivmarkov commented Jan 1, 2026

Uh oh!

danielmeza commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danielmeza commented Jul 30, 2025 •

edited

Loading

danielmeza commented Dec 24, 2025 •

edited

Loading

danielmeza commented Dec 24, 2025 •

edited

Loading