search

thorrak / tiltbridge

Tilt Hydrometer to WiFi Bridge
http://www.tiltbridge.com/
Other
61 stars 27 forks source link

MQTT keepalive is being multiplied by 1000 (so, 30000 instead of 30) #198

Closed bluemodena closed 2 years ago

bluemodena commented 2 years ago

I think there is a bug in the MQTT code where the keep alive is being multiplied by 1000. The value should be in seconds for eclipse-mosquito client. All my other MQTT enable devices use seconds.

1640362453: New client connected from XXX.XXX.X.XXX as GarageMonitor (p2, c1, k15, u'mqtt-user'). 1640362453: New client connected from XXX.XXX.X.XXX as Freezer (p2, c1, k30, u'mqtt-user'). 1640362458: New client connected from XXX.XXX.X.XXX as EspressoClient (p2, c1, k30, u'mqtt-user'). 1640362460: New client connected from XXX.XXX.X.XXX as ChristmasHouseLights-TasmotaPlug (p2, c1, k30, u'mqtt-user'). 1640362464: New client connected from XXX.XXX.X.XXX as CrawlSpaceMonitor (p2, c1, k15, u'mqtt-user'). 1640362467: New client connected from XXX.XXX.X.XXX as EntranceLight (p2, c1, k30, u'mqtt-user').

1640293979: New client connected from XXX.XXX.X.XXX as tiltbridge (p2, c1, k30000, u'mqtt-user').

https://github.com/thorrak/tiltbridge/blob/master/src/sendData.cpp (line#388) mqttClient.setKeepAlive(config.mqttPushEvery * 1000);

thorrak commented 2 years ago

What is the impact of this?

thorrak commented 2 years ago

In doing some research, it looks like TiltBridge deviates from the standard: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901045

bluemodena commented 2 years ago

@thorrak, it is causing the client to disconnect. I'm still using mqtt_v3.

https://mosquitto.org/man/mosquitto-conf-5.html If an MQTT v3.1.1 or v3.1 client specifies a keepalive time greater than max_keepalive they will be sent a CONNACK message with the "identifier rejected" reason code, and disconnected.

In my solution, I have the tiltbridge reading from TiltHydrometers, which then pushes everything to my Mosquitto server (https://hub.docker.com/_/eclipse-mosquitto/) which them publishes to my craftbeerpi4 (https://github.com/avollkopf/craftbeerpi4)

I think all that is needed here is to remove the ' 1000' from https://github.com/thorrak/tiltbridge/blob/master/src/sendData.cpp (line#388) mqttClient.setKeepAlive(config.mqttPushEvery 1000);

thorrak commented 2 years ago

@bluemodena - I managed to locate some hardware here, and am trying to get an update for (amongst other things) this issue. If I upload a beta firmware, would you be willing to test it?

bluemodena commented 2 years ago

@thorrak Yes, I would be happy to. While I am familiar with MQTT, I'm not that familiar with flashing ESP32s... Would this be available for testing using BrewFlasher (as this is what I originally used)?

lbussy commented 2 years ago

Yes:

Updated TiltBridge - BETA - TFT_ESPI - TTGO USB-C to 1.0.3-Beta
Updated TiltBridge - BETA - LCD_SSD1306 - OLED to 1.0.3-Beta
Updated TiltBridge - BETA - D32_Pro_TFT - TFT to 1.0.3-Beta
thorrak commented 2 years ago

I edited @lbussy 's post to remove the SIllyhats one, but the other 3 beta versions are up-to-date and include this fix

bluemodena commented 2 years ago

I have a lot going on at work this week, so I'm late the the party, but the change looks good. Thanks.