-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for MQTT-PAHO 2.0 versioned callbacks with client ID generation and modern dependency management #17
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
Conversation
|
@gonzalocasas 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: gonzalocasas <[email protected]>
Co-authored-by: gonzalocasas <[email protected]>
Co-authored-by: gonzalocasas <[email protected]>
src/compas_eve/mqtt/mqtt_paho.py
Outdated
| if PAHO_MQTT_V2_AVAILABLE: | ||
| self.client = mqtt.Client(callback_api_version=CallbackAPIVersion.VERSION1) | ||
| else: | ||
| self.client = mqtt.Client() # todo: generate client_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add client id generation to this, also allow to override or define the client id passing that in the constructor of the transport class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added client ID generation functionality. The MqttTransport constructor now accepts an optional client_id parameter and automatically generates a unique UUID-based client ID when none is provided. Both paho-mqtt 1.x and 2.x client creation now include the client_id parameter. See commit 99ec693.
|
@copilot update to |
- Add client_id parameter to MqttTransport constructor with auto-generation - Support both explicit client_id and auto-generated UUID-based client_id - Update workflow files to use compas-dev/compas-actions.build@v4 - Update compatibility tests to match new client_id behavior - Add test for custom client_id functionality Co-authored-by: gonzalocasas <[email protected]>
|
@copilot please update to install |
- Move dev dependencies from requirements-dev.txt to setup.py optional dependencies - Update CI workflows to install with [dev] instead of requirements-dev.txt - Update CONTRIBUTING.md to reflect new installation method - Maintain requirements-dev.txt for backward compatibility in setup.py Co-authored-by: gonzalocasas <[email protected]>
Updated dependency management to use [dev] optional dependencies instead of requirements-dev.txt. Development dependencies can now be installed with |
|
@copilot I think you should remove the calls to |
Co-authored-by: gonzalocasas <[email protected]>
Removed the redundant |
|
@copilot remove 3.9 from the python matrix |
Co-authored-by: gonzalocasas <[email protected]>
|
@copilot fix linting issues (e.g. src/compas_eve/mqtt/mqtt_paho.py:35:1: W293 blank line contains whitespace |
Co-authored-by: gonzalocasas <[email protected]>
Fixed the linting issues by removing whitespace from blank lines 35 and 39 in mqtt_paho.py. The file now passes flake8 checks. See commit aa996fb. |
gonzalocasas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
chenkasirer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
This PR adds backward-compatible support for paho-mqtt 2.0 versioned callbacks while maintaining full compatibility with paho-mqtt 1.x installations, includes client ID generation functionality, and modernizes dependency management using optional dependencies.
Problem
The paho-mqtt library version 2.0 introduced breaking changes to the
Client()constructor, requiring acallback_api_versionparameter. Without this parameter, creating an MQTT client in paho-mqtt 2.0 fails with an error. Additionally, the original implementation lacked proper client ID management, which can cause connection issues with MQTT brokers. The project also used outdated dependency management with separate requirements-dev.txt instead of modern optional dependencies.Solution
This implementation automatically detects the available paho-mqtt version and uses the appropriate client creation method with proper client ID handling, while modernizing the dependency management approach:
Key Features
paho-mqtt >=1, <3Usage
Development Setup
Development dependencies are now managed as optional dependencies:
# Install with development dependencies pip install -e .[dev]Files Changed
src/compas_eve/mqtt/mqtt_paho.py: Core implementation with version detection and client ID supportrequirements.txt: Updated paho-mqtt constraint to allow 2.xtests/unit/test_mqtt_paho_compatibility.py: Updated compatibility tests with client ID verificationexamples/mqtt_compatibility_demo.py: User-facing demonstrationCHANGELOG.md: Documentation of changesMANIFEST.in: Include examples in distributionsetup.py: Added [dev] optional dependencies for modern dependency managementCONTRIBUTING.md: Updated development setup instructions.github/workflows/build.yml&.github/workflows/release.yml: Updated to v4 actions, [dev] dependencies, and Python 3.10+ supportThe implementation is minimal and surgical, changing only the necessary lines to support both versions while adding essential client ID functionality and modern dependency management practices, preserving all existing behavior.
Fixes #8.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.