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

How to check the client has received a Connack from the Broker after connect() #163

Open
schnedann opened this issue Mar 29, 2022 · 7 comments

Comments

@schnedann
Copy link

after reading the API Docu and reading the code,
I still have no clue how to check for an successful connect,
so how one can check if the broker sent a Connack, and the
Client is in a Connected state prior to executing a publish?

@pieterconradie
Copy link

Hi @schnedann

I have the same problem, for example when wrong MQTT password is used then mqtt_connect() will return MQTT_OK but only a while later will the MQTT_ERROR_CONNECTION_REFUSED be returned.

I can only offer a hint / workaround. Inspect __mqtt_recv() and search for MQTT_CONTROL_CONNACK. You will see that client->typical_response_time is updated. You can check if this field has been updated to detect the connected state.

I think a better way would be to modify struct mqtt_client and add a state field that gets updated when MQTT_CONTROL_CONNACK is received. All of the other error cases would also need to be catered for.

Best regards,
Pieter
https://piconomix.com

@schnedann
Copy link
Author

I think a better way would be to modify struct mqtt_client and add a state field that gets updated when MQTT_CONTROL_CONNACK is received. All of the other error cases would also need to be catered for.

Hi, thx for the reply. In fact, this is what I did for serving our needs. Its still not clear if I can provide these changes upstream...

@rechrtb
Copy link

rechrtb commented Nov 15, 2022

+1 on having a better way to detect CONNACK has been received.

@pieterconradie
Copy link

Here's my quick fix. I added a bool flag in struct mqtt_client:

struct mqtt_client {
    ...
    /** @brief Flag is set on connection event */
    bool event_connect;
};

I set the flag in __mqtt_recv():

          switch (response.fixed_header.control_type) {
            case MQTT_CONTROL_CONNACK:
                /* release associated CONNECT */
                msg = mqtt_mq_find(&client->mq, MQTT_CONTROL_CONNECT, NULL);
                if (msg == NULL) {
                    client->error = MQTT_ERROR_ACK_OF_UNKNOWN;
                    mqtt_recv_ret = MQTT_ERROR_ACK_OF_UNKNOWN;
                    break;
                }
                msg->state = MQTT_QUEUED_COMPLETE;
                lat_log(LAT_VER, "Release msg type %u", response.fixed_header.control_type);
                /* initialize typical response time */
                client->typical_response_time = (double) (MQTT_PAL_TIME() - msg->time_sent);
                client->last_response_time = (double) (MQTT_PAL_TIME() - msg->time_sent);
                client->min_response_time = (double) (MQTT_PAL_TIME() - msg->time_sent);
                client->max_response_time = (double) (MQTT_PAL_TIME() - msg->time_sent);

                /* check that connection was successful */
                if (response.decoded.connack.return_code != MQTT_CONNACK_ACCEPTED) {
                    if (response.decoded.connack.return_code == MQTT_CONNACK_REFUSED_IDENTIFIER_REJECTED) {
                        client->error = MQTT_ERROR_CONNECT_CLIENT_ID_REFUSED;
                        mqtt_recv_ret = MQTT_ERROR_CONNECT_CLIENT_ID_REFUSED;
                    } else {
                        client->error = MQTT_ERROR_CONNECTION_REFUSED;
                        mqtt_recv_ret = MQTT_ERROR_CONNECTION_REFUSED;
                    }
                    break;
                }
                else {
                    client->event_connect = true;
                }
                break;

Remember to clear the flag in mqtt_init(), mqtt_init_reconnect() and mqtt_reinit()

@acassis
Copy link

acassis commented Nov 25, 2022

Hi @pieterconradie I think your solution is more elegant than mine:

--- a/include/mqtt.h    2021-03-29 14:53:52.000000000 -0300
+++ b/include/mqtt.h    2022-11-10 20:24:11.581983160 -0300
@@ -234,7 +234,8 @@
 enum MQTTErrors {
     MQTT_ERROR_UNKNOWN=INT_MIN,
     __ALL_MQTT_ERRORS(GENERATE_ENUM)
-    MQTT_OK = 1
+    MQTT_OK = 1,
+    MQTT_ACK
 };

 /**
diff -Naur a/src/mqtt.c b/src/mqtt.c
--- a/src/mqtt.c        2021-03-29 14:53:52.000000000 -0300
+++ b/src/mqtt.c        2022-11-10 20:23:41.997057844 -0300
@@ -723,7 +723,11 @@
                 /* initialize typical response time */
                 client->typical_response_time = (double)
(MQTT_PAL_TIME() - msg->time_sent);
                 /* check that connection was successful */
-                if (response.decoded.connack.return_code !=
MQTT_CONNACK_ACCEPTED) {
+                if (response.decoded.connack.return_code ==
MQTT_CONNACK_ACCEPTED) {
+                       client->error = MQTT_ACK;
+                 }
+                 else {
                     if (response.decoded.connack.return_code ==
MQTT_CONNACK_REFUSED_IDENTIFIER_REJECTED) {
                         client->error = MQTT_ERROR_CONNECT_CLIENT_ID_REFUSED;
                         mqtt_recv_ret = MQTT_ERROR_CONNECT_CLIENT_ID_REFUSED;

MQTT-C has an example usage for NuttX RTOS, I will submit a patch to them based on your solution, ok?

@pieterconradie
Copy link

Hi @acassis You are most welcome. Go ahead. Best regards, Pieter

@acassis
Copy link

acassis commented May 12, 2023

@pieterconradie finally I got time to submit the PR to NuttX mainline:
apache/nuttx-apps@e90960d

Thank you very much!

pkarashchenko pushed a commit to apache/nuttx-apps that referenced this issue May 15, 2023
This is a fix implemented by Pieter Conradie as described here:
LiamBindle/MQTT-C#163
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants