Skip to content
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

MQTT v5.0 support #187

Merged
merged 13 commits into from
Jul 11, 2024
Merged

MQTT v5.0 support #187

merged 13 commits into from
Jul 11, 2024

Conversation

ionutab
Copy link
Contributor

@ionutab ionutab commented Jul 11, 2024

Added different callback signatures in the MqttClient for

  • on_connect
  • on_disconnect

to support MQTT v5.0

As per the comments in the paho mqtt client:

        """The callback called when the broker reponds to our connection request.

        Expected signature for callback API version 2::

            connect_callback(client, userdata, connect_flags, reason_code, properties)

        Expected signature for callback API version 1 change with MQTT protocol version:
            * For MQTT v3.1 and v3.1.1 it's::

                connect_callback(client, userdata, flags, rc)

            * For MQTT v5.0 it's::

                connect_callback(client, userdata, flags, reason_code, properties)


        :param Client client: the client instance for this callback
        :param userdata: the private user data as set in Client() or user_data_set()
        :param ConnectFlags connect_flags: the flags for this connection
        :param ReasonCode reason_code: the connection reason code received from the broken.
                       In MQTT v5.0 it's the reason code defined by the standard.
                       In MQTT v3, we convert return code to a reason code, see
                       `convert_connack_rc_to_reason_code()`.
                       `ReasonCode` may be compared to integer.
        :param Properties properties: the MQTT v5.0 properties received from the broker.
                       For MQTT v3.1 and v3.1.1 properties is not provided and an empty Properties
                       object is always used.
        :param dict flags: response flags sent by the broker
        :param int rc: the connection result, should have a value of `ConnackCode`
"""

I'm not sure if anything can be done with the properties param passed in the callbacks.
Let me know if any changes are required.
Thanks!

@ionutab
Copy link
Contributor Author

ionutab commented Jul 11, 2024

I'm not sure how to fix the linter issue.
I'm not sure why it is triggering for the properties but not for other cases in the file like:

    def _on_connect_cb(
        self,
        client: mqtt.Client, # not used
        userdata: typing.Any, # not used
        flags: dict[str, int], # not used
        rc: int,
    ):

🤔

@cyberw
Copy link
Collaborator

cyberw commented Jul 11, 2024

No idea why it works in those other cases. Add # pylint: disable=unused-argument where you need it.

@cyberw
Copy link
Collaborator

cyberw commented Jul 11, 2024

Looks nice. Can you add setting the version to the example in mqtt_ex.py?

@ionutab
Copy link
Contributor Author

ionutab commented Jul 11, 2024

Looks nice. Can you add setting the version to the example in mqtt_ex.py?

I added it in mqtt_custom_client.py

Will add to mqtt_ex.py shortly.

🤔 now that I think about it maybe I should just leave it in mxtt_ex.py only.

What do you think?

@cyberw
Copy link
Collaborator

cyberw commented Jul 11, 2024

Ah, yes, I misread the PR :) But yes, I think it belongs in mqtt_ex.py only!

@ionutab
Copy link
Contributor Author

ionutab commented Jul 11, 2024

Ah, yes, I misread the PR :) But yes, I think it belongs in mqtt_ex.py only!

@ionutab ionutab closed this Jul 11, 2024
@ionutab ionutab reopened this Jul 11, 2024
@ionutab
Copy link
Contributor Author

ionutab commented Jul 11, 2024

please double check the code before merging - I had to alter the MqttUser to pass down the protocol.
thank you

@cyberw cyberw merged commit 260e6cc into SvenskaSpel:master Jul 11, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants