ftp: split list_directory response

This way we get a clear list of files and folders instead of an odd list with F or D prefixes.
mavlink · Jun 4, 2024 · 6deaae8 · 6deaae8
1 parent c56b547
commit 6deaae8
Show file tree

Hide file tree

Showing 10 changed files with 1,065 additions and 291 deletions.
diff --git a/proto b/proto
diff --git a/src/mavsdk/core/mavlink_ftp_client.cpp b/src/mavsdk/core/mavlink_ftp_client.cpp
@@ -360,9 +360,10 @@ void MavlinkFtpClient::process_mavlink_ftp_message(const mavlink_message_t& msg)
                     stop_timer();
                     if (payload->data[0] == ERR_EOF) {
                         std::sort(item.dirs.begin(), item.dirs.end());
-                        item.callback(ClientResult::Success, item.dirs);
+                        std::sort(item.files.begin(), item.files.end());
+                        item.callback(ClientResult::Success, item.dirs, item.files);
                     } else {
-                        item.callback(result_from_nak(payload), {});
+                        item.callback(result_from_nak(payload), {}, {});
                     }
                     terminate_session(*work);
                     work_queue_guard.pop_front();
@@ -956,7 +957,7 @@ bool MavlinkFtpClient::compare_files_start(Work& work, CompareFilesItem& item)
 bool MavlinkFtpClient::list_dir_start(Work& work, ListDirItem& item)
 {
     if (item.path.length() + 1 >= max_data_length) {
-        item.callback(ClientResult::InvalidParameter, {});
+        item.callback(ClientResult::InvalidParameter, {}, {});
         return false;
     }
 
@@ -988,7 +989,8 @@ bool MavlinkFtpClient::list_dir_continue(Work& work, ListDirItem& item, PayloadH
 
     if (payload->size == 0) {
         std::sort(item.dirs.begin(), item.dirs.end());
-        item.callback(ClientResult::Success, item.dirs);
+        std::sort(item.files.begin(), item.files.end());
+        item.callback(ClientResult::Success, item.dirs, item.files);
         return false;
     }
 
@@ -1012,7 +1014,13 @@ bool MavlinkFtpClient::list_dir_continue(Work& work, ListDirItem& item, PayloadH
             continue;
         }
 
-        item.dirs.push_back(entry);
+        if (entry[1] == 'D') {
+            item.dirs.push_back(entry.substr(1, entry.size() - 1));
+        } else if (entry[1] == 'F') {
+            item.files.push_back(entry.substr(1, entry.size() - 1));
+        } else {
+            LogErr() << "Unknown list_dir entry: " << entry;
+        }
     }
 
     work.last_opcode = CMD_LIST_DIRECTORY;
@@ -1350,7 +1358,7 @@ void MavlinkFtpClient::timeout()
             },
             [&](ListDirItem& item) {
                 if (--work->retries == 0) {
-                    item.callback(ClientResult::Timeout, {});
+                    item.callback(ClientResult::Timeout, {}, {});
                     work_queue_guard.pop_front();
                     return;
                 }

diff --git a/src/mavsdk/core/mavlink_ftp_client.h b/src/mavsdk/core/mavlink_ftp_client.h
@@ -62,7 +62,8 @@ class MavlinkFtpClient {
     using ResultCallback = std::function<void(ClientResult)>;
     using UploadCallback = std::function<void(ClientResult, ProgressData)>;
     using DownloadCallback = std::function<void(ClientResult, ProgressData)>;
-    using ListDirectoryCallback = std::function<void(ClientResult, std::vector<std::string>)>;
+    using ListDirectoryCallback =
+        std::function<void(ClientResult, std::vector<std::string>, std::vector<std::string>)>;
     using AreFilesIdenticalCallback = std::function<void(ClientResult, bool)>;
 
     void do_work();
@@ -206,6 +207,7 @@ class MavlinkFtpClient {
         ListDirectoryCallback callback{};
         uint32_t offset{0};
         std::vector<std::string> dirs{};
+        std::vector<std::string> files{};
     };
 
     using Item = std::variant<

diff --git a/src/mavsdk/plugins/ftp/ftp.cpp b/src/mavsdk/plugins/ftp/ftp.cpp
@@ -9,6 +9,7 @@
 
 namespace mavsdk {
 
+using ListDirectoryData = Ftp::ListDirectoryData;
 using ProgressData = Ftp::ProgressData;
 
 Ftp::Ftp(System& system) : PluginBase(), _impl{std::make_unique<FtpImpl>(system)} {}
@@ -37,7 +38,7 @@ void Ftp::list_directory_async(std::string remote_dir, const ListDirectoryCallba
     _impl->list_directory_async(remote_dir, callback);
 }
 
-std::pair<Ftp::Result, std::vector<std::string>> Ftp::list_directory(std::string remote_dir) const
+std::pair<Ftp::Result, Ftp::ListDirectoryData> Ftp::list_directory(std::string remote_dir) const
 {
     return _impl->list_directory(remote_dir);
 }
@@ -102,6 +103,29 @@ Ftp::Result Ftp::set_target_compid(uint32_t compid) const
     return _impl->set_target_compid(compid);
 }
 
+bool operator==(const Ftp::ListDirectoryData& lhs, const Ftp::ListDirectoryData& rhs)
+{
+    return (rhs.paths == lhs.paths) && (rhs.files == lhs.files);
+}
+
+std::ostream& operator<<(std::ostream& str, Ftp::ListDirectoryData const& list_directory_data)
+{
+    str << std::setprecision(15);
+    str << "list_directory_data:" << '\n' << "{\n";
+    str << "    paths: [";
+    for (auto it = list_directory_data.paths.begin(); it != list_directory_data.paths.end(); ++it) {
+        str << *it;
+        str << (it + 1 != list_directory_data.paths.end() ? ", " : "]\n");
+    }
+    str << "    files: [";
+    for (auto it = list_directory_data.files.begin(); it != list_directory_data.files.end(); ++it) {
+        str << *it;
+        str << (it + 1 != list_directory_data.files.end() ? ", " : "]\n");
+    }
+    str << '}';
+    return str;
+}
+
 bool operator==(const Ftp::ProgressData& lhs, const Ftp::ProgressData& rhs)
 {
     return (rhs.bytes_transferred == lhs.bytes_transferred) && (rhs.total_bytes == lhs.total_bytes);

diff --git a/src/mavsdk/plugins/ftp/ftp_impl.cpp b/src/mavsdk/plugins/ftp/ftp_impl.cpp
@@ -74,24 +74,24 @@ void FtpImpl::upload_async(
         });
 }
 
-std::pair<Ftp::Result, std::vector<std::string>> FtpImpl::list_directory(const std::string& path)
+std::pair<Ftp::Result, Ftp::ListDirectoryData> FtpImpl::list_directory(const std::string& path)
 {
-    std::promise<std::pair<Ftp::Result, std::vector<std::string>>> prom;
+    std::promise<std::pair<Ftp::Result, Ftp::ListDirectoryData>> prom;
     auto fut = prom.get_future();
 
-    list_directory_async(path, [&](Ftp::Result result, std::vector<std::string> list) {
-        prom.set_value(std::pair<Ftp::Result, std::vector<std::string>>{result, list});
+    list_directory_async(path, [&](Ftp::Result result, auto& data) {
+        prom.set_value(std::pair<Ftp::Result, Ftp::ListDirectoryData>{result, data});
     });
     return fut.get();
 }
 
 void FtpImpl::list_directory_async(const std::string& path, Ftp::ListDirectoryCallback callback)
 {
     _system_impl->mavlink_ftp_client().list_directory_async(
-        path, [callback, this](MavlinkFtpClient::ClientResult result, auto&& dirs) {
+        path, [callback, this](MavlinkFtpClient::ClientResult result, auto& dirs, auto& files) {
             if (callback) {
-                _system_impl->call_user_callback([temp_callback = callback, result, dirs, this]() {
-                    temp_callback(result_from_mavlink_ftp_result(result), dirs);
+                _system_impl->call_user_callback([=]() {
+                    callback(result_from_mavlink_ftp_result(result), {dirs, files});
                 });
             }
         });

diff --git a/src/mavsdk/plugins/ftp/ftp_impl.h b/src/mavsdk/plugins/ftp/ftp_impl.h
@@ -25,7 +25,7 @@ class FtpImpl : public PluginImplBase {
     void enable() override;
     void disable() override;
 
-    std::pair<Ftp::Result, std::vector<std::string>> list_directory(const std::string& path);
+    std::pair<Ftp::Result, Ftp::ListDirectoryData> list_directory(const std::string& path);
     Ftp::Result create_directory(const std::string& path);
     Ftp::Result remove_directory(const std::string& path);
     Ftp::Result remove_file(const std::string& path);

diff --git a/src/mavsdk/plugins/ftp/include/plugins/ftp/ftp.h b/src/mavsdk/plugins/ftp/include/plugins/ftp/ftp.h
@@ -58,6 +58,29 @@ class Ftp : public PluginBase {
      */
     ~Ftp() override;
 
+    /**
+     * @brief
+     */
+    struct ListDirectoryData {
+        std::vector<std::string> paths{}; /**< @brief The found directories. */
+        std::vector<std::string> files{}; /**< @brief The found files. */
+    };
+
+    /**
+     * @brief Equal operator to compare two `Ftp::ListDirectoryData` objects.
+     *
+     * @return `true` if items are equal.
+     */
+    friend bool operator==(const Ftp::ListDirectoryData& lhs, const Ftp::ListDirectoryData& rhs);
+
+    /**
+     * @brief Stream operator to print information about a `Ftp::ListDirectoryData`.
+     *
+     * @return A reference to the stream.
+     */
+    friend std::ostream&
+    operator<<(std::ostream& str, Ftp::ListDirectoryData const& list_directory_data);
+
     /**
      * @brief Progress data type for file transfer.
      */
@@ -139,7 +162,7 @@ class Ftp : public PluginBase {
     /**
      * @brief Callback type for list_directory_async.
      */
-    using ListDirectoryCallback = std::function<void(Result, std::vector<std::string>)>;
+    using ListDirectoryCallback = std::function<void(Result, ListDirectoryData)>;
 
     /**
      * @brief Lists items from a remote directory.
@@ -155,7 +178,7 @@ class Ftp : public PluginBase {
      *
      * @return Result of request.
      */
-    std::pair<Result, std::vector<std::string>> list_directory(std::string remote_dir) const;
+    std::pair<Result, Ftp::ListDirectoryData> list_directory(std::string remote_dir) const;
 
     /**
      * @brief Creates a remote directory.