adafruit / Adafruit_MQTT_Library

Arduino library for MQTT support
MIT License
571 stars 292 forks source link

Better ping handling #170

Closed Fapiko closed 3 years ago

Fapiko commented 4 years ago

This fixes pings discarding publish messages by always checking for a PUBLISH packet when inside processPacketsUntil. This ensures topic messages will always get handled even if we're pinging.

Also found a bug in Adafruit_MQTT_Client::readPacket which would actually read a byte rather than just returning. This would cause it to fail to read the next packet as the packet type byte had already been read.

I suspect these changes will address issues #141 , #132 , and #83 . I have not tried reproducing them though to verify.

brentru commented 4 years ago

@Fapiko Which hardware did you test this on? I'd like to test on hardware before merging in, and want to use a different hardware target than what was tested.

Fapiko commented 4 years ago

@brentru I have only tested this with the ESP32

brentru commented 4 years ago

@Fapiko Cool - I'll test with an 8266!

cbitter78 commented 3 years ago

I also noticed if you send data that exceeds 93 bytes then the packets are read but it does not find the subscription because it shows the len of the subscription as 0 If I send 93 bytes it works, 94 and it does not.

The data that was sent is

{"name": "vend", "id": "JThpGd", "slot": 2, "args": ["Charles", "Ballast Point             "]}
21:15:24.249 -> Read data:      0 [0x30], 
21:15:24.249 -> Packet Type:        0 [0x30], 
21:15:24.249 -> Read data:        [0x80], 
21:15:24.249 -> Read data:        [0x01], 
21:15:24.249 -> Packet Length:  128
21:15:24.249 -> Read data:        [0x00],   [0x20], c [0x63], b [0x62], i [0x69], t [0x74], t [0x74], e [0x65], 
21:15:24.249 ->     r [0x72], 7 [0x37], 8 [0x38], / [0x2F], f [0x66], e [0x65], e [0x65], d [0x64], 
21:15:24.249 ->     s [0x73], / [0x2F], i [0x69], o [0x6F], t [0x74], - [0x2D], b [0x62], e [0x65], 
21:15:24.249 ->     e [0x65], r [0x72], - [0x2D], c [0x63], o [0x6F], m [0x6D], m [0x6D], a [0x61], 
21:15:24.249 ->     n [0x6E], d [0x64], { [0x7B], " [0x22], n [0x6E], a [0x61], m [0x6D], e [0x65], 
21:15:24.249 ->     " [0x22], : [0x3A],   [0x20], " [0x22], v [0x76], e [0x65], n [0x6E], d [0x64], 
21:15:24.249 ->     " [0x22], , [0x2C],   [0x20], " [0x22], i [0x69], d [0x64], " [0x22], : [0x3A], 
21:15:24.249 ->       [0x20], " [0x22], J [0x4A], T [0x54], h [0x68], p [0x70], G [0x47], d [0x64], 
21:15:24.249 ->     " [0x22], , [0x2C],   [0x20], " [0x22], s [0x73], l [0x6C], o [0x6F], t [0x74], 
21:15:24.249 ->     " [0x22], : [0x3A],   [0x20], 2 [0x32], , [0x2C],   [0x20], " [0x22], a [0x61], 
21:15:24.249 ->     r [0x72], g [0x67], s [0x73], " [0x22], : [0x3A],   [0x20], [ [0x5B], " [0x22], 
21:15:24.249 ->     C [0x43], h [0x68], a [0x61], r [0x72], l [0x6C], e [0x65], s [0x73], " [0x22], 
21:15:24.285 ->     , [0x2C],   [0x20], " [0x22], B [0x42], a [0x61], l [0x6C], l [0x6C], a [0x61], 
21:15:24.285 ->     s [0x73], t [0x74],   [0x20], P [0x50], o [0x6F], i [0x69], n [0x6E], t [0x74], 
21:15:24.285 ->       [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20], 
21:15:24.285 ->       [0x20],   [0x20],   [0x20],   [0x20],   [0x20], " [0x22], ] [0x5D], } [0x7D], 
21:15:24.285 ->     
21:15:24.285 -> Packet len: 131
21:15:24.285 ->     0 [0x30],   [0x80],   [0x01],   [0x00],   [0x20], c [0x63], b [0x62], i [0x69], 
21:15:24.285 ->     t [0x74], t [0x74], e [0x65], r [0x72], 7 [0x37], 8 [0x38], / [0x2F], f [0x66], 
21:15:24.285 ->     e [0x65], e [0x65], d [0x64], s [0x73], / [0x2F], i [0x69], o [0x6F], t [0x74], 
21:15:24.285 ->     - [0x2D], b [0x62], e [0x65], e [0x65], r [0x72], - [0x2D], c [0x63], o [0x6F], 
21:15:24.285 ->     m [0x6D], m [0x6D], a [0x61], n [0x6E], d [0x64], { [0x7B], " [0x22], n [0x6E], 
21:15:24.285 ->     a [0x61], m [0x6D], e [0x65], " [0x22], : [0x3A],   [0x20], " [0x22], v [0x76], 
21:15:24.285 ->     e [0x65], n [0x6E], d [0x64], " [0x22], , [0x2C],   [0x20], " [0x22], i [0x69], 
21:15:24.285 ->     d [0x64], " [0x22], : [0x3A],   [0x20], " [0x22], J [0x4A], T [0x54], h [0x68], 
21:15:24.285 ->     p [0x70], G [0x47], d [0x64], " [0x22], , [0x2C],   [0x20], " [0x22], s [0x73], 
21:15:24.285 ->     l [0x6C], o [0x6F], t [0x74], " [0x22], : [0x3A],   [0x20], 2 [0x32], , [0x2C], 
21:15:24.285 ->       [0x20], " [0x22], a [0x61], r [0x72], g [0x67], s [0x73], " [0x22], : [0x3A], 
21:15:24.285 ->       [0x20], [ [0x5B], " [0x22], C [0x43], h [0x68], a [0x61], r [0x72], l [0x6C], 
21:15:24.285 ->     e [0x65], s [0x73], " [0x22], , [0x2C],   [0x20], " [0x22], B [0x42], a [0x61], 
21:15:24.285 ->     l [0x6C], l [0x6C], a [0x61], s [0x73], t [0x74],   [0x20], P [0x50], o [0x6F], 
21:15:24.285 ->     i [0x69], n [0x6E], t [0x74],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20], 
21:15:24.285 ->       [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20],   [0x20], 
21:15:24.285 ->     " [0x22], ] [0x5D], } [0x7D], 
21:15:24.285 -> Looking for subscription len 0

I tried setting

SUBSCRIPTIONDATALEN: 250 MAXBUFFERSIZE: 250

This had no effect. I still could only send 93 bytes.

This is just an observation, It may suggest a different bug all together.

brentru commented 3 years ago

@cbitter78 Thanks for testing.

This is just an observation, It may suggest a different bug all together.

Could you file this bug separately on issues?

cbitter78 commented 3 years ago

@brentru no problem. Here it is #185

cbitter78 commented 3 years ago

@brentru I have been testing using this branch for a few days now. I have had no issues. It works great!

brentru commented 3 years ago

@cbitter78 Ok, thanks for testing. @Fapiko Could you bump version in https://github.com/adafruit/Adafruit_MQTT_Library/blob/master/library.properties#L2 to 2.1.0 to prepare for release?

The CI will also re-run (it ran too long ago, can't currently re-run) and we may need to fix some small errors if it fails.

Fapiko commented 3 years ago

@brentru Done

Fapiko commented 3 years ago

@brentru Any idea what the CI build fail means?

brentru commented 3 years ago

@Fapiko Yeah. Looks like it's failing on clang-formatting. There's a few formatting things to clean up. https://github.com/adafruit/Adafruit_MQTT_Library/pull/170/checks?check_run_id=1359826162#step:7:7

You'll want to download https://github.com/Sarcasm/run-clang-format/blob/master/run-clang-format.py and run python3 run-clang-format.py -r . in the root directory of this repo.

Fapiko commented 3 years ago

@brentru :+1:

brentru commented 3 years ago

@Fapiko Thanks! LGTM!!