Implement MQTT outbox limits and get_outbox_size()

[mike1703]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

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.

Pull Request Details 📖

Description

This change implements get_outbox_size() on the EspMqttClient to retrieve the amount of bytes currently stored in the MQTT outbox.
Additionally implement a configurable option in MqttClientConfiguration to limit the outbox size to the given amount of bytes so there is no ever growing outbox size that might fill up the entire RAM.

Testing

I tested the limit with this configuration and sent a a lot of bytes

        &MqttClientConfiguration {
            client_id: Some(client_id),
            username,
            password,
            outbox_limit: Some(5000),
            ..Default::default()
        },

and tested it with

    if client.get_outbox_size() > 5100 {
        error!("too much in the outbox, ignore this message");
    } 

The message is shown if no limit is configured and does not show up once a limit of 5000 is set.

            match client.enqueue(
                &topic,
                esp_idf_svc::mqtt::client::QoS::AtLeastOnce,
                false,
                value.as_bytes(),
            ) {
                Ok(message_id) => debug!("enqueued {topic:?} = {value} ({message_id})"),
                Err(error) => error!("cannot enqueue mqtt message: {error}"),
            }

This returns cannot enqueue mqtt message: ERROR if the outbox is full.

[Vollbrecht]

Looks reasonable.The only question is if we should model the no_limit as 0 and not an Option as most other fields here, but i guess that's a nit. ( Assuming no limit is actually 0 and not something like u64::max :D )

Technically its a breaking change since we add a pub field to the struct but well its so minimal its fine from my site.

[ivmarkov]

Looks reasonable.The only question is if we should model the no_limit as 0 and not an Option as most other fields here, but i guess that's a nit. ( Assuming no limit is actually 0 and not something like u64::max :D )

Technically its a breaking change since we add a pub field to the struct but well its so minimal its fine from my site.

I actually prefer Option.

Also, I might very soon introduce 2 breaking changes, so breaking the API in a minor way probably not a big deal.

[mike1703]

Regarding Option vs 0: I thought about going this route but then tried to mimic the style of e.g. buf.out_size. (edit: with two comments about this already, I'm going for the Option variant)

I added a Default::default for the outbox_config_t type as I was missing this. (edit: with option not useful anymore, only set what you have)

Unfortunately I did not find any public documentation on the behavior of the default 0 but looked at sources:
espressif/esp-mqtt@372ab7b introduces this feature (and is still there https://github.com/espressif/esp-mqtt/blob/cea7e959eee68fc974fcae92572a1c37eca72d06/mqtt_client.c#L2119)

[ivmarkov]

OK - thanks a lot!
I'll review in details later today/tmr.

In general I need this too as well - in fact - ASAP - so this is arriving just in time. :)

[mike1703]

made it optional and moved it out of the explicit config to the part where the optional config fields are used

[mike1703]

btw, if you want me to move the change from the added to the breaking section, I can totally do this. I was just not if I should based on your comments

[ivmarkov]

i32 here is a weird choice. ESP-IDF seems to cast the internal uint64_t to i32 (weird?? why?). Regardless, we should instead return a usize.

[mike1703]

Yes, I'm not sure if this was done by accident or on purpose but internally it is by default 0 and if available the uint64_t.
I gave it another try and capped the value at 0 and return it as usize. wdyt?

[ivmarkov]

Thanks!