aws/aws-iot-device-sdk-python-v2

MQTT5: on_publish_received callback input does not match awscrt.mqtt5.ClientOptions.on_publish_callback_fn

blthayer opened this issue · 3 comments

blthayer commented

Describe the issue

The documentation for awsiot.mqtt5_client_builder describes common arguments for all builder functions. The on_publish_received argument indicates:

Callback invoked for all publish packets received by client.

The function should take the following arguments and return nothing:

Taking a look at the source code here, we can see a ClientOptions instance instantiated if not provided:

    client_options = _get(kwargs, 'client_options')
    if client_options is None:
        client_options = awscrt.mqtt5.ClientOptions(
            host_name=_get(kwargs, 'endpoint')
        )

Later on in the same function, here we can see that the on_publish_received callback is set on the client_options:

    if client_options.on_publish_callback_fn is None:
        client_options.on_publish_callback_fn = _get(kwargs, 'on_publish_received')

However, if we look at the documentation for on_publish_callback_fn in awscrt:

  • on_publish_callback_fn (Callable[[PublishReceivedData],]) – Callback for all publish packets received by client.

In conclusion

  • awsiotsdk specifies the callback input as awscrt.mqtt5.PublishPacket
  • but awscrt specifies the callback input as awscrt.mqtt5.PublishReceivedData

Based on the other callbacks, I'm guessing awsiotsdk documentation/type hinting is incorrect and the callback will receive PublishReceivedData, not PublishPacket (note that PublishReceivedData is a dataclass with only one field: publish_packet which is hinted as a PublishPacket instance).

Links

yasminetalby commented

Hello @blthayer ,

Thank you very much for your detailed submission and for bringing this up to our attention.
I have submitted a PR to fix the on_publish_received argument documentation.

We really appreciate your feedback and collaboration.

I wish you a very nice day.

Best,

Yasmine

yasminetalby commented

Thank you very much for bring this up to our attention and helping us improving our SDK documentation. This issue is fixed by 6ee31da

Best regards,

Yasmine

github-actions commented

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.