fpga load: Atomic updating of multiple components

- The MPM function update_component now accepts multiple components to
  be updated in one RPC call.
- Updated the property tree and image loader to match this change.
- Also added DTS loading to the image loader.

5 files changed, 155 insertions, 97 deletions

diff --git a/host/include/uhd/types/component_file.hpp b/host/include/uhd/types/component_file.hpp
index e0761fecb..703c87039 100644
--- a/host/include/uhd/types/component_file.hpp
+++ b/host/include/uhd/types/component_file.hpp
@@ -30,6 +30,8 @@ namespace uhd{ namespace usrp{
         std::vector<uint8_t> data;
     };
 
+    typedef std::vector<component_file_t> component_files_t;
+
 }} // namespace uhd::usrp
 
 #endif /* INCLUDED_UHD_USRP_COMPONENT_FILE_HPP */
diff --git a/host/lib/usrp/mpmd/mpmd_image_loader.cpp b/host/lib/usrp/mpmd/mpmd_image_loader.cpp
index 4c73bc85f..e0b619cba 100644
--- a/host/lib/usrp/mpmd/mpmd_image_loader.cpp
+++ b/host/lib/usrp/mpmd/mpmd_image_loader.cpp
@@ -11,6 +11,7 @@
 #include <uhd/exception.hpp>
 #include <uhd/types/eeprom.hpp>
 #include <uhd/types/component_file.hpp>
+#include <boost/filesystem/convenience.hpp>
 #include <sstream>
 #include <string>
 #include <fstream>
@@ -23,6 +24,54 @@ namespace uhd{ namespace /*anon*/{
     const size_t MD5LEN = 32; // Length of a MD5 hash in chars
 
 /*
+ * Helper function to generate a component_file_t using the input ID and path to file.
+ */
+uhd::usrp::component_file_t generate_component(const std::string& id, const std::string& filepath) {
+    uhd::usrp::component_file_t component_file;
+    // Add an ID to the metadata
+    component_file.metadata["id"] = id;
+    UHD_LOG_TRACE("MPMD IMAGE LOADER",
+                  "Component ID added to the component dictionary: " << id);
+    // Add the filename to the metadata
+    // Remove the path to the filename
+    component_file.metadata["filename"] = boost::filesystem::path(filepath).filename().string();
+    UHD_LOG_TRACE("MPMD IMAGE LOADER",
+                  "Component filename added to the component dictionary: " << filepath);
+    // Add the hash, if a hash file exists
+    const std::string component_hash_filepath = filepath + ".md5";
+    std::ifstream component_hash_ifstream(component_hash_filepath.c_str(), std::ios::binary);
+    std::string component_hash;
+    if (component_hash_ifstream.is_open()) {
+        // TODO: Verify that the hash read is valid, ie only contains 0-9a-f.
+        component_hash.resize(MD5LEN);
+        component_hash_ifstream.read( &component_hash[0], MD5LEN );
+        component_hash_ifstream.close();
+        component_file.metadata["md5"] = component_hash;
+        UHD_LOG_TRACE("MPMD IMAGE LOADER",
+                      "Added component file hash to the component dictionary.");
+    } else {
+        // If there is no hash file, don't worry about it too much
+        UHD_LOG_DEBUG("MPMD IMAGE LOADER", "Could not open component file hash file: "
+                      << component_hash_filepath);
+    }
+
+    // Read the component file image into a structure suitable to sent as a binary string to MPM
+    std::vector<uint8_t> data;
+    std::ifstream component_ifstream(filepath.c_str(), std::ios::binary);
+    if (component_ifstream.is_open()) {
+        data.insert( data.begin(),
+                     std::istreambuf_iterator<char>(component_ifstream),
+                     std::istreambuf_iterator<char>());
+        component_ifstream.close();
+    } else {
+        const std::string err_msg("Component file does not exist: " + filepath);
+        throw uhd::runtime_error(err_msg);
+    }
+    component_file.data = data;
+    return component_file;
+}
+
+/*
  * Function to be registered with uhd_image_loader
  */
 static bool mpmd_image_loader(const image_loader::image_loader_args_t &image_loader_args){
@@ -41,56 +90,36 @@ static bool mpmd_image_loader(const image_loader::image_loader_args_t &image_loa
     uhd::device::sptr usrp = uhd::device::make(dev_addr, uhd::device::USRP);
     uhd::property_tree::sptr tree = usrp->get_tree();
 
-    // Populate the struct that we use to update the FPGA property
-    uhd::usrp::component_file_t component_file;
-    // Add an ID to the metadata
-    component_file.metadata["id"] = "fpga";
-    UHD_LOG_TRACE("MPMD IMAGE LOADER",
-                  "FPGA ID added to the component dictionary");
-    // Add the filename to the metadata
-    // TODO: Current this field is the absolute path on the host. We're letting MPM
-    //       handle cutting off the filename, but it would be better to just pass what we need to.
-    std::string fpga_filepath = image_loader_args.fpga_path;
-    component_file.metadata["filename"] = fpga_filepath;
-    UHD_LOG_TRACE("MPMD IMAGE LOADER",
-                  "FPGA filename added to the component dictionary: " << fpga_filepath);
-    // Add the hash, if a hash file exists
-    std::string fpga_hash_filepath = fpga_filepath + ".md5";
-    std::ifstream fpga_hash_ifstream(fpga_hash_filepath.c_str(), std::ios::binary);
-    std::string fpga_hash;
-    if (fpga_hash_ifstream.is_open()) {
-        // TODO: Verify that the hash read is valid, ie only contains 0-9a-f.
-        fpga_hash.resize(MD5LEN);
-        fpga_hash_ifstream.read( &fpga_hash[0], MD5LEN );
-        fpga_hash_ifstream.close();
-        component_file.metadata["md5"] = fpga_hash;
-        UHD_LOG_TRACE("MPMD IMAGE LOADER", "Added FPGA hash to the component dictionary.");
-    } else {
-        // If there is no hash file, don't worry about it too much
-        UHD_LOG_WARNING("MPMD IMAGE LOADER", "Could not open FPGA hash file: "
-                      << fpga_hash_filepath);
-    }
-
-    // Read the FPGA image into a structure suitable to sent as a binary string to MPM
+    // Generate the component files
     // TODO: We don't have a default image specified because the image will depend on the
     //       device and configuration. We'll probably want to fix this later, but it will
     //       depend on how uhd_images_downloader deposits files.
-    std::vector<uint8_t> data;
-    std::ifstream fpga_ifstream(fpga_filepath.c_str(), std::ios::binary);
-    if (fpga_ifstream.is_open()) {
-        data.insert( data.begin(),
-                     std::istreambuf_iterator<char>(fpga_ifstream),
-                     std::istreambuf_iterator<char>());
-        fpga_ifstream.close();
-    } else {
-        std::string err_msg("FPGA Bitfile does not exist. " + fpga_filepath);
+    uhd::usrp::component_files_t all_component_files;
+    // FPGA component struct
+    const std::string fpga_path = image_loader_args.fpga_path;
+    uhd::usrp::component_file_t comp_fpga = generate_component("fpga", fpga_path);
+    all_component_files.push_back(comp_fpga);
+    // DTS component struct
+    // First, we need to determine the name
+    const std::string base_name = boost::filesystem::change_extension(fpga_path, "").string();
+    if (base_name == fpga_path) {
+        const std::string err_msg("Can't cut extension from FPGA filename... " + fpga_path);
         throw uhd::runtime_error(err_msg);
     }
-    component_file.data = data;
-    UHD_LOG_TRACE("MPMD IMAGE LOADER", "FPGA image read from file.");
+    const std::string dts_path = base_name + ".dts";
+    // Then try to generate it
+    try {
+        uhd::usrp::component_file_t comp_dts = generate_component("dts", dts_path);
+        all_component_files.push_back(comp_dts);
+        UHD_LOG_TRACE("MPMD IMAGE LOADER", "FPGA and DTS images read from file.");
+    } catch (const uhd::runtime_error& ex) {
+        // If we can't find the DTS file, that's fine, continue without it
+        UHD_LOG_WARNING("MPMD IMAGE LOADER", ex.what());
+        UHD_LOG_TRACE("MPMD IMAGE LOADER", "FPGA images read from file.");
+    }
 
     // Call RPC to update the component
-    tree->access<uhd::usrp::component_file_t>("/mboards/0/components/fpga").set(component_file);
+    tree->access<uhd::usrp::component_files_t>("/mboards/0/components/fpga").set(all_component_files);
     UHD_LOG_TRACE("MPMD IMAGE LOADER", "Update component function succeeded.");
 
     return true;
@@ -99,7 +128,9 @@ static bool mpmd_image_loader(const image_loader::image_loader_args_t &image_loa
 
 UHD_STATIC_BLOCK(register_mpm_image_loader){
     // TODO: Update recovery instructions
-    std::string recovery_instructions = "Aborting. Your USRP MPM-enabled device will likely be unusable.";
+    const std::string recovery_instructions = "Aborting. Your USRP MPM-enabled device's update may or may not have\n"
+                                              "completed. The contents of the image files may have been corrupted.\n"
+                                              "Please verify those files as soon as possible.";
 
     //TODO: 'n3xx' doesn't really fit the MPM abstraction, but this is simpler for the time being
     image_loader::register_image_loader("n3xx", mpmd_image_loader, recovery_instructions);
diff --git a/host/lib/usrp/mpmd/mpmd_impl.cpp b/host/lib/usrp/mpmd/mpmd_impl.cpp
index 58b5682c1..f0498cf99 100644
--- a/host/lib/usrp/mpmd/mpmd_impl.cpp
+++ b/host/lib/usrp/mpmd/mpmd_impl.cpp
@@ -36,6 +36,7 @@
 #include <mutex>
 #include <random>
 #include <string>
+#include <vector>
 
 using namespace uhd;
 
@@ -52,20 +53,37 @@ namespace {
     /*************************************************************************
      * Helper functions
      ************************************************************************/
-    uhd::usrp::component_file_t _update_component(
-        const uhd::usrp::component_file_t& comp,
+    uhd::usrp::component_files_t _update_component(
+        const uhd::usrp::component_files_t& comps,
         mpmd_mboard_impl *mb
     ) {
-        std::vector<uint8_t> data = comp.data;
-        std::map<std::string, std::string> metadata;
-        for (const auto& key : comp.metadata.keys()) {
-            metadata[key] = comp.metadata[key];
+        // Construct the arguments to update component
+        std::vector<std::vector<uint8_t>> all_data;
+        std::vector<std::map<std::string, std::string>> all_metadata;
+        // Also construct a copy of just the metadata to store in the property tree
+        uhd::usrp::component_files_t all_comps_copy;
+
+        for  (const auto& comp : comps) {
+            // Make a map for update components args
+            std::map<std::string, std::string> metadata;
+            // Make a component copy to add to the property tree
+            uhd::usrp::component_file_t comp_copy;
+            // Copy the metadata
+            for (const auto& key : comp.metadata.keys()) {
+                metadata[key] = comp.metadata[key];
+                comp_copy.metadata[key] = comp.metadata[key];
+            }
+            // Copy to the update component args
+            all_data.push_back(comp.data);
+            all_metadata.push_back(metadata);
+            // Copy to the property tree
+            all_comps_copy.push_back(comp_copy);
         }
-        mb->rpc->notify_with_token("update_component", metadata, data);
 
-        uhd::usrp::component_file_t comp_copy;
-        comp_copy.metadata = comp.metadata;
-        return comp_copy;
+        // Now call update component
+        mb->rpc->notify_with_token("update_component", all_metadata, all_data);
+
+        return all_comps_copy;
     }
 
 
@@ -174,16 +192,17 @@ namespace {
                 mb->rpc->request<std::vector<std::string>>(
                         "list_updateable_components"
                 );
+        // TODO: Check the 'id' against the registered property
         UHD_LOG_DEBUG("MPMD",
                     "Found " << updateable_components.size() << " updateable motherboard components."
                 );
         for (const auto& comp_name : updateable_components) {
             UHD_LOG_TRACE("MPMD",
                     "Adding motherboard component: " << comp_name);
-            tree->create<uhd::usrp::component_file_t>(mb_path / "components" / comp_name)
-                        .set_coercer([mb](const uhd::usrp::component_file_t& comp_file) {
+            tree->create<uhd::usrp::component_files_t>(mb_path / "components" / comp_name)
+                        .set_coercer([mb](const uhd::usrp::component_files_t& comp_files) {
                     return _update_component(
-                            comp_file,
+                            comp_files,
                             mb
                     );
             })
diff --git a/mpm/python/usrp_mpm/periph_manager/base.py b/mpm/python/usrp_mpm/periph_manager/base.py
index 3cfcc0ce8..4d58b0933 100644
--- a/mpm/python/usrp_mpm/periph_manager/base.py
+++ b/mpm/python/usrp_mpm/periph_manager/base.py
@@ -419,28 +419,24 @@ class PeriphManagerBase(object):
         """
         return [dboard.device_info for dboard in self.dboards]
 
-    def update_component(self, file_metadata, data):
+    def update_component(self, metadata_l, data_l):
         """
         Updates the device component specified by comp_dict
-        :param file_metadata: Dictionary of strings containing metadata
-        :param data: Binary string with the file contents to be written
-        """
-        id_str = file_metadata['id']
-        filename = os.path.basename(file_metadata['filename'])
-        if id_str not in self.updateable_components:
-            self.log.error("{0} not an updateable component ({1})".format(
-                id_str, self.updateable_components.keys()
-            ))
-            raise NotImplementedError("Update component not implemented for {}".format(id_str))
-        self.log.trace("Updating component: {}".format(id_str))
-        if 'md5' in file_metadata:
-            given_hash = file_metadata['md5']
-            comp_hash = md5()
-            comp_hash.update(data)
-            comp_hash = comp_hash.hexdigest()
-            if comp_hash == given_hash:
-                self.log.trace("FPGA bitfile hash matched: {}".format(
-                    comp_hash
+        :param metadata_l: List of dictionary of strings containing metadata
+        :param data_l: List of binary string with the file contents to be written
+        """
+        # We need a 'metadata' and a 'data' for each file we want to update
+        assert (len(metadata_l) == len(data_l)),\
+            "update_component arguments must be the same length"
+        # TODO: Update the manifest file
+
+        # Iterate through the components, updating each in turn
+        for metadata, data in zip(metadata_l, data_l):
+            id_str = metadata['id']
+            filename = os.path.basename(metadata['filename'])
+            if id_str not in self.updateable_components:
+                self.log.error("{0} not an updateable component ({1})".format(
+                    id_str, self.updateable_components.keys()
                 ))
                 raise KeyError("Update component not implemented for {}".format(id_str))
             self.log.trace("Updating component: {}".format(id_str))
@@ -460,24 +456,19 @@ class PeriphManagerBase(object):
                                        comp_hash, given_hash))
                     raise RuntimeError("Component file hash mismatch")
             else:
-                self.log.error("FPGA bitfile hash mismatched:")
-                self.log.error("Calculated {}".format(comp_hash))
-                self.log.error("Given      {}".format(given_hash))
-                raise RuntimeError("FPGA Bitfile hash mismatch")
-        else:
-            self.log.trace("Loading unverified {} image.".format(
-                id_str
-            ))
-        basepath = os.path.join(os.sep, "tmp", "uploads")
-        filepath = os.path.join(basepath, filename)
-        if not os.path.isdir(basepath):
-            self.log.trace("Creating directory {}".format(basepath))
-            os.makedirs(basepath)
-        self.log.trace("Writing data to {}".format(filepath))
-        with open(filepath, 'wb') as f:
-            f.write(data)
-        update_func = getattr(self, self.updateable_components[id_str]['callback'])
-        update_func(filepath, file_metadata)
+                self.log.trace("Loading unverified {} image.".format(
+                    id_str
+                ))
+            basepath = os.path.join(os.sep, "tmp", "uploads")
+            filepath = os.path.join(basepath, filename)
+            if not os.path.isdir(basepath):
+                self.log.trace("Creating directory {}".format(basepath))
+                os.makedirs(basepath)
+            self.log.trace("Writing data to {}".format(filepath))
+            with open(filepath, 'wb') as f:
+                f.write(data)
+            update_func = getattr(self, self.updateable_components[id_str]['callback'])
+            update_func(filepath, metadata)
         return True
 
 
@@ -697,5 +688,3 @@ class PeriphManagerBase(object):
         - src_port: IP port the connection is coming from.
         """
         raise NotImplementedError("commit_xport() not implemented.")
-
-
diff --git a/mpm/python/usrp_mpm/periph_manager/n310.py b/mpm/python/usrp_mpm/periph_manager/n310.py
index 84ab3ae7e..1dab1b572 100644
--- a/mpm/python/usrp_mpm/periph_manager/n310.py
+++ b/mpm/python/usrp_mpm/periph_manager/n310.py
@@ -472,11 +472,15 @@ class n310(PeriphManagerBase):
     # dboard, etc. The host is responsible for providing a compatible image
     # for the N310's current setup.
     binfile_path = '/lib/firmware/n310.bin'
+    dts_path = '/lib/firmware/n310.dts'
     # Override the list of updateable components
     updateable_components = {
         'fpga': {
             'callback': "update_fpga",
         },
+        'dts': {
+            'callback': "update_dts",
+        },
     }
 
     def __init__(self, args):
@@ -913,3 +917,16 @@ class n310(PeriphManagerBase):
             raise RuntimeError("Invalid N310 FPGA bitfile")
         # TODO: Implement reload procedure
         return True
+
+    @no_rpc
+    def update_dts(self, filepath, metadata):
+        """
+        Update the DTS image in the filesystem
+        :param filepath: path to new DTS image
+        :param metadata: Dictionary of strings containing metadata
+        """
+        self.log.trace("Updating DTS with image at {}"
+                       .format(filepath))
+        shutil.copy(filepath, self.dts_path)
+        # TODO: Compile the new dts file into a usable dtbo
+        return True