Race condition when updating and fetching commands

[hamishwillee]

Consider the code below (gist at https://gist.github.com/hamishwillee/9607b3707fb942d59abf#file-gistfile1-txt-L36)

"""
Tests race condition when downloading items
"""

import time
import math
from droneapi.lib import VehicleMode, Location, Command
from pymavlink import mavutil


api = local_connect()
vehicle = api.get_vehicles()[0]


def currentmission():
    """
    Print the current mission
    """
    print "current mission:"
    mycmds = vehicle.commands
    mycmds.download()
    mycmds.wait_valid()
    for cmd in mycmds:
        print cmd


currentmission()

print 'Add dummy waypoint'
vehicle.commands.add(Command( 0, 0, 0, mavutil.mavlink.MAV_FRAME_GLOBAL_RELATIVE_ALT, 
                mavutil.mavlink.MAV_CMD_NAV_WAYPOINT, 0, 0, 
                0, 0, 0, 0, 10, 10, 10))
vehicle.flush() # Send commands

print "Wait 2s for update"
time.sleep(2)

print 'Show mission'
currentmission()

The code works as is, but if the time.sleep(2) is removed then all waypoints are wiped (except the home waypoint).

This shouldn't happen because in theory the flush means that the new waypoint is sent, and this should occur before flush returns - ie before we have a chance to do the new download of the current commands.

I think either we're not doing this properly and the vehicle.command is being wiped by the second call before the flush returns OR the autopilot is confused by our messaging.

@tcr3dr Thoughts?

[hamishwillee]

I THINK this is fixed by #384.

[tcr3dr]

We could stand to have a test for this, heh.

[hamishwillee]

Well, my test worked against dronekit-sitl copter 3.3. But I suspect that proves nothing. Yes, a proper test is a very good idea.

[tcr3dr]

@hamishwillee Check that #439 is a good reduction of the test case?

[hamishwillee]

@tcr3dr Yes, the #430 verifies that the mission is flushed before the subsequent download occurs (and can swamp it)

[tcr3dr]

#439 merged, so this issue should not suffer from a regression.