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 camera frustum marker #181

Open
JStech opened this issue Feb 16, 2021 · 5 comments
Open

Add camera frustum marker #181

JStech opened this issue Feb 16, 2021 · 5 comments

Comments

@JStech
Copy link

JStech commented Feb 16, 2021

I propose adding a marker type that draws a camera frustum given the optical frame, camera intrinsics (sensor_msgs::CameraInfo message), and scale. The marker would look like this:
frustum

And the function signature would be something like,

bool publishCameraFrustum(const Eigen::Isometry3d& pose, const sensor_msgs::CameraInfo& camera_intrinsics,
                          colors color, scales scale, const std::string& ns, std::size_t id)

There's already a function to do this in MoveIt Calibration, and I'd basically copy it here (and then use the version here in MoveIt Calibration). I recently ran into another situation where I'd like to draw a frustum, and so I thought it'd be a good addition to this package. Before writing it, though, I thought I'd double-check that people would want that included, since it would add the dependency on sensor_msgs.

@schornakj
Copy link

If you want to avoid adding the dependency on sensor_msgs you could pass in the intrinsics as an Eigen matrix or some other generic type (though that would be fairly annoying and you'd need an external conversion function anyway).

@JStech
Copy link
Author

JStech commented Feb 16, 2021

Yeah, I really just need the two focal lengths and the image height and width, so I could just make each of those values an argument. I'd prefer using the CameraInfo message, though, because it's convenient and has clear semantics.

@JafarAbdi
Copy link
Collaborator

@JStech rviz_visual_tools already depending on sensor_msgs see CMakeLists.txt & pacakge.xml

@JStech
Copy link
Author

JStech commented Feb 17, 2021

Ah, so it does--I didn't look beyond the main source file, I admit. I'll go ahead and put together a PR for this.

@tonynajjar
Copy link

Cool feature, was this ever implemented or any plans to?

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

No branches or pull requests

4 participants