[MSA] Existing Package Loading Tweaks (moveit#1212)

moveit_setup_assistant/moveit_setup_core_plugins/include/moveit_setup_core_plugins/configuration_files.hpp

@@ -60,6 +60,11 @@ class ConfigurationFiles : public moveit_setup_framework::SetupStep
     package_settings_->setPackagePath(package_path);
   }
 
+  void setPackageName(const std::string& package_name)
+  {
+    package_settings_->setPackageName(package_name);
+  }
+
   /// Populate the 'Files to be generated' list
   void loadFiles();
 

moveit_setup_assistant/moveit_setup_core_plugins/src/configuration_files_widget.cpp

@@ -186,7 +186,28 @@ void ConfigurationFilesWidget::setCheckSelected(bool checked)
 
 void ConfigurationFilesWidget::onPackagePathChanged(const QString& path)
 {
-  setup_step_.setPackagePath(path.toStdString());
+  std::string package_path = path.toStdString();
+  if (package_path == setup_step_.getPackagePath())
+  {
+    return;
+  }
+  setup_step_.setPackagePath(package_path);
+
+  // Determine new potential package name
+  boost::filesystem::path fs_package_path;
+
+  // Remove end slash if there is one
+  if (!package_path.compare(package_path.size() - 1, 1, "/"))
+  {
+    fs_package_path = boost::filesystem::path(package_path.substr(0, package_path.size() - 1));
+  }
+  else
+  {
+    fs_package_path = boost::filesystem::path(package_path);
+  }
+
+  setup_step_.setPackageName(fs_package_path.filename().string());
+
   focusGiven();
 }
 

moveit_setup_assistant/moveit_setup_framework/include/moveit_setup_framework/data/package_settings_config.hpp

@@ -44,18 +44,36 @@ const std::string SETUP_ASSISTANT_FILE = ".setup_assistant";
 class PackageSettingsConfig : public SetupConfig
 {
 public:
+  /**
+   * @brief Overridden method to load THIS config's data variables.
+   *
+   * Not to be confused with loadExisting which is a PackageSettingsConfig-specific
+   * method for loading ALL configs and their data.
+   */
   void loadPrevious(const std::string& package_path, const YAML::Node& node) override;
   YAML::Node saveToYaml() const override;
 
-  void loadExisting(const std::string& package_path);
+  /**
+   * @brief Method for loading the contents of the .setup_assistant file into all the configs
+   *
+   * @param package_path_or_name Either the path to the MoveIt config package folder OR the name of the package.
+   */
+  void loadExisting(const std::string& package_path_or_name);
 
   const std::string& getPackagePath() const
   {
     return config_pkg_path_;
   }
 
+  const std::string& getPackageName() const
+  {
+    return new_package_name_;
+  }
+
   void setPackagePath(const std::string& package_path);
 
+  void setPackageName(const std::string& package_name);
+
   const std::time_t& getGenerationTime() const
   {
     return config_pkg_generated_timestamp_;
@@ -199,7 +217,7 @@ class PackageSettingsConfig : public SetupConfig
   std::string config_pkg_path_;
 
   /// Name of the new package that is being (or going) to be generated, based on user specified save path
-  std::string new_package_name_;
+  std::string new_package_name_{ "unnamed_moveit_config" };
 
   /// Name of the author of this config
   std::string author_name_;

moveit_setup_assistant/moveit_setup_framework/src/package_settings_config.cpp

@@ -62,43 +62,37 @@ YAML::Node PackageSettingsConfig::saveToYaml() const
 void PackageSettingsConfig::setPackagePath(const std::string& package_path)
 {
   config_pkg_path_ = package_path;
+}
 
-  // Determine new package name
-  boost::filesystem::path fs_package_path;
-  // Remove end slash if there is one
-  if (!package_path.compare(package_path.size() - 1, 1, "/"))
-  {
-    fs_package_path = boost::filesystem::path(package_path.substr(0, package_path.size() - 1));
-  }
-  else
-  {
-    fs_package_path = boost::filesystem::path(package_path);
-  }
-
-  // Get the last directory name
-  new_package_name_ = fs_package_path.filename().string();
-
-  // check for empty
-  if (new_package_name_.empty())
-    new_package_name_ = "unknown";
+void PackageSettingsConfig::setPackageName(const std::string& package_name)
+{
+  new_package_name_ = package_name;
 }
 
-void PackageSettingsConfig::loadExisting(const std::string& package_path)
+void PackageSettingsConfig::loadExisting(const std::string& package_path_or_name)
 {
-  if (package_path.empty())
+  if (package_path_or_name.empty())
   {
     throw std::runtime_error("Please specify a configuration package path to load.");
   }
-  // check that the folder exists
-  if (boost::filesystem::is_directory(package_path))
+  // Check if it is a path that exists
+  if (boost::filesystem::is_directory(package_path_or_name))
   {
     // they inputted a full path
-    setPackagePath(package_path);
+    setPackagePath(package_path_or_name);
   }
   else
   {
-    // does not exist, check if its a package
-    std::string share_dir = ament_index_cpp::get_package_share_directory(package_path);
+    // Determine the path from a name
+    /* TODO(dlu): Ideally, the package path is in source so that when we write back to it,
+     *            the changes will be reflected and then we can check them into git.
+     *            However, there's no easy way to determine the source folder from C++.
+     *            You could run colcon list -p --packages-select $PACKAGE_NAME but the
+     *            results are dependent on what folder you are in and opening an external
+     *            process is messy. For now, we just use the share path and rely on the user
+     *            to write back to the proper directory in the ConfigurationFiles step
+     */
+    std::string share_dir = ament_index_cpp::get_package_share_directory(package_path_or_name);
 
     // check that the folder exists
     if (!boost::filesystem::is_directory(share_dir))
@@ -109,6 +103,10 @@ void PackageSettingsConfig::loadExisting(const std::string& package_path)
     setPackagePath(share_dir);
   }
 
+  // Load the package name from the package.xml
+  std::string relative_path;  // we don't use this output value
+  extractPackageNameFromPath(config_pkg_path_, new_package_name_, relative_path);
+
   // Load Config Yaml
   std::string config_path = appendPaths(config_pkg_path_, ".setup_assistant");
   if (!boost::filesystem::is_regular_file(config_path))
@@ -141,7 +139,10 @@ void PackageSettingsConfig::loadExisting(const std::string& package_path)
         yaml_key = backwards_match->second;
       }
 
-      config_data_->get(name)->loadPrevious(config_pkg_path_, title_node[yaml_key]);
+      if (title_node[yaml_key].IsDefined())
+      {
+        config_data_->get(name)->loadPrevious(config_pkg_path_, title_node[yaml_key]);
+      }
     }
   }
   catch (YAML::ParserException& e)  // Catch errors, translate to runtime_error
@@ -183,6 +184,9 @@ void PackageSettingsConfig::loadDependencies()
 void PackageSettingsConfig::collectVariables(std::vector<TemplateVariable>& variables)
 {
   variables.push_back(TemplateVariable("GENERATED_PACKAGE_NAME", new_package_name_));
+
+  // TODO: Add new variables for other fields existing in the package.xml
+  //       i.e. read the version so that the version is not overwritten
   variables.push_back(TemplateVariable("AUTHOR_NAME", author_name_));
   variables.push_back(TemplateVariable("AUTHOR_EMAIL", author_email_));