Tidied AVR GDB memory access command packet classes

CMakeLists.txt

@@ -144,7 +144,6 @@ add_executable(Bloom
     # AVR GDB Server
     src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp
     src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp
-    src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp
     src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp
     src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp
 

src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp

@@ -1,17 +0,0 @@
-#include "AbstractMemoryAccessPacket.hpp"
-
-#include "src/DebugServer/Gdb/ResponsePackets/TargetStopped.hpp"
-#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp"
-#include "src/DebugServer/Gdb/Signal.hpp"
-
-#include "src/Logger/Logger.hpp"
-#include "src/Exceptions/Exception.hpp"
-
-namespace Bloom::DebugServer::Gdb::CommandPackets
-{
-    using ResponsePackets::ErrorResponsePacket;
-
-    using Exceptions::Exception;
-
-
-}

src/DebugServer/Gdb/AvrGdb/CommandPackets/MemoryAccessCommandPacket.hpp

@@ -10,13 +10,21 @@
 namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
 {
     /**
-     * The ReadMemory class implements a structure for "m" packets. Upon receiving these packets, the server is
+     * The MemoryAccessCommandPacket class is a base class for memory access GDB commands that are specific to the AVR
-     * expected to read memory from the target and send it the client.
+     * architecture.
+     *
+     * With the GDB implementation for the AVR architecture, read/write memory commands include a special memory
+     * address. The memory type (FLASH, RAM, EEPROM, etc) is embedded within the 7 most significant bits of the memory
+     * address.
+     *
+     * This class provides functions to extract and remove the memory type from a given memory address.
      */
-    class AbstractMemoryAccessPacket: public Bloom::DebugServer::Gdb::CommandPackets::CommandPacket
+    class MemoryAccessCommandPacket: public Bloom::DebugServer::Gdb::CommandPackets::CommandPacket
     {
     public:
-        explicit AbstractMemoryAccessPacket(const std::vector<unsigned char>& rawPacket): CommandPacket(rawPacket) {};
+        explicit MemoryAccessCommandPacket(const std::vector<unsigned char>& rawPacket)
+            : CommandPacket(rawPacket)
+        {};
 
     protected:
         /**
@@ -32,7 +40,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
          * @return
          */
         Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) {
-            if ((address & AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U) {
+            if ((address & MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U) {
                 return Targets::TargetMemoryType::RAM;
             }
 
@@ -46,8 +54,8 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
          * @return
          */
         std::uint32_t removeMemoryTypeIndicatorFromGdbAddress(std::uint32_t address) {
-            return (address & AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U
+            return (address & MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U
-                ? (address & ~(AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK))
+                ? (address & ~(MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK))
                 : address;
         }
     };

src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp

@@ -14,7 +14,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
     using Exceptions::Exception;
 
     ReadMemory::ReadMemory(const std::vector<unsigned char>& rawPacket)
-        : AbstractMemoryAccessPacket(rawPacket)
+        : MemoryAccessCommandPacket(rawPacket)
     {
         if (this->data.size() < 4) {
             throw Exception("Invalid packet length");
@@ -38,12 +38,15 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
         }
 
         bool conversionStatus = false;
-        this->startAddress = packetSegments.at(0).toUInt(&conversionStatus, 16);
+        const auto gdbStartAddress = packetSegments.at(0).toUInt(&conversionStatus, 16);
 
         if (!conversionStatus) {
             throw Exception("Failed to parse start address from read memory packet data");
         }
 
+        this->memoryType = this->getMemoryTypeFromGdbAddress(gdbStartAddress);
+        this->startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(gdbStartAddress);
+
         this->bytes = packetSegments.at(1).toUInt(&conversionStatus, 16);
 
         if (!conversionStatus) {
@@ -55,9 +58,11 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
         Logger::debug("Handling ReadMemory packet");
 
         try {
-            auto memoryType = this->getMemoryTypeFromGdbAddress(this->startAddress);
+            auto memoryBuffer = targetControllerConsole.readMemory(
-            auto startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(this->startAddress);
+                this->memoryType,
-            auto memoryBuffer = targetControllerConsole.readMemory(memoryType, startAddress, this->bytes);
+                this->startAddress,
+                this->bytes
+            );
 
             auto hexMemoryBuffer = Packet::dataToHex(memoryBuffer);
             debugSession.connection.writePacket(

src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.hpp

@@ -3,7 +3,7 @@
 #include <cstdint>
 #include <optional>
 
-#include "AbstractMemoryAccessPacket.hpp"
+#include "MemoryAccessCommandPacket.hpp"
 
 namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
 {
@@ -11,15 +11,19 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
      * The ReadMemory class implements a structure for "m" packets. Upon receiving these packets, the server is
      * expected to read memory from the target and send it the client.
      */
-    class ReadMemory: public AbstractMemoryAccessPacket
+    class ReadMemory: public MemoryAccessCommandPacket
     {
     public:
         /**
-         * The startAddress sent from the GDB client will include additional bits used to indicate the memory type.
+         * Start address of the memory operation.
-         * These bits have to be removed from the address before it can be used as a start address.
          */
         std::uint32_t startAddress = 0;
 
+        /**
+         * The type of memory to read from.
+         */
+        Targets::TargetMemoryType memoryType = Targets::TargetMemoryType::FLASH;
+
         /**
          * Number of bytes to read.
          */

src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp

@@ -14,7 +14,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
     using namespace Bloom::Exceptions;
 
     WriteMemory::WriteMemory(const std::vector<unsigned char>& rawPacket)
-        : AbstractMemoryAccessPacket(rawPacket)
+        : MemoryAccessCommandPacket(rawPacket)
     {
         if (this->data.size() < 4) {
             throw Exception("Invalid packet length");
@@ -37,12 +37,15 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
         }
 
         bool conversionStatus = false;
-        this->startAddress = packetSegments.at(0).toUInt(&conversionStatus, 16);
+        const auto gdbStartAddress = packetSegments.at(0).toUInt(&conversionStatus, 16);
 
         if (!conversionStatus) {
             throw Exception("Failed to parse start address from write memory packet data");
         }
 
+        this->memoryType = this->getMemoryTypeFromGdbAddress(gdbStartAddress);
+        this->startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(gdbStartAddress);
+
         auto lengthAndBufferSegments = packetSegments.at(1).split(":");
         if (lengthAndBufferSegments.size() != 2) {
             throw Exception(
@@ -67,16 +70,17 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
         Logger::debug("Handling WriteMemory packet");
 
         try {
-            const auto memoryType = this->getMemoryTypeFromGdbAddress(this->startAddress);
+            if (this->memoryType == Targets::TargetMemoryType::FLASH) {
-
-            if (memoryType == Targets::TargetMemoryType::FLASH) {
                 throw Exception(
                     "GDB client requested a flash memory write - This is not currently supported by Bloom."
                 );
             }
 
-            const auto startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(this->startAddress);
+            targetControllerConsole.writeMemory(
-            targetControllerConsole.writeMemory(memoryType, startAddress, this->buffer);
+                this->memoryType,
+                this->startAddress,
+                this->buffer
+            );
 
             debugSession.connection.writePacket(OkResponsePacket());
 

src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.hpp

@@ -3,7 +3,7 @@
 #include <cstdint>
 #include <optional>
 
-#include "AbstractMemoryAccessPacket.hpp"
+#include "MemoryAccessCommandPacket.hpp"
 
 namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
 {
@@ -11,15 +11,22 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
      * The WriteMemory class implements the structure for "M" packets. Upon receiving this packet, the server is
      * expected to write data to the target's memory, at the specified start address.
      */
-    class WriteMemory: public AbstractMemoryAccessPacket
+    class WriteMemory: public MemoryAccessCommandPacket
     {
     public:
         /**
-         * Like with the ReadMemory command packet, the start address carries additional bits that indicate
+         * Start address of the memory operation.
-         * the memory type.
          */
         std::uint32_t startAddress = 0;
 
+        /**
+         * The type of memory to read from.
+         */
+        Targets::TargetMemoryType memoryType = Targets::TargetMemoryType::FLASH;
+
+        /**
+         * Data to write.
+         */
         Targets::TargetMemoryBuffer buffer;
 
         explicit WriteMemory(const std::vector<unsigned char>& rawPacket);