Skip to content

Handle ROS 2 types properly #883

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
Merged
merged 19 commits into from
Oct 27, 2023
Merged

Handle ROS 2 types properly #883

merged 19 commits into from
Oct 27, 2023

Conversation

scottbell
Copy link
Contributor

@scottbell scottbell commented Oct 16, 2023 •
edited
Loading

Public API Changes

None

Description

Handle ROS2 types properly when building type definitions. Don't crash when handling unknown types. Added test coverage for objectutils.

Fixes #834 #822 #821

This was linked to issues Oct 26, 2023
AttributeError: 'float' object has no attribute 'get_fields_and_field_types' #821
Closed
AttributeError: 'float' object has no attribute '__module__'. #822
Closed
@EzraBrooks
Copy link
Contributor

Hey @scottbell, thanks for the PR! This looks more complete than my previous implementation in #840. I'll give this a shot today in my workspace which has experienced some of these issues.

@EzraBrooks EzraBrooks self-requested a review October 26, 2023 17:16
EzraBrooks
EzraBrooks approved these changes Oct 26, 2023
View reviewed changes
Copy link
Contributor

@EzraBrooks EzraBrooks left a comment

Choose a reason for hiding this comment

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

This change worked in my workspace that previously was running PR #840, so this LGTM. Code looks reasonable to me - thanks!

@EzraBrooks EzraBrooks mentioned this pull request Oct 26, 2023
Merged
@EzraBrooks EzraBrooks changed the title Handle RO2 types properly Handle ROS 2 types properly Oct 26, 2023
@scottbell
Copy link
Contributor Author

This change worked in my workspace that previously was running PR #840, so this LGTM. Code looks reasonable to me - thanks!

Thank you for testing!

sea-bass
sea-bass reviewed Oct 26, 2023
View reviewed changes
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Looks good to me as well -- just some minor comments

@scottbell scottbell requested a review from sea-bass October 27, 2023 09:22
@scottbell scottbell mentioned this pull request Oct 27, 2023
Merged
@sea-bass sea-bass merged commit 910163b into RobotWebTools:ros2 Oct 27, 2023
@Hytac
Copy link

Hytac commented Nov 23, 2023

Hi, I'm still using #840.
#883 still giving the error to me
all_service AttributeError: 'float' object has no attribute 'get_fields_and_field_types'
My msg is moveit_msgs/msg/PlanningScene

@sea-bass
Copy link
Contributor

I think ROS 2 needs float32 or float64 if I recall correctly?

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

@EzraBrooks EzraBrooks EzraBrooks approved these changes

@sea-bass sea-bass sea-bass approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@scottbell @EzraBrooks @Hytac @sea-bass