search

tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
599 stars 188 forks source link

MQTT: keepalive, clean disconnect, and more! #446

Closed jmacwhyte closed 2 years ago

jmacwhyte commented 2 years ago

This update adds in various functionality to allow MQTT clients to disconnect and reconnect gracefully. Notably:

  1. New keepalive goroutine to regularly ping the broker according to the specified keepalive interval.
  2. Ability to set keep alive and ping timeout values in the options (with defaults).
  3. If keepalive or any other routine determines the connection is broken (interrupted socket, ping timeout, etc), all routines shut down gracefully and connection status is updated. IsConnected() now accurately shows the connection status, and Disconnect() now works to force a disconnect.
  4. A reconnect can be safely performed by simply calling Connect() again on the existing instance of the mqtt client.

I have tested consecutively disconnecting and reconnecting, and show no memory leaks.

Also,

  1. Retain flag support has been added back in, so clients can set an "away message".
jmacwhyte commented 2 years ago

It looks like commit 2cfc08b560bdb3c6c8e5e0b157061a4551850d15 added in pub.Retain = retained in an attempt to add retain flags, but this is strange because I don't see any Retain variable declared in Eclipse's PublishPacket, so I don't think this code is valid. @mek-x Please let me know if I'm missing something!

mek-x commented 2 years ago

Hi @jmacwhyte, thanks for asking. Retain is part of FixedHeader defined here, which is nested anonymous struct field in PublishPacket (hence Retain can be accessed directly). Besides it must work, I'm using this implementation in my current project ;-)

jmacwhyte commented 2 years ago

@mek-x Aha, I didn't notice FixedHeader was anonymous! Thanks for pointing that out. Either way will work, I guess.

I hope you try out this commit and let me know how it works for you, I think it is a big improvement!

deadprogram commented 2 years ago

@jmacwhyte can you please rebase against dev to resolve the merge conflicts?

jmacwhyte commented 2 years ago

@deadprogram Absolutely, I'll clean it up later today and ping you when it's fixed.

jmacwhyte commented 2 years ago

@deadprogram All good to go! It's now a single commit on the top of dev.

deadprogram commented 2 years ago

Did some testing of my own, and it was working great. Thank you so very much for working on this @jmacwhyte and @mek-x for the previous additions and review.

Now merging.