From 4a10ad4c35eadad3de364d058faf0dceb485c276 Mon Sep 17 00:00:00 2001 From: Nav Date: Sun, 2 May 2021 15:50:07 +0100 Subject: [PATCH] Cleaned up AVR8 part description file loading --- src/TargetController/TargetController.hpp | 5 +- src/Targets/Microchip/AVR/AVR8/Avr8.cpp | 47 ++++++++++++------- src/Targets/Microchip/AVR/AVR8/Avr8.hpp | 6 ++- .../PartDescription/PartDescriptionFile.cpp | 16 +++---- src/Targets/Target.hpp | 13 +++-- 5 files changed, 53 insertions(+), 34 deletions(-) diff --git a/src/TargetController/TargetController.hpp b/src/TargetController/TargetController.hpp index 30d6938c..1f30140f 100644 --- a/src/TargetController/TargetController.hpp +++ b/src/TargetController/TargetController.hpp @@ -118,12 +118,13 @@ namespace Bloom for (auto targetIt = targets.begin(); targetIt != targets.end(); targetIt++) { auto targetName = targetIt->toObject().find("targetName").value().toString() .toLower().toStdString(); + auto targetSignatureHex = mapIt.key().toLower().toStdString(); if (!mapping.contains(targetName)) { mapping.insert({ targetName, - [targetName]() { - return std::make_unique(targetName); + [targetName, targetSignatureHex]() { + return std::make_unique(targetName, TargetSignature(targetSignatureHex)); } }); } diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index e406d66c..d3a69b50 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -40,24 +40,27 @@ void Avr8::preActivationConfigure(const TargetConfig& targetConfig) { } void Avr8::postActivationConfigure() { - auto targetSignature = this->getId(); - auto partDescription = PartDescriptionFile( - targetSignature.toHex(), - (!this->name.empty()) ? std::optional(this->name) : std::nullopt - ); - auto pdSignature = partDescription.getTargetSignature(); - - if (targetSignature != pdSignature) { - // This should never happen. If it does, someone has screwed up the part description mapping file. - throw Exception("Failed to activate target - target signature mismatch.\nThe target signature (\"" - + targetSignature.toHex() + "\") does not match the AVR8 part description signature (\"" - + pdSignature.toHex() + "\"). Please review your target configuration in bloom.json"); + if (!this->partDescription.has_value()) { + this->loadPartDescription(); } - this->partDescription = partDescription; - this->id = partDescription.getTargetSignature(); - this->name = partDescription.getTargetName(); - this->family = partDescription.getFamily(); + /* + * The signature obtained from the device should match what is in the part description file + * + * We don't use this->getId() here as that could return the ID that was extracted from the part description file + * (which it would, if the user specified the exact target name in their project config - see Avr8::getId() and + * TargetController::getSupportedTargets() for more). + */ + auto targetSignature = this->avr8Interface->getDeviceId(); + auto pdSignature = this->partDescription->getTargetSignature(); + + if (targetSignature != pdSignature) { + throw Exception("Failed to validate connected target - target signature mismatch.\nThe target signature" + "(\"" + targetSignature.toHex() + "\") does not match the AVR8 part description signature (\"" + + pdSignature.toHex() + "\"). This will likely be due to an incorrect target name in the configuration file" + + " (bloom.json)." + ); + } } void Avr8::postPromotionConfigure() { @@ -66,6 +69,18 @@ void Avr8::postPromotionConfigure() { this->loadTargetVariants(); } +void Avr8::loadPartDescription() { + auto targetSignature = this->getId(); + auto partDescription = PartDescriptionFile( + targetSignature.toHex(), + (!this->name.empty()) ? std::optional(this->name) : std::nullopt + ); + + this->partDescription = partDescription; + this->name = partDescription.getTargetName(); + this->family = partDescription.getFamily(); +} + void Avr8::loadPadDescriptors() { auto& targetParameters = this->getTargetParameters(); diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp index f7b93eda..4383f490 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp @@ -73,9 +73,13 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit */ virtual void loadTargetVariants(); + void loadPartDescription(); + public: explicit Avr8() = default; - Avr8(const std::string& name): name(name) {}; + Avr8(const std::string& name, const TargetSignature& signature): name(name) { + this->id = signature; + }; /* * The functions below implement the Target interface for AVR8 targets. diff --git a/src/Targets/Microchip/AVR/AVR8/PartDescription/PartDescriptionFile.cpp b/src/Targets/Microchip/AVR/AVR8/PartDescription/PartDescriptionFile.cpp index 14870465..86a54ad0 100644 --- a/src/Targets/Microchip/AVR/AVR8/PartDescription/PartDescriptionFile.cpp +++ b/src/Targets/Microchip/AVR/AVR8/PartDescription/PartDescriptionFile.cpp @@ -14,40 +14,40 @@ PartDescriptionFile::PartDescriptionFile(const std::string& targetSignatureHex, if (mapping.contains(qTargetSignatureHex)) { // We have a match for the target signature. auto descriptionFilesJsonArray = mapping.find(qTargetSignatureHex).value().toArray(); - auto descriptionFiles = std::vector(); + auto matchingDescriptionFiles = std::vector(); std::copy_if( descriptionFilesJsonArray.begin(), descriptionFilesJsonArray.end(), - std::back_inserter(descriptionFiles), + std::back_inserter(matchingDescriptionFiles), [&targetName] (const QJsonValue& value) { auto pdTargetName = value.toObject().find("targetName")->toString().toLower().toStdString(); return !targetName.has_value() || (targetName.has_value() && targetName.value() == pdTargetName); } ); - if (targetName.has_value() && descriptionFiles.empty()) { + if (targetName.has_value() && matchingDescriptionFiles.empty()) { throw Exception("Failed to resolve target description file for target \"" + targetName.value() + "\" - target signature \"" + targetSignatureHex + "\" does not belong to target with name \"" + targetName.value() + "\". Please review your bloom.json configuration."); } - if (descriptionFiles.size() == 1) { + if (matchingDescriptionFiles.size() == 1) { // Attempt to load the XML part description file auto descriptionFilePath = QString::fromStdString(Application::getApplicationDirPath()) + "/" - + descriptionFiles.front().toObject().find("targetDescriptionFilePath")->toString(); + + matchingDescriptionFiles.front().toObject().find("targetDescriptionFilePath")->toString(); Logger::debug("Loading AVR8 part description file: " + descriptionFilePath.toStdString()); this->init(descriptionFilePath); - } else if (descriptionFiles.size() > 1) { + } else if (matchingDescriptionFiles.size() > 1) { /* * There are numerous part description files mapped to this target signature. There's really not * much we can do at this point, so we'll just instruct the user to use a more specific target name. */ QStringList targetNames; std::transform( - descriptionFiles.begin(), - descriptionFiles.end(), + matchingDescriptionFiles.begin(), + matchingDescriptionFiles.end(), std::back_inserter(targetNames), [](const QJsonValue& descriptionFile) { return QString("\"" + descriptionFile.toObject().find("targetName")->toString().toLower() + "\""); diff --git a/src/Targets/Target.hpp b/src/Targets/Target.hpp index 7b334d1a..002ec2a4 100644 --- a/src/Targets/Target.hpp +++ b/src/Targets/Target.hpp @@ -139,13 +139,12 @@ namespace Bloom::Targets * begin the target configuration and activation process, we are able to learn a lot more about the target. * For AVR8 targets, we extract the target signature shortly after activation, and with that signature we find * the appropriate part description file, which has all of the information regarding the target that we could - * possibly need. - * So, by the time we have activated the target, we will know a lot more about it, and it is at this point, where - * we may want to promote it to a more specific target class (from the generic Avr8 target class). The generic - * AVR8 target class will attempt to promote the target to one that is more specific to the target's AVR8 family - * (ATmega, XMega, Tiny, etc). These classes can then also perform promotion of their own, if required, where - * they could promote to a class that's not only specific to an AVR8 family, but to a particular target model (for - * example, a target class that was written specifically for the ATmega328P target). + * possibly need. So, by the time we have activated the target, we will know a lot more about it, and it is at + * this point, where we may want to promote it to a more specific target class (from the generic Avr8 target + * class). The generic AVR8 target class will attempt to promote the target to one that is more specific to + * the target's AVR8 family (ATmega, XMega, Tiny, etc). These classes can then also perform promotion of their + * own, if required, where they could promote to a class that's not only specific to an AVR8 family, but to a + * particular target model (for example, a target class that was written specifically for the ATmega328P target). * * This method should attempt to promote the current target class to one that is more specific to the connected * target, with the information it currently holds on the target.