bertmelis / espMqttClient

MQTT 3.1.1 client library for the Espressif devices ESP8266 and ESP32 on the Arduino framework.
https://www.emelis.net/espMqttClient/
MIT License
92 stars 21 forks source link

fix _onPubcomp #111

Closed MichaelDvP closed 10 months ago

MichaelDvP commented 10 months ago

With receiving Pubcomp the communicaion is finished.

bertmelis commented 10 months ago

Another bug? 😢

MichaelDvP commented 10 months ago

Yes, but mosquitto ignores it, only the ioBroker mqtt-broker throws a lot of errors because of the extra pubcomp.

bertmelis commented 10 months ago

Ouch, that's a stupid copy-paste error of me.

Obviously we don't need to send a pubcomp when receiving a pubcomp. 🤦

MichaelDvP commented 10 months ago

BTW: is this: https://github.com/bertmelis/espMqttClient/blob/490a94ab25e994dfe4905ffc0a155d0257ade404/src/MqttClient.cpp#L44 fixed value for _timeout intended or should it be EMC_TX_TIMEOUT from the config?

bertmelis commented 10 months ago

BTW: is this:

https://github.com/bertmelis/espMqttClient/blob/490a94ab25e994dfe4905ffc0a155d0257ade404/src/MqttClient.cpp#L44

fixed value for _timeout intended or should it be EMC_TX_TIMEOUT from the config?

Yes. I believe so. This compile time variable also isn't in the docs.

bertmelis commented 10 months ago

The code was actually tested for this scenario but Mosquitto is forgiving and doesn't disconnect.

See https://github.com/bertmelis/espMqttClient/blob/main/test/test_client_native/test_client_native.cpp#L226

Maybe I should add a delay in the test to give Mosquitto some time to realize something went wrong...???

MichaelDvP commented 10 months ago

Maybe I should add a delay in the test to give Mosquitto some time to realize something went wrong...???

No, mosquitto just knows the communication is successfull and ignores the extra pubcomp. There is no errormessage, it just works. Other brokers get confused by this packet. The ioBroker-mqtt i've used does not like it, i tested with mosquitto, it works, so i first thought it was an error in ioBroker,

bertmelis commented 10 months ago

That's it! I just test with Mosquitto (in Github actions but also in my home setup).

Thanks for the find!