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 python-modbus-pip to python.yaml #32010

Merged
merged 7 commits into from
Feb 15, 2022
Merged

Add python-modbus-pip to python.yaml #32010

merged 7 commits into from
Feb 15, 2022

Conversation

tgaspar
Copy link
Contributor

@tgaspar tgaspar commented Feb 2, 2022

Please add the python-modbus-pip dependency to the rosdep database.

Package name:

python-modbus-pip

Package Upstream Source:

Link to source repository: https://github.com/riptideio/pymodbus

Purpose of using this:

There already is an entry for python-modbus in the YAML. However, the Ubuntu and Debian repositories have outdated versions, 1.3.2 and 1.2.0 respectively.

When installing from PyPI we get a more recent version - 2.5.3.

I suggest to keep the outdated python-modbus inside the YAML to not break compatibility.

Distro packaging links:

Links to Distribution Packages

@tgaspar tgaspar requested a review from a team as a code owner February 2, 2022 11:58
@github-actions github-actions bot added the rosdep Issue/PR is for a rosdep key label Feb 2, 2022
rosdep/python.yaml Outdated Show resolved Hide resolved
@@ -3694,6 +3694,13 @@ python-pymavlink:
python-pymodbus:
debian: [python-pymodbus]
ubuntu: [python-pymodbus]
python-pymodbus-pip:
Copy link
Contributor

Choose a reason for hiding this comment

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

One last change, since these are identical please make the python3- key an alias to the python- key.

python-pymodbus-pip: &migrate_eol_2027_04_30_python3_pymodbus_pip
  debian:
    pip:
      packages: [pymodbus]
  ubuntu:
    pip:
      packages: [pymodbus]
...
python3-pymodbus-pip: *migrate_eol_2027_04_30_python3_pymodbus_pip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will do! Quick question though: how do I determine the EOL? Or better, how did you determine it?

Copy link
Contributor

Choose a reason for hiding this comment

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

how do I determine the EOL? Or better, how did you determine it?

I know we're transitioning to Ubuntu Jammy for ROS Rolling at the moment, so I assumed both that it counts as a supported platform and that it has the furthest away EOL date of April 2027.

@tgaspar
Copy link
Contributor Author

tgaspar commented Feb 10, 2022

I could use a little help here. I am not sure why I am getting the following nosetest error:

Package 'python-pymodbus' could not be found for ubuntu focal on amd64

and

Package 'python-pymodbus' could not be found for ubuntu jammy on amd64
Mostly because these are the lines of the YAML that remained untouched by my PR.

@ivanpauno
Copy link
Contributor

@tgaspar the PR checker sometimes checks more lines than needed (see #31439).
We will need to fix the python-pymodbus entry in another PR and rebase this one (or you can also fix that here).

@ivanpauno
Copy link
Contributor

ivanpauno commented Feb 10, 2022

To be more clear, this probably fixes the issue:

python-pymodbus:
  debian: [python-pymodbus]
  ubuntu:
    '*': [python-pymodbus]
    jammy: null

@tgaspar
Copy link
Contributor Author

tgaspar commented Feb 10, 2022

@ivanpauno thanks for the suggestion. Given the errors I received before I added both jammy and focal. Hope that's fine ...

rosdep/python.yaml Outdated Show resolved Hide resolved
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@sloretz sloretz merged commit 1478abe into ros:master Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rosdep Issue/PR is for a rosdep key
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants