From 6bea419e1be57d1625e4d299ed7d9f06f7c13b66 Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Thu, 4 Aug 2022 21:07:16 +0100
Subject: [PATCH] Added check to confirm the user's selected AVR8 physical
 interface is supported by the selected target

---
 src/Targets/CMakeLists.txt                    |  1 +
 src/Targets/Microchip/AVR/AVR8/Avr8.cpp       | 41 +++++++++++++++++++
 .../Microchip/AVR/AVR8/PhysicalInterface.cpp  | 14 +++++++
 .../Microchip/AVR/AVR8/PhysicalInterface.hpp  |  9 ++++
 4 files changed, 65 insertions(+)
 create mode 100644 src/Targets/Microchip/AVR/AVR8/PhysicalInterface.cpp

diff --git a/src/Targets/CMakeLists.txt b/src/Targets/CMakeLists.txt
index f2da2b71..2fb2b4d2 100755
--- a/src/Targets/CMakeLists.txt
+++ b/src/Targets/CMakeLists.txt
@@ -5,5 +5,6 @@ target_sources(
         ${CMAKE_CURRENT_SOURCE_DIR}/TargetRegister.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/AVR/AVR8/Avr8.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/AVR/AVR8/Avr8TargetConfig.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/AVR/AVR8/PhysicalInterface.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp
 )
diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp
index 0a51b12e..4b2b0448 100644
--- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp
+++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp
@@ -4,6 +4,7 @@
 #include <bitset>
 #include <limits>
 #include <thread>
+#include <algorithm>
 
 #include "src/Logger/Logger.hpp"
 #include "src/Helpers/Paths.hpp"
@@ -32,6 +33,46 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         if (this->family.has_value()) {
             this->avr8DebugInterface->setFamily(this->family.value());
 
+            if (!this->supportedPhysicalInterfaces.contains(this->targetConfig->physicalInterface)) {
+                /*
+                 * The user has selected a physical interface that does not appear to be supported by the selected
+                 * target.
+                 *
+                 * Bloom's target description files provide a list of supported physical interfaces for each target
+                 * (which is how this->supportedPhysicalInterfaces is populated), but it's possible that this list may
+                 * be wrong/incomplete. For this reason, we don't throw an exception here. Instead, we just present the
+                 * user with a warning and a list of physical interfaces known to be supported by their selected target.
+                 */
+                const auto physicalInterfaceNames = getPhysicalInterfaceNames();
+
+                std::string supportedPhysicalInterfaceList = std::accumulate(
+                    this->supportedPhysicalInterfaces.begin(),
+                    this->supportedPhysicalInterfaces.end(),
+                    std::string(),
+                    [&physicalInterfaceNames] (const std::string& string, PhysicalInterface physicalInterface) {
+                        if (physicalInterface == PhysicalInterface::ISP) {
+                            /*
+                             * Don't include the ISP interface in the list of supported interfaces, as doing so may
+                             * mislead the user into thinking the ISP interface can be used for debugging operations.
+                             */
+                            return string;
+                        }
+
+                        return string + "\n - " + physicalInterfaceNames.at(physicalInterface);
+                    }
+                );
+
+                Logger::warning(
+                    "\nThe selected target (" + this->name + ") does not support the selected physical interface ("
+                        + physicalInterfaceNames.at(this->targetConfig->physicalInterface) + "). Target activation "
+                        "will likely fail. The target supports the following physical interfaces: \n"
+                        + supportedPhysicalInterfaceList + "\n\nFor physical interface configuration values, see "
+                        + Paths::homeDomainName() + "/docs/configuration/avr8-physical-interfaces. \n\nIf this "
+                        "information is incorrect, please report this to Bloom developers via "
+                        + Paths::homeDomainName() + "/report-issue.\n"
+                );
+            }
+
         } else {
             if (this->targetConfig->physicalInterface == PhysicalInterface::JTAG) {
                 throw InvalidConfig(
diff --git a/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.cpp b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.cpp
new file mode 100644
index 00000000..1f1f5d14
--- /dev/null
+++ b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.cpp
@@ -0,0 +1,14 @@
+#include "PhysicalInterface.hpp"
+
+namespace Bloom::Targets::Microchip::Avr::Avr8Bit
+{
+    std::map<PhysicalInterface, std::string> getPhysicalInterfaceNames() {
+        return std::map<PhysicalInterface, std::string>({
+            {PhysicalInterface::ISP, "ISP"},
+            {PhysicalInterface::DEBUG_WIRE, "debugWire"},
+            {PhysicalInterface::PDI, "PDI"},
+            {PhysicalInterface::JTAG, "JTAG"},
+            {PhysicalInterface::UPDI, "UPDI"},
+        });
+    }
+}
diff --git a/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp
index ed765975..0a77997e 100644
--- a/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp
+++ b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp
@@ -1,6 +1,8 @@
 #pragma once
 
 #include <cstdint>
+#include <map>
+#include <string>
 
 namespace Bloom::Targets::Microchip::Avr::Avr8Bit
 {
@@ -12,4 +14,11 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         PDI,
         UPDI,
     };
+
+    /**
+     * Returns a mapping of physical interfaces to their marketing name.
+     *
+     * @return
+     */
+    std::map<PhysicalInterface, std::string> getPhysicalInterfaceNames();
 }