Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enqueued qos0 dispatch MQTT_EVENT_PUBLISHED (IDFGH-12511) #273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nebkat
Copy link
Contributor

@nebkat nebkat commented Apr 1, 2024

Dispatch MQTT_EVENT_PUBLISHED when an enqueued QOS0 message is actually published.

@github-actions github-actions bot changed the title Enqueued qos0 dispatch MQTT_EVENT_PUBLISHED Enqueued qos0 dispatch MQTT_EVENT_PUBLISHED (IDFGH-12511) Apr 1, 2024
@euripedesrocha
Copy link
Collaborator

Hi @nebkat thanks for your contribution. Could you clarify your use case here?

@nebkat
Copy link
Contributor Author

nebkat commented Apr 5, 2024

Hi @euripedesrocha!

When you esp_mqtt_client_enqueue a qos0 message it gets added to the outbox but there is no confirmation that it actually got sent to the broker.

Qos1+ can be used to receive the explicit confirmation back but I do not need that level of guarantee, I just want to know when the message got sent (including for tracking # of messages successfully published in this session).

esp_mqtt_client_publish also doesn't work for me because I'm publishing in an event handler than needs to return quickly.

And lastly, I would like to experiment with semaphore synchronized publishing which would be easier if all messages triggered MQTT_EVENT_PUBLISHED.

@euripedesrocha
Copy link
Collaborator

@nebkat thanks for clarification.
We don't have the event for QoS 0 messages because it at first don't make sense given that they are a QoS level that is used when user don't want to track the message.
Could you clarify why you want to have the synchronized publishing?

@nebkat
Copy link
Contributor Author

nebkat commented Apr 8, 2024

These are technically two separate issues - with QoS 0 we might not care whether the broker actually successfully processes the message once it is received, but we may still want to know whether the client ever attempted to send the message in the first place (since we queued it for asynchronous sending and otherwise have no way of knowing).

In other words: it is ok if my broker or network connection somehow lose the message, but it seems wasteful to have to depend on the broker to observe the behavior of the client (by upgrading to QoS 1/2).

With this change I use MQTT_EVENT_PUBLISHED to count the total number of messages and bytes published across all QoS.

For synchronization, I would like to be able to do:

// in handler
case ESP_MQTT_PUBLISH:
    xTaskNotifyGive(task_handle);

// in task
esp_mqtt_client_enqueue(...); // QoS0
// Possibly do something here
if (!ulTaskNotifyTake(true, pdMS_TO_TICKS(1000))) return; // Stop publishing batch if previous message fails
esp_mqtt_client_enqueue(...); // QoS0
// Possibly do something here
if (!ulTaskNotifyTake(true, pdMS_TO_TICKS(1000))) return; // Stop publishing batch if previous message fails

This is not possible with esp_mqtt_client_publish. Furthermore, I believe using esp_mqtt_client_publish will skip ahead of any enqueued messages which is undesirable for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants