From 28802a38ddcb4e2213b4bac978ba81e956e9fb78 Mon Sep 17 00:00:00 2001 From: David Sidrane Date: Tue, 26 May 2015 14:00:49 -1000 Subject: [PATCH] removed firmware common, made GetInfo part of the firmware checke class fixed firmware path so that code compiles --- .../basic_file_server_backend.hpp | 90 ++++++++++++--- .../include/uavcan_posix/firmware_common.hpp | 107 ------------------ .../include/uavcan_posix/firmware_path.hpp | 67 +++++------ .../uavcan_posix/firmware_version_checker.hpp | 81 ++++++++++++- 4 files changed, 184 insertions(+), 161 deletions(-) delete mode 100644 libuavcan_drivers/posix/include/uavcan_posix/firmware_common.hpp diff --git a/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp b/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp index 7a02ee11e1..c71a177bb9 100644 --- a/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp +++ b/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp @@ -23,8 +23,7 @@ #include #include #include - -#include "firmware_common.hpp" +#include 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; - fw.getFileInfo(path.c_str()); - out_crc64 = fw.descriptor.image_crc; - out_size = fw.descriptor.image_size; - // 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 - out_type.flags = uavcan::protocol::file::EntryType::FLAG_FILE | - uavcan::protocol::file::EntryType::FLAG_READABLE; - rv = 0; + using namespace std; + out_size = 0; + out_crc64 = 0; + + rv = -ENOENT; + uint8_t buffer[512]; + + int fd = ::open(path.c_str(), O_RDONLY); + + 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 - ssize_t len = ::read(fd, out_buffer, inout_size); + //todo uses a read at offset to fill on EAGAIN + 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() { } + + + + }; } diff --git a/libuavcan_drivers/posix/include/uavcan_posix/firmware_common.hpp b/libuavcan_drivers/posix/include/uavcan_posix/firmware_common.hpp deleted file mode 100644 index 31a139349f..0000000000 --- a/libuavcan_drivers/posix/include/uavcan_posix/firmware_common.hpp +++ /dev/null @@ -1,107 +0,0 @@ -/**************************************************************************** -* -* Copyright (c) 2015 PX4 Development Team. All rights reserved. -* Author: Pavel Kirienko -* David Sidrane -* -****************************************************************************/ - -#ifndef UAVCAN_POSIX_FIRMWARE_COMMON_HPP_INCLUDED -#define UAVCAN_POSIX_FIRMWARE_COMMON_HPP_INCLUDED - -#include -#include -#include -#include -#include - -#include -#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 diff --git a/libuavcan_drivers/posix/include/uavcan_posix/firmware_path.hpp b/libuavcan_drivers/posix/include/uavcan_posix/firmware_path.hpp index a59ce1ec0e..4777a11a36 100644 --- a/libuavcan_drivers/posix/include/uavcan_posix/firmware_path.hpp +++ b/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 - * a member function inside the firmware version checker class. + * This class manages the sole copies of the cache path and the base path of the fw + * 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_; - BasePathString cache_path_; + static char getPathSeparator() + { + return static_cast(uavcan::protocol::file::Path::SEPARATOR); + } /** * This type is used internally for the full path to file */ typedef uavcan::MakeString::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(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(getFirmwareCachePath()); + addSlash(base_path_); + 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); - } }; } diff --git a/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp b/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp index c53b6e2c08..beeb9ce8c1 100644 --- a/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp +++ b/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp @@ -17,8 +17,8 @@ #include #include +#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;