-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
AP_Mount: Add parameter to control relative pan option for servo mounts #16433
Conversation
I'm all for this this fixes a bug that some users reported. But I think it would be bettter if you use a MNT()_OPTIONS flags parameter. |
@spark404 We discussed in DevCall today...a few comments:
|
Sure, no problem :-) I've registered on the discord server, so it's best to use that to find the best date. But today (jan 26th) 2 pm CEST could work, tomorrow as well. |
* Change from a dedicated parameter to a reusable options field
That makes sense, I've made the changes. Please advice if this is the preferred way of doing this. Other discussions still pending on a meeting. |
Reading this more closely this just seems like a bug to me so I don't think we even need a parameter. Can anyone imagine a case where the servo outputs wouldn't be relative to the vehicle's heading when doing ROI? |
I can't think of any, but didn't want to risk overlooking something. So a parameter seemed the prudent action. I've looked through the other gimbals and I couldn't find a place where relative pan was set to false other than in AP_Mount_Servo. |
For this PR I think you can just change those couple of falses to trues and we can merge it. On a related note I think we will need a parameter and an auxiliary switch to allow users to control whether they want the yaw to remain locked in earth frame or in body frame when controlling the gimbal's pan via RC. This is scope creep though so not required to get this PR in. |
Done, just pushed the commit.
Nice, sounds like a useful feature. |
@rmackay9 I was planning a mission in QGroundControl and actually found a "use case". The gimbal orientation as set by QGC actually uses the earth reference to plan the gimbal angles. See attached screen grab. I'm still sure this behaviour would be troublesome when used with smarter gimbals that do use the vehicle orientation. Like storm gimbals in serial or MAVLink mode will probably not point in the right direction. (Unless either QGC or ArduCopter is being smart about this and compensates the angles for the gimbal relative to the vehicle yaw somewhere in the code. I don't know enough about the internals of the mission planning stuff to answer that) |
OK, I fell in the rabbit hole I guess, but here is a blurb on relative panning in AP_Mount and specifically from the AP_Mount_Servo point of view. So, we have these different modes for Gimbals. Each mode has their own needs and assumptions about the world around it. Starting with the easy ones and mainly focusing on the yaw (or pan) angle. Additional Notes (edited):
MAV_MOUNT_MODE_RETRACT: MAV_MOUNT_MODE_NEUTRAL: MAV_MOUNT_MODE_MAVLINK_TARGETING: MAV_MOUNT_MODE_RC_TARGETING: MAV_MOUNT_MODE_GPS_POINT: Taking these three together as they are from a gimbal orientation perspective quite similar. The goals it to make the gimbal point to a location for which we supply GPS coordinates. Considering only the yaw angle, this is done by calculating the bearing from the current position of the UAV to the indicated location. This bearing should be compared with the current heading of the UAV and the difference should be used as the input for the yaw angle. Another case that could be made is that as the bearing calculated is with respect to the earth frame the angle should be set according to the earth frame reference. In that case it’s left up to the gimbal to compensate for any yaw rotation. After writing all this down, my takeaway is that whether or not the servo mount should use relative pan is upto the “intelligence” in the gimbal. If the gimbal can determine its own heading, providing earth frame angles would work. If the gimbal knows only about its own initialization state and rotates itself to around that position with the angles provided, the logical behaviour would be to have the UAV calculate the yaw angles relative to the UAV yaw. Assuming that a servo mount is in general not an intelligent mount (hope I’m not offending any mounts here) making all calculations relative to the vehicle provides the most consistent behaviour. This means that GCS and missions shouldn’t assume to provide angles in ef, or additional code would need to be in the fc to calculate from ef to UAV frame when determining angles. I hope this makes any sense and would love some feedback from people that know a lot more about the code and the intensions behind it. I could be completely wrong about all of this and would love to understand why. Semi-related food for thought; When running through the calculations in AP_Mount_Backend for yaw angles in noticed that an approximation is used for both the distance between GPS coordinates and bearing. I’m assuming this is to avoid an overload in floating point angle calculation. However by design this introduces and error, which on relatively short distances could be non-trivial. I calculated scenario with a target at 66 meters and bearing -116° and the mount code calculated 101 meters distance and -106° bearing. This roughly translates to the camera pointing a spot 37 meter away from the intended target. For a wide view camera this might be an acceptable error, but with (extreme) zoom cameras this error might place the target out of frame. |
@Hwurzburg here is a Testflight (SITL/Gazebo with the patch) : https://youtu.be/5L8sH2xFCvs As far as I can test it works pretty well with planned missions including tracking a ROI. This is the 3d gimbal used in the video: khancyr/ardupilot_gazebo#46 |
ping, should this be discussed in a DevCall? |
@spark404 can you attend Ardupilot's EU Developer Call ? |
@amilcarlucas Yes, I'll try to be there. I should be able to make it if I got the timezone conversion right :-) |
The current code in AP_Mount_Servo sets the bool relative_pan to false. As far as I understand it this makes it calculate angles without calculating the pan angle relative to the current angle of the vehicle. When setting gimbal ROI targets separate from the ROI targets of the drone this results unexpected angles. I expected the gimbal to point to the ROI, regardless of the current yaw of the vehicle.
To adjust the behaviour i added a parameter MNT(2)_REL_PAN to change this from hardcoded to configurable behaviour. The default remains false which is the current hardcoded setting.
Limited testing done by installing this firmware with patch on a quadcopter and feeding several coordinates to the gimbal using MAV_CMD_DO_MOUNT_CONTROL.