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

Draw RelativePose2DStampedConstraint constraints #107

Merged

Conversation

efernandez
Copy link
Collaborator

This includes:

  • Dynamically generate display properties for the constraint sources
  • Cache the constraint sources properties config so it's applied when
    the properties are later created
  • Create Variable visual + property, as we do for the Constraint

Below you can see how the display looks like, its properties and visual objects, and what can be done to inspect a fuse_msgs::SerializedGraph message using this PR. Before only the variables were rendered.

simplescreenrecorder-2019-09-23_17 43 38

I've recently merged this same thing into our fork so we get some testing on it: clearpathrobotics#2

Also note this requires rviz >= 1.13.4, but if that's an issue, I can make it work with a lower version. The reason is that I try to re-use code for the covariance property and visual, but it requires their code to be duplicated. With rviz >= 1.13.4, at least I don't have to duplicate the visual code, although I still have for the property one.

@efernandez
Copy link
Collaborator Author

@svwilliams Could you paste the test failure error here, please? I suspect it's related with the version of rviz being used, since 1.13.4 is the latest one and you probably have an older one in your test infrastructure.

@svwilliams
Copy link
Contributor

[2019-09-26T15:40:19.753Z] Starting >>> fuse_viz

[2019-09-26T15:40:58.529Z] --- stderr: fuse_viz

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/mapped_covariance_property.cpp:31:0:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h: In constructor ‘rviz::MappedCovarianceVisual::MappedCovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:74:115: error: ‘rviz::CovarianceVisual::CovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’ is private within this context

[2019-09-26T15:40:58.529Z]      : CovarianceVisual(scene_manager, parent_node, is_local_rotation, is_visible, pos_scale, ori_scale, ori_offset)

[2019-09-26T15:40:58.529Z]                                                                                                                    ^

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:33:0,

[2019-09-26T15:40:58.529Z]                  from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/mapped_covariance_property.cpp:31:

[2019-09-26T15:40:58.529Z] /opt/locusrobotics/hotdog/dev/ros1/include/rviz/default_plugin/covariance_visual.h:93:3: note: declared private here

[2019-09-26T15:40:58.529Z]    CovarianceVisual( Ogre::SceneManager* scene_manager, Ogre::SceneNode* parent_node, bool is_local_rotation, bool is_visible = true, float pos_scale = 1.0f, float ori_scale = 0.1f, float ori_offset = 0.1f);

[2019-09-26T15:40:58.529Z]    ^~~~~~~~~~~~~~~~

[2019-09-26T15:40:58.529Z] make[2]: *** [CMakeFiles/fuse_viz.dir/src/mapped_covariance_property.cpp.o] Error 1

[2019-09-26T15:40:58.529Z] make[2]: *** Waiting for unfinished jobs....

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/relative_pose_2d_stamped_constraint_property.cpp:36:0:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h: In constructor ‘rviz::MappedCovarianceVisual::MappedCovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:74:115: error: ‘rviz::CovarianceVisual::CovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’ is private within this context

[2019-09-26T15:40:58.529Z]      : CovarianceVisual(scene_manager, parent_node, is_local_rotation, is_visible, pos_scale, ori_scale, ori_offset)

[2019-09-26T15:40:58.529Z]                                                                                                                    ^

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:33:0,

[2019-09-26T15:40:58.529Z]                  from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/relative_pose_2d_stamped_constraint_property.cpp:36:

[2019-09-26T15:40:58.529Z] /opt/locusrobotics/hotdog/dev/ros1/include/rviz/default_plugin/covariance_visual.h:93:3: note: declared private here

[2019-09-26T15:40:58.529Z]    CovarianceVisual( Ogre::SceneManager* scene_manager, Ogre::SceneNode* parent_node, bool is_local_rotation, bool is_visible = true, float pos_scale = 1.0f, float ori_scale = 0.1f, float ori_offset = 0.1f);

[2019-09-26T15:40:58.529Z]    ^~~~~~~~~~~~~~~~

[2019-09-26T15:40:58.529Z] make[2]: *** [CMakeFiles/fuse_viz.dir/src/relative_pose_2d_stamped_constraint_property.cpp.o] Error 1

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/relative_pose_2d_stamped_constraint_visual.cpp:36:0:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h: In constructor ‘rviz::MappedCovarianceVisual::MappedCovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’:

[2019-09-26T15:40:58.529Z] /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:74:115: error: ‘rviz::CovarianceVisual::CovarianceVisual(Ogre::SceneManager*, Ogre::SceneNode*, bool, bool, float, float, float)’ is private within this context

[2019-09-26T15:40:58.529Z]      : CovarianceVisual(scene_manager, parent_node, is_local_rotation, is_visible, pos_scale, ori_scale, ori_offset)

[2019-09-26T15:40:58.529Z]                                                                                                                    ^

[2019-09-26T15:40:58.529Z] In file included from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/include/fuse_viz/mapped_covariance_visual.h:33:0,

[2019-09-26T15:40:58.529Z]                  from /home/ec2-user/workspace/test_rosdistro_fuse_PR-107/package/fuse_viz/src/relative_pose_2d_stamped_constraint_visual.cpp:36:

[2019-09-26T15:40:58.529Z] /opt/locusrobotics/hotdog/dev/ros1/include/rviz/default_plugin/covariance_visual.h:93:3: note: declared private here

[2019-09-26T15:40:58.529Z]    CovarianceVisual( Ogre::SceneManager* scene_manager, Ogre::SceneNode* parent_node, bool is_local_rotation, bool is_visible = true, float pos_scale = 1.0f, float ori_scale = 0.1f, float ori_offset = 0.1f);

[2019-09-26T15:40:58.529Z]    ^~~~~~~~~~~~~~~~

[2019-09-26T15:40:58.529Z] make[2]: *** [CMakeFiles/fuse_viz.dir/src/relative_pose_2d_stamped_constraint_visual.cpp.o] Error 1

[2019-09-26T15:40:58.529Z] make[1]: *** [CMakeFiles/fuse_viz.dir/all] Error 2

[2019-09-26T15:40:58.529Z] make: *** [all] Error 2

[2019-09-26T15:40:58.529Z] ---

[2019-09-26T15:40:58.529Z] Failed   <<< fuse_viz	[ Exited with code 2 ]

@efernandez
Copy link
Collaborator Author

Thanks, it's that.

Let me know if you prefer me to copy the CovarianceVisual code from https://github.com/ros-visualization/rviz/blob/melodic-devel/src/rviz/default_plugin/covariance_visual.h and https://github.com/ros-visualization/rviz/blob/melodic-devel/src/rviz/default_plugin/covariance_visual.cpp, or you'll upgrade rviz to 1.13.4.

It's a shame they don't allow to easily compound those classes 😞
I'd revisit this in a few weeks/months and try to contribute something to rviz so we can do this a lot cleaner, but it'd be great if we could get this in now. :)

@svwilliams
Copy link
Contributor

I'll take a look at everything in the next couple of days.

@svwilliams
Copy link
Contributor

At least for now, I'm trying to maintain compatibility back to Kinetic. So if you need functionality that doesn't go back that far, I'd say you need to duplicate the CovarianceVisual code.

Or you can always release fuse_viz as a separate package, outside of the main fuse repo. Then you can add whatever version restrictions you want.

@efernandez
Copy link
Collaborator Author

I've reverted the last commit, which is the one that requires rviz 1.13.4. Now it should build.
I've also roslint the code.

Copy link
Contributor

@svwilliams svwilliams left a comment

Choose a reason for hiding this comment

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

There is definitely some repetition between the pose visual/property and the relative pose visual/property, but I don't think it's worth refactoring anything at this point.

fuse_viz/include/fuse_viz/conversions.h Outdated Show resolved Hide resolved
fuse_viz/src/serialized_graph_display.cpp Show resolved Hide resolved
This includes:
* Dynamically generate display properties for the constraint sources
* Cache the constraint sources properties config so it's applied when
  the properties are later created
* Create Variable visual + property, as we do for the Constraint
This reverts commit 8b2e683.

This allows to use the rviz version shipped with Kinetic, and we only
need to revert this commit when we decide to drop backwards
compatibility with Kinetic or older versions of rviz.
@efernandez
Copy link
Collaborator Author

@svwilliams I addressed your comments and rebased this on top of the current devel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants