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

Yaw mode manual auto #9146

Closed
wants to merge 10 commits into from
Closed

Yaw mode manual auto #9146

wants to merge 10 commits into from

Conversation

Stifael
Copy link
Contributor

@Stifael Stifael commented Mar 23, 2018

Based on the discussion here.

What this PR does:

  • move logic defined by MIS_YAWMODE to mc_pos_control
  • Navigator has higher priority than mc_pos_control to set yaw (only applies to auto)
    • Navigator sends yaw if: waypoint contains yaw (!=NAN), ROI is turned on but mount has no yaw control
  • MIS_YAWMODE has an affect on manual (pleas give feedback if that makes sense)
    • 0: yaw is not changed and is equal to the yaw when MIS_YAWMODE switched to 0
    • 1: yaw is controlled from sticks
    • 2: heading points towards home if available. otherwise mode 0 applies.
    • 3: heading points away from home if available. otherwise 1 mode 0 applies.

Although MIS_YAWMODE is not used in the navigator anymore, I kept this parameter for legacy reason.

@dagar, I was very confused about the heading_sp_update method (https://github.com/PX4/Firmware/compare/yaw_mode_manual_auto?expand=1#diff-9b5a77642a641b107c8f8a966de34acdL222), in particular about the different flags. For instance, to me it is not clear why the heading update method gets entered if the vehicle is a vtol...

The biggest advantage of PR is that for MC, the triplets are no longer sent continuously during mission.

@Stifael Stifael requested a review from dagar March 23, 2018 11:16
@dagar
Copy link
Member

dagar commented Mar 23, 2018

I think the MIS_YAWMODE parameter can move to MPC and be renamed appropriately. Generally we want to minimize parameter churn to not break configurations, but in this case the vast majority will be unaffected. We should still make note of the change and call it out in the release notes.

@Stifael Stifael force-pushed the yaw_mode_manual_auto branch from d8d38e8 to 6d1bf30 Compare March 26, 2018 06:12
@Stifael Stifael changed the title [WIP] Yaw mode manual auto Yaw mode manual auto Mar 26, 2018
@Stifael
Copy link
Contributor Author

Stifael commented Mar 27, 2018

@dagar I replaced it.
However, I am still wondering if this breaks anything vtol related, mainly because of this check here:
https://github.com/PX4/Firmware/pull/9146/files#diff-9b5a77642a641b107c8f8a966de34acdL222

@Stifael
Copy link
Contributor Author

Stifael commented Mar 29, 2018

@RomanBapst do you have any insight about vtol yaw handling in the navigator? In particular, why this line was required?

@dagar
Copy link
Member

dagar commented Mar 29, 2018

Obviously we need to check every case, but I'm reasonably certain the proper thing to do is not treat VTOL any differently. The vtol "controller" is capable of overriding the attitude setpoint for things like weathervane mode (ie yaw rate crippling).

@Stifael
Copy link
Contributor Author

Stifael commented Apr 3, 2018

@dagar, you are saying that here it should check for VTOL as well?

@dagar
Copy link
Member

dagar commented Apr 3, 2018

A single is_rotary_wing check should be sufficient. The is_rotary_wing flag is true when a VTOL is in MC mode.

@dagar
Copy link
Member

dagar commented Jul 7, 2018

Sorry, we lost track of this PR. Let me know when you're ready to move forward with it.

@dagar dagar added this to the Release v2.0.0 milestone Jul 7, 2018
@Stifael Stifael mentioned this pull request Jul 23, 2018
@Stifael
Copy link
Contributor Author

Stifael commented Aug 13, 2018

close this since obsolete

@Stifael Stifael closed this Aug 13, 2018
@LorenzMeier LorenzMeier deleted the yaw_mode_manual_auto branch January 21, 2019 10:47
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.

2 participants