QOS0 incorrect outbox expiry deletion when MQTT_REPORT_DELETED_MESSAGES is defined (IDFGH-14021)

[Totrasmek]

Checklist

• Checked the issue tracker for similar issues to ensure this is not a duplicate
• Read the documentation to confirm the issue is not addressed there and your configuration is set correctly
• Tested with the latest version to ensure the issue hasn't been fixed

How often does this bug occurs?

always

Expected behavior

When setting MQTT_REPORT_DELETED_MESSAGES, I expect to receive an MQTT_EVENT_DELETED posted to the event handler when the esp_mqtt_task calls mqtt_delete_expired_messages(client) for each QOS0 message that has been in the outbox > OUTBOX_EXPIRED_TIMEOUT_MS. This is the behaviour for QOS1 and QOS2 messages.

Actual behavior (suspected bug)

When my outbox has only QOS0 messages, I receive no MQTT_EVENT_DELETED posted to the event handler, and only the first message in the outbox is deleted if it has been in the outbox > OUTBOX_EXPIRED_TIMEOUT_MS.

This is because QOS0 messages are given a msg_id of 0 and mqtt_delete_expired_messages stops deleting outbox items once their msg_id is 0 as seen here:

while ((msg_id = outbox_delete_single_expired(client->outbox, platform_tick_get_ms(), OUTBOX_EXPIRED_TIMEOUT_MS)) > 0) {
        client->event.event_id = MQTT_EVENT_DELETED;
        client->event.msg_id = msg_id;
        if (esp_mqtt_dispatch_event(client) != ESP_OK) {
            ESP_LOGE(TAG, "Failed to post event on deleting message id=%d", msg_id);
        }
    }

This might be easily fixed by changing the conditional from > to >= on the msg_id

Error logs or terminal output

No response

Steps to reproduce the behavior

1. Ensure your event handler will log MQTT_EVENT_DELETED
2. Turn on MQTT_REPORT_DELETED_MESSAGES
3. Log the outbox size in esp_mqtt_task just before and after mqtt_delete_expired_messages(client)
4. Continuously enqueue to the outbox QOS0 messages
5. Add some high latency to the network to force messages to reach past the OUTBOX_EXPIRED_TIMEOUT_MS by using a local broker and simulated latency through a command like sudo tc qdisc add dev wlp0s20f3 root netem delay 2000ms
6. Observe that you will never see MQTT_EVENT_DELETED
7. Observe that the outbox will only ever drop by the size of a single message at a time, even though your outbox is likely filled with many expired messages.

Project release version

I am using commit cac1552 shipped with ESP-IDF v5.2,v5.3 and v5.4

System architecture

other (details in Additional context)

Operating system

Linux

Operating system version

Ubuntu 22.04

Shell

Bash

Additional context

No response

[euripedesrocha]

Hi @Totrasmek thanks for reporting and providing the PR with the fix. I'll review and test it.