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

publish callback in wrong place for QOS > 0 #163

Open
davydnorris opened this issue May 1, 2019 · 3 comments
Open

publish callback in wrong place for QOS > 0 #163

davydnorris opened this issue May 1, 2019 · 3 comments

Comments

@davydnorris
Copy link

The user defined publish callback is set to be called when a message is successfully sent to the MQTT server. For QOS0 this is OK, but for QOS1 and QOS2 the rest of the publish transaction is being ignored.

In these cases the correct place to call the publishCb callback in mqtt.c would be after line 388 for QOS1 and after line 406 for QOS2:

          case MQTT_MSG_TYPE_PUBACK:
            if (client->mqtt_state.pending_msg_type == MQTT_MSG_TYPE_PUBLISH && client->mqtt_state.pending_msg_id == msg_id) {
/********** QOS1 here ************/
              MQTT_INFO("MQTT: received MQTT_MSG_TYPE_PUBACK, finish QoS1 publish\r\n");
            }

            break;
          case MQTT_MSG_TYPE_PUBREC:
            client->mqtt_state.outbound_message = mqtt_msg_pubrel(&client->mqtt_state.mqtt_connection, msg_id);
            if (QUEUE_Puts(&client->msgQueue, client->mqtt_state.outbound_message->data, client->mqtt_state.outbound_message->length) == -1) {
              MQTT_INFO("MQTT: Queue full\r\n");
            }
            break;
          case MQTT_MSG_TYPE_PUBREL:
            client->mqtt_state.outbound_message = mqtt_msg_pubcomp(&client->mqtt_state.mqtt_connection, msg_id);
            if (QUEUE_Puts(&client->msgQueue, client->mqtt_state.outbound_message->data, client->mqtt_state.outbound_message->length) == -1) {
              MQTT_INFO("MQTT: Queue full\r\n");
            }
            break;
          case MQTT_MSG_TYPE_PUBCOMP:
            if (client->mqtt_state.pending_msg_type == MQTT_MSG_TYPE_PUBLISH && client->mqtt_state.pending_msg_id == msg_id) {
/********** QOS2 here ************/
              MQTT_INFO("MQTT: receive MQTT_MSG_TYPE_PUBCOMP, finish QoS2 publish\r\n");
            }
            break;

In addition, there would need to be a check for QOS0 in the current spot where it's called (line 463) but I can't see where the QOS value for the transaction can be accessed.

@davydnorris
Copy link
Author

I discovered this little wrinkle because I'm deep sleeping my device regularly, and so have transaction counting in place to work out when the device has completed and can sleep. The publishCb function was being called too early for QOS1 and the unit was sleeping before the last transaction had completely finished.

@davydnorris
Copy link
Author

OK I found where the pending QoS can be retrieved so line 463 would need to be:

	if ((client->connState == MQTT_DATA || client->connState == MQTT_KEEPALIVE_SEND)
			&& client->mqtt_state.pending_msg_type == MQTT_MSG_TYPE_PUBLISH) {
		if (client->publishedCb && client->mqtt_state.pending_publish_qos == QOS0)
			client->publishedCb((uint32_t*) client);
	}

@davydnorris
Copy link
Author

davydnorris commented May 2, 2019

So it turns out that mqtt_state.pending_publish_qos is never set in the code so it's always 0 (QOS0). This would need to be set just before sending the message at line 744:

client->mqtt_state.pending_publish_qos = mqtt_msg_get_qos(dataBuffer);

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

1 participant