From 667a8edc7a082cc465f47fe8c826e92df0618b14 Mon Sep 17 00:00:00 2001 From: Karim Abdelrahman Date: Wed, 16 Apr 2025 16:26:11 +0300 Subject: [PATCH] McuSupport: fix optional packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCU plugin didn't have to handle "optional" property in MCU kit descriptions. I believe this is mainly because "optional" was always "false" for all the previous MCU targets. A newly introduced kit, with one package set "optional" to "true", manifested the issue. The user couldn't create a kit without providing a valid path to that optional package. The user should be able to leave the optional package path empty or provide a valid path. This change introduces a new Groupbox widget to display the optional packages. An optional package with empty path is considered valid. Task-number: QTCREATORBUG-32777 Change-Id: I5f27952efc77bc5e4351ab4c013669eb6056810f Reviewed-by: Sivert Krøvel Reviewed-by: Alessandro Portale --- src/plugins/mcusupport/mcuabstractpackage.h | 1 + src/plugins/mcusupport/mcupackage.cpp | 38 +++++++++++++------ src/plugins/mcusupport/mcupackage.h | 5 +++ .../mcusupport/mcusupportoptionspage.cpp | 23 ++++++++++- src/plugins/mcusupport/mcusupportsdk.cpp | 2 + src/plugins/mcusupport/mcutargetdescription.h | 1 + src/plugins/mcusupport/mcutargetfactory.cpp | 1 + src/plugins/mcusupport/test/packagemock.h | 1 + src/plugins/mcusupport/test/unittest.cpp | 3 ++ 9 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/plugins/mcusupport/mcuabstractpackage.h b/src/plugins/mcusupport/mcuabstractpackage.h index 98d69144aa8..751d312133d 100644 --- a/src/plugins/mcusupport/mcuabstractpackage.h +++ b/src/plugins/mcusupport/mcuabstractpackage.h @@ -34,6 +34,7 @@ public: virtual QString label() const = 0; virtual QString cmakeVariableName() const = 0; virtual QString environmentVariableName() const = 0; + virtual bool isOptional() const = 0; virtual bool isAddToSystemPath() const = 0; virtual QStringList versions() const = 0; diff --git a/src/plugins/mcusupport/mcupackage.cpp b/src/plugins/mcusupport/mcupackage.cpp index ff66956a532..15c5ef9b202 100644 --- a/src/plugins/mcusupport/mcupackage.cpp +++ b/src/plugins/mcusupport/mcupackage.cpp @@ -41,6 +41,7 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, const QStringList &versions, const QString &downloadUrl, const McuPackageVersionDetector *versionDetector, + const bool optional, const bool addToSystemPath, const Utils::PathChooser::Kind &valueType, const bool allowNewerVersionKey) @@ -53,6 +54,7 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, , m_cmakeVariableName(cmakeVarName) , m_environmentVariableName(envVarName) , m_downloadUrl(downloadUrl) + , m_optional(optional) , m_addToSystemPath(addToSystemPath) , m_valueType(valueType) { @@ -93,6 +95,11 @@ QString McuPackage::environmentVariableName() const return m_environmentVariableName; } +bool McuPackage::isOptional() const +{ + return m_optional; +} + bool McuPackage::isAddToSystemPath() const { return m_addToSystemPath; @@ -190,25 +197,34 @@ McuPackage::Status McuPackage::status() const return m_status; } +bool McuPackage::isOptionalAndEmpty() const +{ + return m_status == Status::EmptyPath && isOptional(); +} + bool McuPackage::isValidStatus() const { return m_status == Status::ValidPackage || m_status == Status::ValidPackageMismatchedVersion - || m_status == Status::ValidPackageVersionNotDetected; + || m_status == Status::ValidPackageVersionNotDetected || isOptionalAndEmpty(); } void McuPackage::updateStatusUi() { - switch (m_status) { - case Status::ValidPackage: + if (isOptionalAndEmpty()) { m_infoLabel->setType(InfoLabel::Ok); - break; - case Status::ValidPackageMismatchedVersion: - case Status::ValidPackageVersionNotDetected: - m_infoLabel->setType(InfoLabel::Warning); - break; - default: - m_infoLabel->setType(InfoLabel::NotOk); - break; + } else { + switch (m_status) { + case Status::ValidPackage: + m_infoLabel->setType(InfoLabel::Ok); + break; + case Status::ValidPackageMismatchedVersion: + case Status::ValidPackageVersionNotDetected: + m_infoLabel->setType(InfoLabel::Warning); + break; + default: + m_infoLabel->setType(InfoLabel::NotOk); + break; + } } m_infoLabel->setText(statusText()); } diff --git a/src/plugins/mcusupport/mcupackage.h b/src/plugins/mcusupport/mcupackage.h index 7b9ef646506..b3366d1db87 100644 --- a/src/plugins/mcusupport/mcupackage.h +++ b/src/plugins/mcusupport/mcupackage.h @@ -39,6 +39,7 @@ public: const QString &downloadUrl = {}, const McuPackageVersionDetector *versionDetector = nullptr, const bool addToPath = false, + const bool optional = false, const Utils::PathChooser::Kind &valueType = Utils::PathChooser::Kind::ExistingDirectory, const bool allowNewerVersionKey = false); @@ -50,6 +51,7 @@ public: QString label() const override; QString cmakeVariableName() const override; QString environmentVariableName() const override; + bool isOptional() const override; bool isAddToSystemPath() const override; QStringList versions() const override; @@ -77,6 +79,8 @@ private: void updatePath(); void updateStatusUi(); + bool isOptionalAndEmpty() const; + SettingsHandler::Ptr settingsHandler; Utils::PathChooser *m_fileChooser = nullptr; @@ -95,6 +99,7 @@ private: const QString m_cmakeVariableName; const QString m_environmentVariableName; const QString m_downloadUrl; + const bool m_optional; const bool m_addToSystemPath; const Utils::PathChooser::Kind m_valueType; diff --git a/src/plugins/mcusupport/mcusupportoptionspage.cpp b/src/plugins/mcusupport/mcusupportoptionspage.cpp index d1f0f3f86c9..348a0d2435a 100644 --- a/src/plugins/mcusupport/mcusupportoptionspage.cpp +++ b/src/plugins/mcusupport/mcusupportoptionspage.cpp @@ -55,8 +55,10 @@ private: QMap m_packageWidgets; QMap m_mcuTargetPacketWidgets; QFormLayout *m_packagesLayout = nullptr; + QFormLayout *m_optionalPackagesLayout = nullptr; QGroupBox *m_qtForMCUsSdkGroupBox = nullptr; QGroupBox *m_packagesGroupBox = nullptr; + QGroupBox *m_optionalPackagesGroupBox = nullptr; QGroupBox *m_mcuTargetsGroupBox = nullptr; QComboBox *m_mcuTargetsComboBox = nullptr; QGroupBox *m_kitCreationGroupBox = nullptr; @@ -122,6 +124,14 @@ McuSupportOptionsWidget::McuSupportOptionsWidget(McuSupportOptions &options, m_packagesGroupBox->setLayout(m_packagesLayout); } + { + m_optionalPackagesGroupBox = new QGroupBox(Tr::tr("Optional")); + m_optionalPackagesGroupBox->setFlat(true); + mainLayout->addWidget(m_optionalPackagesGroupBox); + m_optionalPackagesLayout = new QFormLayout; + m_optionalPackagesGroupBox->setLayout(m_optionalPackagesLayout); + } + { m_mcuTargetsInfoLabel = new Utils::InfoLabel; mainLayout->addWidget(m_mcuTargetsInfoLabel); @@ -187,6 +197,10 @@ void McuSupportOptionsWidget::updateStatus() const bool ready = valid && mcuTarget; m_mcuTargetsGroupBox->setVisible(ready); m_packagesGroupBox->setVisible(ready && !mcuTarget->packages().isEmpty()); + m_optionalPackagesGroupBox->setVisible( + ready && std::ranges::any_of(mcuTarget->packages(), [](McuPackagePtr p) { + return p->isOptional(); + })); m_kitCreationGroupBox->setVisible(ready); m_mcuTargetsInfoLabel->setVisible(valid && m_options.sdkRepository.mcuTargets.isEmpty()); if (m_mcuTargetsInfoLabel->isVisible()) { @@ -266,6 +280,10 @@ void McuSupportOptionsWidget::showMcuTargetPackages() m_packagesLayout->removeRow(0); } + while (m_optionalPackagesLayout->rowCount() > 0) { + m_optionalPackagesLayout->removeRow(0); + } + std::set packages; for (const auto &package : mcuTarget->packages()) { @@ -285,7 +303,10 @@ void McuSupportOptionsWidget::showMcuTargetPackages() package->setPath(macroExpander->expand(package->defaultPath())); } }); - m_packagesLayout->addRow(package->label(), packageWidget); + if (package->isOptional()) + m_optionalPackagesLayout->addRow(package->label(), packageWidget); + else + m_packagesLayout->addRow(package->label(), packageWidget); packageWidget->show(); } diff --git a/src/plugins/mcusupport/mcusupportsdk.cpp b/src/plugins/mcusupport/mcusupportsdk.cpp index 39082010da5..2710462b91d 100644 --- a/src/plugins/mcusupport/mcusupportsdk.cpp +++ b/src/plugins/mcusupport/mcusupportsdk.cpp @@ -61,6 +61,7 @@ McuPackagePtr createQtForMCUsPackage(const SettingsHandler::Ptr &settingsHandler {}, // versions {}, // downloadUrl nullptr, // versionDetector + false, // optional false, // addToPath Utils::PathChooser::Kind::ExistingDirectory, // valueType true)}; // useNewestVersionKey @@ -706,6 +707,7 @@ static PackageDescription parsePackage(const QJsonObject &cmakeEntry) detectionPaths, versions, parseVersionDetection(cmakeEntry), + cmakeEntry["optional"].toBool(), cmakeEntry["addToSystemPath"].toBool(), parseLineEditType(cmakeEntry["type"])}; } diff --git a/src/plugins/mcusupport/mcutargetdescription.h b/src/plugins/mcusupport/mcutargetdescription.h index 3b6d783320a..296de46f4fe 100644 --- a/src/plugins/mcusupport/mcutargetdescription.h +++ b/src/plugins/mcusupport/mcutargetdescription.h @@ -33,6 +33,7 @@ struct PackageDescription Utils::FilePaths detectionPaths; QStringList versions; VersionDetection versionDetection; + bool optional; bool shouldAddToSystemPath; Utils::PathChooser::Kind type; }; //struct PackageDescription diff --git a/src/plugins/mcusupport/mcutargetfactory.cpp b/src/plugins/mcusupport/mcutargetfactory.cpp index 120e2ce89ca..a392d1f3b3b 100644 --- a/src/plugins/mcusupport/mcutargetfactory.cpp +++ b/src/plugins/mcusupport/mcutargetfactory.cpp @@ -137,6 +137,7 @@ McuPackagePtr McuTargetFactory::createPackage(const PackageDescription &pkgDesc) pkgDesc.versions, {}, createVersionDetection(pkgDesc.versionDetection), + pkgDesc.optional, pkgDesc.shouldAddToSystemPath, pkgDesc.type}}; } diff --git a/src/plugins/mcusupport/test/packagemock.h b/src/plugins/mcusupport/test/packagemock.h index ab2e425ef54..037063165da 100644 --- a/src/plugins/mcusupport/test/packagemock.h +++ b/src/plugins/mcusupport/test/packagemock.h @@ -29,6 +29,7 @@ public: MOCK_METHOD(bool, isValidStatus, (), (const)); MOCK_METHOD(QString, cmakeVariableName, (), (const)); MOCK_METHOD(QString, environmentVariableName, (), (const)); + MOCK_METHOD(bool, isOptional, (), (const)); MOCK_METHOD(bool, isAddToSystemPath, (), (const)); MOCK_METHOD(bool, writeToSettings, (), (const)); MOCK_METHOD(void, readFromSettings, ()); diff --git a/src/plugins/mcusupport/test/unittest.cpp b/src/plugins/mcusupport/test/unittest.cpp index f45fd668e6d..eba2d80be14 100644 --- a/src/plugins/mcusupport/test/unittest.cpp +++ b/src/plugins/mcusupport/test/unittest.cpp @@ -196,6 +196,7 @@ const PackageDescription {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; const McuTargetDescription::Platform platformDescription{id, @@ -851,6 +852,7 @@ void McuSupportTest::test_useFallbackPathForToolchainWhenPathFromSettingsIsNotAv {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; McuTargetDescription::Toolchain toolchainDescription{armGcc, {}, compilerDescription, {}}; @@ -875,6 +877,7 @@ void McuSupportTest::test_usePathFromSettingsForToolchainPath() {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; McuTargetDescription::Toolchain toolchainDescription{armGcc, {}, compilerDescription, {}};