FileManagerTest: add random drops to test retry behavior

src/comm/MockLinkFileServer.cc

@@ -36,9 +36,22 @@ MockLinkFileServer::MockLinkFileServer(uint8_t systemIdServer, uint8_t component
     _errMode(errModeNone),
     _systemIdServer(systemIdServer),
     _componentIdServer(componentIdServer),
-    _mockLink(mockLink)
+    _mockLink(mockLink),
+    _lastReplyValid(false),
+    _lastReplySequence(0),
+    _randomDropsEnabled(false)
 {
+    srand(0); // make sure unit tests are deterministic
+}
+
+void MockLinkFileServer::ensureNullTemination(FileManager::Request* request)
+{
+    if (request->hdr.size < sizeof(request->data)) {
+        request->data[request->hdr.size] = '\0';
 
+    } else {
+        request->data[sizeof(request->data)-1] = '\0';
+    }
 }
 
 /// @brief Handles List command requests. Only supports root folder paths.
@@ -51,6 +64,8 @@ void MockLinkFileServer::_listCommand(uint8_t senderSystemId, uint8_t senderComp
     QString                     path;
     uint16_t                    outgoingSeqNumber = _nextSeqNumber(seqNumber);
 
+    ensureNullTemination(request);
+
     // We only support root path
     path = (char *)&request->data[0];
     if (!path.isEmpty() && path != "/") {
@@ -103,6 +118,8 @@ void MockLinkFileServer::_openCommand(uint8_t senderSystemId, uint8_t senderComp
     QString                     path;
     uint16_t                    outgoingSeqNumber = _nextSeqNumber(seqNumber);
     
+    ensureNullTemination(request);
+
     size_t cchPath = strnlen((char *)request->data, sizeof(request->data));
     Q_ASSERT(cchPath != sizeof(request->data));
     Q_UNUSED(cchPath); // Fix initialized-but-not-referenced warning on release builds
@@ -270,9 +287,24 @@ void MockLinkFileServer::handleFTPMessage(const mavlink_message_t& message)
     if (requestFTP.target_system != _systemIdServer) {
         return;
     }
-    
+
     FileManager::Request* request = (FileManager::Request*)&requestFTP.payload[0];
 
+	if (_randomDropsEnabled) {
+	    if (rand() % 3 == 0) {
+	        qDebug() << "FileServer: Random drop of incoming packet";
+	        return;
+	    }
+	}
+
+	if (_lastReplyValid && request->hdr.seqNumber + 1 == _lastReplySequence) {
+	    // this is the same request as the one we replied to last. It means the (n)ack got lost, and the GCS
+	    // resent the request
+	    qDebug() << "FileServer: resending response";
+	    _mockLink->respondWithMavlinkMessage(_lastReply);
+	    return;
+	}
+
     uint16_t incomingSeqNumber = request->hdr.seqNumber;
     uint16_t outgoingSeqNumber = _nextSeqNumber(incomingSeqNumber);
     
@@ -361,20 +393,27 @@ void MockLinkFileServer::_sendNak(uint8_t targetSystemId, uint8_t targetComponen
 /// @brief Emits a Request through the messageReceived signal.
 void MockLinkFileServer::_sendResponse(uint8_t targetSystemId, uint8_t targetComponentId, FileManager::Request* request, uint16_t seqNumber)
 {
-    mavlink_message_t   mavlinkMessage;
-    
     request->hdr.seqNumber = seqNumber;
+    _lastReplySequence = seqNumber;
+    _lastReplyValid = true;
     
     mavlink_msg_file_transfer_protocol_pack_chan(_systemIdServer,    // System ID
                                                  0,                  // Component ID
                                                  _mockLink->mavlinkChannel(),
-                                                 &mavlinkMessage,    // Mavlink Message to pack into
+                                                 &_lastReply,    // Mavlink Message to pack into
                                                  0,                  // Target network
                                                  targetSystemId,
                                                  targetComponentId,
                                                  (uint8_t*)request); // Payload
+
+	if (_randomDropsEnabled) {
+	    if (rand() % 3 == 0) {
+	        qDebug() << "FileServer: Random drop of outgoing packet";
+	        return;
+	    }
+	}
     
-    _mockLink->respondWithMavlinkMessage(mavlinkMessage);
+    _mockLink->respondWithMavlinkMessage(_lastReply);
 }
 
 /// @brief Generates the next sequence number given an incoming sequence number. Handles generating

src/comm/MockLinkFileServer.h

@@ -70,6 +70,8 @@ public:
     /// @brief The set of files supported by the mock server for testing purposes. Each one represents a different edge case for testing.
     static const FileTestCase rgFileTestCases[cFileTestCases];
     
+    void enableRandromDrops(bool enable) { _randomDropsEnabled = enable; }
+
 signals:
     /// You can connect to this signal to be notified when the server receives a Terminate command.
     void terminateCommandReceived(void);
@@ -89,6 +91,9 @@ private:
     void _resetCommand(uint8_t senderSystemId, uint8_t senderComponentId, uint16_t seqNumber);
     uint16_t _nextSeqNumber(uint16_t seqNumber);
     
+    /// if request is a string, this ensures it's null-terminated
+    static void ensureNullTemination(FileManager::Request* request);
+
     QStringList _fileList;  ///< List of files returned by List command
     
     static const uint8_t    _sessionId;
@@ -97,6 +102,12 @@ private:
     const uint8_t           _systemIdServer;    ///< System ID for server
     const uint8_t           _componentIdServer; ///< Component ID for server
     MockLink*               _mockLink;          ///< MockLink to communicate through
+
+    bool _lastReplyValid;
+    uint16_t _lastReplySequence;
+    mavlink_message_t _lastReply;
+
+    bool _randomDropsEnabled;
 };
 
 #endif

src/qgcunittest/FileManagerTest.cc

@@ -127,6 +127,9 @@ void FileManagerTest::_listTest(void)
     Q_ASSERT(_fileManager);
     Q_ASSERT(_multiSpy);
     Q_ASSERT(_multiSpy->checkNoSignals() == true);
+
+    // test the automatic retry behavior by enabling random drops
+    _fileServer->enableRandromDrops(true);
     
     // FileManager::listDirectory signalling as follows:
     //  Emits a listEntry signal for each list entry
@@ -192,6 +195,8 @@ void FileManagerTest::_listTest(void)
         _multiSpy->clearAllSignals();
         _fileServer->setErrorMode(MockLinkFileServer::errModeNone);
     }
+
+    _fileServer->enableRandromDrops(false);
 }
 
 #if 0