hirotakaster / MQTT

MQTT for Photon, Spark Core
Other
216 stars 118 forks source link

subscribe() fails when giving MQTTQOS1 as second argument #12

Closed Lahorde closed 8 years ago

Lahorde commented 8 years ago

Sometimes I do not receive an MQTT event on a given topic. Events are received by other clients, I checked it with mosquitto_sub. So I tried to modify qos in :

mqttClient.connect("roomba_photon", NULL,  MQTTQOS2, 0, NULL); 

But problem still remains, so I tried :

mqttClient.subscribe(topic, MQTTQOS1);

but in this case no messages are received on topic, changing to MQTTQOS0 works fine.

hirotakaster commented 8 years ago

Hi Lahorde, I recommend you implement re-connect method or connect to the mqtt-server when you need. In my case, when keeping connect mqtt client with server suddenly disconnect or tcp connection is fail. That's reason is not based mqtt client library, I think photon firmware bug or wifi connection problem. And FYI, as you know mqtt use tcp connection on wifi, keeping the tcp connection is high power consumption in my test on photon. so I recommend you would be better connect to mqtt server when you need only.

Lahorde commented 8 years ago

Hi hirotakaster, As you can see I already reconnect to mqtt when connection drops (loop() code : https://github.com/OpHaCo/roomba_wifi/blob/master/roomba-control.ino I lost some packets because of frequents connection drops. But before discovering it I tried to modify qos and saw problem descibed in this issue. No I have no more disconnections and performed some tests checking no packets are lost.

By investigating given issue, I saw :

mqttClient.connect("roomba_photon", NULL,  MQTTQOS2, 0, NULL); 
mqttClient.subscribe(topic, MQTTQOS1);

won't have any effect on qos, in MQTT.h are defined qos levels

#define MQTTQOS0        (0 << 1)
#define MQTTQOS1        (1 << 1)
#define MQTTQOS2        (2 << 1)

If I take connection methods, on underlying implementation, qos must be set at bit pos 3-4 in CONNECT frame buffer byte 10(refer http://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/mqtt-v3r1.html#connect), In code it is done with :

if (willTopic) {
            v = 0x06|(willQos<<3)|(willRetain<<5);

But if I give MQTTQOS1 as parameter qos position won't be correct as it will be shifted by 1 once again. Maybe you can move MQTTQOSx definitions in source and add another enum/define for QOS to be provided to public MQTT methods.

Issue with

mqttClient.subscribe(topic, MQTTQOS1);

is quite similar, first check done in this method :

    if (qos < 0 || qos > 1)
        return false;

if I give MQTTQOS1 It will never subscribe anything!

I can send you a pull request with modifications if you want.

hirotakaster commented 8 years ago

oh, I see. please send a pull request and contribute your commit. thank you.