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

Add Fixedwing path following as a separate controller state #21376

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

Jaeyoung-Lim
Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim commented Mar 28, 2023

Solved Problem

This PR adds an additional FW path navigation state, which uses position, tangent and optionally curvature to do path tracking.

Currently this was done by overriding control_auto_position, but since it is using more of the derivative setpoints, we thought it should be separated out as a separate mode

The behavior is as the following.
In offboard,

  • When velocity setpoints are available set FW state as FW_POSCTRL_MODE_AUTO_PATH
  • When only position setpoints are available, set FW state as FW_POSCTRL_MODE_AUTO, which would end up using control_auto_position

Fixes #{Github issue ID}

Test coverage

  • Tested in SITL Gazebo
    • with normal mission mode
    • with offboard path tracking

Additional Context

  • Offboard seems to be also completely broken, so fixed it along the way

@Jaeyoung-Lim Jaeyoung-Lim changed the title Add fw state for path following Add Fixedwing path following as a separate controller state Mar 28, 2023
@Jaeyoung-Lim Jaeyoung-Lim marked this pull request as ready for review March 28, 2023 13:26
@Jaeyoung-Lim Jaeyoung-Lim force-pushed the pr-fw-offboard branch 3 times, most recently from 7e5aec0 to e3db31f Compare March 28, 2023 15:13
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.

I agree on the separating it into the new control_auto_path() method (as opposed to control_auto_position()). Let's just be very careful with the state machine, that one is hard to see through anymore.

This commit separates offboard path following as a separate state inside the FW Poscontrol module.

This is a cleanup on clearly defining fw pos control behaviors
@Jaeyoung-Lim
Copy link
Member Author

Jaeyoung-Lim commented Dec 27, 2023

@sfuhrer Rebased and tested with ROS2: https://review.px4.io/plot_app?log=3e50cfeb-ffb5-41fa-9fab-ad5b797207c8

I agree on the separating it into the new control_auto_path() method (as opposed to control_auto_position()). Let's just be very careful with the state machine, that one is hard to see through anymore.

Fully agreed, the logic is no longer tractable. I think we should try to hash out the global variables so we can unit test the conditions.Should we revisit #21382 ?

Also, would be great if we can finally get rid of pos_sp_triplets 😄

@Jaeyoung-Lim
Copy link
Member Author

@sfuhrer I don't think the CI failure is related to this PR (happening on a multirotor)

sfuhrer
sfuhrer previously approved these changes Jan 9, 2024
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.

Yes, lets get this in and move on to your next one!

src/modules/fw_pos_control/FixedwingPositionControl.cpp Outdated Show resolved Hide resolved
@sfuhrer
Copy link
Contributor

sfuhrer commented Jan 9, 2024

Also, would be great if we can finally get rid of pos_sp_triplets

What of the triplets is annoying you most? Maybe we can find a way iteratively work towards a better interface. Eg we can starve out PositionSetpoint further, until we can completely delete it.

Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
@Jaeyoung-Lim
Copy link
Member Author

What of the triplets is annoying you most? Maybe we can find a way iteratively work towards a better interface. Eg we can starve out PositionSetpoint further, until we can completely delete it.

@sfuhrer For me it seems like a superset of all the information we need downstream, and a lot of the information is actually unused or not properly populated, depending on the state of the controller.

@Jaeyoung-Lim Jaeyoung-Lim merged commit c8e041a into main Jan 9, 2024
87 of 89 checks passed
@Jaeyoung-Lim Jaeyoung-Lim deleted the pr-fw-offboard branch January 9, 2024 15:35
somebody-once-told-me pushed a commit to somebody-once-told-me/PX4-Autopilot that referenced this pull request Jan 13, 2024
* Separate offboard path setpoints as fixedwing pos control state

This commit separates offboard path following as a separate state inside the FW Poscontrol module.

This is a cleanup on clearly defining fw pos control behaviors

* Update src/modules/fw_pos_control/FixedwingPositionControl.cpp

Co-authored-by: Silvan Fuhrer <silvan@auterion.com>

* Fix format

---------

Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
Peize-Liu pushed a commit to Peize-Liu/PX4-Autopilot that referenced this pull request Mar 24, 2024
* Separate offboard path setpoints as fixedwing pos control state

This commit separates offboard path following as a separate state inside the FW Poscontrol module.

This is a cleanup on clearly defining fw pos control behaviors

* Update src/modules/fw_pos_control/FixedwingPositionControl.cpp

Co-authored-by: Silvan Fuhrer <silvan@auterion.com>

* Fix format

---------

Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants