Skip to content

uxrce_client: option to set publication rate per topic#24765

Merged
sfuhrer merged 3 commits into
PX4:mainfrom
GuillaumeLaine:uxrce-set-poll-rate
May 27, 2025
Merged

uxrce_client: option to set publication rate per topic#24765
sfuhrer merged 3 commits into
PX4:mainfrom
GuillaumeLaine:uxrce-set-poll-rate

Conversation

@GuillaumeLaine
Copy link
Copy Markdown
Contributor

@GuillaumeLaine GuillaumeLaine commented Apr 25, 2025

Changes

  • Adds the option to manually rate limit DDS publications per topic, by setting a value for rate_limit (Hz) in dds_topics.yaml
  • The default publication rates remains untouched, with the default polling rate still set to UXRCE_DEFAULT_POLL_RATE = 10ms / 100Hz across all topics
  • Renamed UXRCE_DEFAULT_POLL_RATE -> UXRCE_DEFAULT_POLL_INTERVAL_MS for clarity, because it could suggest being a frequency rather than a time interval

Related Discussions

@dakejahl
Copy link
Copy Markdown
Contributor

awesome!

Copy link
Copy Markdown
Member

@beniaminopozzan beniaminopozzan left a comment

Choose a reason for hiding this comment

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

Thanks @GuillaumeLaine !

Because you added publish_interval_ms, could you set it immediately during the generation of dds_topic.h to either the desired interval 1e3/rate_limit or the default interval so that later on you just call orb_set_interval(fds[idx].fd, send_subscriptions[idx].publish_interval_ms) ?

No need to have check the values twice.

While you are on it, could you rename UXRCE_DEFAULT_POLL_RATE to UXRCE_DEFAULT_POLL_INTERVAL_MS?
Thanks!

@hamishwillee
Copy link
Copy Markdown
Contributor

Don't forget an associated docs update please @GuillaumeLaine

@azerupi
Copy link
Copy Markdown

azerupi commented Apr 30, 2025

Other PR that was closed for the same feature: #23473 (posting for visibility)

@GuillaumeLaine
Copy link
Copy Markdown
Contributor Author

GuillaumeLaine commented May 23, 2025

@beniaminopozzan Thanks for your suggestion, I've applied it
@hamishwillee I've updated the associated docs, let me know if they are adequate: PX4/PX4-user_guide#3661

EDIT: I was just told docs are now pushed to this repo directly. I've pushed the docs here and closing the other docs PR

Comment thread src/modules/uxrce_dds_client/dds_topics.yaml Outdated
@github-actions
Copy link
Copy Markdown
Contributor

No flaws found

Copy link
Copy Markdown
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Looks good to me, would appreciate another review by @beniaminopozzan though as he previously requested changes.

Comment thread src/modules/uxrce_dds_client/dds_topics.yaml Outdated
Copy link
Copy Markdown
Member

@beniaminopozzan beniaminopozzan left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks @GuillaumeLaine !

Comment thread docs/en/middleware/uxrce_dds.md
@sfuhrer sfuhrer merged commit e1167f0 into PX4:main May 27, 2025
66 of 67 checks passed
@@ -428,9 +428,11 @@ publications:

- topic: /fmu/out/vehicle_odometry
type: px4_msgs::msg::VehicleOdometry
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW, this all looks good.

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.

6 participants