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

Offboard trajectory_setpoint conflict when in auto modes #18306

Closed
benjinne opened this issue Sep 28, 2021 · 6 comments
Closed

Offboard trajectory_setpoint conflict when in auto modes #18306

benjinne opened this issue Sep 28, 2021 · 6 comments
Labels
bug Mode: Offboard Offboard mode control

Comments

@benjinne
Copy link
Contributor

Describe the bug
Offboard trajectory_setpoint sent while in an auto mode conflict with flight_tasks sending trajectory_setpoint since they both publish to the same topic. It appears everything is working correctly when sending trajectory_setpoints at the 10hz recommended by the offboard tutorial and in an auto mode, however this is because flight_tasks publishes at 50hz and is fast enough to essentially "override" the setpoints sent by the 10hz offboard publisher. When publishing at 100hz to trajectory_setpoint from offboard, the auto mode is essentially ignored and the offboard setpoint overrides the internal flight_tasks module.

To Reproduce
Steps to reproduce the behavior:

  1. Launch simulator and set PX4 to HOLD mode
  2. Send only trajectory_setpoints using offboard at 10hz and nothing happens
  3. Send only trajectory_setpoints using offboard at 100hz and observe HOLD mode is overriden

Expected behavior
There needs a way for trajectory_setpoint to be distinguished if it's coming from offboard or flight_tasks. When in offboard mode, flight_tasks no longer send trajectory_setpoints, but if in any auto mode, a conflict is possible if offboard setpoints are sent faster than the flight_task

@dagar dagar added the bug label Sep 28, 2021
@dagar
Copy link
Member

dagar commented Sep 28, 2021

This type of potential conflict needs to be prevented more fundamentally.

Are you using Mavlink or ROS2/FastRTPS?

In the mavlink case this should already be prevented until the system is actually in OFFBOARD mode.

if (vehicle_status.nav_state == vehicle_status_s::NAVIGATION_STATE_OFFBOARD) {
// only publish setpoint once in OFFBOARD
setpoint.timestamp = hrt_absolute_time();
_trajectory_setpoint_pub.publish(setpoint);
}

@benjinne
Copy link
Contributor Author

I'm using ROS2/FastRTPS

@dagar
Copy link
Member

dagar commented Sep 28, 2021

The quickest fix is to respect the nav_state before publishing (like mavlink).

The better fix would be to publish the offboard setpoint on a different topic (same msg) and the consumer (the position controller) can decide which to respect based on the system state.

@benjinne
Copy link
Contributor Author

I'm working on a fix, and got multicopter position controller working, but for the rover, why is there 2 updates for trajectory_setpoint_sub?

_trajectory_setpoint_sub.update(&_trajectory_setpoint);

and
_trajectory_setpoint_sub.update(&_trajectory_setpoint);

@dagar
Copy link
Member

dagar commented Sep 29, 2021

I'm working on a fix, and got multicopter position controller working, but for the rover, why is there 2 updates for trajectory_setpoint_sub?

_trajectory_setpoint_sub.update(&_trajectory_setpoint);

and

_trajectory_setpoint_sub.update(&_trajectory_setpoint);

I don't think it's intentional, it's probably a result of how things evolved to this point with OFFBOARD setpoints previously getting to the controllers through a different topic.

@benjinne
Copy link
Contributor Author

benjinne commented Dec 7, 2022

Closing since mavlink handles this issue and using ROS2 is experimental and assumes the offboard computer must stop sending setpoints if a pilot takes over.

@benjinne benjinne closed this as completed Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Mode: Offboard Offboard mode control
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants