Skip to content

Fix CONNECT for large MQTT payloads #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 12, 2019
Merged

Fix CONNECT for large MQTT payloads #3

merged 5 commits into from
Aug 12, 2019

Conversation

brentru
Copy link
Member

@brentru brentru commented Aug 12, 2019

The remaining length used by connect is incompatible with large payloads (>127 bytes). An instance of this is when a long (> 300 byte) password, such as a JSON Web Token, or a long client identifier is provided.

  • The remaining length calculation used by this library has been removed in favor of using the algorithm described by the MQTT V3.1.1 specification (https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718030).
  • Default keepalive value sent by connect() increased to 60 seconds to ensure the MQTT broker rejects incorrect connections, instead of hanging _wait_for_msg().
  • Fixed header bytearray removed, switching to a variable bytearray.
  • Variables within connect renamed to correct names per the MQTT spec.

Tested with a PyPortal running CircuitPython Latest, two tests were run to verify compatibility with existing (small) payloads and large connect payloads...

MiniMQTT Example - Adafruit IO WiFi - testing small (<127 byte) connect payload.

Connecting to Adafruit IO...
25346.9: DEBUG - Creating new socket
25347.0: DEBUG - Attempting to establish secure MQTT connection...
appemnding...
25349.8: DEBUG - Sending CONNECT to broker
25349.9: DEBUG - Fixed Header: bytearray(b'\x10>\x00')
Variable Header: bytearray(b'\x04MQTT\x04\xc2\x00<')
25350.2: DEBUG - Receiving CONNACK packet from broker
Connected to Adafruit IO! Listening for topic changes on brubell/feeds/onoff

Connecting to Google's Cloud IoT MQTT Server - testing large (512B+) connect payload.

Attempting to connect to mqtt.googleapis.com
25400.9: DEBUG - Creating new socket
25400.9: DEBUG - Attempting to establish secure MQTT connection...
25403.5: DEBUG - Sending CONNECT to broker
25403.5: DEBUG - Fixed Header: bytearray(b'\x10\xb6\x04\x00')
Variable Header: bytearray(b'\x04MQTT\x04\xc2\x00<')
25403.8: DEBUG - Receiving CONNACK packet from broker
Connected to MQTT Broker!
Flags: 0
 RC: 0

@brentru brentru added the bug Something isn't working label Aug 12, 2019
@brentru brentru requested a review from a team August 12, 2019 21:27
@ladyada ladyada merged commit f3c0cdf into adafruit:master Aug 12, 2019
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Aug 13, 2019
Updating https://github.com/adafruit/Adafruit_CircuitPython_SGP30 to 2.0.3 from 2.0.2:
  > Fixed typo in README

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1331 to 1.1.0 from 1.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1331#4 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_VCNL4040 to 1.1.0 from 1.0.0:
  > Merge branch 'master' of github.com:adafruit/Adafruit_CircuitPython_VCNL4040
  > tweaked example code
  > adding lux and white scaling w/ gain/IT

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to v1.0.1 from v1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#3 from brentru/fix-connack-payload
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#2 from brentru/update-aio-code
rtwfroody pushed a commit to rtwfroody/Adafruit_CircuitPython_MiniMQTT that referenced this pull request Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants