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

publish vehicle rates setpoint topic #24262

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bdilman
Copy link
Contributor

@bdilman bdilman commented Jan 28, 2025

Solved Problem

This PR adds the vehicle_rates_setpoint under published dds topics. ROS2 applications can with this change subscribe to thrust and rate setpoints.

Expected terminal output:

[uxrce_dds_client] successfully created rt/fmu/out/vehicle_rates_setpoint data writer, topic id: 266

Fixes #{Github issue ID}

Solution

  • Add ... for ...
  • Refactor ...

Changelog Entry

For release notes:

Feature/Bugfix XYZ
New parameter: XYZ_Z
Documentation: Need to clarify page ... / done, read docs.px4.io/...

Alternatives

We could also ...

Test coverage

Context

Related links, screenshot before/after, video

@bdilman bdilman requested a review from sfuhrer January 28, 2025 14:55
Copy link

github-actions bot commented Jan 28, 2025

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 264 byte (0.01 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +264  +0.0%    +264    .text
  +0.6%    +212  +0.6%    +212    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +45  +0.0%     +45    [section .text]
  +0.1%      +4  +0.1%      +4    ../../src/modules/vtol_att_control/vtol_type.cpp
  +0.2%      +3  +0.2%      +3    ../../src/systemcmds/ver/ver.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +44  [ = ]       0    .debug_frame
+0.0%    +142  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
  +0.1%    +146  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%    +266  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.5%    +292  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  -0.1%      -1  [ = ]       0    task/task_cancelpt.c
+0.0%    +194  [ = ]       0    .debug_loc
  +0.6%    +194  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%     +38  [ = ]       0    .debug_str
  +0.2%     +38  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%     +60  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +1.7%     +60  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +32  [ = ]       0    [section .strtab]
+0.0%     +32  [ = ]       0    .symtab
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +1.1%     +32  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
-2.3%    -264  [ = ]       0    [Unmapped]
+0.0%    +832  +0.0%    +264    TOTAL

px4_fmu-v6x [Total VM Diff: 248 byte (0.01 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +248  +0.0%    +248    .text
  +0.6%    +212  +0.6%    +212    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +32  +0.0%     +32    [section .text]
  +0.1%      +4  +0.1%      +4    ../../src/modules/vtol_att_control/vtol_type.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +44  [ = ]       0    .debug_frame
+0.0%    +142  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
  +0.1%    +146  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%    +266  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.5%    +292  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  -0.1%      -1  [ = ]       0    task/task_cancelpt.c
+0.0%    +194  [ = ]       0    .debug_loc
  +0.6%    +194  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%     +38  [ = ]       0    .debug_str
  +0.2%     +38  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%     +60  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +1.7%     +60  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +32  [ = ]       0    [section .strtab]
+0.0%     +32  [ = ]       0    .symtab
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +1.1%     +32  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
-0.3%    -248  [ = ]       0    [Unmapped]
+0.0%    +832  +0.0%    +248    TOTAL

Updated: 2025-01-28T16:09:42

@bdilman bdilman marked this pull request as ready for review January 28, 2025 15:58
Copy link
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.

Agree that it is useful for some applications, e.g. to smoothen the hand-over from a PX4-internally controlled flight mode to an external one that is interfacing over rate/thrust setpoints.

@beniaminopozzan anything to add from your side?

@beniaminopozzan
Copy link
Member

@sfuhrer given the quite high rate of the topic I'd double check that the overall PX4 TX rate isn't getting too high in particular when serial connection is used. Last time I checked the client was sending all messages to the agent even if the topics had no subscribers.

@sfuhrer
Copy link
Contributor

sfuhrer commented Jan 29, 2025

@sfuhrer given the quite high rate of the topic I'd double check that the overall PX4 TX rate isn't getting too high in particular when serial connection is used. Last time I checked the client was sending all messages to the agent even if the topics had no subscribers.

Good point. How do you check the TX rate load? And how is it again, is the message sent to ROS whenever there is a uORB topic publication?

Alternatively we could consider VehicleAttitudeSetpoint, that is updated at a lower rate already.

@beniaminopozzan
Copy link
Member

How do you check the TX rate load?

The uxrce_dds_client status command will give you an estimate of TX and RX rate:

if (_connected) {
PX4_INFO("Payload tx: %i B/s", _last_payload_tx_rate);
PX4_INFO("Payload rx: %i B/s", _last_payload_rx_rate);
}

And how is it again, is the message sent to ROS whenever there is a uORB topic publication?

Even if there is no subscriber in the ROS 2 network the TX value will remain non-zero.
It can be also checked in sim.

@beniaminopozzan
Copy link
Member

Behaviour in sim pre-pr:

pxh> uxrce_dds_client status
INFO  [uxrce_dds_client] Running, connected
INFO  [uxrce_dds_client] Using transport:     udp
INFO  [uxrce_dds_client] Agent IP:            127.0.0.1
INFO  [uxrce_dds_client] Agent port:          8888
INFO  [uxrce_dds_client] Custom participant:  no
INFO  [uxrce_dds_client] Localhost only:      no
INFO  [uxrce_dds_client] Payload tx:          75298 B/s
INFO  [uxrce_dds_client] Payload rx:          0 B/s
INFO  [uxrce_dds_client] timesync converged: true
uxrce_dds_client: cycle: 10008 events, 33620000us elapsed, 3359.31us avg, min 0us max 8000us 3433.727us rms
uxrce_dds_client: cycle interval: 10009 events, 3358.98us avg, min 0us max 8000us 3433.556us rms

post pr:

pxh> uxrce_dds_client status
INFO  [uxrce_dds_client] Running, connected
INFO  [uxrce_dds_client] Using transport:     udp
INFO  [uxrce_dds_client] Agent IP:            127.0.0.1
INFO  [uxrce_dds_client] Agent port:          8888
INFO  [uxrce_dds_client] Custom participant:  no
INFO  [uxrce_dds_client] Localhost only:      no
INFO  [uxrce_dds_client] Payload tx:          78578 B/s
INFO  [uxrce_dds_client] Payload rx:          0 B/s
INFO  [uxrce_dds_client] timesync converged: true
uxrce_dds_client: cycle: 14090 events, 41616000us elapsed, 2953.58us avg, min 4000us max 8000us 2689.561us rms
uxrce_dds_client: cycle interval: 14091 events, 2953.37us avg, min 0us max 8000us 2689.465us rms

It is not that bad tbh

@bdilman
Copy link
Contributor Author

bdilman commented Jan 30, 2025

@beniaminopozzan Thank you for your review and testing!

I have just checked the topic frequencies inside a dockerized external mode and they're indeed published at a very high rate and even in absence of a subscriber:

ros2 topic hz /fmu/out/vehicle_rates_setpoint
average rate: 100.664
	min: 0.001s max: 0.028s std dev: 0.00492s window: 103
average rate: 94.286
	min: 0.001s max: 0.040s std dev: 0.00538s window: 191
ros2 topic info /fmu/out/vehicle_rates_setpoint
Type: px4_msgs/msg/VehicleRatesSetpoint
Publisher count: 1
Subscription count: 0

If you find the data rate increase still okay, I actually would like to proceed with the PR. In parallel checking options to avoid this topic to be a necessity on the external mode side.

@Jaeyoung-Lim
Copy link
Member

@beniaminopozzan The message rate of the simulation and the hardware might be different and therefore the implication.

I would be a bit mindful on publishing high rate streams by default which people rarely use. The dds topic yaml is supposed to be user configurable :)

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.

4 participants