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

[Draft] Fix SITL offboard attitude for VTOL_TYPE_RESERVED2 #13122

Closed
wants to merge 3 commits into from

Conversation

ThomasRigi
Copy link
Member

@ThomasRigi ThomasRigi commented Oct 8, 2019

Describe problem solved by the proposed pull request
Partly fixes #13092 :
Fixes offboard attitude control for VTOLs of type VTOL_TYPE_RESERVED2 in SITL.

Does not work with HIL and the big question is why?

Test data / coverage
Successfully tested in SITL with MAVSDK function Offboard::set_attitude(Offboard::Attitude attitude):
https://logs.px4.io/plot_app?log=870a9ef1-c346-4a18-beb9-1afb8c6a287a

Not working in HIL (with Pixhawk 4 as hardware) :
https://logs.px4.io/plot_app?log=f714ce4f-828c-45bc-8753-f95fd6b2bbab

Not tested on real drones.

Describe your preferred solution
Changed the ORB ID of the attitude setpoint publication for VTOL_TYPE_RESERVED2. Other VTOL_TYPE_* vehicles can easily be added in the switch. Downside: needs a new ORB Publication with the new API.

Describe possible alternatives
Could also use vehicle_status.is_vtol flag , but that would need another ORB subscription to access that information.
Also, with the old ORB API, it would be possible to only have one publication in the header file and set the ORB ID accordingly in the switch before publication

Additional context
I could not figure out why it doesn't work in HIL, that's a big point of confusion for me. Would be good if someone else could have a look at that. :)

@julianoes
Copy link
Contributor

I'm confused. What's VTOL_TYPE_RESERVED2?

@ThomasRigi
Copy link
Member Author

@julianoes All standard VTOL frames I have looked at use this MAV_TYPE, so I thought they were equivalent. Maybe @sfuhrer or @RomanBapst can confirm?

@sfuhrer
Copy link
Contributor

sfuhrer commented Oct 8, 2019

@julianoes All standard VTOL frames I have looked at use this MAV_TYPE, so I thought they were equivalent. Maybe @sfuhrer or @RomanBapst can confirm?

Looks like you're correct. VTOL_TYPE_RESERVED2 is MAV_TYPE 22, which we fly for standard VTOL. No clue why it's not MAV_TYPE_VTOL_QUADROTOR (20) actually... @RomanBapst ?

https://github.com/mavlink/c_library_v2/blob/31cc5bb39e1cb69f5e608e8e2cac8a4d57332e1c/common/common.h#L91-L95

@ThomasRigi
Copy link
Member Author

MAV_TYPE_VTOL_QUADROTOR (20) has a "tailsitter" comment... (doesn't make too much sense to me though)

@julianoes
Copy link
Contributor

Aha, I see.

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct but please run make format.

@@ -1397,6 +1397,7 @@ MavlinkReceiver::handle_message_set_attitude_target(mavlink_message_t *msg)
case MAV_TYPE_OCTOROTOR:
case MAV_TYPE_TRICOPTER:
case MAV_TYPE_HELICOPTER:
case MAV_TYPE_VTOL_RESERVED2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this specific only for this VTOL type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how thrust is handled in the other VTOL types, so I preferred to leave it blank for those types. I only work on quadplanes.

Also, my newly added switch on lines 1410-1417 eventually needs to be updated with the other VTOL types (preferably by someone who knows the other types, not that I make a stupid mistake)

Copy link
Contributor

@jlecoeur jlecoeur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasRigi (and @irsdkv in #13140), you both report that you tested your PRs with MAVSDK, however the two solutions are not the same. Is it really needed to publish on topic mc_virtual_attitude_setpoint?

Comment on lines +1400 to 1401
case MAV_TYPE_VTOL_RESERVED2:
att_sp.thrust_body[2] = -set_attitude_target.thrust;
Copy link
Contributor

@jlecoeur jlecoeur Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work only in MC mode, att_sp.thrust_body[0] should be used when in fixed-wing mode.

Copy link
Contributor

@irsdkv irsdkv Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@irsdkv
Copy link
Contributor

irsdkv commented Oct 9, 2019

@ThomasRigi
Copy link
Member Author

As my PR is much more of a stub than @irsdkv's, I will close this one.

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.

[VTOL] Offboard attitude control not working
6 participants