Camera display for Mission Waypoints issues and specifications clarification

stefandunca commented 4 years ago

We have reports from pilots of Waypoints Camera Yaw not matching the final executed commands Looking throughout the code I see some unexpected behavior. With this issue, I would also like to clarify the specification.

Expected Behavior

[ ] Enabling one waypoint will show camera waypoints to all the following waypoints. If Vehicle heading is set for one waypoint it will propagate the vehicle yaw for the next waypoints as well
- is this intended? Looking into the implementation it looks like intended.
- while recomputing mission flight status _missionFlightStatus is used to store the last vehicle yaw and then to propagate it to the following waypoints https://github.com/mavlink/qgroundcontrol/blob/988cd6a9ecefca7503422e0bd9c62d5cad2a21ea/src/MissionManager/MissionController.cc#L1633 Simple mission Enabling camera yaw for one waypoint will show the expected camera yaw to the following waypoints Enabling vehicle yaw will reset the camera orientation for the others as well. The following has it set to 45 and the next one (last) to 0.

Current Behavior

[ ] Setting the current vehicle yaw it will be set for the last one as well. This looks like an issue and not intended
[ ] In some corner cases, setting vehicle yaw won't be taken into consideration. This also looks like an issue

Steps to Reproduce:

Create a mission containing four waypoints
Enable camera gimbal for the second waypoint
Enable Vehicle Heading (vehicle yaw in the code) for the last one, the fourth one. At this point, the third waypoint changes the camera direction
Enable vehicle heading for the third and set it to 45. The second changes as well.
Set vehicle heading for the second to 45, nothing happens. Set the gimbal yaw to 45, now the third and fourth changes as well.

System Information

Latest master.

DonLakeFlyer commented 4 years ago

Enabling one waypoint will show camera waypoints to all the following waypoints. If Vehicle heading is set for one waypoint it will propagate the vehicle yaw for the next waypoints as well

This is intended. The assumption is that if you set a gimbal angle and/or vehicle heading those settings will persist through the rest of the waypoints until you change them to something else. It also assume that the NaN setting for yaw on the subsequent waypoints will leave the heading alone as opposed to going back to say pointing at the next waypoint. If that is incorrect let me know.

Your repro steps indicate a separate bug in the gimbal calc code.

DonLakeFlyer commented 4 years ago

So at least with PX4 a NaN in yaw causes the vehicle to go back to pointing towards next waypoint. In looking at the mavlink spec, it somewhat clear that this is what it should do. So QGC code is incorrect here.

Also the value is labeled as Heading when it should be labeled as Yaw.

hamishwillee commented 4 years ago

My understanding of the spec is that an explicit yaw (heading) in a waypoint lasts until the next waypoint. If a NaN is sent then the behaviour is unspecified by MAVLink - ie the theory is that it devolves to flight-stack specific behaviour. On PX4 the default behaviour is to point to next waypoint, but this can be changed to home or away from home in parameters. I have a PR in place to make this something that can be set using an explicit message: https://github.com/mavlink/mavlink/pull/1297

So at least with PX4 a NaN in yaw causes the vehicle to go back to pointing towards next waypoint. In looking at the mavlink spec, it somewhat clear that this is what it should do. So QGC code is incorrect here.

So to be pedantic, it will point towards whatever the system setting says. I think perhaps we need a message to publish this (rather than forcing you to get a parameter?)

Also the value is labeled as Heading when it should be labeled as Yaw.

Yes.

I don't know what the gimbal setting should do. QGC exports a plan that says this is done with MAV_CMD_DO_MOUNT_CONTROL but there is nothing to say whether the setting should propagate across waypoints, and the way it points depends a bit on mount mode.

Is the general assumption that commands apply to specific waypoint, or that they apply until cancelled? I don't think we have such an assumption, which means we really need to say for every message.

DonLakeFlyer commented 4 years ago

lasts until the next waypoint

I think mentioning that in the spec would be helpful.

Is the general assumption that commands apply to specific waypoint, or that they apply until cancelled

Apply until cancelled.

mavlink / qgroundcontrol