removed firmware common, made GetInfo part of the firmware checke class fixed firmware path so that code compiles

libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp

@@ -23,8 +23,7 @@
 #include <uavcan/protocol/file/EntryType.hpp>
 #include <uavcan/protocol/file/Read.hpp>
 #include <uavcan/protocol/file_server.hpp>
-
+#include <uavcan/data_type.hpp>
-#include "firmware_common.hpp"
 
 namespace uavcan_posix
 {
@@ -33,29 +32,74 @@ namespace uavcan_posix
  */
 class BasicFileSeverBackend : public uavcan::IFileServerBackend
 {
+
     enum { FilePermissions = 438 };   ///< 0o666
 
-protected:
+public:
     /**
+     *
      * Back-end for uavcan.protocol.file.GetInfo.
      * Implementation of this method is required.
      * On success the method must return zero.
      */
+
     virtual int16_t getInfo(const Path& path, uint64_t& out_crc64, uint32_t& out_size, EntryType& out_type)
     {
+
         int rv = uavcan::protocol::file::Error::INVALID_VALUE;
+        uavcan::DataTypeSignatureCRC crc;
 
         if (path.size() > 0)
         {
-            FirmwareCommon fw;
+            using namespace std;
-            fw.getFileInfo(path.c_str());
+            out_size = 0;
-            out_crc64 = fw.descriptor.image_crc;
+            out_crc64 = 0;
-            out_size = fw.descriptor.image_size;
+
-            // TODO Using fixed flag FLAG_READABLE until we add file permission checks to return actual value.
+            rv = -ENOENT;
-            // TODO Check whether the object pointed by path is a file or a directory
+            uint8_t buffer[512];
-            out_type.flags = uavcan::protocol::file::EntryType::FLAG_FILE |
+
-                             uavcan::protocol::file::EntryType::FLAG_READABLE;
+            int fd = ::open(path.c_str(), O_RDONLY);
-            rv = 0;
+
+            if (fd >= 0)
+            {
+                int len = 0;
+
+                do
+                {
+
+                    len = ::read(fd, buffer, sizeof(buffer));
+
+                    if (len > 0)
+                    {
+
+                        out_size += len;
+                        crc.add(buffer, len);
+
+                    }
+                    else if (len < 0)
+                    {
+                        rv = EIO;
+                        goto out_close;
+                    }
+
+                }
+                while(len > 0);
+
+                out_crc64 = crc.get();
+
+                // We can assume the path is to a file and the file is readable.
+                out_type.flags = uavcan::protocol::file::EntryType::FLAG_READABLE |
+                                 uavcan::protocol::file::EntryType::FLAG_FILE;
+
+                // TODO Using fixed flag FLAG_READABLE until we add file permission checks to return actual value.
+                // TODO Check whether the object pointed by path is a file or a directory
+                // On could ad call to stat() to determine if the path is to a file or a directory but the
+                // what are the return parameters in this case?
+
+                rv = 0;
+            out_close:
+                close(fd);
+            }
         }
         return rv;
     }
@@ -67,12 +111,16 @@ protected:
      * if the end of file is reached.
      * On success the method must return zero.
      */
+
     virtual int16_t read(const Path& path, const uint32_t offset, uint8_t* out_buffer, uint16_t& inout_size)
     {
+
         int rv = uavcan::protocol::file::Error::INVALID_VALUE;
 
         if (path.size() > 0)
         {
+
+
             int fd = open(path.c_str(), O_RDONLY);
 
             if (fd < 0)
@@ -81,30 +129,38 @@ protected:
             }
             else
             {
+
                 if (::lseek(fd, offset, SEEK_SET) < 0)
                 {
                     rv = errno;
                 }
                 else
                 {
-                    // TODO use a read at offset to fill on EAGAIN
+                    //todo uses a read at offset to fill on EAGAIN
-                    ssize_t len = ::read(fd, out_buffer, inout_size);
+                    ssize_t len  = ::read(fd, out_buffer, inout_size);
 
-                    if (len < 0)
+                    if (len <  0)
                     {
                         rv = errno;
                     }
                     else
                     {
-                        inout_size = len;
+
+                        inout_size =  len;
                         rv = 0;
                     }
                 }
-                (void)close(fd);
+                close(fd);
             }
         }
         return rv;
     }
+
+    BasicFileSeverBackend() { }
+
+
+
+
 };
 
 }

libuavcan_drivers/posix/include/uavcan_posix/firmware_common.hpp

@@ -1,107 +0,0 @@
-/****************************************************************************
-*
-*   Copyright (c) 2015 PX4 Development Team. All rights reserved.
-*      Author: Pavel Kirienko <pavel.kirienko@gmail.com>
-*              David Sidrane <david_s5@usa.net>
-*
-****************************************************************************/
-
-#ifndef UAVCAN_POSIX_FIRMWARE_COMMON_HPP_INCLUDED
-#define UAVCAN_POSIX_FIRMWARE_COMMON_HPP_INCLUDED
-
-#include <cstdint>
-#include <cstdio>
-#include <cstring>
-#include <fcntl.h>
-#include <cerrno>
-
-#include <uavcan/protocol/file/Path.hpp>
-#include "firmware_path.hpp"
-
-namespace uavcan_posix
-{
-/**
- * Firmware file validation logic.
- * TODO Rename - FirmwareCommon is a bad name as it doesn't reflect the purpose of this class.
- * TODO Returning value via member variable is not a proper way of doing it. Probably the whole class should
- *      be replaced with a static function.
- */
-class FirmwareCommon
-{
-    static uavcan::uint64_t getAppDescriptorSignature()
-    {
-        uavcan::uint64_t ull = 0;
-        std::memcpy(&ull, "APDesc00", 8);
-        return ull;
-    }
-
-public:
-    struct AppDescriptor
-    {
-        uint8_t signature[sizeof(uavcan::uint64_t)];
-        uint64_t image_crc;
-        uint32_t image_size;
-        uint32_t vcs_commit;
-        uint8_t major_version;
-        uint8_t minor_version;
-        uint8_t reserved[6];
-    };
-
-    AppDescriptor descriptor;
-
-    int getFileInfo(const char* path)
-    {
-        using namespace std;
-
-        const unsigned MaxChunk = 512 / sizeof(uint64_t);
-
-        const uint64_t signature = getAppDescriptorSignature();
-
-        int rv = -ENOENT;
-        uint64_t chunk[MaxChunk];
-        int fd = open(path, O_RDONLY);
-
-        if (fd >= 0)
-        {
-            AppDescriptor* pdescriptor = NULL;
-
-            while (pdescriptor == NULL)
-            {
-                int len = read(fd, chunk, sizeof(chunk));
-
-                if (len == 0)
-                {
-                    break;
-                }
-
-                if (len < 0)
-                {
-                    rv = -errno;
-                    goto out_close;
-                }
-
-                uint64_t* p = &chunk[0];
-
-                do
-                {
-                    if (*p == signature)
-                    {
-                        pdescriptor = (AppDescriptor*) p;
-                        descriptor = *pdescriptor;
-                        rv = 0;
-                        break;
-                    }
-                }
-                while (p++ <= &chunk[MaxChunk - (sizeof(AppDescriptor) / sizeof(chunk[0]))]);
-            }
-
-        out_close:
-            (void)close(fd);
-        }
-        return rv;
-    }
-};
-
-}
-
-#endif // Include guard

libuavcan_drivers/posix/include/uavcan_posix/firmware_path.hpp

@@ -17,11 +17,14 @@ namespace uavcan_posix
 {
 /**
  * Firmware Path Management.
- * FIXME Seems like the only purpose of this class is to initialize some directory. It probably should be replaced to
+ * This class manages the sole copies of the cache path and the base path of the fw
- *       a member function inside the firmware version checker class.
+ * This prevent having to have large stack allocations to manipulate the 2 paths.
+ * It ensures that the paths have the proper slash as endings for concatenation.
+ * It also provides the creation of the folders  and encapsulates the paths to
  */
 class FirmwarePath
 {
+public:
     enum { MaxBasePathLength = 128 };
 
     /**
@@ -34,24 +37,28 @@ class FirmwarePath
      */
     enum { MaxPathLength = uavcan::protocol::file::Path::FieldTypes::path::MaxSize + MaxBasePathLength };
 
-    BasePathString base_path_;
+    static char getPathSeparator()
-    BasePathString cache_path_;
+    {
+        return static_cast<char>(uavcan::protocol::file::Path::SEPARATOR);
+    }
 
     /**
      * This type is used internally for the full path to file
      */
     typedef uavcan::MakeString<MaxPathLength>::Type PathString;
 
+
+
+private:
+
+    BasePathString base_path_;
+    BasePathString cache_path_;
+
     /**
      * The folder where the files will be copied and read from
      */
     static const char* getCacheDir() { return "c"; }
 
-    static char getPathSeparator()
-    {
-        return static_cast<char>(uavcan::protocol::file::Path::SEPARATOR);
-    }
-
     static void addSlash(BasePathString& path)
     {
         if (path.back() != getPathSeparator())
@@ -68,6 +75,21 @@ class FirmwarePath
         }
     }
 
+    void setFirmwareBasePath(const char* path)
+    {
+        base_path_ = path;
+    }
+
+    void setFirmwareCachePath(const char* path)
+    {
+        cache_path_ = path;
+    }
+
+public:
+    const BasePathString& getFirmwareBasePath() const { return base_path_; }
+
+    const BasePathString& getFirmwareCachePath() const { return cache_path_; }
+
     /**
      * Creates the Directories were the files will be stored
      *
@@ -90,7 +112,7 @@ class FirmwarePath
             if (len > 0 && len < base_path_.MaxSize)
             {
                 setFirmwareBasePath(base_path);
-                removeSlash(getFirmwareBasePath());
+                removeSlash(base_path_);
                 const char* path = getFirmwareBasePath().c_str();
 
                 setFirmwareCachePath(path);
@@ -111,8 +133,8 @@ class FirmwarePath
                     rv = mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO);
                 }
 
-                addSlash(getFirmwareBasePath());
+                addSlash(base_path_);
-                addSlash(getFirmwareCachePath());
+                addSlash(cache_path_);
 
                 if (rv >= 0)
                 {
@@ -126,27 +148,6 @@ class FirmwarePath
         }
         return rv;
     }
-
-    void setFirmwareBasePath(const char* path)
-    {
-        base_path_ = path;
-    }
-
-    void setFirmwareCachePath(const char* path)
-    {
-        cache_path_ = path;
-    }
-
-public:
-    const BasePathString& getFirmwareBasePath() const { return base_path_; }
-
-    const BasePathString& getFirmwareCachePath() const { return cache_path_; }
-
-    /// TODO Rename. Init is a bad name, as it not initializes the object, but modifies the FS.
-    int init(const char* path)
-    {
-        return createFwPaths(path);
-    }
 };
 
 }

libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp

@@ -17,8 +17,8 @@
 #include <dirent.h>
 
 #include <uavcan/protocol/firmware_update_trigger.hpp>
+#include "firmware_path.hpp"
 
-#include "firmware_common.hpp"
 
 namespace uavcan_posix
 {
@@ -32,6 +32,18 @@ class FirmwareVersionChecker : public uavcan::IFirmwareVersionChecker
 
     FirmwarePath* paths_;
 
+    struct AppDescriptor
+    {
+        uint8_t signature[sizeof(uavcan::uint64_t)];
+        uint64_t image_crc;
+        uint32_t image_size;
+        uint32_t vcs_commit;
+        uint8_t major_version;
+        uint8_t minor_version;
+        uint8_t reserved[6];
+    };
+
+
     int copyIfNot(const char* srcpath, const char* destpath)
     {
         // Does the file exist
@@ -93,6 +105,61 @@ class FirmwareVersionChecker : public uavcan::IFirmwareVersionChecker
         return rv;
     }
 
+    __attribute__((optimize("O0")))
+    static int getFileInfo(const char* path, AppDescriptor & descriptor)
+    {
+        using namespace std;
+
+        const unsigned MaxChunk = 512 / sizeof(uint64_t);
+
+        uint64_t signature = 0;
+        std::memcpy(&signature, "APDesc00", 8);
+
+
+        int rv = -ENOENT;
+        uint64_t chunk[MaxChunk];
+        int fd = open(path, O_RDONLY);
+
+        if (fd >= 0)
+        {
+            AppDescriptor* pdescriptor = NULL;
+
+            while (pdescriptor == NULL)
+            {
+                int len = read(fd, chunk, sizeof(chunk));
+
+                if (len == 0)
+                {
+                    break;
+                }
+
+                if (len < 0)
+                {
+                    rv = -errno;
+                    goto out_close;
+                }
+
+                uint64_t* p = &chunk[0];
+
+                do
+                {
+                    if (*p == signature)
+                    {
+                        pdescriptor = (AppDescriptor*) p;
+                        descriptor = *pdescriptor;
+                        rv = 0;
+                        break;
+                    }
+                }
+                while (p++ <= &chunk[MaxChunk - (sizeof(AppDescriptor) / sizeof(chunk[0]))]);
+            }
+
+        out_close:
+            (void)close(fd);
+        }
+        return rv;
+    }
+
 protected:
     /**
      * This method will be invoked when the class obtains a response to GetNodeInfo request.
@@ -108,6 +175,7 @@ protected:
      * @return                          True - the class will begin sending update requests.
      *                                  False - the node will be ignored, no request will be sent.
      */
+    __attribute__((optimize("O0")))
     virtual bool shouldRequestFirmwareUpdate(uavcan::NodeID node_id,
                                              const uavcan::protocol::GetNodeInfo::Response& node_info,
                                              FirmwareFilePath& out_firmware_file_path)
@@ -164,13 +232,18 @@ protected:
                             int cr = copyIfNot(full_src_path.c_str(), full_dst_path.c_str());
 
                             // We have a file, is it a valid image
-                            FirmwareCommon fw;
+                            AppDescriptor descriptor;
 
-                            if (cr == 0 && fw.getFileInfo(full_dst_path.c_str()) == 0)
+                            std::memset(&descriptor,0, sizeof(descriptor));
+
+                            if (cr == 0 && getFileInfo(full_dst_path.c_str(), descriptor) == 0)
                             {
+                                volatile AppDescriptor descriptorC = descriptor;
+                                descriptorC.reserved[1]++;
+
                                 if (node_info.software_version.image_crc == 0 ||
                                     (node_info.software_version.major == 0 && node_info.software_version.minor == 0) ||
-                                    fw.descriptor.image_crc != node_info.software_version.image_crc)
+                                    descriptor.image_crc != node_info.software_version.image_crc)
                                 {
                                     rv = true;
                                     out_firmware_file_path = pfile->d_name;