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

Next generation AWS IoT Client SDK for Python using the AWS Common Runtime
Apache License 2.0
408 stars 213 forks source link

on_connection_resumed issue - sample app incorrect #60

Closed js1972 closed 4 years ago

js1972 commented 4 years ago

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug Unable to use on_connection_resumed. When I set the on_connection_resumed parameter when setting up the mqtt_connection like this:

mqtt_connection = mqtt_connection_builder.mtls_from_path(
endpoint=config["ENDPOINT"],
cert_filepath=args["cert"],
pri_key_filepath=args["key"],
client_bootstrap=client_bootstrap,
ca_filepath=None,
client_id=config["DEVICEID"],
clean_session=False,
keep_alive_secs=60,
will=lwt,
on_connection_interrupted=on_connection_interupted,
on_connection_resumed=on_connection_resumed)

The connection resumed callback is defined like so:

def on_connection_resumed(connection, return_code, session_present, **kwargs):
    print('connection resumed with return code {}, session present {}'.format(return_code, session_present))

At runtime when I disconnect the internet and then reconnect to make a reconnection occur I get this python exception:

Exception ignored in: <class 'TypeError'>
Traceback (most recent call last): 
File "/home/jason/.virtualenvs/sg/lib/python3.6/site-packages/awscrt/mqtt.py", line 215, in _on_connection_resumed
session_present=session_present) 
TypeError: on_connection_resumed() missing 1 required positional argument: 'return_code'

It turns out this is just because of an error in the sample programs and also the doc comments in aws-crt-python/awscrt/awsiot_mqtt_connection_builder.py.

You need to remove the return_code parameter so that the signature is like so:

def on_connection_resumed(connection, session_present, **kwargs):
bretambrose commented 4 years ago

What versions of the sdk (and crt) are you using? Looking at the callback invocation (https://github.com/awslabs/aws-crt-python/blame/master/awscrt/mqtt.py#L326) it seems like it has correctly used return_code for a while now.

github-actions[bot] commented 4 years ago

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

bretambrose commented 4 years ago

Unsure why the bot zapped this one. I still wanted to do a little testing on it.

bretambrose commented 4 years ago

I ran the sample, disconnected for long enough that the channel shut down, then restored the network and the mqtt connection recovered and I got a well-formed on_connection_resumed invocation. My best guess is that maybe you had (or we had, unsure) a temporary mismatch between the SDK and the crt in terms of the callback signature.

js1972 commented 4 years ago

Seems to be running ok for me now too.. Happy for you to close. ;-)