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

Revamp how we get network information in ros2doctor. #910

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

clalancette
Copy link
Contributor

We already depend on psutil, which can give us information for ros2doctor in a cross-platform way. Lean more heavily into using this, which should help us deal with cross-platform a little more smoothly.

I will note that this slightly changes the output of ros2doctor on Linux. Prior to this PR, the flags output would look something like:

flags        : 4163<MULTICAST,BROADCAST,UP,RUNNING>

After this PR, the output looks like this:

flags        : UP,BROADCAST,RUNNING,MULTICAST

That is, we lose the decimal bitmask associated with the device.

This should fix #906

We already depend on psutil, which can give us information
for ros2doctor in a cross-platform way.  Lean more heavily
into using this, which should help us deal with cross-platform
a little more smoothly.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor Author

Another CI after the fix to flake8:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

This is way much better for cross-platform support, lgtm!

@clalancette
Copy link
Contributor Author

Going ahead and merging this one in, thanks for the review.

@clalancette clalancette merged commit 30092b6 into rolling Jun 13, 2024
3 checks passed
@clalancette clalancette deleted the clalancette/revamp-doctor-network branch June 13, 2024 18:40
@fujitatomoya
Copy link
Collaborator

@clalancette what do you say backport to jazzy, iron and humble? looks like they all have this problem and already use psutil, so it should be straight-forward. if that is fine, i can take care of backports.

@clalancette
Copy link
Contributor Author

@clalancette what do you say backport to jazzy, iron and humble? looks like they all have this problem and already use psutil, so it should be straight-forward. if that is fine, i can take care of backports.

The big problem with doing that is that we'd change the output on an already-released distribution. I guess we could kind of reconstruct that information there by putting the POSIX_NET_FLAGS back and mapping from strings to hex numbers, but that would be wrong on macOS.

@fujitatomoya
Copy link
Collaborator

ah that is true. this changes the format, there could be some script or tools relying on the current format for released distros. okay, i am not gonna do this at this moment.

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.

ros2doctor has incorrect bitmask for MULTICAST on macOS 14.4
2 participants