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

Fix: deleted <robotNamespace> in depth_camera.sdf.jinja #983

Closed
wants to merge 1 commit into from

Conversation

ChanJoon
Copy link

@ChanJoon ChanJoon commented May 14, 2023

In previous commit, @Cavalletta98 added tag in this model.

e5836d3

but this had a serious impact on PX4-Avoidance and other use case of depth camera in custom UAV model.
The difference in the presence of this tag is as follows:

$ roslaunch local_planner local_planner-depth_camera.launch

if exists:
$rostopic list

# copied only related topics
/iris_obs_avoid/camera/depth/camera_info
/iris_obs_avoid/camera/depth/image_raw
/iris_obs_avoid/camera/depth/points
/iris_obs_avoid/camera/parameter_descriptions
/iris_obs_avoid/camera/parameter_updates
/iris_obs_avoid/camera/rgb/camera_info
/iris_obs_avoid/camera/rgb/image_raw
/iris_obs_avoid/camera/rgb/image_raw/compressed
/iris_obs_avoid/camera/rgb/image_raw/compressed/parameter_descriptions
/iris_obs_avoid/camera/rgb/image_raw/compressed/parameter_updates
/iris_obs_avoid/camera/rgb/image_raw/compressedDepth
/iris_obs_avoid/camera/rgb/image_raw/compressedDepth/parameter_descriptions
/iris_obs_avoid/camera/rgb/image_raw/compressedDepth/parameter_updates
/iris_obs_avoid/camera/rgb/image_raw/theora
/iris_obs_avoid/camera/rgb/image_raw/theora/parameter_descriptions
/iris_obs_avoid/camera/rgb/image_raw/theora/parameter_updates

else (this commit version or before e5836d3)

# copied only related topics
/camera/depth/camera_info
/camera/depth/image_raw
/camera/depth/points
/camera/parameter_descriptions
/camera/parameter_updates
/camera/rgb/camera_info
/camera/rgb/image_raw
/camera/rgb/image_raw/compressed
/camera/rgb/image_raw/compressed/parameter_descriptions
/camera/rgb/image_raw/compressed/parameter_updates
/camera/rgb/image_raw/compressedDepth
/camera/rgb/image_raw/compressedDepth/parameter_descriptions
/camera/rgb/image_raw/compressedDepth/parameter_updates
/camera/rgb/image_raw/theora
/camera/rgb/image_raw/theora/parameter_descriptions
/camera/rgb/image_raw/theora/parameter_updates

Since PX4-Avoidance utilizes the "/camera/depth/points" topic in their launch file, it might be more logical or appropriate to exclude this tag.
<arg name="pointcloud_topics" default="[/camera/depth/points]"/>

I know that using a specific robot namespace is easier for users to customize and utilize for models, but this ultimately depends on the user's needs.

@Cavalletta98
Copy link
Contributor

The problem was about simulating multiple drones for PX4 avoidance software. In this way you can have separate topics with namespace. If you remove It,It Will be impossible to simulate multiple drones

@ChanJoon
Copy link
Author

ChanJoon commented May 14, 2023

The problem was about simulating multiple drones for PX4 avoidance software. In this way you can have separate topics with namespace. If you remove It,It Will be impossible to simulate multiple drones

Thank you for your quick feedback!
As you said, when we use multiple drones, we should use <robotNamespace> in our camera model.

But I think it is depend on the user who wants to simulate multiple drones or different camera topics.
Given the reliance of PX4-Autopilot and PX4-Avoidance on this repository, it may seem more intuitive to remove the tag. (I personally encountered some challenges while trying to use local_planner_depth-camera.launch for both PX4-Avoidance and my own navigation system.)

Are there any other files that use this repository to simulate multiple drones? @Cavalletta98

@Cavalletta98
Copy link
Contributor

I really don't know

Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

Why not update the avoidance package to include the namespace?

@Cavalletta98
Copy link
Contributor

Probably was not possible. I didn't remember

@ChanJoon
Copy link
Author

ChanJoon commented May 14, 2023

It looks great to update the avoidance package.

I think we have two options. Delete this tag or Update avoidance launch file (i.e <arg name="pointcloud_topics" default="[/iris_obs_avoid/camera/depth/points]"/>)

@Cavalletta98
Copy link
Contributor

The problem Is that even if we update the avoidance with this fix,something more should be done. Because when i did this fix,It solved only the fact that you can see the camera of the different drones in rviz.
It should be discussed with the guys of avoidance software in order to do all the things for simulating multiple drones, and so decide where Is the right place to put this tag. Can you please open and issue on avoidance?

@ChanJoon
Copy link
Author

As far as I know, @Jaeyoung-Lim is one of the maintainers of the avoidance package.

@Jaeyoung-Lim Could you tell me your opinions?

@Jaeyoung-Lim
Copy link
Member

@ChanJoon I would say it is the downstream package(avoidance) that would need to be updated

@ChanJoon
Copy link
Author

@ChanJoon I would say it is the downstream package(avoidance) that would need to be updated

Thanks. I'll close this request and open a new one in the avoidance soon.

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.

3 participants