From b8449f9fb255db21169a053c8e262f1814efdbec Mon Sep 17 00:00:00 2001 From: "David V. Lu" Date: Thu, 5 May 2022 22:34:45 -0400 Subject: [PATCH] [MSA] Exising Package Loading Tweaks --- .../configuration_files.hpp | 5 ++ .../src/configuration_files_widget.cpp | 23 +++++++- .../data/package_settings_config.hpp | 22 +++++++- .../src/package_settings_config.cpp | 56 ++++++++++--------- 4 files changed, 77 insertions(+), 29 deletions(-) diff --git a/moveit_setup_assistant/moveit_setup_core_plugins/include/moveit_setup_core_plugins/configuration_files.hpp b/moveit_setup_assistant/moveit_setup_core_plugins/include/moveit_setup_core_plugins/configuration_files.hpp index 698c3a7c01..17090c7aec 100644 --- a/moveit_setup_assistant/moveit_setup_core_plugins/include/moveit_setup_core_plugins/configuration_files.hpp +++ b/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(); diff --git a/moveit_setup_assistant/moveit_setup_core_plugins/src/configuration_files_widget.cpp b/moveit_setup_assistant/moveit_setup_core_plugins/src/configuration_files_widget.cpp index 1b8c59ce6e..48a4efc8ce 100644 --- a/moveit_setup_assistant/moveit_setup_core_plugins/src/configuration_files_widget.cpp +++ b/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(); } diff --git a/moveit_setup_assistant/moveit_setup_framework/include/moveit_setup_framework/data/package_settings_config.hpp b/moveit_setup_assistant/moveit_setup_framework/include/moveit_setup_framework/data/package_settings_config.hpp index 65c333089e..f51254e384 100644 --- a/moveit_setup_assistant/moveit_setup_framework/include/moveit_setup_framework/data/package_settings_config.hpp +++ b/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_; diff --git a/moveit_setup_assistant/moveit_setup_framework/src/package_settings_config.cpp b/moveit_setup_assistant/moveit_setup_framework/src/package_settings_config.cpp index 2ac9910e90..56474818c4 100644 --- a/moveit_setup_assistant/moveit_setup_framework/src/package_settings_config.cpp +++ b/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& 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_));