search

mavlink / qgroundcontrol

Cross-platform ground control station for drones (Android, iOS, Mac OS, Linux, Windows)
http://qgroundcontrol.io
3.31k stars 3.63k forks source link

Messed up waypoints on QGC when sending them with second MAVLink source #1747

Closed flyver closed 9 years ago

flyver commented 9 years ago

This bug occurs when the QGC is running and you send new waypoints from another source (second telemetry port). Then the QGC gets confused and mixes the points from the previous mission and the current mission in some invalid mission. Refresh and Get does not fix the issues. The issue is fixed when pixhawk is restarted or QGC is restarted(sometimes). I am not sure the exact cause but it happens all the time. Found that the bug could be prevented when you first clear the current mission in the QGC's interface, then upload the mission through telem 2 and then GET the new waipoints from the QGC. My guess is that the bug happens when there are some waypoints displayed on the interface of the QGC and then mission is uploaded through another interface.

DonLakeFlyer commented 9 years ago

I don't have a lot of experience with the mission protocol but I don't think what you are describing fits that: http://www.qgroundcontrol.org/mavlink/waypoint_protocol. QGC must drive "asking" for waypoints. The vehicle can't just send waypoints at any time.

flyver commented 9 years ago

This is not the problem. The problem is that when you "ask" for the waypoints, they get tangled with the previous waypoints.

dogmaphobic commented 9 years ago

I have not had a chance to look at the code but it seems QGC is appending the waypoints instead of replacing them.

LorenzMeier commented 9 years ago

I think this is due to the internal double-storage (one for transmission, one for for navigation-use) for the autopilot. The MAVLink instances probably need to copy the current storage ID before using it (whereas they currently assume that the one they have is the current one).

LorenzMeier commented 9 years ago

I recommend you make these static: https://github.com/PX4/Firmware/blob/master/src/modules/mavlink/mavlink_mission.h#L110-L119

And additionally introduce a transfer_mavlink_instance pointer or index which is locked using a mutex on transfer start and refuses transfers on other ports while one instance is active. That should bind the two instances onto the same view of the data.

Alternatively you can modify the receiver class to first read from the mission uorb topic before it changes the dataman ID. This is a fringe use case, I'm happy to look at this later but I don't have the bandwidth right now. It would make a great contribution and would be welcome upstream if you need this functionality.