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

Visualze Frustum #491

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Visualze Frustum #491

merged 5 commits into from
Jan 17, 2025

Conversation

BA-Utkarsh
Copy link
Contributor

@BA-Utkarsh BA-Utkarsh commented Dec 26, 2024

🎉 New feature

Summary

This PR mainly adds the visualization of Frustum.
We could see it was present in gazebo classic and from gazebo garden onwards the plugin/feature is not available.

Test it

$ Build gazebo from source.
$ . install/setup.sh
$ gz sim examples/worlds/visualize_frustum.sdf

Test Ref images,

  1. Play the simulation.
    frustum-1

  2. Select the topic from scroll down.
    frustum-2

  3. Refresh it to get the "logical_camera/frustum" topic.
    frustum-3

  4. Subcriibed to "logical_camera/frustum".
    frustum-4

  5. Final output
    frustum-6

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed
  • All tests passed
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Supporting PRs

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@BA-Utkarsh
Copy link
Contributor Author

@BA-Utkarsh BA-Utkarsh requested a review from ahcorde January 10, 2025 07:44
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

please check linters

  /github/workspace/src/LogicalCameraSensor.cc:185:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/src/LogicalCameraSensor.cc:195:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/src/LogicalCameraSensor.cc:196:  Lines should be <= 80 characters long  [whitespace/line_length] [2]

Signed-off-by: Utkarsh <[email protected]>
@BA-Utkarsh BA-Utkarsh requested a review from ahcorde January 11, 2025 10:27
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

thanks for the contribution. Just minor comments.

@@ -36,9 +36,13 @@ class gz::sensors::LogicalCameraSensorPrivate
/// \brief node to create publisher
public: transport::Node node;

public: transport::Node node_logic;
Copy link
Contributor

Choose a reason for hiding this comment

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

add doxygen comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@@ -56,6 +60,8 @@ class gz::sensors::LogicalCameraSensorPrivate

/// \brief Msg containg info on models detected by logical camera
msgs::LogicalCameraImage msg;

msgs::LogicalCameraSensor msg_logic;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
msgs::LogicalCameraSensor msg_logic;
/// \brief Msg containing logical camera frustum info.
public: msgs::LogicalCameraSensor msgLogic;

Looks like we forgot to add public to msg above as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

/// \brief publisher to publish logical camera messages.
public: transport::Node::Publisher pub;

public: transport::Node::Publisher pub_logic;
Copy link
Contributor

Choose a reason for hiding this comment

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

use camelCase and add doxygen comment:

Suggested change
public: transport::Node::Publisher pub_logic;
/// \brief Publisher to publish logical camera frustum information
public: transport::Node::Publisher pubLogic;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

if (!this->dataPtr->pub)
{
gzerr << "Unable to create publisher on topic[" << this->Topic() << "].\n";
return false;
}

if (!this->dataPtr->pub_logic)
{
gzerr << "Unable to create publisher on topic[" << this->Topic() << "].\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gzerr << "Unable to create publisher on topic[" << this->Topic() << "].\n";
gzerr << "Unable to create publisher on topic[" << this->Topic() << "/frustum].\n";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@iche033 iche033 merged commit ecb2681 into gazebosim:gz-sensors9 Jan 17, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic 🪵 jetty Gazebo Jetty
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants