adafruit / Adafruit_CircuitPython_MiniMQTT

MQTT Client Library for CircuitPython
Other
72 stars 50 forks source link

do not share CONNECT variable header #186

Closed vladak closed 7 months ago

vladak commented 7 months ago

This straightforward change prevents unwanted reuse of certain fields in the CONNECT variable header. Another way to fix this would be to use copy.copy() however that does not seem to be present in CP.

vladak commented 7 months ago

Tested with CPython 3.11.2 using this code:

#!/usr/bin/env python3

import logging
import socket
import ssl
import time
import pytest

import adafruit_minimqtt.adafruit_minimqtt as MQTT

def test_keepalive(keep_alive_timeout):
    logging.basicConfig()
    logger = logging.getLogger(__name__)
    logger.setLevel(logging.DEBUG)

    host = "172.40.0.3"
    port = 1883
    mqtt_client = MQTT.MQTT(
        broker=host,
        port=port,
        socket_pool=socket,
        ssl_context=ssl.create_default_context(),
        connect_retries=1,
        recv_timeout=5,
        keep_alive=keep_alive_timeout,
    )

    logger.debug(f"connecting with keep alive = {keep_alive_timeout}")
    mqtt_client.connect()
    mqtt_client.disconnect()

test_keepalive(8)
test_keepalive(16)

and checked that the keepalive value in the CONNECT packets match the 2 values used by the program.

vladak commented 7 months ago

Tested with Adafruit CircuitPython 8.2.6 on 2023-09-12; Adafruit QT Py ESP32-S3 no psram with ESP32S3 using this code:

#!/usr/bin/env python3

import adafruit_logging as logging
import socketpool
import ssl
import sys
import time
import wifi

from secrets import secrets

import adafruit_minimqtt.adafruit_minimqtt as MQTT

def main():
    logger = logging.getLogger(__name__)
    logger.setLevel(logging.DEBUG)

    logger.info("Connecting to wifi")
    wifi.radio.connect(secrets["SSID"], secrets["password"], timeout=10)
    logger.info(f"Connected to {secrets['SSID']}")
    logger.debug(f"IP: {wifi.radio.ipv4_address}")

    test_keepalive(8)
    test_keepalive(16)

def test_keepalive(keep_alive_timeout):
    logger = logging.getLogger(__name__)
    logger.setLevel(logging.DEBUG)

    pool = socketpool.SocketPool(wifi.radio)

    host = "172.40.0.3"
    port = 1883
    mqtt_client = MQTT.MQTT(
        broker=host,
        port=port,
        socket_pool=pool,
        ssl_context=ssl.create_default_context(),
        connect_retries=1,
        recv_timeout=5,
        keep_alive=keep_alive_timeout,
    )

    mqtt_client.logger = logger

    logger.debug(f"connecting with keep alive = {keep_alive_timeout}")
    mqtt_client.connect()
    mqtt_client.disconnect()

if __name__ == "__main__":
    try:
        main()
    except KeyboardInterrupt:
        sys.exit(0)

The output on the console:

]0;🐍172.40.0.23 | code.py | 8.2.6\371.009: INFO - Connecting to wifi
371.011: INFO - Connected to XXX
371.013: DEBUG - IP: 172.40.0.23
371.015: DEBUG - connecting with keep alive = 8
371.017: DEBUG - Attempting to connect to MQTT broker (attempt #0)
371.018: DEBUG - Attempting to establish MQTT connection...
371.020: INFO - Establishing an INSECURE connection to 172.40.0.3:1883
371.025: DEBUG - Sending CONNECT to broker...
371.026: DEBUG - Fixed Header: bytearray(b'\x10\x13\x00')
371.028: DEBUG - Variable Header: bytearray(b'\x04MQTT\x04\x02\x00\x08')
371.031: DEBUG - Receiving CONNACK packet from broker
371.036: DEBUG - Got message type: 0x20
371.038: DEBUG - Resetting reconnect backoff
371.039: DEBUG - Sending DISCONNECT packet to broker
371.041: DEBUG - Closing socket
371.045: DEBUG - connecting with keep alive = 16
371.047: DEBUG - Attempting to connect to MQTT broker (attempt #0)
371.048: DEBUG - Attempting to establish MQTT connection...
371.050: INFO - Establishing an INSECURE connection to 172.40.0.3:1883
371.057: DEBUG - Sending CONNECT to broker...
371.058: DEBUG - Fixed Header: bytearray(b'\x10\x13\x00')
371.060: DEBUG - Variable Header: bytearray(b'\x04MQTT\x04\x02\x00\x10')
371.063: DEBUG - Receiving CONNACK packet from broker
371.069: DEBUG - Got message type: 0x20
371.071: DEBUG - Resetting reconnect backoff
371.072: DEBUG - Sending DISCONNECT packet to broker
371.074: DEBUG - Closing socket
]0;🐍172.40.0.23 | Done | 8.2.6\
Code done running.

and the before/after diff:

--- /tmp/before.out 2023-11-19 21:54:10.000000000 +0100
+++ /tmp/after.out  2023-11-19 21:54:10.000000000 +0100
@@ -18,8 +18,8 @@
 DEBUG - Attempting to establish MQTT connection...
 INFO - Establishing an INSECURE connection to 172.40.0.3:1883
 DEBUG - Sending CONNECT to broker...
-DEBUG - Fixed Header: bytearray(b'\x10\x14\x00')
-DEBUG - Variable Header: bytearray(b'\x04MQTT\x04\x02\x00\x18')
+DEBUG - Fixed Header: bytearray(b'\x10\x13\x00')
+DEBUG - Variable Header: bytearray(b'\x04MQTT\x04\x02\x00\x10')
 DEBUG - Receiving CONNACK packet from broker
 DEBUG - Got message type: 0x20
 DEBUG - Resetting reconnect backoff
FoamyGuy commented 7 months ago

Resolves: #185