MissionManagerTest protocol hardening tests

- Fixed bugs found to unit test

src/MissionManager/MissionManager.h

@@ -67,12 +67,24 @@ public:
     /// Returns a copy of the current set of mission items. Caller is responsible for
     /// freeing returned object.
     QmlObjectListModel* copyMissionItems(void);
+    
+    /// Error codes returned in error signal
+    typedef enum {
+        InternalError,
+        AckTimeoutError,        ///< Timed out waiting for response from vehicle
+        ProtocolOrderError,     ///< Incorrect protocol sequence from vehicle
+        RequestRangeError,      ///< Vehicle requested item out of range
+        ItemMismatchError,      ///< Vehicle returned item with seq # different than requested
+        VehicleError,           ///< Vehicle returned error
+        MissingRequestsError,   ///< Vehicle did not request all items during write sequence
+    } ErrorCode_t;
 
 signals:
     // Public signals
     void canEditChanged(bool canEdit);
     void newMissionItemsAvailable(void);
     void inProgressChanged(bool inProgress);
+    void error(int errorCode, const QString& errorMsg);
     
 private slots:
     void _mavlinkMessageReceived(const mavlink_message_t& message);
@@ -86,7 +98,7 @@ private:
         AckMissionRequest,  ///< MISSION_REQUEST is expected, or MISSION_ACK to end sequence
     } AckType_t;
     
-    void _startAckTimeout(AckType_t ack, const mavlink_message_t& message);
+    void _startAckTimeout(AckType_t ack);
     bool _stopAckTimeout(AckType_t expectedAck);
     void _sendTransactionComplete(void);
     void _handleMissionCount(const mavlink_message_t& message);
@@ -95,6 +107,10 @@ private:
     void _handleMissionAck(const mavlink_message_t& message);
     void _requestNextMissionItem(int sequenceNumber);
     void _clearMissionItems(void);
+    void _sendError(ErrorCode_t errorCode, const QString& errorMsg);
+    void _retryWrite(void);
+    void _retryRead(void);
+    bool _retrySequence(AckType_t ackType);
 
 private:
     Vehicle*            _vehicle;
@@ -104,7 +120,6 @@ private:
 
     QTimer*             _ackTimeoutTimer;
     AckType_t           _retryAck;
-    mavlink_message_t   _retryMessage;
     int                 _retryCount;
     
     int                 _expectedSequenceNumber;
@@ -113,7 +128,7 @@ private:
     
     QmlObjectListModel  _missionItems;
     
-    static const int _ackTimeoutMilliseconds= 1000;
+    static const int _ackTimeoutMilliseconds= 500;
     static const int _maxRetryCount = 5;
 };
 

src/MissionManager/MissionManagerTest.h

@@ -42,9 +42,11 @@ private slots:
     
     void _readEmptyVehicle(void);
     void _roundTripItems(void);
+    void _testWriteFailureHandling(void);
     
 private:
     void _checkInProgressValues(bool inProgress);
+    void _writeItems(MockLinkMissionItemHandler::FailureMode_t failureMode, MissionManager::ErrorCode_t errorCode, bool failFirstTimeOnly);
 
     MockLink*       _mockLink;
     MissionManager* _missionManager;
@@ -53,6 +55,7 @@ private:
         canEditChangedSignalIndex = 0,
         newMissionItemsAvailableSignalIndex,
         inProgressChangedSignalIndex,
+        errorSignalIndex,
         maxSignalIndex
     };
     
@@ -60,6 +63,7 @@ private:
         canEditChangedSignalMask =              1 << canEditChangedSignalIndex,
         newMissionItemsAvailableSignalMask =    1 << newMissionItemsAvailableSignalIndex,
         inProgressChangedSignalMask =           1 << inProgressChangedSignalIndex,
+        errorSignalMask =                       1 << errorSignalIndex,
     };
 
     MultiSignalSpy*     _multiSpy;

src/Vehicle/Vehicle.cc

@@ -85,6 +85,7 @@ Vehicle::Vehicle(LinkInterface* link, int vehicleId, MAV_AUTOPILOT firmwareType)
     , _satelliteLock(0)
     , _wpm(NULL)
     , _updateCount(0)
+    , _missionManager(NULL)
     , _armed(false)
     , _base_mode(0)
     , _custom_mode(0)
@@ -164,6 +165,9 @@ Vehicle::Vehicle(LinkInterface* link, int vehicleId, MAV_AUTOPILOT firmwareType)
 
 Vehicle::~Vehicle()
 {
+    delete _missionManager;
+    _missionManager = NULL;
+    
     // Stop listening for system messages
     disconnect(UASMessageHandler::instance(), &UASMessageHandler::textMessageCountChanged,  this, &Vehicle::_handleTextMessage);
     // Disconnect any previously connected active MAV

src/comm/MockLink.cc

@@ -66,17 +66,18 @@ union px4_custom_mode {
     float data_float;
 };
 
-MockLink::MockLink(MockConfiguration* config) :
-    _name("MockLink"),
-    _connected(false),
-    _vehicleSystemId(128),     // FIXME: Pull from eventual parameter manager
-    _vehicleComponentId(200),  // FIXME: magic number?
-    _inNSH(false),
-    _mavlinkStarted(false),
-    _mavBaseMode(MAV_MODE_FLAG_MANUAL_INPUT_ENABLED | MAV_MODE_FLAG_CUSTOM_MODE_ENABLED),
-    _mavState(MAV_STATE_STANDBY),
-    _autopilotType(MAV_AUTOPILOT_PX4),
-    _fileServer(NULL)
+MockLink::MockLink(MockConfiguration* config)
+    : _missionItemHandler(this)
+    , _name("MockLink")
+    , _connected(false)
+    , _vehicleSystemId(128)     // FIXME: Pull from eventual parameter manager
+    , _vehicleComponentId(200)  // FIXME: magic number?
+    , _inNSH(false)
+    , _mavlinkStarted(false)
+    , _mavBaseMode(MAV_MODE_FLAG_MANUAL_INPUT_ENABLED | MAV_MODE_FLAG_CUSTOM_MODE_ENABLED)
+    , _mavState(MAV_STATE_STANDBY)
+    , _autopilotType(MAV_AUTOPILOT_PX4)
+    , _fileServer(NULL)
 {
     _config = config;
     union px4_custom_mode   px4_cm;
@@ -88,10 +89,8 @@ MockLink::MockLink(MockConfiguration* config) :
     _fileServer = new MockLinkFileServer(_vehicleSystemId, _vehicleComponentId, this);
     Q_CHECK_PTR(_fileServer);
     
-    _missionItemHandler = new MockLinkMissionItemHandler(this);
-    Q_CHECK_PTR(_missionItemHandler);
-
     moveToThread(this);
+    
     _loadParams();
     QObject::connect(this, &MockLink::_incomingBytes, this, &MockLink::_handleIncomingBytes);
 }
@@ -149,6 +148,8 @@ void MockLink::run(void)
     QObject::disconnect(&_timer1HzTasks, &QTimer::timeout, this, &MockLink::_run1HzTasks);
     QObject::disconnect(&_timer10HzTasks, &QTimer::timeout, this, &MockLink::_run10HzTasks);
     QObject::disconnect(&_timer50HzTasks, &QTimer::timeout, this, &MockLink::_run50HzTasks);
+    
+    _missionItemHandler.shutdown();
 }
 
 void MockLink::_run1HzTasks(void)
@@ -303,8 +304,7 @@ void MockLink::_handleIncomingMavlinkBytes(const uint8_t* bytes, int cBytes)
             continue;
         }
         
-        Q_ASSERT(_missionItemHandler);
-        if (_missionItemHandler->handleMessage(msg)) {
+        if (_missionItemHandler.handleMessage(msg)) {
             continue;
         }
 
@@ -668,3 +668,7 @@ void MockLink::_handleCommandLong(const mavlink_message_t& msg)
     }
 }
 
+void MockLink::setMissionItemFailureMode(MockLinkMissionItemHandler::FailureMode_t failureMode, bool firstTimeOnly)
+{
+    _missionItemHandler.setMissionItemFailureMode(failureMode, firstTimeOnly);
+}

src/comm/MockLink.h

@@ -85,6 +85,23 @@ public:
     bool disconnect(void);
 
     LinkConfiguration* getLinkConfiguration() { return _config; }
+    
+    /// Sets a failure mode for unit testing
+    ///     @param failureMode Type of failure to simulate
+    ///     @param firstTimeOnly true: fail first call, success subsequent calls, false: fail all calls
+    void setMissionItemFailureMode(MockLinkMissionItemHandler::FailureMode_t failureMode, bool firstTimeOnly);
+    
+    /// Called to send a MISSION_ACK message while the MissionManager is in idle state
+    void sendUnexpectedMissionAck(MAV_MISSION_RESULT ackType) { _missionItemHandler.sendUnexpectedMissionAck(ackType); }
+    
+    /// Called to send a MISSION_ITEM message while the MissionManager is in idle state
+    void sendUnexpectedMissionItem(void) { _missionItemHandler.sendUnexpectedMissionItem(); }
+    
+    /// Called to send a MISSION_REQUEST message while the MissionManager is in idle state
+    void sendUnexpectedMissionRequest(void) { _missionItemHandler.sendUnexpectedMissionRequest(); }
+    
+    /// Reset the state of the MissionItemHandler to no items, no transactions in progress.
+    void resetMissionItemHandler(void) { _missionItemHandler.reset(); }
 
 signals:
     /// @brief Used internally to move data to the thread.
@@ -126,7 +143,7 @@ private:
     float _floatUnionForParam(int componentId, const QString& paramName);
     void _setParamFloatUnionIntoMap(int componentId, const QString& paramName, float paramFloat);
 
-    MockLinkMissionItemHandler* _missionItemHandler;
+    MockLinkMissionItemHandler  _missionItemHandler;
 
     QString _name;
     bool    _connected;

src/comm/MockLinkMissionItemHandler.h

@@ -26,6 +26,7 @@
 
 #include <QObject>
 #include <QMap>
+#include <QTimer>
 
 #include "QGCMAVLink.h"
 #include "QGCLoggingCategory.h"
@@ -40,11 +41,55 @@ class MockLinkMissionItemHandler : public QObject
 
 public:
     MockLinkMissionItemHandler(MockLink* mockLink);
+    ~MockLinkMissionItemHandler();
+    
+    // Prepares for destruction on correct thread
+    void shutdown(void);
 
     /// @brief Called to handle mission item related messages. All messages should be passed to this method.
     ///         It will handle the appropriate set.
     /// @return true: message handled
     bool handleMessage(const mavlink_message_t& msg);
+    
+    typedef enum {
+        FailNone,                           // No failures
+        FailReadRequestListNoResponse,      // Don't send MISSION_COUNT in response to MISSION_REQUEST_LIST
+        FailReadRequest0NoResponse,         // Don't send MISSION_ITEM in response to MISSION_REQUEST item 0
+        FailReadRequest1NoResponse,         // Don't send MISSION_ITEM in response to MISSION_REQUEST item 1
+        FailReadRequest0IncorrectSequence,  // Respond to MISSION_REQUEST 0 with incorrect sequence number in  MISSION_ITEM
+        FailReadRequest1IncorrectSequence,  // Respond to MISSION_REQUEST 1 with incorrect sequence number in  MISSION_ITEM
+        FailReadRequest0ErrorAck,           // Respond to MISSION_REQUEST 0 with MISSION_ACK error
+        FailReadRequest1ErrorAck,           // Respond to MISSION_REQUEST 1 bogus MISSION_ACK error
+        FailWriteRequest0NoResponse,        // Don't respond to MISSION_COUNT with MISSION_REQUEST 0
+        FailWriteRequest1NoResponse,        // Don't respond to MISSION_ITEM 0 with MISSION_REQUEST 1
+        FailWriteRequest0IncorrectSequence, // Respond to MISSION_COUNT 0 with MISSION_REQUEST with wrong sequence number
+        FailWriteRequest1IncorrectSequence, // Respond to MISSION_ITEM 0 with MISSION_REQUEST with wrong sequence number
+        FailWriteRequest0ErrorAck,          // Respond to MISSION_COUNT 0 with MISSION_ACK error
+        FailWriteRequest1ErrorAck,          // Respond to MISSION_ITEM 0 with MISSION_ACK error
+        FailWriteFinalAckNoResponse,        // Don't send the final MISSION_ACK
+        FailWriteFinalAckErrorAck,          // Send an error as the final MISSION_ACK
+        FailWriteFinalAckMissingRequests,   // Send the MISSION_ACK before all items have been requested
+    } FailureMode_t;
+
+    /// Sets a failure mode for unit testing
+    ///     @param failureMode Type of failure to simulate
+    ///     @param firstTimeOnly true: fail first call, success subsequent calls, false: fail all calls
+    void setMissionItemFailureMode(FailureMode_t failureMode, bool firstTimeOnly);
+    
+    /// Called to send a MISSION_ACK message while the MissionManager is in idle state
+    void sendUnexpectedMissionAck(MAV_MISSION_RESULT ackType);
+    
+    /// Called to send a MISSION_ITEM message while the MissionManager is in idle state
+    void sendUnexpectedMissionItem(void);
+    
+    /// Called to send a MISSION_REQUEST message while the MissionManager is in idle state
+    void sendUnexpectedMissionRequest(void);
+    
+    /// Reset the state of the MissionItemHandler to no items, no transactions in progress.
+    void reset(void) { _missionItems.clear(); }
+
+private slots:
+    void _missionItemResponseTimeout(void);
 
 private:
     void _handleMissionRequestList(const mavlink_message_t& msg);
@@ -52,6 +97,8 @@ private:
     void _handleMissionItem(const mavlink_message_t& msg);
     void _handleMissionCount(const mavlink_message_t& msg);
     void _requestNextMissionItem(int sequenceNumber);
+    void _sendAck(MAV_MISSION_RESULT ackType);
+    void _startMissionItemResponseTimer(void);
 
 private:
     MockLink* _mockLink;
@@ -61,6 +108,10 @@ private:
     
     typedef QMap<uint16_t, mavlink_mission_item_t>   MissionList_t;
     MissionList_t   _missionItems;
+    
+    QTimer* _missionItemResponseTimer;
+    FailureMode_t   _failureMode;
+    bool            _failureFirstTimeOnly;
 };
 
 #endif