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.

Sign up for GitHub

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

Jump to bottom

Bump paho-mqtt client to version 2.1.0 #136130

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from
Draft

Bump paho-mqtt client to version 2.1.0 #136130

wants to merge 6 commits into from
+30 −27

Conversation

jbouwh
Copy link
Contributor

@jbouwh jbouwh commented Jan 20, 2025 •
edited
Loading

Proposed change

Bump paho-mqtt client to version 2.1.0

Bump pyeconet==0.1.23 -> 0.1.26

  • Minor changes between 0.1.23 and 0.1.25 the big reason for the 0.1.26 change now is that the API was updated recently and has broken usage report (energy usage/water usage) reporting. See: w1ll1am23/pyeconet@v0.1.23...v0.1.26

Dependencies

The following libraries are incompatible and need to be updated to allow the upgrade:

The type stub types-paho-mqtt was removed from the dependencies.

The libraries are temporary removed from this PR to allow paho-mqtt to upgrade.

The following libraries have a dependency and may need attention as they have a dependency:

paho-mqtt==2.1.0
├── aiomqtt==2.3.0 [requires: paho-mqtt>=2.1.0,<3.0.0] -> Okay!
│   ├── deebot-client==11.0.0 [requires: aiomqtt>=2.0.0,<3.0]
│   ├── letpot==0.2.0 [requires: aiomqtt>=2.0,<3.0]
│   └── yolink-api==0.4.7 [requires: aiomqtt>=2.0.0,<3.0.0]
├── ibmiotf==0.3.4 [requires: paho-mqtt>=1.3.1]
├── microBeesPy==0.3.5 [requires: paho-mqtt]
├── pyeconet==0.1.23 [requires: paho-mqtt>=1.5.0]
├── pyezviz==0.2.1.2 [requires: paho-mqtt]
├── pysmappee==0.2.29 [requires: paho-mqtt>=1.5.0]
└── tuya-device-sharing-sdk==0.2.1 [requires: paho-mqtt]

TODO Checks on depending libraries:

  • ibmiotf==0.3.4 used for watson_iot, no active users reported. Version 0.4.0 was released Aug 9, 2018. Library does not seem to be maintained anymore. It looks the library only references the paho.mqtt.client.MQTT_ERR_SUCCESS constant and does not call the paho client it self. So no changes needed.
  • microBeesPy==0.3.5 used for microbees, 2 active users reported. It seems MQTT is not used in HA, so it might not have impact. The config initializes without issues when the new paho client is loaded.
  • pyeconet==0.1.23 -> 0.1.26 (bump included in this PR)
  • pyezviz==0.2.1.2 The backend lib needs a bump: Bump paho-mqtt from 1.6.1 to 2.1.0 RenierM26/pyEzvizApi#16. It is not likely the HA integartion will fail without the bump as no MQTT is used.
  • pysmappee==0.2.29
  • tuya-device-sharing-sdk==0.2.1 The Tuya integration will not break and work without any changes (tested in a dev). The MQTT features from the SDK are not used.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @emontnemery, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (mqtt) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of mqtt can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign mqtt Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

jbouwh
jbouwh commented Jan 20, 2025
View reviewed changes
homeassistant/components/mqtt/async_client.py
Comment on lines +54 to +60
self._in_callback_mutex = NullLock() # type: ignore[assignment]
self._callback_mutex = NullLock() # type: ignore[assignment]
self._msgtime_mutex = NullLock() # type: ignore[assignment]
self._out_message_mutex = NullLock() # type: ignore[assignment]
self._in_message_mutex = NullLock() # type: ignore[assignment]
self._reconnect_delay_mutex = NullLock() # type: ignore[assignment]
self._mid_generate_mutex = NullLock() # type: ignore[assignment]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed since the new version has type hints now

homeassistant/components/mqtt/client.py
@@ -117,7 +116,7 @@

MAX_PACKETS_TO_READ = 500

type SocketType = socket.socket | ssl.SSLSocket | mqtt.WebsocketWrapper | Any
type SocketType = socket.socket | ssl.SSLSocket | mqtt._WebsocketWrapper | Any # noqa: SLF001
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems needed still, but the WebsocketWrapper class was made private (renamed)

homeassistant/components/mqtt/client.py Outdated Show resolved Hide resolved
homeassistant/components/mqtt/client.py
client_id,
protocol=proto,
transport=transport,
transport=transport, # type: ignore[arg-type]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New typing requires a literal string

homeassistant/components/mqtt/client.py
@@ -533,7 +533,7 @@ def _increase_socket_buffer_size(self, sock: SocketType) -> None:
try:
# Some operating systems do not allow us to set the preferred
# buffer size. In that case we try some other size options.
sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, new_buffer_size)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, new_buffer_size) # type: ignore[union-attr]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems still to be working when executed in the debugger. Not sure about the type warnings.

homeassistant/components/mqtt/client.py
Comment on lines +1219 to +1221
async def _async_wait_for_mid_or_raise(
self, mid: int | None, result_code: int
) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of an error mid will be None

homeassistant/components/mqtt/client.py
Comment on lines -1272 to +1279
matcher = MQTTMatcher()
matcher = MQTTMatcher() # type: ignore[no-untyped-call]
matcher[subscription] = True

return lambda topic: next(matcher.iter_match(topic), False)
return lambda topic: next(matcher.iter_match(topic), False) # type: ignore[no-untyped-call]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the library seems not to be typed yet

requirements_all.txt Outdated Show resolved Hide resolved
requirements_all.txt
roombapy==1.8.1
#roombapy==1.8.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary removed this to allow testing the mqtt integration

requirements_test.txt
@@ -41,7 +41,6 @@ types-beautifulsoup4==4.12.0.20241020
types-caldav==1.3.0.20241107
types-chardet==0.1.5
types-decorator==5.1.8.20240310
types-paho-mqtt==1.6.0.20240321
Copy link
Contributor Author

@jbouwh jbouwh Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the type stub for the old version. Note that the package explicitly needs to be uninstalled. Just updating the test dependencies is not enough.

@joostlek
Copy link
Member

cc @Lash-L @bsmappee @RenierM26 @Orhideous

@Lash-L
Copy link
Contributor

Lash-L commented Jan 20, 2025

@allenporter and I were just talking about this last week.

We should be able to get this done

@epenet epenet mentioned this pull request Jan 24, 2025
Draft
19 tasks
@Tabisch Tabisch mentioned this pull request Jan 25, 2025
Open
@Tabisch
Copy link

Tabisch commented Jan 25, 2025

I have created a PR for roombapy, to hopefully get that updated.
pschmitt/roombapy#405

@jbouwh
Copy link
Contributor Author

jbouwh commented Jan 25, 2025

pschmitt/roombapy#405

Thanks, the PR looks good

@jbouwh
Copy link
Contributor Author

jbouwh commented Jan 25, 2025

pschmitt/roombapy#405

Thanks, the PR looks good

Please leave a comment about the version when the library is bumped

@jbouwh jbouwh added dependency and removed bugfix labels Jan 25, 2025
@allenporter allenporter mentioned this pull request Jan 28, 2025
Merged
19 tasks
@allenporter
Copy link
Contributor

roborock bump with paho-mqtt 2.1.0 compatibility is in #136727

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emontnemery emontnemery Awaiting requested review from emontnemery emontnemery will be requested when the pull request is marked ready for review emontnemery is a code owner

@bdraco bdraco Awaiting requested review from bdraco bdraco will be requested when the pull request is marked ready for review bdraco is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@bdraco bdraco

@emontnemery emontnemery

Labels
by-code-owner cla-signed core dependency has-tests integration: mqtt Quality Scale: platinum small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jbouwh @joostlek @Lash-L @Tabisch @allenporter @bdraco @emontnemery