-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Conversation
I'm confused. What's |
@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 ? |
MAV_TYPE_VTOL_QUADROTOR (20) has a "tailsitter" comment... (doesn't make too much sense to me though) |
Aha, I see. |
There was a problem hiding this 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: |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this 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
?
case MAV_TYPE_VTOL_RESERVED2: | ||
att_sp.thrust_body[2] = -set_attitude_target.thrust; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now checked and filling in true thrust_body
field
(Due to this switch)
In case of mc mode we should use this topic for VTOL: |
As my PR is much more of a stub than @irsdkv's, I will close this one. |
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. :)