Replaced messy AVR8 target config approach with new Avr8TargetConfig object

src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp

@@ -53,14 +53,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
     using Bloom::Targets::TargetRegisterType;
     using Bloom::Targets::TargetRegisters;
 
-    void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) {
+    void EdbgAvr8Interface::configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) {
-        this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE);
+        this->targetConfig = targetConfig;
 
-        if (targetConfig.jsonObject.contains("disableDebugWirePreDisconnect")) {
+        this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE);
-            this->disableDebugWireOnDeactivate = targetConfig.jsonObject.find(
-                "disableDebugWirePreDisconnect"
-            )->toBool();
-        }
     }
 
     void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& config) {
@@ -158,7 +154,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
 
         this->setParameter(
             Avr8EdbgParameters::PHYSICAL_INTERFACE,
-            getAvr8PhysicalInterfaceToIdMapping().at(this->physicalInterface)
+            getAvr8PhysicalInterfaceToIdMapping().at(this->targetConfig->physicalInterface)
         );
     }
 
@@ -225,7 +221,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
                 this->activatePhysical();
 
             } catch (const Avr8CommandFailure& activationException) {
-                if (this->physicalInterface == PhysicalInterface::DEBUG_WIRE
+                if (this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE
                     && activationException.code == Avr8CommandFailureCode::DEBUGWIRE_PHYSICAL_ERROR
                 ) {
                     throw DebugWirePhysicalInterfaceError(
@@ -246,7 +242,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
 
     void EdbgAvr8Interface::deactivate() {
         if (this->targetAttached) {
-            if (this->physicalInterface == PhysicalInterface::DEBUG_WIRE && this->disableDebugWireOnDeactivate) {
+            if (
+                this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE
+                && this->targetConfig->disableDebugWireOnDeactivate
+            ) {
                 try {
                     this->disableDebugWire();
                     Logger::warning(
@@ -336,7 +335,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
             throw Avr8CommandFailure("AVR8 Get device ID command failed", response);
         }
 
-        return response.extractSignature(this->physicalInterface);
+        return response.extractSignature(this->targetConfig->physicalInterface);
     }
 
     void EdbgAvr8Interface::setBreakpoint(std::uint32_t address) {

src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp

@@ -81,11 +81,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          *
          * @param targetConfig
          */
-        void configure(const TargetConfig& targetConfig) override;
+        void configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) override;
-
-        void setPhysicalInterface(Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface) override {
-            this->physicalInterface = physicalInterface;
-        }
 
         /**
          * Configures the target family. For some physical interfaces, the target family is required in order
@@ -263,6 +259,11 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         EdbgInterface& edbgInterface;
 
+        /**
+         * Project's AVR8 target configuration.
+         */
+        std::optional<Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig> targetConfig;
+
         /**
          * The target family is taken into account when configuring the AVR8 Generic protocol on the EDBG device.
          *
@@ -284,13 +285,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         Avr8ConfigVariant configVariant = Avr8ConfigVariant::NONE;
 
-        /**
-         * Currently, the AVR8 Generic protocol supports 4 physical interfaces: debugWire, JTAG, PDI and UPDI.
-         * The desired physical interface must be selected by setting the "AVR8_PHY_PHYSICAL" parameter.
-         */
-        Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface =
-            Targets::Microchip::Avr::Avr8Bit::PhysicalInterface::DEBUG_WIRE;
-
         /**
          * EDBG-based debug tools require target specific parameters such as memory locations, page sizes and
          * register addresses. It is the AVR8 target's responsibility to obtain the required information and pass it
@@ -332,24 +326,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         bool targetAttached = false;
 
-        /**
-         * Because the debugWire module requires control of the reset pin on the target, enabling this module will
-         * effectively mean losing control of the reset pin. This means users won't be able to use other
-         * interfaces that require access to the reset pin, such as ISP, until the debugWire module is disabled.
-         *
-         * The AVR8 Generic protocol provides a function for temporarily disabling the debugWire module on the target.
-         * This doesn't change the DWEN fuse and its affect is only temporary - the debugWire module will be
-         * reactivated upon the user cycling the power to the target.
-         *
-         * Bloom is able to temporarily disable the debugWire module, automatically, upon deactivating of the
-         * target (which usually occurs after a debug session has ended). This allows users to program the target via
-         * ISP, after they've finished a debug session. After programming the target, the user will need to cycle the
-         * target power before Bloom can gain access for another debug session.
-         *
-         * See disableDebugWire() method below.
-         */
-        bool disableDebugWireOnDeactivate = false;
-
         /**
          * This mapping allows us to determine which config variant to select, based on the target family and the
          * selected physical interface.
@@ -415,8 +391,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
                     auto configVariantsByPhysicalInterface = configVariantsByFamily
                         .at(this->family.value());
 
-                    if (configVariantsByPhysicalInterface.contains(this->physicalInterface)) {
+                    if (configVariantsByPhysicalInterface.contains(this->targetConfig->physicalInterface)) {
-                        return configVariantsByPhysicalInterface.at(this->physicalInterface);
+                        return configVariantsByPhysicalInterface.at(this->targetConfig->physicalInterface);
                     }
                 }
 
@@ -441,8 +417,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
                     {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI},
                 };
 
-                if (physicalInterfacesToConfigVariants.contains(this->physicalInterface)) {
+                if (physicalInterfacesToConfigVariants.contains(this->targetConfig->physicalInterface)) {
-                    return physicalInterfacesToConfigVariants.at(this->physicalInterface);
+                    return physicalInterfacesToConfigVariants.at(this->targetConfig->physicalInterface);
                 }
             }
 

src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp

@@ -3,6 +3,8 @@
 #include <cstdint>
 #include <set>
 
+#include "src/Targets/Microchip/AVR/AVR8/Avr8TargetConfig.hpp"
+
 #include "src/Targets/Microchip/AVR/TargetSignature.hpp"
 #include "src/Targets/Microchip/AVR/AVR8/Family.hpp"
 #include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp"
@@ -11,7 +13,6 @@
 #include "src/Targets/TargetState.hpp"
 #include "src/Targets/TargetRegister.hpp"
 #include "src/Targets/TargetMemory.hpp"
-#include "src/ProjectConfig.hpp"
 
 namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8
 {
@@ -42,12 +43,9 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8
          * Configures the interface. Any debug tool -> target interface specific configuration should take
          * place here.
          *
-         * For example, the EdbgAvr8Interface implementation configures the physical interface and config
-         * variant here.
-         *
          * @param targetConfig
          */
-        virtual void configure(const TargetConfig& targetConfig) = 0;
+        virtual void configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) = 0;
 
         /**
          * Sets the target family, independent of other configuration.
@@ -56,13 +54,6 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8
          */
         virtual void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) = 0;
 
-        /**
-         * Sets the selected physical interface.
-         *
-         * @param physicalInterface
-         */
-        virtual void setPhysicalInterface(Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface) = 0;
-
         /**
          * Should accept Avr8 target parameters for configuration of the interface.
          *

src/Targets/Microchip/AVR/AVR8/Avr8.cpp

@@ -28,27 +28,13 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
     void Avr8::preActivationConfigure(const TargetConfig& targetConfig) {
         Target::preActivationConfigure(targetConfig);
 
-        auto physicalInterface = targetConfig.jsonObject.find("physicalInterface")->toString().toLower().toStdString();
+        this->targetConfig = Avr8TargetConfig(targetConfig);
-        auto availablePhysicalInterfaces = Avr8::getPhysicalInterfacesByName();
-
-        if (physicalInterface.empty() || !availablePhysicalInterfaces.contains(physicalInterface)) {
-            throw InvalidConfig("Invalid or missing physical interface config parameter for AVR8 target.");
-        }
-
-        const auto selectedPhysicalInterface = availablePhysicalInterfaces.at(physicalInterface);
-
-        if (selectedPhysicalInterface == PhysicalInterface::DEBUG_WIRE) {
-            Logger::warning("AVR8 debugWire interface selected - the DWEN fuse will need to be enabled");
-        }
-
-        this->physicalInterface = selectedPhysicalInterface;
-        this->avr8DebugInterface->setPhysicalInterface(this->physicalInterface.value());
 
         if (this->family.has_value()) {
             this->avr8DebugInterface->setFamily(this->family.value());
 
         } else {
-            if (this->physicalInterface == PhysicalInterface::JTAG) {
+            if (this->targetConfig->physicalInterface == PhysicalInterface::JTAG) {
                 throw InvalidConfig(
                     "The JTAG physical interface cannot be used with an ambiguous target name"
                     " - please specify the exact name of the target in your configuration file. "
@@ -56,7 +42,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
                 );
             }
 
-            if (this->physicalInterface == PhysicalInterface::UPDI) {
+            if (this->targetConfig->physicalInterface == PhysicalInterface::UPDI) {
                 throw InvalidConfig(
                     "The UPDI physical interface cannot be used with an ambiguous target name"
                     " - please specify the exact name of the target in your configuration file. "
@@ -65,29 +51,17 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
             }
         }
 
-        if (targetConfig.jsonObject.contains("updateDwenFuseBit")) {
+        if (
-            this->updateDwenFuseBit = targetConfig.jsonObject.value(
+            this->targetConfig->updateDwenFuseBit && this->avrIspInterface == nullptr
-                "updateDwenFuseBit"
+            && this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE
-            ).toBool();
-
-            if (this->updateDwenFuseBit
-                && this->avrIspInterface == nullptr
-                && this->physicalInterface == PhysicalInterface::DEBUG_WIRE
         ) {
             Logger::warning(
                 "The connected debug tool (or associated driver) does not provide any ISP interface. "
                 "Bloom will be unable to update the DWEN fuse bit in the event of a debugWire activation failure."
             );
         }
-        }
 
-        if (targetConfig.jsonObject.contains("cycleTargetPowerPostDwenUpdate")) {
+        this->avr8DebugInterface->configure(this->targetConfig.value());
-            this->cycleTargetPowerPostDwenUpdate = targetConfig.jsonObject.value(
-                "cycleTargetPowerPostDwenUpdate"
-            ).toBool();
-        }
-
-        this->avr8DebugInterface->configure(targetConfig);
 
         if (this->avrIspInterface != nullptr) {
             this->avrIspInterface->configure(targetConfig);
@@ -144,7 +118,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
             this->avr8DebugInterface->activate();
 
         } catch (const Exceptions::DebugWirePhysicalInterfaceError& debugWireException) {
-            if (!this->updateDwenFuseBit) {
+            if (!this->targetConfig->updateDwenFuseBit) {
                 throw TargetOperationFailure(
                     "Failed to activate debugWire physical interface - check target connection and DWEN fuse "
                         "bit. Bloom can manage the DWEN fuse bit automatically. For instructions on enabling this "
@@ -160,7 +134,10 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
                 this->writeDwenFuseBit(true);
 
                 // If the debug tool provides a TargetPowerManagementInterface, attempt to cycle the target power
-                if (this->targetPowerManagementInterface != nullptr && this->cycleTargetPowerPostDwenUpdate) {
+                if (
+                    this->targetPowerManagementInterface != nullptr
+                    && this->targetConfig->cycleTargetPowerPostDwenUpdate
+                ) {
                     Logger::info("Cycling target power");
 
                     Logger::debug("Disabling target power");

src/Targets/Microchip/AVR/AVR8/Avr8.hpp

@@ -17,7 +17,7 @@
 
 #include "TargetDescription/TargetDescriptionFile.hpp"
 
-#include "src/ProjectConfig.hpp"
+#include "Avr8TargetConfig.hpp"
 
 namespace Bloom::Targets::Microchip::Avr::Avr8Bit
 {
@@ -127,9 +127,10 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* avr8DebugInterface = nullptr;
         DebugToolDrivers::TargetInterfaces::Microchip::Avr::AvrIspInterface* avrIspInterface = nullptr;
 
+        std::optional<Avr8TargetConfig> targetConfig;
+
         std::string name;
         std::optional<Family> family;
-        std::optional<PhysicalInterface> physicalInterface;
         std::optional<TargetDescription::TargetDescriptionFile> targetDescriptionFile;
 
         std::optional<TargetParameters> targetParameters;
@@ -138,25 +139,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         std::map<TargetRegisterType, TargetRegisterDescriptors> targetRegisterDescriptorsByType;
         std::map<TargetMemoryType, TargetMemoryDescriptor> targetMemoryDescriptorsByType;
 
-        bool updateDwenFuseBit = false;
-        bool cycleTargetPowerPostDwenUpdate = true;
-
-        /**
-         * Users are required to set their desired physical interface in their Bloom configuration. This would take
-         * the form of a string, so we map the available options to the appropriate enums.
-         */
-        static inline auto getPhysicalInterfacesByName() {
-            using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface;
-
-            return std::map<std::string, PhysicalInterface>({
-                {"debugwire", PhysicalInterface::DEBUG_WIRE},
-                {"debug-wire", PhysicalInterface::DEBUG_WIRE},
-                {"pdi", PhysicalInterface::PDI},
-                {"jtag", PhysicalInterface::JTAG},
-                {"updi", PhysicalInterface::UPDI},
-            });
-        };
-
         /**
          * Resolves the appropriate TDF for the AVR8 target and populates this->targetDescriptionFile.
          */