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

Add support for paho-mqtt >= 2.0.0 via legacy callback API. #2916

Merged
merged 1 commit into from
May 2, 2024

Conversation

raineth
Copy link
Contributor

@raineth raineth commented May 2, 2024

This is all that's required to make these scripts work properly on paho-mqtt 2.0.0, but it doesn't use the new callback API. Per the documentation, VERSION1 callbacks are deprecated but will be supported until paho-mqtt 3.

This is all that's required to make these scripts work properly on
paho-mqtt 2.0.0, but it doesn't use the new callback API.  Per the
documentation, VERSION1 callbacks are deprecated but will be supported
until paho-mqtt 3.
@raineth
Copy link
Contributor Author

raineth commented May 2, 2024

This would close issue #2840.

I see a similar change in pr #2841, but this one retains compatibility with older versions of paho-mqtt.

@zuckschwerdt
Copy link
Collaborator

I guess we could even do without the conditional? There is a disscussion in #2840 and a change as part of #2841.

Can you test if that is true? I.e. will something like these still work with paho version 1?

mqttc = mqtt.Client(mqtt.CallbackAPIVersion.VERSION1)

mqtt_client = mqtt.Client(callback_api_version=mqtt.CallbackAPIVersion.VERSION1, client_id="RTL_433_Test")

@raineth
Copy link
Contributor Author

raineth commented May 2, 2024

I guess we could even do without the conditional? There is a disscussion in #2840 and a change as part of #2841.

Can you test if that is true? I.e. will something like these still work with paho version 1?

The conditional is necessary, mqtt.CallbackAPIVersion does not exist prior to paho v2.

>>> paho.mqtt.__version__
'1.6.1'
>>> paho.mqtt.client.CallbackAPIVersion
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'paho.mqtt.client' has no attribute 'CallbackAPIVersion'

vs

>>> paho.mqtt.__version__
'2.0.0'
>>> paho.mqtt.client.CallbackAPIVersion
<enum 'CallbackAPIVersion'>

@gdt
Copy link
Collaborator

gdt commented May 2, 2024

wow, this was merged super fast, with no opportunity for commenting.

Note that paho 2.0.1 has just been released which makes compat of some things a lot better. I'm not sure it affects this issue, or just other parts. The essence is a move to keyword args so that calls with only keyword and no positional args can work in both 1.6.1 and 2.0.1.

@zuckschwerdt
Copy link
Collaborator

Sorry, it looked like the previous discussion was concluded.
AFAICS, there is no real change here, just the minimal needed conditional to allow Paho 2.0?

PRs to further improve this very welcome – or better even someone to take over the review and approvable of HA script changes, I'm not using this script and can't do QA.

@gdt
Copy link
Collaborator

gdt commented May 3, 2024

It might well have concluded, and it's quite possible that the change in 2.0.1 is not relevant. The entire paho-mqtt API break is very unfortunate. (I haven't had to the time to really pay attention and am starting to think about updating pkgsrc to paho-mqtt 2.0.1, but there is rtl_433, weewx, and some of my own scripts.)

@raineth
Copy link
Contributor Author

raineth commented May 3, 2024

Unfortunately, paho.mqtt.python 2.0.0 has been out for almost 3 months and the hasattr() method seems to be the only way to support that installed base until 2.1.0 percolates out into the wild. (It looks like 2.1.0 was just released on Apr 29, and it isn't in Debian unstable yet or even properly updated on the Eclipse website for paho.) I don't think paho.mqtt.python 2.0.0 is in any distro's stable release, though, so maybe it will make sense to revert this change in a few months. It's a shame they didn't manage to avoid the breaking change in the first place, but at least unmaintained code/examples will start working again soon.

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.

3 participants