From a971e92a589dbff54a868c41dfd19e7b105245c6 Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Sun, 8 Dec 2024 23:33:39 +0000
Subject: [PATCH] Tidying

---
 .../Protocols/WchLink/WchLinkInterface.hpp    | 10 ++++----
 .../WCH/WchLinkDebugInterface.cpp             | 24 +++++++++++++++----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp
index 01bef226..ed1e6901 100644
--- a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp
+++ b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp
@@ -56,6 +56,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink
 
         template <class CommandType>
         auto sendCommandAndWaitForResponse(const CommandType& command) {
+            using Services::StringService;
             const auto rawCommand = command.getRawCommand();
 
             /*
@@ -86,14 +87,15 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink
             }
 
             if (rawResponse[0] == 0x81) {
-                throw Exceptions::DeviceCommunicationFailure{"Error response"};
+                throw Exceptions::DeviceCommunicationFailure{
+                    "Error response to command 0x" + StringService::toHex(command.commandId)
+                };
             }
 
             if (rawResponse[1] != command.commandId) {
                 throw Exceptions::DeviceCommunicationFailure{
-                    "Missing/invalid command ID in response from device 0x"
-                        + Services::StringService::toHex(rawResponse[1]) + " - expected: 0x"
-                        + Services::StringService::toHex(command.commandId)
+                    "Missing/invalid command ID in response from device 0x" + StringService::toHex(rawResponse[1])
+                        + " - expected: 0x" + StringService::toHex(command.commandId)
                 };
             }
 
diff --git a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp
index 6c5208e9..6571f4cb 100644
--- a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp
+++ b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp
@@ -239,8 +239,8 @@ namespace DebugToolDrivers::Wch
                  * command.
                  *
                  * This sometimes leads to exceptions occurring on the target, when the program buffer contains certain
-                 * instructions before the partial block write command is serviced. This is why we clear the program
-                 * buffer before invoking the partial block write command.
+                 * instructions before the partial block write command is invoked. This is why we clear the program
+                 * buffer beforehand.
                  */
                 this->riscVTranslator.clearProgramBuffer();
                 this->wchLinkInterface.writeFlashPartialBlock(startAddress, buffer);
@@ -324,6 +324,16 @@ namespace DebugToolDrivers::Wch
                 this->flashProgramOpcodes
             );
 
+            /*
+             * Would this not be better placed in endProgrammingSession()? We could persist the command type we invoked
+             * to perform the write, and if required, reattach at the end of the programming session.
+             *
+             * I don't think that would work, because the target needs to be accessible for other operations whilst in
+             * programming mode. We may perform other operations in between program memory writes, but that wouldn't
+             * work if we left the target in an inaccessible state between writes. So I think we have to reattach here.
+             *
+             * TODO: Review after v2.0.0.
+             */
             this->deactivate();
             this->wchLinkInterface.sendCommandAndWaitForResponse(Commands::Control::GetDeviceInfo{});
             this->activate();
@@ -350,7 +360,13 @@ namespace DebugToolDrivers::Wch
     }
 
     void WchLinkDebugInterface::enableProgrammingMode() {
-        // Nothing to do here
+        /*
+         * Nothing to do here
+         *
+         * We cannot prepare the WCH-Link tool for a programming session here, as the preparation process differs
+         * across the two types of flash write commands (full and partial block write). We don't know which command
+         * we'll be utilising at this point, so we perform the preparation in writeMemory().
+         */
     }
 
     void WchLinkDebugInterface::disableProgrammingMode() {
@@ -360,7 +376,7 @@ namespace DebugToolDrivers::Wch
     void WchLinkDebugInterface::applyAccessRestrictions(TargetMemorySegmentDescriptor& memorySegmentDescriptor) {
         if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) {
             /*
-             * Actually, we *can* write to flash memory whilst in debug mode (via a partial page write), but we don't
+             * Actually, we *can* write to flash memory whilst in debug mode (via a partial block write), but we don't
              * need to, so I'm just going to block it, for now.
              */
             memorySegmentDescriptor.debugModeAccess.writeable = false;