PX4-Autopilot icon indicating copy to clipboard operation
PX4-Autopilot copied to clipboard

Add offboard mode switch module for simplifying offboard for ros2

Open Jaeyoung-Lim opened this issue 1 year ago • 8 comments

Solved Problem

When using offboard mode with ros2, the sender needs to be careful to not publish the setpoints while not in offboard mode.

This can be achieved typically by subscribing to a vehicle status message, and publishing the setpoints only when the vehicle enters offboard mode.

However, if the person implementing the offboard mode is not responsible, this may be prone to errors by intefering with other controllers.

Fixes https://github.com/PX4/PX4-Autopilot/issues/22512

@ayhamalharbat FYI

Solution

  • Separate the offboard topic into a dedicated topic.
  • Remove the need for publishing offboard control mode separately

Changelog Entry

For release notes:

Bugfix Separate offboard setpoints

Alternatives

We could also keep the interface transparent, and require the ros2 node to be responsible for making sure that it does not interfere with other topics

Test coverage

Tested in Gazebo, with gz_omnicopter

Jaeyoung-Lim avatar Dec 12 '23 20:12 Jaeyoung-Lim

@Jaeyoung-Lim The idea here is that the new module monitors the offboard_* topic, and if they are valid then it publishes the offboard_control_mode topic and only when the vehicle enter OFFBOARD mode then the offboard_* topics republished standard setpoints? Did I get it right?

beniaminopozzan avatar Dec 17 '23 17:12 beniaminopozzan

@beniaminopozzan Yes!

Jaeyoung-Lim avatar Dec 17 '23 17:12 Jaeyoung-Lim

Great, is there a reason for having only the logic for trajectory, attitude and rate setpoint? what about thrust and torque and direct?

beniaminopozzan avatar Dec 17 '23 20:12 beniaminopozzan

about thrust and torque and direct?

@beniaminopozzan No real reason, just wasnt able to test with it yet

One thing missing from this is sanity checking the combination of setpoints (e.g. you cant set thrust and torque with rates at the same time)

Jaeyoung-Lim avatar Dec 18 '23 04:12 Jaeyoung-Lim

@MaEtUgR we are still waiting on your review!

mrpollo avatar Jan 30 '24 16:01 mrpollo

@beniaminopozzan I have intentionally left them out, since I think for those I think it might be just better to publish directly to the actuators due to the time delay introduced by publishing/republishing.

I also wouldnt know how to test it if we have the interface other than SITL, which is not representative of doing this on hardware.

If we intend to add such interface, I would do it in a separate PR

Jaeyoung-Lim avatar Jan 30 '24 19:01 Jaeyoung-Lim

@Jaeyoung-Lim oh I see, that makes sense unless the expected increase in latency is proven wrong by real hardware tests.

Nevertheless, this further increase the complexity of the offboard interface, now users have to keep in mind that some offboard setpoints have to be paired with the offboardControlMode messages while other setpoints don't.

Wouldn't it be cleaner at this point, instead of having a centralized offboard mode swither, to distribute this on every module that uses the "setpoint" messages? Something like: if mc_att_control subcribes to vehicle_attitude_setpoints, then it will subscribe to offboard_attitude_setpoint too. Internally it will use offboard_attitude_setpoint instead of vehicle_attitude_setpoints if vehicle_control_mode says offboard + attitude.

Conficting modes situations will be addressed at the single controller level, which will check VehicleControlMode for knowing which setpoint is shall use

beniaminopozzan avatar Feb 03 '24 12:02 beniaminopozzan

Wouldn't it be cleaner at this point, instead of having a centralized offboard mode swither, to distribute this on every module that uses the "setpoint" messages? Something like: if mc_att_control subcribes to vehicle_attitude_setpoints, then it will subscribe to offboard_attitude_setpoint too. Internally it will use offboard_attitude_setpoint instead of vehicle_attitude_setpoints if vehicle_control_mode says offboard + attitude.

I have been trying to avoid this, as by doing this we would create a new "flight mode" for offboard mode and all controller modules need to be aware of the "offboard" interface. For me the value of offboard mode is that the control modules do not know whether the vehicle is in offboard mode and therefore we can test individual structures by simply turning on and off different levels of control handled by commander. If somebody is implementing a new controller, they do not need to implement a special "offboard" interface to test it.

Regarding completeness, I agree with you that it would be nice to support direct actuator setpoints. However, this PR solves a problem where a lot of people are struggling to use the offboard interface due to the "handshaking" behavior it requires(need to monitor vehicle flight mode before publishing the setpoints). The people struggling to do this are rarely people using direct actuator inputs.

On the other hand, I do not have the capacity to test the direct actuator setpoint interface for this PR. So if anyone would be interested in contributing it would be great. But otherwise, it will be hard to include it as part of this PR.

Note that this PR does not interfere/replace the previous way of engaging offboard mode. It just adds a simplified interface to test it.

Jaeyoung-Lim avatar Feb 05 '24 09:02 Jaeyoung-Lim