diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp index 60f72f3c..ee6dcbc7 100644 --- a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp @@ -8,8 +8,6 @@ #include "src/DebugServer/Gdb/ResponsePackets/EmptyResponsePacket.hpp" #include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" -#include "src/Targets/TargetBreakpoint.hpp" - #include "src/Services/StringService.hpp" #include "src/Logger/Logger.hpp" diff --git a/src/DebugToolDrivers/Protocols/CmsisDap/CmsisDapInterface.hpp b/src/DebugToolDrivers/Protocols/CmsisDap/CmsisDapInterface.hpp index b73ef117..0d21979b 100644 --- a/src/DebugToolDrivers/Protocols/CmsisDap/CmsisDapInterface.hpp +++ b/src/DebugToolDrivers/Protocols/CmsisDap/CmsisDapInterface.hpp @@ -109,10 +109,6 @@ namespace DebugToolDrivers::Protocols::CmsisDap private: /** * All CMSIS-DAP devices employ the USB HID interface for communication. - * - * For many CMSIS-DAP devices, the USB HID interface parameters (interface number, endpoint config, etc) vary - * amongst devices, so we'll need to be able to preActivationConfigure the CMSISDAPInterface from a - * higher level. For an example, see the constructor of the AtmelIce device class. */ Usb::HidInterface usbHidInterface; @@ -120,7 +116,7 @@ namespace DebugToolDrivers::Protocols::CmsisDap * Some CMSIS-DAP debug tools fail to operate properly when we send commands too quickly. Even if we've * received a response from every previous command. * - * Because of this, we may need to enforce a minimum time gap between sending CMSIS commands. + * Because of this, we may need to enforce a minimum interval between sending CMSIS commands. */ std::chrono::milliseconds commandDelay = std::chrono::milliseconds{0}; std::int64_t lastCommandSentTimeStamp = 0; diff --git a/src/DebugToolDrivers/Wch/DeviceInfo.hpp b/src/DebugToolDrivers/Wch/DeviceInfo.hpp index 07c752d5..9ee642e7 100644 --- a/src/DebugToolDrivers/Wch/DeviceInfo.hpp +++ b/src/DebugToolDrivers/Wch/DeviceInfo.hpp @@ -12,10 +12,5 @@ namespace DebugToolDrivers::Wch public: WchFirmwareVersion firmwareVersion; std::optional variant; - - explicit DeviceInfo(WchFirmwareVersion firmwareVersion, std::optional variant) - : firmwareVersion(firmwareVersion) - , variant(variant) - {} }; } diff --git a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp index f0fb4ab2..e40ec049 100644 --- a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp +++ b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp @@ -58,7 +58,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink } void WchLinkInterface::setClockSpeed(WchLinkTargetClockSpeed speed, WchTargetId targetId) { - const auto speedIdsBySpeed = BiMap{ + static const auto speedIdsBySpeed = BiMap{ {WchLinkTargetClockSpeed::CLK_6000_KHZ, 0x01}, {WchLinkTargetClockSpeed::CLK_4000_KHZ, 0x02}, {WchLinkTargetClockSpeed::CLK_400_KHZ, 0x03}, @@ -155,7 +155,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink const auto response = this->sendCommandAndWaitForResponse( Commands::PreparePartialFlashBlockWrite{ - static_cast(startAddress + (packetSize * i)), + static_cast(startAddress + (packetSize * i)), static_cast(segmentSize) } ); diff --git a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.hpp b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.hpp index 38c6b1d2..be1aba6b 100644 --- a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.hpp +++ b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.hpp @@ -30,7 +30,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink : public ::DebugToolDrivers::Protocols::RiscVDebugSpec::DebugTransportModuleInterface { public: - static constexpr Targets::TargetMemorySize MAX_PARTIAL_BLOCK_WRITE_SIZE = 64; + static constexpr auto MAX_PARTIAL_BLOCK_WRITE_SIZE = Targets::TargetMemorySize{64}; WchLinkInterface(Usb::UsbInterface& usbInterface, Usb::UsbDevice& usbDevice); diff --git a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp index 50940081..746d80d7 100644 --- a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp +++ b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp @@ -273,7 +273,7 @@ namespace DebugToolDrivers::Wch return this->wchLinkInterface.eraseProgramMemory(); } - // Ignore other (non-program memory) erase requests, for now. + Logger::debug("Ignoring erase operation on `" + memorySegmentDescriptor.key + "` segment - not supported"); } void WchLinkDebugInterface::enableProgrammingMode() { @@ -282,7 +282,7 @@ namespace DebugToolDrivers::Wch * * 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(). + * we'll be utilising at this point. */ } @@ -326,14 +326,14 @@ namespace DebugToolDrivers::Wch ) }; - static constexpr auto ebreakBytes = std::to_array({ + static constexpr auto EBREAK_OPCODE = std::to_array({ static_cast(::Targets::RiscV::Opcodes::Ebreak), static_cast(::Targets::RiscV::Opcodes::Ebreak >> 8), static_cast(::Targets::RiscV::Opcodes::Ebreak >> 16), static_cast(::Targets::RiscV::Opcodes::Ebreak >> 24) }); - static constexpr auto compressedEbreakBytes = std::to_array({ + static constexpr auto COMPRESSED_EBREAK_OPCODE = std::to_array({ static_cast(::Targets::RiscV::Opcodes::EbreakCompressed), static_cast(::Targets::RiscV::Opcodes::EbreakCompressed >> 8) }); @@ -343,8 +343,8 @@ namespace DebugToolDrivers::Wch softwareBreakpoint.memorySegmentDescriptor, softwareBreakpoint.address, softwareBreakpoint.size == 2 - ? TargetMemoryBufferSpan{compressedEbreakBytes} - : TargetMemoryBufferSpan{ebreakBytes} + ? TargetMemoryBufferSpan{COMPRESSED_EBREAK_OPCODE} + : TargetMemoryBufferSpan{EBREAK_OPCODE} ); this->softwareBreakpointRegistry.insert(softwareBreakpoint); diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/MemoryRegionManager/RegionItem.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/MemoryRegionManager/RegionItem.cpp index 2b2147a2..fcaec74e 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/MemoryRegionManager/RegionItem.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/MemoryRegionManager/RegionItem.cpp @@ -100,7 +100,7 @@ namespace Widgets } auto addressType = this->getSelectedAddressInputType(); - const auto memoryAddressRange = this->memorySegmentDescriptor.addressRange; + const auto& memoryAddressRange = this->memorySegmentDescriptor.addressRange; const auto memoryAddressRangeStr = QString{ "0x" + QString::number(memoryAddressRange.startAddress, 16).toUpper() + QString(" -> ") diff --git a/src/ProjectConfig.hpp b/src/ProjectConfig.hpp index 5c497bff..fd57d079 100644 --- a/src/ProjectConfig.hpp +++ b/src/ProjectConfig.hpp @@ -15,19 +15,9 @@ * multiple debug environments, each environment must be assigned a unique name that can be used to identify the * environment. This is why the 'environments' parameter is a YAML map, with the key being the environment name. * - * On application start up, we extract the config from this YAML file and generate a ProjectConfig object. + * On application start up, we extract the config from this YAML file and construct a ProjectConfig object. * See Application::loadProjectConfiguration() for more on this. * - * Some config parameters are specific to certain entities within Bloom, but have no significance across the - * rest of the application. For example, AVR8 targets require the 'physicalInterface' and other AVR8 specific - * parameters. These are used to configure AVR8 targets, but have no significance across the rest of the - * application. This is why some configuration structs (like TargetConfig) include a YAML::Node member. - * When instances of these structs are passed to the appropriate entities, any configuration required by those - * entities is extracted from the YAML::Node member. This means we don't have to worry about any entity specific - * config parameters at the application level. We can simply extract what we need at an entity level and the rest - * of the application can remain oblivious. For an example on extracting entity specific config, see - * Avr8TargetConfig::Avr8TargetConfig() and Avr8::preActivationConfigure(). - * * For more on project configuration, see Bloom documentation at https://bloom.oscillate.io/docs/configuration */ @@ -36,8 +26,7 @@ * * Please don't define any target specific configuration here, unless it applies to *all* targets across * the application. If a target requires specific config, it should be extracted from the YAML::Node (targetNode) - * member. This should be done in Target::preActivationConfigure(), to which an instance of TargetConfig is passed. - * See the comment above on entity specific config for more on this. + * member. */ struct TargetConfig { @@ -73,8 +62,8 @@ struct TargetConfig std::optional reserveSteppingBreakpoint = std::nullopt; /** - * For extracting any target specific configuration. See Avr8TargetConfig::Avr8TargetConfig() and - * Avr8::preActivationConfigure() for an example of this. + * For extracting any target specific configuration. See Avr8TargetConfig::Avr8TargetConfig() for an example of + * this. */ YAML::Node targetNode; diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 831dd08a..5febdd21 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -139,13 +139,8 @@ namespace TargetController std::optional activeAtomicSession = std::nullopt; - /** - * The TargetController should be the sole owner of the target and debugTool. They are constructed and - * destroyed within the TargetController. Under no circumstance should ownership of these resources be - * transferred to any other component within Bloom. - */ - std::unique_ptr target = nullptr; std::unique_ptr debugTool = nullptr; + std::unique_ptr target = nullptr; std::map< Commands::CommandType, @@ -171,8 +166,7 @@ namespace TargetController * If program caching is enabled, all program memory reads will be serviced by the cache, if we have the data. * * Most targets only have a single memory segment for program memory, but some may have multiple program - * memories, across multiple address spaces. We have a single cache (TargetMemoryCache object) for each - * memory segment. + * memories, across multiple address spaces. We have a single cache for each memory segment. */ std::map programMemoryCachesBySegmentId;