Fixed issue with automatic config variant selection, where we were not properly handling XMEGA targets with the JTAG physical interface.

Also introduced new AVR8 families, for D series targets.
Also moved AVR family param outside of TargetParameters struct

build/scripts/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php

@@ -10,6 +10,9 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
     const AVR8_FAMILY_MEGA = 'MEGA';
     const AVR8_FAMILY_TINY = 'TINY';
     const AVR8_FAMILY_XMEGA = 'XMEGA';
+    const AVR8_FAMILY_DB = 'DB';
+    const AVR8_FAMILY_DA = 'DA';
+    const AVR8_FAMILY_DD = 'DD';
     const AVR8_FAMILY_OTHER = 'OTHER';
 
     const AVR8_PHYSICAL_INTERFACE_JTAG = 'JTAG';
@@ -77,6 +80,15 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
             } else if (stristr($deviceAttributes['family'], 'mega') !== false) {
                 $this->family = self::AVR8_FAMILY_MEGA;
 
+            } else if (strtolower(trim($deviceAttributes['family'])) == 'avr da') {
+                $this->family = self::AVR8_FAMILY_DA;
+
+            } else if (strtolower(trim($deviceAttributes['family'])) == 'avr db') {
+                $this->family = self::AVR8_FAMILY_DB;
+
+            } else if (strtolower(trim($deviceAttributes['family'])) == 'avr dd') {
+                $this->family = self::AVR8_FAMILY_DD;
+
             } else {
                 $this->family = self::AVR8_FAMILY_OTHER;
             }
@@ -338,6 +350,10 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
             return $failures;
         }
 
+        if (is_null($this->family) || $this->family == self::AVR8_FAMILY_OTHER) {
+            $failures[] = 'Unknown AVR8 family';
+        }
+
         if (is_null($this->stackPointerRegisterStartAddress)) {
             $failures[] = 'Missing stack pointer register start address.';
         }

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

@@ -144,5 +144,4 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
         DATA = 0x84,
         FAILED = 0xA0,
     };
-
 }

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

@@ -69,7 +69,6 @@ void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) {
     auto physicalInterface = targetConfig.jsonObject.find("physicalInterface")->toString().toLower().toStdString();
 
     auto availablePhysicalInterfaces = EdbgAvr8Interface::physicalInterfacesByName;
-    auto availableConfigVariants = EdbgAvr8Interface::configVariantsByPhysicalInterface;
 
     if (physicalInterface.empty()
         || availablePhysicalInterfaces.find(physicalInterface) == availablePhysicalInterfaces.end()
@@ -79,16 +78,18 @@ void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) {
 
     auto selectedPhysicalInterface = availablePhysicalInterfaces.find(physicalInterface)->second;
 
-    if (availableConfigVariants.find(selectedPhysicalInterface) == availableConfigVariants.end()) {
-        throw InvalidConfig("Invalid config variant deduced from physical interface.");
-    }
-
     if (selectedPhysicalInterface == Avr8PhysicalInterface::DEBUG_WIRE) {
         Logger::warning("AVR8 debugWire interface selected - the DWEN fuse will need to be enabled");
     }
 
     this->physicalInterface = selectedPhysicalInterface;
-    this->configVariant = availableConfigVariants.find(selectedPhysicalInterface)->second;
+
+    if (this->physicalInterface == Avr8PhysicalInterface::JTAG && !this->family.has_value()) {
+        throw InvalidConfig("Cannot use JTAG physical interface with ambiguous target name - please specify the"
+            " exact name of the target in your configuration file. See https://bloom.oscillate.io/docs/supported-targets");
+    }
+
+    this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE);
 }
 
 void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& config) {
@@ -110,6 +111,18 @@ void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& con
         throw Exception("Failed to find status register size");
     }
 
+    if (this->configVariant == Avr8ConfigVariant::NONE) {
+        auto configVariant = this->resolveConfigVariant();
+
+        if (!configVariant.has_value()) {
+            throw Exception("Failed to resolve config variant for the selected "
+                "physical interface and AVR8 family. The selected physical interface is not known to be supported "
+                "by the AVR8 family.");
+        }
+
+        this->configVariant = configVariant.value();
+    }
+
     if (this->configVariant == Avr8ConfigVariant::XMEGA) {
         if (!config.appSectionPdiOffset.has_value()) {
             throw Exception("Missing required parameter: APPL_BASE_ADDR");
@@ -850,7 +863,7 @@ TargetRegisters EdbgAvr8Interface::readGeneralPurposeRegisters(std::set<std::siz
     auto output = TargetRegisters();
 
     auto registers = this->readMemory(
-        this->targetParameters.family == Family::XMEGA ? Avr8MemoryType::REGISTER_FILE : Avr8MemoryType::SRAM,
+        this->family == Family::XMEGA ? Avr8MemoryType::REGISTER_FILE : Avr8MemoryType::SRAM,
         this->targetParameters.gpRegisterStartAddress.value_or(0x00),
         this->targetParameters.gpRegisterSize.value_or(32)
     );

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

@@ -41,6 +41,14 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         EdbgInterface& edbgInterface;
 
+        /**
+         * The target family is taken into account when configuring the AVR8 Generic protocol on the EDBG device.
+         *
+         * We use this to determine which config variant to select.
+         * See EdbgAvr8Interface::resolveConfigVariant() for more.
+         */
+        std::optional<Targets::Microchip::Avr::Avr8Bit::Family> family;
+
         /**
          * The AVR8 Generic protocol provides two functions: Debugging and programming. The desired function must be
          * configured via the setting of the "AVR8_CONFIG_FUNCTION" parameter.
@@ -52,13 +60,13 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          * The "AVR8_CONFIG_VARIANT" parameter allows us to determine which target parameters are required by the
          * debug tool.
          */
-        Avr8ConfigVariant configVariant;
+        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.
          */
-        Avr8PhysicalInterface physicalInterface;
+        Avr8PhysicalInterface physicalInterface = Avr8PhysicalInterface::NONE;
 
         /**
          * EDBG-based debug tools require target specific parameters such as memory locations, page sizes and
@@ -123,17 +131,100 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
         };
 
         /**
-         * Although users can supply the desired config variant via their Bloom configuration, this is not required.
+         * This mapping allows us to determine which config variant to select, based on the target family and the
-         * This mapping allows us to determine which config variant to select, based on the selected physical
+         * selected physical interface.
-         * interface.
          */
-        static inline std::map<Avr8PhysicalInterface, Avr8ConfigVariant> configVariantsByPhysicalInterface = {
+        static inline auto getConfigVariantsByFamilyAndPhysicalInterface() {
+            using Targets::Microchip::Avr::Avr8Bit::Family;
+            return std::map<Family, std::map<Avr8PhysicalInterface, Avr8ConfigVariant>>({
+                {
+                    Family::MEGA,
+                    {
+                        {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG},
+                        {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE},
+                    }
+                },
+                {
+                    Family::TINY,
+                    {
+                        {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG},
+                        {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE},
+                    }
+                },
+                {
+                    Family::XMEGA,
+                    {
+                        {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::XMEGA},
+                        {Avr8PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA},
+                    }
+                },
+                {
+                    Family::DA,
+                    {
+                        {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI},
+                    }
+                },
+                {
+                    Family::DB,
+                    {
+                        {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI},
+                    }
+                },
+                {
+                    Family::DD,
+                    {
+                        {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI},
+                    }
+                },
+            });
+        };
+
+        /**
+         * Will attempt to resolve the config variant with the information currently held.
+         *
+         * @return
+         */
+        std::optional<Avr8ConfigVariant> resolveConfigVariant() {
+            if (this->family.has_value()) {
+                auto configVariantsByFamily = EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface();
+
+                if (configVariantsByFamily.contains(this->family.value())) {
+                    auto configVariantsByPhysicalInterface = configVariantsByFamily
+                        .at(this->family.value());
+
+                    if (configVariantsByPhysicalInterface.contains(this->physicalInterface)) {
+                        return configVariantsByPhysicalInterface.at(this->physicalInterface);
+                    }
+                }
+
+            } else {
+                /*
+                 * If there is no family set, we may be able to resort to a simpler mapping of physical interfaces
+                 * to config variants. But this will only work if the selected physical interface is *NOT* JTAG.
+                 *
+                 * This is because JTAG is the only physical interface that could map to two different config
+                 * variants (MEGAJTAG and XMEGA). The only way we can figure out which config variant to use is if we
+                 * know the target family.
+                 *
+                 * This is why we don't allow users to use ambiguous target names (such as the generic "avr8" target
+                 * name), when using the JTAG physical interface. We won't be able to resolve the correct target
+                 * variant. Users are required to specify the exact target name in their config, when using the JTAG
+                 * physical interface. That way, this->family will be set by the time resolveConfigVariant() is called.
+                 */
+                static std::map<Avr8PhysicalInterface, Avr8ConfigVariant> physicalInterfacesToConfigVariants = {
                     {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE},
                     {Avr8PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA},
-            {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG},
                     {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI},
                 };
 
+                if (physicalInterfacesToConfigVariants.contains(this->physicalInterface)) {
+                    return physicalInterfacesToConfigVariants.at(this->physicalInterface);
+                }
+            }
+
+            return std::nullopt;
+        }
+
         /**
          * Sets an AVR8 parameter on the debug tool. See the Avr8EdbgParameters class and protocol documentation
          * for more on available parameters.
@@ -338,6 +429,16 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         virtual void configure(const TargetConfig& targetConfig) override;
 
+        /**
+         * Configures the target family. For some physical interfaces, the target family is required in order
+         * properly configure the EDBG tool. See EdbgAvr8Interface::resolveConfigVariant() for more.
+         *
+         * @param family
+         */
+        void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) override {
+            this->family = family;
+        }
+
         /**
          * Accepts target parameters from the AVR8 target instance and sends the necessary target parameters to the
          * debug tool.

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

@@ -5,6 +5,7 @@
 
 #include "src/Targets/Microchip/AVR/TargetSignature.hpp"
 #include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp"
+#include "src/Targets/Microchip/AVR/AVR8/Family.hpp"
 #include "src/Targets/TargetState.hpp"
 #include "src/Targets/TargetRegister.hpp"
 #include "src/Targets/TargetMemory.hpp"
@@ -36,6 +37,13 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8
          */
         virtual void configure(const TargetConfig& targetConfig) = 0;
 
+        /**
+         * Sets the target family, independent of other configuration.
+         *
+         * @param family
+         */
+        virtual void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) = 0;
+
         /**
          * Should accept Avr8 target parameters for configuration of the interface.
          *

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

@@ -33,6 +33,10 @@ using namespace Exceptions;
 void Avr8::preActivationConfigure(const TargetConfig& targetConfig) {
     Target::preActivationConfigure(targetConfig);
 
+    if (this->family.has_value()) {
+        this->avr8Interface->setFamily(this->family.value());
+    }
+
     this->avr8Interface->configure(targetConfig);
 }
 
@@ -61,6 +65,11 @@ void Avr8::postActivationConfigure() {
 }
 
 void Avr8::postPromotionConfigure() {
+    if (!this->family.has_value()) {
+        throw Exception("Failed to resolve AVR8 family");
+    }
+
+    this->avr8Interface->setFamily(this->family.value());
     this->avr8Interface->setTargetParameters(this->getTargetParameters());
 }
 
@@ -276,7 +285,6 @@ TargetParameters& Avr8::getTargetParameters() {
     if (!this->targetParameters.has_value()) {
         assert(this->targetDescriptionFile.has_value());
         this->targetParameters = TargetParameters();
-        this->targetParameters->family = this->family;
         auto& propertyGroups = this->targetDescriptionFile->getPropertyGroupsMappedByName();
 
         auto flashMemorySegment = this->targetDescriptionFile->getFlashMemorySegment();

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

@@ -62,6 +62,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         explicit Avr8() = default;
         Avr8(const std::string& name, const TargetSignature& signature): name(name) {
             this->id = signature;
+            this->loadTargetDescriptionFile();
         };
 
         /*

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

@@ -7,5 +7,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         MEGA,
         XMEGA,
         TINY,
+        DB,
+        DA,
+        DD,
     };
 }

src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp

@@ -141,9 +141,9 @@ Family TargetDescriptionFile::getFamily() const {
         throw Exception("Could not find target family name in target description file.");
     }
 
-    if (familyNameToEnums.find(familyName) == familyNameToEnums.end()) {
+    if (!familyNameToEnums.contains(familyName)) {
         throw Exception("Unknown family name in target description file.");
     }
 
-    return familyNameToEnums.find(familyName)->second;
+    return familyNameToEnums.at(familyName);
 }

src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp

@@ -36,6 +36,9 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription
                 {"avr tiny", Family::TINY},
                 {"tinyavr", Family::TINY},
                 {"tinyavr 2", Family::TINY},
+                {"avr da", Family::DA},
+                {"avr db", Family::DB},
+                {"avr dd", Family::DD},
             };
         };
 

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

@@ -10,8 +10,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
 {
     struct TargetParameters
     {
-        std::optional<Family> family;
-
         std::optional<std::uint32_t> bootSectionStartAddress;
         std::optional<std::uint32_t> gpRegisterStartAddress;
         std::optional<std::uint32_t> gpRegisterSize;