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: replace ref interface with binary interface in uxr create entity methods #27145

Merged
merged 5 commits into from
May 29, 2024

Conversation

srmainwaring
Copy link
Contributor

@srmainwaring srmainwaring commented May 23, 2024

Replace the uxr_buffer_create_*_ref methods with uxr_buffer_create_*_bin. This eliminates the need for a dds_xrce_profile.xml on the agent side and simplifies future work to #ifdef messages for inclusion as everything is now self contained the AP_DDS library.

Testing

  • Run the standard launch examples, dropping the ref:=... argument.

Flash

  • There is an increase in flash of approx 2k.
  • This is due to the additional strings required to label topics and types.
  • Strings used to reference the data readers and writers in the XML ref file have been removed.

See Also

- Add topic name and type to topic table.
- Use binary creation functions for participants and topics.
- Add constant for domain ID.
- Create publishers and datawriters by binary
- Create subscribers, datareaders and services by binary
- Add extra fields to the services table.
- Remove dds_xrce_profile.xml
- Document additional service table fields
- Add QoS struct to topic and service tables
- Replace profile labels with enums.

Signed-off-by: Rhys Mainwaring <[email protected]>
@srmainwaring srmainwaring force-pushed the prs/pr-dds-create-entity-bin branch from 83d8073 to 3b09948 Compare May 23, 2024 18:10
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

Awesome!! This is a significant advance in our DDS support - this allows us to start using IFDEF's to enable only some topics and save resources and improve security only enabling what's actually needed. Many thanks for doing this.

libraries/AP_DDS/AP_DDS_Topic_Table.h Outdated Show resolved Hide resolved
.type_name = "tf2_msgs::msg::dds_::TFMessage_",
.qos = {
.durability = UXR_DURABILITY_VOLATILE,
.reliability = UXR_RELIABILITY_RELIABLE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure we should use reliable for our high rate TF?

Copy link
Contributor Author

@srmainwaring srmainwaring May 25, 2024

Choose a reason for hiding this comment

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

These are the subscribers. The XML did not specify a reliability for these topics

- /ap/joy
- /ap/tf
- /ap/cmd_vel
- /ap/cmd_gps_pose

If the default QoS (struct) initialised each of it's fields to 0 we get:

typedef struct uxrQoS_t
{
    uxrQoSDurability durability = UXR_DURABILITY_TRANSIENT_LOCAL;
    uxrQoSReliability reliability = UXR_RELIABILITY_RELIABLE;
    uxrQoSHistory history = UXR_HISTORY_KEEP_LAST;
    uint16_t depth = 0;
} uxrQoS_t;

See create_entities_bin.h. Now I'm not certain that the XML is doing the same. Perhaps we should set reliability to BEST_EFFORT for all subscribers?

Switch to BEST_EFFORT in bf1962d

Copy link
Collaborator

Choose a reason for hiding this comment

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

For these I think best effort to save resources.

@tridge tridge merged commit 88926a3 into ArduPilot:master May 29, 2024
91 checks passed
@srmainwaring srmainwaring deleted the prs/pr-dds-create-entity-bin branch May 29, 2024 08:24
@IamPete1 IamPete1 removed the DevCallEU label Jun 4, 2024
srmainwaring added a commit to srmainwaring/ardupilot_wiki that referenced this pull request Jul 17, 2024
Update the launch command which has changed in `master`. The `dds-xrce-profile.xml` is not required after version `4.5`.

See: ArduPilot/ardupilot#27145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants