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

AP_DDS: update dds profile, eliminate need for integration service #24880

Merged

Conversation

srmainwaring
Copy link
Contributor

@srmainwaring srmainwaring commented Sep 5, 2023

Use type and service name mapping rules to ensure AP_DDS service calls are automatically mapped between DDS and ROS 2. This eliminates the need to run the eProsima Integration Service to access AP_DDS services from the ROS 2 CLI.

Additionally the arm_motors service has been placed in the ap namespace to be consistent with other topics.

Details

  • Use type and topic name mapping rules in the arm_motors service profile.
  • Remove the integration service configuration file.
  • Update the service section in the README and document the topic and service mapping rules.

Figure: arm_motors service request/response pair with types and names in eProsima fastddsmonitor.
dds-monitor-services

Testing

  • Configure for SITL with ./waf configure --board sitl --enable-dds
  • Build ./waf build
  • Run a SITL session with DDS enabled and use UDP as a transport layer
ros2 launch ardupilot_sitl sitl_dds_udp.launch.py \
transport:=udp4 \
refs:=$(ros2 pkg prefix ardupilot_sitl)/share/ardupilot_sitl/config/dds_xrce_profile.xml \
synthetic_clock:=True \
wipe:=False \
model:=quad speedup:=1 \
slave:=0 \
instance:=0 \
defaults:=$(ros2 pkg prefix ardupilot_sitl)/share/ardupilot_sitl/config/default_params/copter.parm,$(ros2 pkg prefix ardupilot_sitl)/share/ardupilot_sitl/config/default_params/dds_udp.parm \
sim_address:=127.0.0.1 \
master:=tcp:127.0.0.1:5760 \
sitl:=127.0.0.1:5501
  • Check services are available
$ ros2 service list                        
/ap/arm_motors
  • Run the service example and check output in mavproxy
$ ros2 service call /ap/arm_motors ardupilot_msgs/srv/ArmMotors "{arm: True}"
requester: making request: ardupilot_msgs.srv.ArmMotors_Request(arm=True)

response:
ardupilot_msgs.srv.ArmMotors_Response(result=True)

- Use type and topic name mapping rules in the arm_motors service profile.
- Remove the integration service configuration file.
- Update the service section in the README and document the topic and service mapping rules.

Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

Tried testing, ran into incomplete build instructions. I ran the waf build, but should I have instread run a colcon build?

Opened up #24885 as a fix for ardupilot_sitl not declaring a runtime dependency on micro_ros_agent.

libraries/AP_DDS/README.md Show resolved Hide resolved
libraries/AP_DDS/README.md Show resolved Hide resolved
Copy link
Member

@arshPratap arshPratap left a comment

Choose a reason for hiding this comment

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

@srmainwaring I have tested the changes on my system and the services are running perfectly..Some minor changes but otherwise looking great
Thanks

.req_profile_label = "",
.rep_profile_label = "ArmMotors_Replier",
.rep_profile_label = "arm_motors__replier",
Copy link
Member

Choose a reason for hiding this comment

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

Lets rename this to arm_motors__rr or something similar.Quite similar to how we do it the topic and data reader/writer profiler names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arshPratap - I'll make these changes in a follow up PR. Also would like to standardise the node name to lowercase and rename to /ardupilot_dds. There are a few other house-keeping changes as well, so may sweep them up in a PR other stylistic / non-functional edits.

libraries/AP_DDS/README.md Show resolved Hide resolved
libraries/AP_DDS/dds_xrce_profile.xml Show resolved Hide resolved
@Ryanf55 Ryanf55 self-requested a review September 6, 2023 15:05
Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

image
Works now.
Needed to build micro_ros_agent, ardupilot_sitl, and ardupilot_msgs

@peterbarker peterbarker merged commit eeb5227 into ArduPilot:master Sep 6, 2023
84 checks passed
@srmainwaring srmainwaring deleted the prs/pr-dds-service-profile branch September 6, 2023 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants