adafruit / Adafruit_MQTT_Library

Arduino library for MQTT support
MIT License
573 stars 291 forks source link

topic lenght - current code only reads the LSB part #102

Closed jwende closed 3 years ago

jwende commented 6 years ago

https://github.com/adafruit/Adafruit_MQTT_Library/blob/974f4b8713a83d01b040a0d23b798d3f2978d990/Adafruit_MQTT.cpp#L464

would result in an error if the topicname is larger than 255 bytes

jjunimd commented 6 years ago

Should it be changed to topiclen = buffer[4]; ?

jwende commented 6 years ago

please see the spec at: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html

so the current code only works if the message is smaller than 128 bytes and the topic length is smaller than 256 bytes

jjunimd commented 6 years ago

I am currently successfully sending 415 byte messages. To do this, I had to set MAXBUFFERSIZE=1000, but made no other changes.

This works well for 5000-20000 packets, then locks up somewhere in mitt.publish(). I have reduced the package size to ~20 bytes and it still locks up at same place, so the lockup seems unrelated to message size.

jjunimd commented 6 years ago

Sorry, I made one other change: Adafruit_MQTT_Client::sendPacket() modified to eliminate limit on packet size, i.e. replaced: uint16_t sendlen = len > 250 ? 250 : len; with: uint16_t sendlen = len;

jwende commented 6 years ago

Adafruit_MQTT::publishPacket creates a correct packet structure (variable length / two bytes length for topic) and imho you did the correct change to the sendPacket() method. So your issue might be related to the underlying network stack.

My issue was related to the receiving part in uint8_t Adafruit_MQTT::subscribePacket

jjunimd commented 6 years ago

I hope it's not the low-level network stack as that will be almost impossible for me to debug.

If I send my 415 byte messages at a rate of 1 every 3 seconds, I can reproducibly send 5200-6000 messages before hanging in mqtt.publish(). (Note exact number varies each time by 10-200 bytes).

If I send the same messages at 2-3 per second, I can send 18000-20000 packets before hanging. This is the exact opposite to what I expected. Any ideas?

jwende commented 6 years ago

Not really - what QoS are you using ?

jjunimd commented 6 years ago

QoS = 0 (trying to keep it simple)

jwende commented 6 years ago

ok - so no issues with keeping sequences, msg id's and aknowledgements.

Not sure if you can monitor the heap size of your device - it might indicate if there are any leaks ...

jjunimd commented 6 years ago

I'm using the MemoryFree library to monitor free memory (space between top of heap and bottom of stack) with every message and it does not change. I think that rules out memory leaks.

I'm sure there is a clue in the fact that sending messages faster allows more messages to be sent before lockup. Note, at the high data rate, it takes less time to get to lockup, but many more messages have been successfully sent.