search

ankraft / ACME-oneM2M-CSE

An open source CSE Middleware for Education.
https://acmecse.net/
BSD 3-Clause "New" or "Revised" License
23 stars 16 forks source link

Fixed paho MQTT implementation for V2 #153

Closed Luke1734 closed 4 months ago

Luke1734 commented 4 months ago

In a recent commit, titled "Updated types to latest Paho version" (commit hash f1921cb), updates were made to bring the python types in line with the changes made in the latest paho mqtt version. However, there were several breaking changes introduced in this version. See: https://eclipse.dev/paho/files/paho.mqtt.python/html/migrations.html

Notably the syntax was changed such that MQTTv3 and v5 now use the same function signature. However this new signature is different to the old signature from previous paho mqtt versions, which was used previously. In updating the typing information for the parameters of the function in the commit mentioned above, the _onSubscribe function of the MQTTConnection.py file was updated to include this new extra parameter accidentally, yet all the other functions were not changed, resulting in a crash whenever MQTT was enabled. This is because it tries to call this function with the old signature since mqtt.CallbackAPIVERSION1 was set in the MQTTClient constructor, yet the _onSubscribe function uses the new signature.

One fix is to simply remove this erroneously added "properties" parameter from the _onSubscribe function, and things would work as before for now, however this is deprecated. This, it would make better sense to follow the new syntax, and I have updated the code to follow this new function signature.

ankraft commented 4 months ago

Thanks a lot for doing the QA and creating the PR! I totally missed that. Also, mypy did find the one signature change but not the others.

ankraft commented 4 months ago

Adding in next commit: fixing the signature change for on_disconnect as well as fixes for some smaller error code conversions