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

Ros2 add launch and remappings #43

Merged
merged 21 commits into from
May 13, 2024
Merged

Conversation

delihus
Copy link
Contributor

@delihus delihus commented Apr 24, 2024

Added dynamically loaded components with their plugins in Gazebo Ignition.

Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
Comment on lines 57 to 69
for component in node["components"]:
if component["type"] == "LDR01" or component["type"] == "LDR06":
actions.append(get_launch_description("slamtec_rplidar", package, namespace, component))

if component["type"] == "LDR13":
actions.append(get_launch_description("ouster_os", package, namespace, component))

if component["type"] == "LDR20":
actions.append(get_launch_description("velodyne", package, namespace, component))

if component["type"] == "CAM01":
actions.append(get_launch_description("orbbec_astra", package, namespace, component))

Copy link
Contributor

Choose a reason for hiding this comment

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

Make it easier to maintain

Suggested change
for component in node["components"]:
if component["type"] == "LDR01" or component["type"] == "LDR06":
actions.append(get_launch_description("slamtec_rplidar", package, namespace, component))
if component["type"] == "LDR13":
actions.append(get_launch_description("ouster_os", package, namespace, component))
if component["type"] == "LDR20":
actions.append(get_launch_description("velodyne", package, namespace, component))
if component["type"] == "CAM01":
actions.append(get_launch_description("orbbec_astra", package, namespace, component))
component_to_launch = {
"LDR01": "slamtec_rplidar",
"LDR06": "slamtec_rplidar",
"LDR13": "ouster_os",
"LDR20": "velodyne",
"CAM01": "orbbec_astra"
}
for component in node["components"]:
component_type = component["type"]
if component_type in component_to_launch:
launch_description = get_launch_description(component_to_launch[component_type], package, namespace, component)
actions.append(launch_description)

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we can even inplace and rid off get_launch_description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah, I prefer get_launch_description to stay.

Copy link
Contributor

@rafal-gorecki rafal-gorecki Apr 29, 2024

Choose a reason for hiding this comment

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

I think it could be done better (these test a bit scary me). First of all i will leave only test here and others useful function move to other file.

This is how I imagined these tests:

  1. Create a list of object types
  2. Create a list containing the test case: (namespaces, device_name, result)
  3. In a loop, loop through all sensors and namespaces.
    a. load_component
    b. check link name
    c. check sensor name.

Yaml loading tests can be added to this.
And similarly, first a few scenarios, e.g. without parent_name, should cause an error.

Let me know what you think, maybe I can help you with it, because I know you work less often now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So. We have changed the idea of our urdf. Changes applied in this pr.

Comment on lines 13 to 14
use_gpu="True"
simulation_engine="ignition-gazebo"
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably unncecesary

Suggested change
use_gpu="True"
simulation_engine="ignition-gazebo"
simulation_engine="ignition-gazebo"

@rafal-gorecki rafal-gorecki self-requested a review May 13, 2024 11:20
@rafal-gorecki rafal-gorecki merged commit cdd86f0 into ros2 May 13, 2024
1 check passed
@rafal-gorecki rafal-gorecki deleted the ros2-add-launch-and-remappings branch May 13, 2024 11:20
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.

2 participants