More tidying

src/DebugServer/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp

@@ -3,7 +3,6 @@
 #include "src/DebugServer/Gdb/ResponsePackets/ResponsePacket.hpp"
 
 #include "src/Services/StringService.hpp"
-#include "src/Helpers/BiMap.hpp"
 #include "src/Exceptions/Exception.hpp"
 
 namespace DebugServer::Gdb::RiscVGdb::CommandPackets
@@ -48,24 +47,34 @@ namespace DebugServer::Gdb::RiscVGdb::CommandPackets
 
         Logger::info("Handling ReadMemoryMap packet");
 
-        static const auto gdbMemoryTypesBySegmentType = BiMap<TargetMemorySegmentType, std::string>{
+        static const auto gdbMemoryTypeFromSegment = [] (
-            {TargetMemorySegmentType::FLASH, "flash"},
+            const Targets::TargetMemorySegmentDescriptor& segmentDescriptor
-            {TargetMemorySegmentType::RAM, "ram"},
+        ) -> std::optional<std::string> {
-            {TargetMemorySegmentType::IO, "ram"},
+            switch (segmentDescriptor.type) {
-            {TargetMemorySegmentType::ALIASED, "flash"}, // TODO: Assumption made here. Will hold for now. Review later
+                case TargetMemorySegmentType::FLASH:
+                case TargetMemorySegmentType::ALIASED: {
+                    return "flash";
+                }
+                case TargetMemorySegmentType::RAM:
+                case TargetMemorySegmentType::IO: {
+                    return "ram";
+                }
+                default: {
+                    return std::nullopt;
+                }
+            }
         };
 
         auto memoryMap = std::string{"<memory-map>\n"};
 
-        for (const auto& [segmentKey, segmentDescriptor] : gdbTargetDescriptor.systemAddressSpaceDescriptor.segmentDescriptorsByKey) {
+        for (const auto& segmentDescriptor : gdbTargetDescriptor.systemAddressSpaceDescriptor.segmentDescriptorsByKey | std::views::values) {
-            const auto gdbMemType = gdbMemoryTypesBySegmentType.valueAt(segmentDescriptor.type);
+            const auto gdbMemType = gdbMemoryTypeFromSegment(segmentDescriptor);
             if (!gdbMemType.has_value()) {
                 continue;
             }
 
             const auto segmentWritable = (
-                segmentDescriptor.debugModeAccess.writeable
+                segmentDescriptor.debugModeAccess.writeable || segmentDescriptor.programmingModeAccess.writeable
-                || segmentDescriptor.programmingModeAccess.writeable
             );
 
             memoryMap += "<memory type=\"" + (!segmentWritable ? "rom" : *gdbMemType) + "\" start=\"0x"

src/DebugToolDrivers/Protocols/RiscVDebug/DebugTranslator.cpp

@@ -20,7 +20,6 @@
 #include "src/Targets/RiscV/Opcodes/Sw.hpp"
 #include "src/Targets/RiscV/Opcodes/Addi.hpp"
 
-#include "TriggerModule/Registers/TriggerSelect.hpp"
 #include "TriggerModule/Registers/TriggerInfo.hpp"
 #include "TriggerModule/Registers/TriggerData1.hpp"
 #include "TriggerModule/Registers/MatchControl.hpp"
@@ -88,9 +87,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
         }
 
         if (this->debugModuleDescriptor.hartIndices.size() > 1) {
-            Logger::debug(
+            Logger::debug("Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size()));
-                "Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size())
-            );
             Logger::warning("Bloom only supports debugging a single RISC-V hart - selecting first available");
         }
 
@@ -353,11 +350,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
         if (triggerDescriptor.supportedTypes.contains(TriggerType::MATCH_CONTROL)) {
             using TriggerModule::Registers::MatchControl;
 
-            this->writeCpuRegister(
+            this->writeCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerDescriptor.index);
-                CpuRegisterNumber::TRIGGER_SELECT,
-                TriggerModule::Registers::TriggerSelect{triggerDescriptor.index}.value()
-            );
-
             this->writeCpuRegister(
                 CpuRegisterNumber::TRIGGER_DATA_1,
                 MatchControl{
@@ -650,9 +643,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
 
         static constexpr auto MAX_TRIGGER_INDEX = 10;
         for (auto triggerIndex = TriggerModule::TriggerIndex{0}; triggerIndex <= MAX_TRIGGER_INDEX; ++triggerIndex) {
-            const auto selectRegValue = TriggerModule::Registers::TriggerSelect{triggerIndex}.value();
+            const auto writeSelectError = this->tryWriteCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerIndex);
-
-            const auto writeSelectError = this->tryWriteCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, selectRegValue);
             if (writeSelectError == AbstractCommandError::EXCEPTION) {
                 break;
             }
@@ -664,7 +655,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
                 };
             }
 
-            if (this->readCpuRegister(CpuRegisterNumber::TRIGGER_SELECT) != selectRegValue) {
+            if (this->readCpuRegister(CpuRegisterNumber::TRIGGER_SELECT) != triggerIndex) {
                 break;
             }
 
@@ -786,7 +777,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
                 .registerNumber = number,
                 .transfer = true,
                 .flags = flags,
-                .size= RegisterAccessControlField::RegisterSize::SIZE_32
+                .size = RegisterAccessControlField::RegisterSize::SIZE_32
             }.value(),
             .commandType = AbstractCommandRegister::CommandType::REGISTER_ACCESS
         });
@@ -1315,11 +1306,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug
         if (triggerDescriptor.supportedTypes.contains(TriggerType::MATCH_CONTROL)) {
             using TriggerModule::Registers::MatchControl;
 
-            this->writeCpuRegister(
+            this->writeCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerDescriptor.index);
-                CpuRegisterNumber::TRIGGER_SELECT,
-                TriggerModule::Registers::TriggerSelect{triggerDescriptor.index}.value()
-            );
-
             this->writeCpuRegister(CpuRegisterNumber::TRIGGER_DATA_1, MatchControl{}.value());
             return;
         }

src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerInfo.hpp

@@ -30,7 +30,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers
         [[nodiscard]] std::set<TriggerType> getSupportedTriggerTypes() const {
             auto output = std::set<TriggerType>{};
 
-            static constexpr auto types = std::to_array<TriggerType>({
+            static constexpr auto TYPES = std::to_array<TriggerType>({
                 TriggerType::LEGACY,
                 TriggerType::MATCH_CONTROL,
                 TriggerType::INSTRUCTION_COUNT,
@@ -40,7 +40,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers
                 TriggerType::EXTERNAL,
             });
 
-            for (const auto& type : types) {
+            for (const auto& type : TYPES) {
                 if (this->info & (0x01 << static_cast<std::uint8_t>(type))) {
                     output.insert(type);
                 }

src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerSelect.hpp

@@ -1,24 +0,0 @@
-#pragma once
-
-#include <cstdint>
-
-#include "src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/TriggerModule.hpp"
-
-namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers
-{
-    /**
-     * TODO: Given the single, full width bit field, is this struct really necessary? Review.
-     */
-    struct TriggerSelect
-    {
-        TriggerIndex index;
-
-        constexpr explicit TriggerSelect(TriggerIndex index)
-            : index(index)
-        {}
-
-        [[nodiscard]] constexpr RegisterValue value() const {
-            return static_cast<RegisterValue>(this->index);
-        }
-    };
-}

src/TargetController/TargetControllerComponent.cpp

@@ -3,6 +3,7 @@
 #include <filesystem>
 #include <typeindex>
 #include <algorithm>
+#include <ranges>
 
 #include "src/Targets/Microchip/Avr8/TargetDescriptionFile.hpp"
 #include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp"
@@ -1046,7 +1047,7 @@ namespace TargetController
         };
         auto commitOperations = std::vector<CommitOperation>{};
 
-        for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) {
+        for (const auto& writeOperation : session.writeOperationsBySegmentId | std::views::values) {
             auto& segmentCache = this->getProgramMemoryCache(writeOperation.memorySegmentDescriptor);
 
             // Can the program memory cache facilitate diffing with all regions in this write operation?
@@ -1101,8 +1102,8 @@ namespace TargetController
              * before constructing the delta segments.
              */
             auto cacheData = segmentCache.data;
-            for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) {
+            for (const auto& breakpointsByAddress : this->softwareBreakpointRegistry | std::views::values) {
-                for (const auto& [address, breakpoint] : breakpointsByAddress) {
+                for (const auto& breakpoint : breakpointsByAddress | std::views::values) {
                     if (breakpoint.memorySegmentDescriptor != writeOperation.memorySegmentDescriptor) {
                         continue;
                     }
@@ -1169,11 +1170,11 @@ namespace TargetController
     }
 
     void TargetControllerComponent::abandonDeltaProgrammingSession(const DeltaProgramming::Session& session) {
-        for (const auto& [segmentId, eraseOperation] : session.eraseOperationsBySegmentId) {
+        for (const auto& eraseOperation: session.eraseOperationsBySegmentId | std::views::values) {
             this->eraseTargetMemory(eraseOperation.addressSpaceDescriptor, eraseOperation.memorySegmentDescriptor);
         }
 
-        for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) {
+        for (const auto& writeOperation : session.writeOperationsBySegmentId | std::views::values) {
             for (const auto& region : writeOperation.regions) {
                 this->writeTargetMemory(
                     writeOperation.addressSpaceDescriptor,