diff --git a/build/scripts/Avr8TargetDescriptionFiles.php b/build/scripts/Avr8TargetDescriptionFiles.php index 79a03366..77527a32 100644 --- a/build/scripts/Avr8TargetDescriptionFiles.php +++ b/build/scripts/Avr8TargetDescriptionFiles.php @@ -40,15 +40,13 @@ $avrTdfs = TargetDescriptionFiles\Factory::loadAvr8Tdfs(); print "Processing " . count($avrTdfs) . " AVR8 TDFs...\n\n"; -$processedTargetIds = []; - foreach ($avrTdfs as $avrTdf) { print "Processing AVR8 TDF for target " . $avrTdf->targetName . "\n"; $strippedTargetName = str_replace([' '] , '_', $avrTdf->targetName); $id = strtolower($strippedTargetName); - if (in_array($id, $processedTargetIds)) { + if (in_array($id, $tdfMapping)) { print "\033[31m" . "\n"; print "FATAL ERROR: duplicate AVR8 target ID detected: " . $id . "\n\n" . "TDF Path: " . realpath($avrTdf->filePath); @@ -94,12 +92,11 @@ foreach ($avrTdfs as $avrTdf) { exit(1); } - $tdfMapping[strtolower($avrTdf->signature->toHex())][] = [ - 'targetName' => $strippedTargetName, - 'targetDescriptionFilePath' => $relativeDestinationFilePath, + $tdfMapping[$id] = [ + 'name' => $strippedTargetName, + 'signature' => $avrTdf->signature->toHex(), + 'tdfPath' => $relativeDestinationFilePath, ]; - - $processedTargetIds[] = $id; } if (file_put_contents(AVR_TDF_MAPPING_FILE_PATH, json_encode($tdfMapping, JSON_PRETTY_PRINT)) === false) { diff --git a/src/DebugServer/CMakeLists.txt b/src/DebugServer/CMakeLists.txt index 2dd23c0d..ee55b87e 100755 --- a/src/DebugServer/CMakeLists.txt +++ b/src/DebugServer/CMakeLists.txt @@ -8,11 +8,10 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/GdbDebugServerConfig.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/Connection.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/DebugSession.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/TargetDescriptor.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/ResponsePackets/SupportedFeaturesResponse.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/CommandPacket.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/SupportedFeaturesQuery.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/ReadRegisters.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/WriteRegister.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/ContinueExecution.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/StepExecution.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/InterruptExecution.cpp @@ -30,6 +29,9 @@ target_sources( # AVR GDB RSP Server ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/AvrGdbRsp.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/TargetDescriptor.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadRegisters.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadRegister.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/WriteRegister.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadMemoryMap.cpp diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp index 096a43ad..031a9d7a 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp @@ -1,6 +1,9 @@ #include "AvrGdbRsp.hpp" // Command packets +#include "CommandPackets/ReadRegister.hpp" +#include "CommandPackets/ReadRegisters.hpp" +#include "CommandPackets/WriteRegister.hpp" #include "CommandPackets/ReadMemory.hpp" #include "CommandPackets/WriteMemory.hpp" #include "CommandPackets/ReadMemoryMap.hpp" @@ -34,6 +37,9 @@ namespace Bloom::DebugServer::Gdb::AvrGdb std::unique_ptr AvrGdbRsp::resolveCommandPacket( const RawPacket& rawPacket ) { + using AvrGdb::CommandPackets::ReadRegister; + using AvrGdb::CommandPackets::ReadRegisters; + using AvrGdb::CommandPackets::WriteRegister; using AvrGdb::CommandPackets::ReadMemory; using AvrGdb::CommandPackets::WriteMemory; using AvrGdb::CommandPackets::ReadMemoryMap; @@ -42,6 +48,18 @@ namespace Bloom::DebugServer::Gdb::AvrGdb using AvrGdb::CommandPackets::FlashDone; if (rawPacket.size() >= 2) { + if (rawPacket[1] == 'p') { + return std::make_unique(rawPacket); + } + + if (rawPacket[1] == 'g') { + return std::make_unique(rawPacket); + } + + if (rawPacket[1] == 'P') { + return std::make_unique(rawPacket); + } + if (rawPacket[1] == 'm') { return std::make_unique(rawPacket, this->gdbTargetDescriptor.value()); } diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.cpp new file mode 100644 index 00000000..cdfc02cd --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.cpp @@ -0,0 +1,96 @@ +#include "ReadRegister.hpp" + +#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" +#include "src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp" + +#include "src/Targets/TargetRegister.hpp" + +#include "src/Services/StringService.hpp" +#include "src/Logger/Logger.hpp" + +#include "src/Exceptions/Exception.hpp" + +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets +{ + using Services::TargetControllerService; + + using Targets::TargetRegister; + using Targets::TargetRegisterDescriptors; + + using ResponsePackets::ResponsePacket; + using ResponsePackets::ErrorResponsePacket; + + using Exceptions::Exception; + + ReadRegister::ReadRegister(const RawPacket& rawPacket) + : CommandPacket(rawPacket) + { + if (this->data.size() < 2) { + throw Exception("Invalid packet length"); + } + + this->registerId = static_cast( + std::stoi(std::string(this->data.begin() + 1, this->data.end())) + ); + } + + void ReadRegister::handle(DebugSession& debugSession, TargetControllerService& targetControllerService) { + Logger::info("Handling ReadRegister packet"); + + try { + Logger::debug("Reading GDB register ID: " + std::to_string(this->registerId)); + + if (this->registerId == TargetDescriptor::PROGRAM_COUNTER_GDB_REGISTER_ID) { + /* + * GDB has requested the program counter. We can't access this in the same way as we do with other + * registers. + */ + const auto programCounter = targetControllerService.getProgramCounter(); + + debugSession.connection.writePacket( + ResponsePacket(Services::StringService::toHex(Targets::TargetMemoryBuffer({ + static_cast(programCounter), + static_cast(programCounter >> 8), + static_cast(programCounter >> 16), + static_cast(programCounter >> 24), + }))) + ); + + return; + } + + const auto& targetDescriptor = debugSession.gdbTargetDescriptor; + const auto& gdbRegisterDescriptor = targetDescriptor.gdbRegisterDescriptorsById.at(this->registerId); + + const auto targetRegisterDescriptorId = targetDescriptor.getTargetRegisterDescriptorIdFromGdbRegisterId( + this->registerId + ); + + if (!targetRegisterDescriptorId.has_value()) { + throw Exception("GDB requested an invalid/unknown register"); + } + + auto registerValue = targetControllerService.readRegisters({*targetRegisterDescriptorId}).front().value; + + // GDB expects register values to be in LSB. + std::reverse(registerValue.begin(), registerValue.end()); + + if (registerValue.size() < gdbRegisterDescriptor.size) { + /* + * The register on the target is smaller than the size expected by GDB. + * + * Insert the rest of the bytes. + */ + registerValue.insert(registerValue.end(), (gdbRegisterDescriptor.size - registerValue.size()), 0x00); + } + + debugSession.connection.writePacket( + ResponsePacket(Services::StringService::toHex(registerValue)) + ); + + } catch (const Exception& exception) { + Logger::error("Failed to read general registers - " + exception.getMessage()); + debugSession.connection.writePacket(ErrorResponsePacket()); + } + } +} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.hpp new file mode 100644 index 00000000..fa5f765d --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegister.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" + +#include "src/DebugServer/Gdb/RegisterDescriptor.hpp" + +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets +{ + /** + * The ReadRegister class implements a structure for the "p" command packet. In response to this packet, the server + * is expected to send register values for the requested register. + */ + class ReadRegister: public Gdb::CommandPackets::CommandPacket + { + public: + GdbRegisterId registerId; + + explicit ReadRegister(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/CommandPackets/ReadRegisters.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.cpp similarity index 50% rename from src/DebugServer/Gdb/CommandPackets/ReadRegisters.cpp rename to src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.cpp index ac39d936..bdcdf56e 100644 --- a/src/DebugServer/Gdb/CommandPackets/ReadRegisters.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.cpp @@ -1,6 +1,7 @@ #include "ReadRegisters.hpp" #include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" +#include "src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp" #include "src/Targets/TargetRegister.hpp" @@ -9,12 +10,12 @@ #include "src/Exceptions/Exception.hpp" -namespace Bloom::DebugServer::Gdb::CommandPackets +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets { using Services::TargetControllerService; using Targets::TargetRegister; - using Targets::TargetRegisterDescriptors; + using Targets::TargetRegisterDescriptorIds; using ResponsePackets::ResponsePacket; using ResponsePackets::ErrorResponsePacket; @@ -23,63 +24,54 @@ namespace Bloom::DebugServer::Gdb::CommandPackets ReadRegisters::ReadRegisters(const RawPacket& rawPacket) : CommandPacket(rawPacket) - { - if (this->data.size() >= 2 && this->data.front() == 'p') { - // This command packet is requesting a specific register - this->registerNumber = static_cast( - std::stoi(std::string(this->data.begin() + 1, this->data.end())) - ); - } - } + {} void ReadRegisters::handle(DebugSession& debugSession, TargetControllerService& targetControllerService) { Logger::info("Handling ReadRegisters packet"); try { const auto& targetDescriptor = debugSession.gdbTargetDescriptor; - auto descriptors = TargetRegisterDescriptors(); + auto descriptorIds = TargetRegisterDescriptorIds(); - if (this->registerNumber.has_value()) { - Logger::debug("Reading register number: " + std::to_string(this->registerNumber.value())); - descriptors.insert( - targetDescriptor.getTargetRegisterDescriptorFromNumber(this->registerNumber.value()) + // Read all target registers mapped to a GDB register + for (const auto& [gdbRegisterId, gdbRegisterDescriptor] : targetDescriptor.gdbRegisterDescriptorsById) { + const auto registerDescriptorId = targetDescriptor.getTargetRegisterDescriptorIdFromGdbRegisterId( + gdbRegisterId ); - } else { - // Read all target registers mapped to a GDB register - for (const auto& registerNumber : targetDescriptor.getRegisterNumbers()) { - descriptors.insert(targetDescriptor.getTargetRegisterDescriptorFromNumber(registerNumber)); + if (registerDescriptorId.has_value()) { + descriptorIds.insert(*registerDescriptorId); } } - auto registerSet = targetControllerService.readRegisters(descriptors); + auto registerSet = targetControllerService.readRegisters(descriptorIds); /* - * Sort each register by their respective GDB register number - this will leave us with a collection of + * Sort each register by their respective GDB register ID - this will leave us with a collection of * registers in the order expected by the GDB client. */ std::sort( registerSet.begin(), registerSet.end(), - [this, &targetDescriptor] (const TargetRegister& registerA, const TargetRegister& registerB) { - return targetDescriptor.getRegisterNumberFromTargetRegisterDescriptor(registerA.descriptor) < - targetDescriptor.getRegisterNumberFromTargetRegisterDescriptor(registerB.descriptor); + [this, &targetDescriptor] (const TargetRegister& regA, const TargetRegister& regB) { + return targetDescriptor.getGdbRegisterIdFromTargetRegisterDescriptorId(regA.descriptorId).value() < + targetDescriptor.getGdbRegisterIdFromTargetRegisterDescriptorId(regB.descriptorId).value(); } ); /* - * Finally, reverse the register values (as they're all currently in MSB, but GDB expects them in LSB), ensure - * that each register value size matches the size in the associated GDB register descriptor, implode the - * values, convert to hexadecimal form and send to the GDB client. + * Reverse the register values (as they're all currently in MSB, but GDB expects them in LSB), ensure that + * each register value size matches the size in the associated GDB register descriptor and implode the + * values. */ auto registers = std::vector(); for (auto& reg : registerSet) { std::reverse(reg.value.begin(), reg.value.end()); - const auto gdbRegisterNumber = targetDescriptor.getRegisterNumberFromTargetRegisterDescriptor( - reg.descriptor + const auto gdbRegisterId = targetDescriptor.getGdbRegisterIdFromTargetRegisterDescriptorId( + reg.descriptorId ).value(); - const auto& gdbRegisterDescriptor = targetDescriptor.getRegisterDescriptorFromNumber(gdbRegisterNumber); + const auto& gdbRegisterDescriptor = targetDescriptor.gdbRegisterDescriptorsById.at(gdbRegisterId); if (reg.value.size() < gdbRegisterDescriptor.size) { reg.value.insert(reg.value.end(), (gdbRegisterDescriptor.size - reg.value.size()), 0x00); @@ -88,12 +80,19 @@ namespace Bloom::DebugServer::Gdb::CommandPackets registers.insert(registers.end(), reg.value.begin(), reg.value.end()); } + // Finally, include the program counter (which GDB expects to reside at the end) + const auto programCounter = targetControllerService.getProgramCounter(); + registers.insert(registers.end(), static_cast(programCounter)); + registers.insert(registers.end(), static_cast(programCounter >> 8)); + registers.insert(registers.end(), static_cast(programCounter >> 16)); + registers.insert(registers.end(), static_cast(programCounter >> 24)); + debugSession.connection.writePacket( ResponsePacket(Services::StringService::toHex(registers)) ); } catch (const Exception& exception) { - Logger::error("Failed to read general registers - " + exception.getMessage()); + Logger::error("Failed to read registers - " + exception.getMessage()); debugSession.connection.writePacket(ErrorResponsePacket()); } } diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.hpp new file mode 100644 index 00000000..cce6fc66 --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadRegisters.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include + +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" + +#include "src/DebugServer/Gdb/RegisterDescriptor.hpp" + +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets +{ + /** + * The ReadRegisters class implements a structure for the "g" command packet. In response to this packet, the + * server is expected to send register values for all registers. + */ + class ReadRegisters: public Gdb::CommandPackets::CommandPacket + { + public: + explicit ReadRegisters(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.cpp new file mode 100644 index 00000000..a9ffa4f9 --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.cpp @@ -0,0 +1,110 @@ +#include "WriteRegister.hpp" + +#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" + +#include "src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp" + +#include "src/Logger/Logger.hpp" +#include "src/Exceptions/Exception.hpp" + +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets +{ + using Services::TargetControllerService; + + using Targets::TargetRegister; + using Targets::TargetRegisterDescriptors; + + using ResponsePackets::ResponsePacket; + using ResponsePackets::OkResponsePacket; + using ResponsePackets::ErrorResponsePacket; + + using Exceptions::Exception; + + WriteRegister::WriteRegister(const RawPacket& rawPacket) + : CommandPacket(rawPacket) + { + // The P packet updates a single register + auto packet = std::string(this->data.begin(), this->data.end()); + + if (packet.size() < 4) { + throw Exception("Invalid WriteRegister command packet - insufficient data in packet."); + } + + if (packet.find('=') == std::string::npos) { + throw Exception("Invalid WriteRegister command packet - unexpected format"); + } + + const auto packetSegments = QString::fromStdString(packet).split("="); + this->registerId = static_cast(packetSegments.front().mid(1).toUInt(nullptr, 16)); + this->registerValue = Packet::hexToData(packetSegments.back().toStdString()); + + if (this->registerValue.empty()) { + throw Exception("Invalid WriteRegister command packet - missing register value"); + } + + std::reverse(this->registerValue.begin(), this->registerValue.end()); + } + + void WriteRegister::handle(DebugSession& debugSession, TargetControllerService& targetControllerService) { + Logger::info("Handling WriteRegister packet"); + + try { + if (this->registerId == TargetDescriptor::PROGRAM_COUNTER_GDB_REGISTER_ID) { + targetControllerService.setProgramCounter( + static_cast( + (this->registerValue.size() >= 1 ? this->registerValue[0] : 0x00) << 24 + | (this->registerValue.size() >= 2 ? this->registerValue[1] : 0x00) << 16 + | (this->registerValue.size() >= 3 ? this->registerValue[2] : 0x00) << 8 + | (this->registerValue.size() >= 4 ? this->registerValue[3] : 0x00) + ) + ); + + debugSession.connection.writePacket(OkResponsePacket()); + return; + } + + const auto& gdbTargetDescriptor = debugSession.gdbTargetDescriptor; + const auto descriptorId = gdbTargetDescriptor.getTargetRegisterDescriptorIdFromGdbRegisterId( + this->registerId + ); + + if (!descriptorId.has_value()) { + throw Exception("Invalid/unknown register"); + } + + const auto& descriptor = gdbTargetDescriptor.targetDescriptor.registerDescriptorsById.at(*descriptorId); + + if (this->registerValue.size() > descriptor.size) { + // Attempt to trim the higher zero-value bytes from the register value, until we reach the correct size. + for (auto i = this->registerValue.size() - 1; i >= descriptor.size; --i) { + if (this->registerValue.at(i) != 0x00) { + // If we reach a non-zero byte, we cannot trim anymore without changing the data + break; + } + + this->registerValue.erase(this->registerValue.begin() + i); + } + + if (this->registerValue.size() > descriptor.size) { + const auto& gdbRegisterDescriptor = gdbTargetDescriptor.gdbRegisterDescriptorsById.at( + this->registerId + ); + throw Exception( + "Cannot set value for " + gdbRegisterDescriptor.name + " - value size exceeds register size." + ); + } + } + + targetControllerService.writeRegisters({ + TargetRegister(descriptor.id, this->registerValue) + }); + + debugSession.connection.writePacket(OkResponsePacket()); + + } catch (const Exception& exception) { + Logger::error("Failed to write registers - " + exception.getMessage()); + debugSession.connection.writePacket(ErrorResponsePacket()); + } + } +} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.hpp new file mode 100644 index 00000000..7ea37dcd --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteRegister.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" + +#include "src/DebugServer/Gdb/RegisterDescriptor.hpp" + +#include "src/Targets/TargetRegister.hpp" +#include "src/Targets/TargetMemory.hpp" + +namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets +{ + /** + * The WriteRegister class implements the structure for "P" packets. + */ + class WriteRegister: public Gdb::CommandPackets::CommandPacket + { + public: + GdbRegisterId registerId; + Targets::TargetMemoryBuffer registerValue; + + explicit WriteRegister(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp b/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp index d49d3010..53a32a12 100644 --- a/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp +++ b/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp @@ -19,164 +19,143 @@ namespace Bloom::DebugServer::Gdb::AvrGdb {Targets::TargetMemoryType::FLASH, 0}, {Targets::TargetMemoryType::RAM, 0x00800000U}, {Targets::TargetMemoryType::EEPROM, 0x00810000U}, - } + }, + {}, + {}, + {} ) { this->loadRegisterMappings(); } - std::optional TargetDescriptor::getRegisterNumberFromTargetRegisterDescriptor( - const Targets::TargetRegisterDescriptor& registerDescriptor - ) const { - return this->targetRegisterDescriptorsByGdbNumber.valueAt(registerDescriptor); - } - - const RegisterDescriptor& TargetDescriptor::getRegisterDescriptorFromNumber(GdbRegisterNumber number) const { - const auto registerDescriptorIt = this->registerDescriptorsByGdbNumber.find(number); - if (registerDescriptorIt.has_value()) { - return (*registerDescriptorIt)->second; - } - - throw Exception( - "Unknown register from GDB - register number (" + std::to_string(number) - + ") not mapped to any GDB register descriptor." - ); - } - - const TargetRegisterDescriptor& TargetDescriptor::getTargetRegisterDescriptorFromNumber( - GdbRegisterNumber number - ) const { - const auto targetRegisterDescriptorIt = this->targetRegisterDescriptorsByGdbNumber.find(number); - if (targetRegisterDescriptorIt.has_value()) { - return (*targetRegisterDescriptorIt)->second; - } - - throw Exception( - "Unknown register from GDB - register number (" + std::to_string(number) - + ") not mapped to any target register descriptor." - ); - } - - const std::vector& TargetDescriptor::getRegisterNumbers() const { - return this->registerNumbers; - } - void TargetDescriptor::loadRegisterMappings() { - const auto& registerDescriptorsByType = this->targetDescriptor.registerDescriptorsByType; - if (!registerDescriptorsByType.contains(TargetRegisterType::STATUS_REGISTER)) { - throw Exception("Missing status register descriptor"); - } - - if (!registerDescriptorsByType.contains(TargetRegisterType::STACK_POINTER)) { - throw Exception("Missing stack pointer register descriptor"); - } - - if (!registerDescriptorsByType.contains(TargetRegisterType::PROGRAM_COUNTER)) { - throw Exception("Missing program counter register descriptor"); - } - - if ( - !registerDescriptorsByType.contains(TargetRegisterType::GENERAL_PURPOSE_REGISTER) - || registerDescriptorsByType.at(TargetRegisterType::GENERAL_PURPOSE_REGISTER).size() != 32 - ) { - throw Exception("Unexpected general purpose register count"); - } - - /* - * For AVR targets, avr-gdb defines 35 registers in total: - * - * Register number 0 through 31 are general purpose registers - * Register number 32 is the status register (SREG) - * Register number 33 is the stack pointer register - * Register number 34 is the program counter register - */ - - // Generate 35 register numbers (0 -> 34) - std::iota(this->registerNumbers.begin(), this->registerNumbers.end(), 0); - - /* - * Worth noting that gpRegisterDescriptors will always be sorted in the correct order, from register 0 to 31. - * - * Hmm, but the sorting is based on the start address (see TargetRegisterDescriptor::<() for more). So - * effectively, we're assuming that the registers will be laid out in the correct order, in memory. I think - * this assumption is fair. - */ - const auto& gpRegisterDescriptors = registerDescriptorsByType.at( + const auto generalPurposeTargetRegisterDescriptorIds = this->targetDescriptor.registerDescriptorIdsForType( TargetRegisterType::GENERAL_PURPOSE_REGISTER ); - // General purpose registers - GdbRegisterNumber regNumber = 0; - for (const auto& descriptor : gpRegisterDescriptors) { - this->registerDescriptorsByGdbNumber.insert(std::pair( - regNumber, - RegisterDescriptor( - regNumber, - 1, - "General Purpose Register " + std::to_string(regNumber) - ) - )); + const auto statusTargetRegisterDescriptorIds = this->targetDescriptor.registerDescriptorIdsForType( + TargetRegisterType::STATUS_REGISTER + ); - this->targetRegisterDescriptorsByGdbNumber.insert(std::pair( - regNumber, - descriptor - )); + const auto stackPointerTargetRegisterDescriptorIds = this->targetDescriptor.registerDescriptorIdsForType( + TargetRegisterType::STACK_POINTER + ); - regNumber++; + if (generalPurposeTargetRegisterDescriptorIds.size() != 32) { + throw Exception("Unexpected general purpose register count"); } - // Status, stack pointer and program counter registers - const auto statusDescriptor = RegisterDescriptor( - 32, + if (statusTargetRegisterDescriptorIds.empty()) { + throw Exception("Missing status register descriptor"); + } + + if (stackPointerTargetRegisterDescriptorIds.empty()) { + throw Exception("Missing stack pointer register descriptor"); + } + + /* + * For AVR targets, GDB defines 35 registers in total: + * + * - Register ID 0 through 31 are general purpose registers + * - Register ID 32 is the status register (SREG) + * - Register ID 33 is the stack pointer register + * - Register ID 34 is the program counter + * + * For AVR targets, we don't have a target register descriptor for the program counter, so we don't map that + * GDB register ID (34) to anything here. Instead, the register command packet handlers (ReadRegisters, + * WriteRegister, etc) will handle any operations involving that GDB register. + */ + + // General purpose registers + GdbRegisterId gdbRegisterId = 0; + for (const auto descriptorId : generalPurposeTargetRegisterDescriptorIds) { + auto gdbRegisterDescriptor = RegisterDescriptor( + gdbRegisterId, + 1, + "General Purpose Register " + std::to_string(gdbRegisterId) + ); + + this->gdbRegisterIdsByTargetRegisterDescriptorId.emplace(descriptorId, gdbRegisterDescriptor.id); + this->targetRegisterDescriptorIdsByGdbRegisterId.emplace(gdbRegisterDescriptor.id, descriptorId); + + this->gdbRegisterDescriptorsById.emplace(gdbRegisterDescriptor.id, std::move(gdbRegisterDescriptor)); + + gdbRegisterId++; + } + + const auto& statusTargetRegisterDescriptor = this->targetDescriptor.registerDescriptorsById.at( + *(statusTargetRegisterDescriptorIds.begin()) + ); + + auto statusGdbRegisterDescriptor = RegisterDescriptor( + TargetDescriptor::STATUS_GDB_REGISTER_ID, 1, "Status Register" ); - this->registerDescriptorsByGdbNumber.insert(std::pair(statusDescriptor.number, statusDescriptor)); - this->targetRegisterDescriptorsByGdbNumber.insert(std::pair( - statusDescriptor.number, - *(registerDescriptorsByType.at(TargetRegisterType::STATUS_REGISTER).begin()) - )); + if (statusTargetRegisterDescriptor.size > statusGdbRegisterDescriptor.size) { + throw Exception("AVR8 status target register size exceeds the GDB register size."); + } - const auto stackPointerDescriptor = RegisterDescriptor( - 33, + this->gdbRegisterIdsByTargetRegisterDescriptorId.emplace( + statusTargetRegisterDescriptor.id, + statusGdbRegisterDescriptor.id + ); + this->targetRegisterDescriptorIdsByGdbRegisterId.emplace( + statusGdbRegisterDescriptor.id, + statusTargetRegisterDescriptor.id + ); + + this->gdbRegisterDescriptorsById.emplace( + statusGdbRegisterDescriptor.id, + std::move(statusGdbRegisterDescriptor) + ); + + const auto& stackPointerTargetRegisterDescriptor = this->targetDescriptor.registerDescriptorsById.at( + *(stackPointerTargetRegisterDescriptorIds.begin()) + ); + + auto stackPointerGdbRegisterDescriptor = RegisterDescriptor( + TargetDescriptor::STACK_POINTER_GDB_REGISTER_ID, 2, "Stack Pointer Register" ); - this->registerDescriptorsByGdbNumber.insert(std::pair(stackPointerDescriptor.number, stackPointerDescriptor)); - this->targetRegisterDescriptorsByGdbNumber.insert(std::pair( - stackPointerDescriptor.number, - *(registerDescriptorsByType.at(TargetRegisterType::STACK_POINTER).begin()) - )); + if (stackPointerTargetRegisterDescriptor.size > stackPointerGdbRegisterDescriptor.size) { + throw Exception("AVR8 stack pointer target register size exceeds the GDB register size."); + } - const auto programCounterDescriptor = RegisterDescriptor( - 34, + this->gdbRegisterIdsByTargetRegisterDescriptorId.emplace( + stackPointerTargetRegisterDescriptor.id, + stackPointerGdbRegisterDescriptor.id + ); + this->targetRegisterDescriptorIdsByGdbRegisterId.emplace( + stackPointerGdbRegisterDescriptor.id, + stackPointerTargetRegisterDescriptor.id + ); + + this->gdbRegisterDescriptorsById.emplace( + stackPointerGdbRegisterDescriptor.id, + std::move(stackPointerGdbRegisterDescriptor) + ); + + /* + * We acknowledge the GDB program counter register here, but we don't map it to any target register descriptors. + * + * This is because we can't access the program counter on AVR targets in the same way we do with other + * registers. We don't have a register descriptor for the program counter. We have to treat it as a special + * case in the register access command packet handlers. See CommandPackets::ReadRegister, + * CommandPackets::WriteRegister, etc for more. + */ + auto programCounterGdbRegisterDescriptor = RegisterDescriptor( + TargetDescriptor::PROGRAM_COUNTER_GDB_REGISTER_ID, 4, "Program Counter" ); - this->registerDescriptorsByGdbNumber.insert(std::pair( - programCounterDescriptor.number, - programCounterDescriptor - )); - this->targetRegisterDescriptorsByGdbNumber.insert(std::pair( - programCounterDescriptor.number, - *(registerDescriptorsByType.at(TargetRegisterType::PROGRAM_COUNTER).begin()) - )); - - if (registerDescriptorsByType.at(TargetRegisterType::STATUS_REGISTER).size() > statusDescriptor.size) { - throw Exception("AVR8 status target register size exceeds the GDB register size."); - } - - if (registerDescriptorsByType.at(TargetRegisterType::STACK_POINTER).size() > stackPointerDescriptor.size) { - throw Exception("AVR8 stack pointer target register size exceeds the GDB register size."); - } - - if ( - registerDescriptorsByType.at(TargetRegisterType::PROGRAM_COUNTER).size() > programCounterDescriptor.size - ) { - throw Exception("AVR8 program counter size exceeds the GDB register size."); - } + this->gdbRegisterDescriptorsById.emplace( + programCounterGdbRegisterDescriptor.id, + std::move(programCounterGdbRegisterDescriptor) + ); } } diff --git a/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp b/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp index 7066f0b2..081235a3 100644 --- a/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp +++ b/src/DebugServer/Gdb/AvrGdb/TargetDescriptor.hpp @@ -2,52 +2,18 @@ #include "src/DebugServer/Gdb/TargetDescriptor.hpp" -#include "src/Helpers/BiMap.hpp" - namespace Bloom::DebugServer::Gdb::AvrGdb { class TargetDescriptor: public DebugServer::Gdb::TargetDescriptor { public: - BiMap registerDescriptorsByGdbNumber = {}; - BiMap targetRegisterDescriptorsByGdbNumber = {}; + static constexpr auto STATUS_GDB_REGISTER_ID = 32; + static constexpr auto STACK_POINTER_GDB_REGISTER_ID = 33; + static constexpr auto PROGRAM_COUNTER_GDB_REGISTER_ID = 34; explicit TargetDescriptor(const Targets::TargetDescriptor& targetDescriptor); - /** - * Should retrieve the GDB register number, given a target register descriptor. Or std::nullopt if the target - * register descriptor isn't mapped to any GDB register. - * - * @param registerDescriptor - * @return - */ - std::optional getRegisterNumberFromTargetRegisterDescriptor( - const Targets::TargetRegisterDescriptor& registerDescriptor - ) const override; - - /** - * Should retrieve the GDB register descriptor for a given GDB register number. - * - * @param number - * @return - */ - const RegisterDescriptor& getRegisterDescriptorFromNumber(GdbRegisterNumber number) const override; - - /** - * Should retrieve the mapped target register descriptor for a given GDB register number. - * - * @param number - * @return - */ - const Targets::TargetRegisterDescriptor& getTargetRegisterDescriptorFromNumber( - GdbRegisterNumber number - ) const override; - - const std::vector& getRegisterNumbers() const override; - private: - std::vector registerNumbers = std::vector(35); - /** * For AVR targets, avr-gdb defines 35 registers in total: * diff --git a/src/DebugServer/Gdb/CommandPackets/GenerateSvd.cpp b/src/DebugServer/Gdb/CommandPackets/GenerateSvd.cpp index eb7a0539..404921e4 100644 --- a/src/DebugServer/Gdb/CommandPackets/GenerateSvd.cpp +++ b/src/DebugServer/Gdb/CommandPackets/GenerateSvd.cpp @@ -142,40 +142,34 @@ namespace Bloom::DebugServer::Gdb::CommandPackets auto peripheralsByName = std::map(); - for (const auto& [registerType, registerDescriptors] : targetDescriptor.registerDescriptorsByType) { - if (registerDescriptors.empty()) { + for (const auto& [descriptorId, registerDescriptor] : targetDescriptor.registerDescriptorsById) { + if ( + !registerDescriptor.startAddress.has_value() + || !registerDescriptor.name.has_value() + || registerDescriptor.name->empty() + || !registerDescriptor.groupName.has_value() + || ( + registerDescriptor.type != Targets::TargetRegisterType::OTHER + && registerDescriptor.type != Targets::TargetRegisterType::PORT_REGISTER + ) + ) { continue; } - for (const auto& registerDescriptor : registerDescriptors) { - if ( - !registerDescriptor.startAddress.has_value() - || !registerDescriptor.name.has_value() - || registerDescriptor.name->empty() - || !registerDescriptor.groupName.has_value() - || ( - registerDescriptor.type != Targets::TargetRegisterType::OTHER - && registerDescriptor.type != Targets::TargetRegisterType::PORT_REGISTER - ) - ) { - continue; - } + auto peripheralIt = peripheralsByName.find(*registerDescriptor.groupName); - auto peripheralIt = peripheralsByName.find(*registerDescriptor.groupName); + if (peripheralIt == peripheralsByName.end()) { + auto peripheral = Peripheral{ + .name = QString::fromStdString( + *registerDescriptor.groupName + ).replace(QChar(' '), QChar('_')).toUpper(), + .baseAddress = baseAddressOffset + }; - if (peripheralIt == peripheralsByName.end()) { - auto peripheral = Peripheral{ - .name = QString::fromStdString( - *registerDescriptor.groupName - ).replace(QChar(' '), QChar('_')).toUpper(), - .baseAddress = baseAddressOffset - }; - - peripheralIt = peripheralsByName.insert(std::pair(*registerDescriptor.groupName, peripheral)).first; - } - - peripheralIt->second.registerDescriptors.insert(registerDescriptor); + peripheralIt = peripheralsByName.insert(std::pair(*registerDescriptor.groupName, peripheral)).first; } + + peripheralIt->second.registerDescriptors.insert(registerDescriptor); } auto peripheralsElement = document.createElement("peripherals"); @@ -213,7 +207,7 @@ namespace Bloom::DebugServer::Gdb::CommandPackets ); registerElement.appendChild( - createElement("access", registerDescriptor.writable ? "read-write" : "read-only") + createElement("access", registerDescriptor.access.writable ? "read-write" : "read-only") ); registersElement.appendChild(registerElement); diff --git a/src/DebugServer/Gdb/CommandPackets/ReadRegisters.hpp b/src/DebugServer/Gdb/CommandPackets/ReadRegisters.hpp deleted file mode 100644 index 7b032058..00000000 --- a/src/DebugServer/Gdb/CommandPackets/ReadRegisters.hpp +++ /dev/null @@ -1,35 +0,0 @@ -#pragma once - -#include - -#include "CommandPacket.hpp" - -#include "src/DebugServer/Gdb/RegisterDescriptor.hpp" - -namespace Bloom::DebugServer::Gdb::CommandPackets -{ - /** - * The ReadRegisters class implements a structure for "g" and "p" command packets. In response to these - * packets, the server is expected to send register values for all registers (for "g" packets) or for a single - * register (for "p" packets). - */ - class ReadRegisters: public CommandPacket - { - public: - /** - * "p" packets include a register number to indicate which register is requested for reading. When this is set, - * the server is expected to respond with only the value of the requested register. - * - * If the register number is not supplied (as is the case with "g" packets), the server is expected to respond - * with values for all registers. - */ - std::optional registerNumber; - - explicit ReadRegisters(const RawPacket& rawPacket); - - void handle( - DebugSession& debugSession, - Services::TargetControllerService& targetControllerService - ) override; - }; -} diff --git a/src/DebugServer/Gdb/CommandPackets/WriteRegister.cpp b/src/DebugServer/Gdb/CommandPackets/WriteRegister.cpp deleted file mode 100644 index a9f12b38..00000000 --- a/src/DebugServer/Gdb/CommandPackets/WriteRegister.cpp +++ /dev/null @@ -1,86 +0,0 @@ -#include "WriteRegister.hpp" - -#include "src/DebugServer/Gdb/ResponsePackets/TargetStopped.hpp" -#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" -#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" - -#include "src/Targets/TargetRegister.hpp" - -#include "src/Logger/Logger.hpp" -#include "src/Exceptions/Exception.hpp" - -namespace Bloom::DebugServer::Gdb::CommandPackets -{ - using Services::TargetControllerService; - - using Targets::TargetRegister; - using Targets::TargetRegisterDescriptors; - - using ResponsePackets::ResponsePacket; - using ResponsePackets::OkResponsePacket; - using ResponsePackets::ErrorResponsePacket; - - using Exceptions::Exception; - - WriteRegister::WriteRegister(const RawPacket& rawPacket) - : CommandPacket(rawPacket) - { - // The P packet updates a single register - auto packet = std::string(this->data.begin(), this->data.end()); - - if (packet.size() < 4) { - throw Exception("Invalid P command packet - insufficient data in packet."); - } - - if (packet.find('=') == std::string::npos) { - throw Exception("Invalid P command packet - unexpected format"); - } - - auto packetSegments = QString::fromStdString(packet).split("="); - this->registerNumber = static_cast(packetSegments.front().mid(1).toUInt(nullptr, 16)); - this->registerValue = Packet::hexToData(packetSegments.back().toStdString()); - std::reverse(this->registerValue.begin(), this->registerValue.end()); - } - - void WriteRegister::handle(DebugSession& debugSession, TargetControllerService& targetControllerService) { - Logger::info("Handling WriteRegister packet"); - - try { - auto targetRegisterDescriptor = debugSession.gdbTargetDescriptor.getTargetRegisterDescriptorFromNumber( - this->registerNumber - ); - - const auto valueSize = this->registerValue.size(); - if (valueSize > 0 && valueSize > targetRegisterDescriptor.size) { - // Attempt to trim the higher zero-value bytes from the register value, until we reach the correct size. - for (auto i = this->registerValue.size() - 1; i >= targetRegisterDescriptor.size; i--) { - if (this->registerValue.at(i) != 0x00) { - // If we reach a non-zero byte, we cannot trim anymore without changing the data - break; - } - - this->registerValue.erase(this->registerValue.begin() + i); - } - - if (this->registerValue.size() > targetRegisterDescriptor.size) { - const auto& gdbRegisterDescriptor = debugSession.gdbTargetDescriptor.getRegisterDescriptorFromNumber( - this->registerNumber - ); - throw Exception("Cannot set value for " + gdbRegisterDescriptor.name - + " - value size exceeds register size." - ); - } - } - - targetControllerService.writeRegisters({ - TargetRegister(targetRegisterDescriptor, this->registerValue) - }); - - debugSession.connection.writePacket(OkResponsePacket()); - - } catch (const Exception& exception) { - Logger::error("Failed to write registers - " + exception.getMessage()); - debugSession.connection.writePacket(ErrorResponsePacket()); - } - } -} diff --git a/src/DebugServer/Gdb/CommandPackets/WriteRegister.hpp b/src/DebugServer/Gdb/CommandPackets/WriteRegister.hpp deleted file mode 100644 index 47dcccc9..00000000 --- a/src/DebugServer/Gdb/CommandPackets/WriteRegister.hpp +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -#include - -#include "CommandPacket.hpp" -#include "src/Targets/TargetRegister.hpp" - -namespace Bloom::DebugServer::Gdb::CommandPackets -{ - /** - * The WriteRegisters class implements the structure for "P" packets. Upon receiving this packet, - * server is expected to update a register value to the target. - */ - class WriteRegister: public CommandPacket - { - public: - int registerNumber = 0; - std::vector registerValue; - - explicit WriteRegister(const RawPacket& rawPacket); - - void handle( - DebugSession& debugSession, - Services::TargetControllerService& targetControllerService - ) override; - }; -} diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.cpp b/src/DebugServer/Gdb/GdbRspDebugServer.cpp index f0f7e9b7..4f53ea7a 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.cpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.cpp @@ -21,8 +21,6 @@ #include "CommandPackets/InterruptExecution.hpp" #include "CommandPackets/ContinueExecution.hpp" #include "CommandPackets/StepExecution.hpp" -#include "CommandPackets/ReadRegisters.hpp" -#include "CommandPackets/WriteRegister.hpp" #include "CommandPackets/SetBreakpoint.hpp" #include "CommandPackets/RemoveBreakpoint.hpp" #include "CommandPackets/Monitor.hpp" @@ -280,14 +278,6 @@ namespace Bloom::DebugServer::Gdb return std::make_unique(rawPacket); } - if (rawPacketString[1] == 'g' || rawPacketString[1] == 'p') { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 'P') { - return std::make_unique(rawPacket); - } - if (rawPacketString[1] == 'c') { return std::make_unique(rawPacket); } diff --git a/src/DebugServer/Gdb/RegisterDescriptor.hpp b/src/DebugServer/Gdb/RegisterDescriptor.hpp index 3e235cb5..6f4e0cf3 100644 --- a/src/DebugServer/Gdb/RegisterDescriptor.hpp +++ b/src/DebugServer/Gdb/RegisterDescriptor.hpp @@ -5,27 +5,28 @@ namespace Bloom::DebugServer::Gdb { - using GdbRegisterNumber = int; + using GdbRegisterId = std::uint16_t; /* * GDB defines a set of registers for each target architecture. * - * Each register in the set is assigned a register number, which is used to identify the register. + * Each register in the set is assigned an ID, which is used to identify the registers. Although the mapping of + * registers to IDs is hardcoded in GDB, GDB server implementations are expected to be aware of this mapping. */ struct RegisterDescriptor { - GdbRegisterNumber number; + GdbRegisterId id; std::uint16_t size; std::string name; - RegisterDescriptor(GdbRegisterNumber number, std::uint16_t size, const std::string& name) - : number(number) + RegisterDescriptor(GdbRegisterId id, std::uint16_t size, const std::string& name) + : id(id) , size(size) , name(name) {}; bool operator == (const RegisterDescriptor& other) const { - return this->number == other.number; + return this->id == other.id; } bool operator != (const RegisterDescriptor& other) const { @@ -33,7 +34,7 @@ namespace Bloom::DebugServer::Gdb } bool operator < (const RegisterDescriptor& rhs) const { - return this->number < rhs.number; + return this->id < rhs.id; } bool operator > (const RegisterDescriptor& rhs) const { @@ -64,7 +65,7 @@ namespace std public: std::size_t operator () (const Bloom::DebugServer::Gdb::RegisterDescriptor& descriptor) const { // We use the GDB register number as the hash, as it is unique to the register. - return static_cast(descriptor.number); + return static_cast(descriptor.id); } }; } diff --git a/src/DebugServer/Gdb/TargetDescriptor.cpp b/src/DebugServer/Gdb/TargetDescriptor.cpp new file mode 100644 index 00000000..d37cffad --- /dev/null +++ b/src/DebugServer/Gdb/TargetDescriptor.cpp @@ -0,0 +1,64 @@ +#include "TargetDescriptor.hpp" + +namespace Bloom::DebugServer::Gdb +{ + TargetDescriptor::TargetDescriptor( + const Targets::TargetDescriptor& targetDescriptor, + const BiMap& memoryOffsetsByType, + std::map gdbRegisterDescriptorsById, + std::map gdbRegisterIdsByTargetRegisterDescriptorId, + std::map targetRegisterDescriptorIdsByGdbRegisterId + ) + : targetDescriptor(targetDescriptor) + , memoryOffsetsByType(memoryOffsetsByType) + , memoryOffsets(memoryOffsetsByType.getValues()) + , gdbRegisterDescriptorsById(gdbRegisterDescriptorsById) + , gdbRegisterIdsByTargetRegisterDescriptorId(gdbRegisterIdsByTargetRegisterDescriptorId) + , targetRegisterDescriptorIdsByGdbRegisterId(targetRegisterDescriptorIdsByGdbRegisterId) + {} + + std::uint32_t TargetDescriptor::getMemoryOffset(Targets::TargetMemoryType memoryType) const { + return this->memoryOffsetsByType.valueAt(memoryType).value_or(0); + } + + Targets::TargetMemoryType TargetDescriptor::getMemoryTypeFromGdbAddress(std::uint32_t address) const { + // Start with the largest offset until we find a match + for ( + auto memoryOffsetIt = this->memoryOffsets.rbegin(); + memoryOffsetIt != this->memoryOffsets.rend(); + ++memoryOffsetIt + ) { + if ((address & *memoryOffsetIt) == *memoryOffsetIt) { + return this->memoryOffsetsByType.at(*memoryOffsetIt); + } + } + + return Targets::TargetMemoryType::FLASH; + } + + std::optional TargetDescriptor::getGdbRegisterIdFromTargetRegisterDescriptorId( + Targets::TargetRegisterDescriptorId targetRegisterDescriptorId + ) const { + const auto gdbRegisterIdIt = this->gdbRegisterIdsByTargetRegisterDescriptorId.find( + targetRegisterDescriptorId + ); + + if (gdbRegisterIdIt != this->gdbRegisterIdsByTargetRegisterDescriptorId.end()) { + return gdbRegisterIdIt->second; + } + + return std::nullopt; + } + + std::optional TargetDescriptor::getTargetRegisterDescriptorIdFromGdbRegisterId( + GdbRegisterId gdbRegisterId + ) const { + const auto registerDescriptorIdIt = this->targetRegisterDescriptorIdsByGdbRegisterId.find(gdbRegisterId); + + if (registerDescriptorIdIt != this->targetRegisterDescriptorIdsByGdbRegisterId.end()) { + return registerDescriptorIdIt->second; + } + + return std::nullopt; + } +} diff --git a/src/DebugServer/Gdb/TargetDescriptor.hpp b/src/DebugServer/Gdb/TargetDescriptor.hpp index 06b2f3bf..be143f05 100644 --- a/src/DebugServer/Gdb/TargetDescriptor.hpp +++ b/src/DebugServer/Gdb/TargetDescriptor.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "src/Helpers/BiMap.hpp" #include "src/Targets/TargetDescriptor.hpp" @@ -21,21 +22,19 @@ namespace Bloom::DebugServer::Gdb { public: Targets::TargetDescriptor targetDescriptor; + std::map gdbRegisterDescriptorsById; explicit TargetDescriptor( const Targets::TargetDescriptor& targetDescriptor, - const BiMap& memoryOffsetsByType - ) - : targetDescriptor(targetDescriptor) - , memoryOffsetsByType(memoryOffsetsByType) - , memoryOffsets(memoryOffsetsByType.getValues()) - {} + const BiMap& memoryOffsetsByType, + std::map gdbRegisterDescriptorsById, + std::map gdbRegisterIdsByTargetRegisterDescriptorId, + std::map targetRegisterDescriptorIdsByGdbRegisterId + ); virtual ~TargetDescriptor() = default; - virtual std::uint32_t getMemoryOffset(Targets::TargetMemoryType memoryType) const { - return this->memoryOffsetsByType.valueAt(memoryType).value_or(0); - } + std::uint32_t getMemoryOffset(Targets::TargetMemoryType memoryType) const; /** * Helper method to extract the target memory type (Flash, RAM, etc) from a GDB memory address. @@ -43,58 +42,35 @@ namespace Bloom::DebugServer::Gdb * @param address * @return */ - Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) const { - // Start with the largest offset until we find a match - for ( - auto memoryOffsetIt = this->memoryOffsets.rbegin(); - memoryOffsetIt != this->memoryOffsets.rend(); - ++memoryOffsetIt - ) { - if ((address & *memoryOffsetIt) == *memoryOffsetIt) { - return this->memoryOffsetsByType.at(*memoryOffsetIt); - } - } - - return Targets::TargetMemoryType::FLASH; - } + Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) const; /** - * Should retrieve the GDB register number, given a target register descriptor. Or std::nullopt if the target - * register descriptor isn't mapped to any GDB register. + * Should retrieve the GDB register ID, given a target register descriptor ID. Or std::nullopt if the + * target register descriptor ID isn't mapped to any GDB register. * - * @param registerDescriptor + * @param registerDescriptorId * @return */ - virtual std::optional getRegisterNumberFromTargetRegisterDescriptor( - const Targets::TargetRegisterDescriptor& registerDescriptor - ) const = 0; + std::optional getGdbRegisterIdFromTargetRegisterDescriptorId( + Targets::TargetRegisterDescriptorId targetRegisterDescriptorId + ) const; /** - * Should retrieve the GDB register descriptor for a given GDB register number. + * Should retrieve the mapped target register descriptor ID for a given GDB register ID. * - * @param number + * This function may return std::nullopt if the GDB register ID maps to something that isn't considered a + * register on our end. For example, for AVR targets, the GDB register ID 34 maps to the program counter. But + * the program counter is not treated like any other register in Bloom (there's no TargetRegisterDescriptor for + * it). So in that case, the GDB register ID is not mapped to any target register descriptor ID. + * + * @param gdbRegisterId * @return */ - virtual const RegisterDescriptor& getRegisterDescriptorFromNumber(GdbRegisterNumber number) const = 0; + std::optional getTargetRegisterDescriptorIdFromGdbRegisterId( + GdbRegisterId gdbRegisterId + ) const; - /** - * Should retrieve the mapped target register descriptor for a given GDB register number. - * - * @param number - * @return - */ - virtual const Targets::TargetRegisterDescriptor& getTargetRegisterDescriptorFromNumber( - GdbRegisterNumber number - ) const = 0; - - /** - * Should return all allocated GDB register numbers for the target. - * - * @return - */ - virtual const std::vector& getRegisterNumbers() const = 0; - - private: + protected: /** * When GDB sends us a memory address, the memory type (Flash, RAM, EEPROM, etc) is embedded within. This is * done by ORing the address with some constant. For example, for AVR targets, RAM addresses are ORed with @@ -109,5 +85,8 @@ namespace Bloom::DebugServer::Gdb * Sorted set of the known memory offsets (see memoryOffsetsByType). */ std::set memoryOffsets; + + std::map gdbRegisterIdsByTargetRegisterDescriptorId; + std::map targetRegisterDescriptorIdsByGdbRegisterId; }; } diff --git a/src/DebugToolDrivers/DebugTool.hpp b/src/DebugToolDrivers/DebugTool.hpp index 7234f13d..b79d4f4a 100644 --- a/src/DebugToolDrivers/DebugTool.hpp +++ b/src/DebugToolDrivers/DebugTool.hpp @@ -3,6 +3,11 @@ #include "TargetInterfaces/TargetPowerManagementInterface.hpp" #include "TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp" +#include "src/Targets/Microchip/AVR/AVR8/Avr8TargetConfig.hpp" +#include "src/Targets/Microchip/AVR/AVR8/Family.hpp" +#include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp" +#include "src/Targets/TargetRegister.hpp" + #include "TargetInterfaces/Microchip/AVR/AvrIspInterface.hpp" namespace Bloom @@ -67,7 +72,12 @@ namespace Bloom * * @return */ - virtual DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* getAvr8DebugInterface() { + virtual DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* getAvr8DebugInterface( + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig, + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters, + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById + ) { return nullptr; } @@ -81,7 +91,9 @@ namespace Bloom * * @return */ - virtual DebugToolDrivers::TargetInterfaces::Microchip::Avr::AvrIspInterface* getAvrIspInterface() { + virtual DebugToolDrivers::TargetInterfaces::Microchip::Avr::AvrIspInterface* getAvrIspInterface( + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig + ) { return nullptr; } diff --git a/src/DebugToolDrivers/Microchip/EdbgDevice.cpp b/src/DebugToolDrivers/Microchip/EdbgDevice.cpp index 1685b8b5..d0fea6aa 100644 --- a/src/DebugToolDrivers/Microchip/EdbgDevice.cpp +++ b/src/DebugToolDrivers/Microchip/EdbgDevice.cpp @@ -66,7 +66,6 @@ namespace Bloom::DebugToolDrivers ); } - this->edbgAvr8Interface = std::make_unique(this->edbgInterface.get()); this->edbgAvrIspInterface = std::make_unique(this->edbgInterface.get()); this->setInitialised(true); @@ -81,6 +80,27 @@ namespace Bloom::DebugToolDrivers UsbDevice::close(); } + TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* EdbgDevice::getAvr8DebugInterface( + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig, + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters, + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById + ) { + if (this->edbgAvr8Interface == nullptr) { + this->edbgAvr8Interface = std::make_unique( + this->edbgInterface.get(), + targetConfig, + targetFamily, + targetParameters, + targetRegisterDescriptorsById + ); + + this->configureAvr8Interface(); + } + + return this->edbgAvr8Interface.get(); + } + std::string EdbgDevice::getSerialNumber() { using namespace CommandFrames::Discovery; using ResponseFrames::Discovery::ResponseId; diff --git a/src/DebugToolDrivers/Microchip/EdbgDevice.hpp b/src/DebugToolDrivers/Microchip/EdbgDevice.hpp index c6d273dd..b588aafa 100644 --- a/src/DebugToolDrivers/Microchip/EdbgDevice.hpp +++ b/src/DebugToolDrivers/Microchip/EdbgDevice.hpp @@ -50,11 +50,16 @@ namespace Bloom::DebugToolDrivers */ void close() override; - TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* getAvr8DebugInterface() override { - return this->edbgAvr8Interface.get(); - } + TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* getAvr8DebugInterface( + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig, + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters, + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById + ) override; - TargetInterfaces::Microchip::Avr::AvrIspInterface* getAvrIspInterface() override { + TargetInterfaces::Microchip::Avr::AvrIspInterface* getAvrIspInterface( + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig + ) override { return this->edbgAvrIspInterface.get(); } @@ -153,5 +158,9 @@ namespace Bloom::DebugToolDrivers * @return */ std::uint16_t getCmsisHidReportSize(); + + virtual void configureAvr8Interface() { + return; + } }; } diff --git a/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.cpp b/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.cpp index 3a2a5f5f..35f7a6c3 100644 --- a/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.cpp +++ b/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.cpp @@ -18,7 +18,6 @@ namespace Bloom::DebugToolDrivers try { EdbgDevice::init(); - this->edbgAvr8Interface->setReactivateJtagTargetPostProgrammingMode(true); } catch (const DeviceNotFound& exception) { /* @@ -40,4 +39,8 @@ namespace Bloom::DebugToolDrivers throw exception; } } + + void MplabPickit4::configureAvr8Interface() { + this->edbgAvr8Interface->setReactivateJtagTargetPostProgrammingMode(true); + } } diff --git a/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.hpp b/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.hpp index a8ca45f4..1ab4c8e1 100644 --- a/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.hpp +++ b/src/DebugToolDrivers/Microchip/MplabPickit4/MplabPickit4.hpp @@ -36,5 +36,8 @@ namespace Bloom::DebugToolDrivers } void init() override; + + protected: + void configureAvr8Interface() override; }; } diff --git a/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.cpp b/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.cpp index 7253cbcc..732cfaa1 100644 --- a/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.cpp +++ b/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.cpp @@ -18,7 +18,6 @@ namespace Bloom::DebugToolDrivers try { EdbgDevice::init(); - this->edbgAvr8Interface->setReactivateJtagTargetPostProgrammingMode(true); } catch (const DeviceNotFound& exception) { /* @@ -48,4 +47,8 @@ namespace Bloom::DebugToolDrivers throw exception; } } + + void MplabSnap::configureAvr8Interface() { + this->edbgAvr8Interface->setReactivateJtagTargetPostProgrammingMode(true); + } } diff --git a/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.hpp b/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.hpp index e541829c..3c71d2cf 100644 --- a/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.hpp +++ b/src/DebugToolDrivers/Microchip/MplabSnap/MplabSnap.hpp @@ -39,5 +39,8 @@ namespace Bloom::DebugToolDrivers } void init() override; + + protected: + void configureAvr8Interface() override; }; } diff --git a/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.cpp b/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.cpp index e5f689cb..bdf913bb 100644 --- a/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.cpp +++ b/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.cpp @@ -11,9 +11,7 @@ namespace Bloom::DebugToolDrivers ) {} - void XplainedPro::init() { - EdbgDevice::init(); - + void XplainedPro::configureAvr8Interface() { this->edbgAvr8Interface->setMaximumMemoryAccessSizePerRequest(256); } } diff --git a/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.hpp b/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.hpp index 408363ca..52d66e93 100644 --- a/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.hpp +++ b/src/DebugToolDrivers/Microchip/XplainedPro/XplainedPro.hpp @@ -27,6 +27,7 @@ namespace Bloom::DebugToolDrivers return "Xplained Pro"; } - void init() override; + protected: + void configureAvr8Interface() override; }; } diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp index 8c17c76f..848cbb5d 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp @@ -1,6 +1,7 @@ #include "EdbgAvr8Interface.hpp" #include +#include #include #include "src/Services/PathService.hpp" @@ -80,71 +81,26 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr using Bloom::Targets::TargetRegister; using Bloom::Targets::TargetRegisterDescriptor; using Bloom::Targets::TargetRegisterDescriptors; + using Bloom::Targets::TargetRegisterDescriptorId; + using Bloom::Targets::TargetRegisterDescriptorIds; using Bloom::Targets::TargetRegisterType; using Bloom::Targets::TargetRegisters; - EdbgAvr8Interface::EdbgAvr8Interface(EdbgInterface* edbgInterface) + EdbgAvr8Interface::EdbgAvr8Interface( + EdbgInterface* edbgInterface, + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig, + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters, + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById + ) : edbgInterface(edbgInterface) + , targetConfig(targetConfig) + , family(targetFamily) + , targetParameters(targetParameters) + , targetRegisterDescriptorsById(targetRegisterDescriptorsById) + , configVariant(EdbgAvr8Interface::resolveConfigVariant(targetFamily, targetConfig.physicalInterface)) {} - void EdbgAvr8Interface::configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) { - this->targetConfig = targetConfig; - - this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE); - } - - void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& config) { - this->targetParameters = config; - - if (!config.stackPointerRegisterLowAddress.has_value()) { - throw DeviceInitializationFailure("Failed to find stack pointer register start address"); - } - - if (!config.stackPointerRegisterSize.has_value()) { - throw DeviceInitializationFailure("Failed to find stack pointer register size"); - } - - if (!config.statusRegisterStartAddress.has_value()) { - throw DeviceInitializationFailure("Failed to find status register start address"); - } - - if (!config.statusRegisterSize.has_value()) { - throw DeviceInitializationFailure("Failed to find status register size"); - } - - if (this->configVariant == Avr8ConfigVariant::NONE) { - auto configVariant = this->resolveConfigVariant(); - - if (!configVariant.has_value()) { - throw DeviceInitializationFailure("Failed to resolve config variant for the selected " - "physical interface and AVR8 family. The selected physical interface is not known to be supported " - "by the AVR8 family." - ); - } - - this->configVariant = configVariant.value(); - } - - switch (this->configVariant) { - case Avr8ConfigVariant::DEBUG_WIRE: - case Avr8ConfigVariant::MEGAJTAG: { - this->setDebugWireAndJtagParameters(); - break; - } - case Avr8ConfigVariant::XMEGA: { - this->setPdiParameters(); - break; - } - case Avr8ConfigVariant::UPDI: { - this->setUpdiParameters(); - break; - } - default: { - break; - } - } - } - void EdbgAvr8Interface::init() { if (this->configVariant == Avr8ConfigVariant::XMEGA) { // Default PDI clock to 4MHz @@ -177,8 +133,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr this->setParameter( Avr8EdbgParameters::PHYSICAL_INTERFACE, - getAvr8PhysicalInterfaceToIdMapping().at(this->targetConfig->physicalInterface) + getAvr8PhysicalInterfaceToIdMapping().at(this->targetConfig.physicalInterface) ); + + this->setTargetParameters(); } void EdbgAvr8Interface::stop() { @@ -267,7 +225,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr } catch (const Avr8CommandFailure& activationException) { if ( - this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE + this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE && ( activationException.code == Avr8CommandFailureCode::DEBUGWIRE_PHYSICAL_ERROR || activationException.code == Avr8CommandFailureCode::FAILED_TO_ENABLE_OCD @@ -292,8 +250,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr void EdbgAvr8Interface::deactivate() { if (this->targetAttached) { if ( - this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE - && this->targetConfig->disableDebugWireOnDeactivate + this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE + && this->targetConfig.disableDebugWireOnDeactivate ) { try { this->disableDebugWire(); @@ -385,7 +343,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr throw Avr8CommandFailure("AVR8 Get device ID command failed", responseFrame); } - return responseFrame.extractSignature(this->targetConfig->physicalInterface); + return responseFrame.extractSignature(this->targetConfig.physicalInterface); } void EdbgAvr8Interface::setBreakpoint(TargetMemoryAddress address) { @@ -418,7 +376,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr } } - TargetRegisters EdbgAvr8Interface::readRegisters(const TargetRegisterDescriptors& descriptors) { + TargetRegisters EdbgAvr8Interface::readRegisters(const TargetRegisterDescriptorIds& descriptorIds) { /* * This function needs to be fast. Insight eagerly requests the values of all known registers that it can * present to the user. It does this on numerous occasions (target stopped, user clicked refresh, etc). This @@ -439,7 +397,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr auto output = TargetRegisters(); // Group descriptors by type and resolve the address range for each type - auto descriptorsByType = std::map>(); + auto descriptorIdsByType = std::map>(); /* * An address range is just an std::pair of addresses - the first being the start address, the second being @@ -450,7 +408,12 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr using AddressRange = std::pair; auto addressRangeByType = std::map(); - for (const auto& descriptor : descriptors) { + for (const auto& descriptorId : descriptorIds) { + const auto descriptorIt = this->targetRegisterDescriptorsById.find(descriptorId); + assert(descriptorIt != this->targetRegisterDescriptorsById.end()); + + const auto& descriptor = descriptorIt->second; + if (!descriptor.startAddress.has_value()) { Logger::debug( "Attempted to read register in the absence of a start address - register name: " @@ -459,18 +422,18 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr continue; } - descriptorsByType[descriptor.type].insert(&descriptor); + descriptorIdsByType[descriptor.type].insert(descriptor.id); const auto startAddress = descriptor.startAddress.value(); const auto endAddress = startAddress + (descriptor.size - 1); - const auto addressRangeit = addressRangeByType.find(descriptor.type); + const auto addressRangeIt = addressRangeByType.find(descriptor.type); - if (addressRangeit == addressRangeByType.end()) { + if (addressRangeIt == addressRangeByType.end()) { addressRangeByType[descriptor.type] = AddressRange(startAddress, endAddress); } else { - auto& addressRange = addressRangeit->second; + auto& addressRange = addressRangeIt->second; if (startAddress < addressRange.first) { addressRange.first = startAddress; @@ -486,16 +449,17 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * Now that we have our address ranges and grouped descriptors, we can perform a single read call for each * register type. */ - for (const auto&[registerType, descriptors] : descriptorsByType) { + for (const auto&[registerType, descriptorIds] : descriptorIdsByType) { const auto& addressRange = addressRangeByType[registerType]; const auto startAddress = addressRange.first; const auto endAddress = addressRange.second; - const auto bufferSize = (endAddress - startAddress) + 1; + const auto readSize = (endAddress - startAddress) + 1; - const auto memoryType = (registerType != TargetRegisterType::GENERAL_PURPOSE_REGISTER) ? - Avr8MemoryType::SRAM + const auto memoryType = (registerType != TargetRegisterType::GENERAL_PURPOSE_REGISTER) + ? Avr8MemoryType::SRAM : (this->configVariant == Avr8ConfigVariant::XMEGA || this->configVariant == Avr8ConfigVariant::UPDI - ? Avr8MemoryType::REGISTER_FILE : Avr8MemoryType::SRAM); + ? Avr8MemoryType::REGISTER_FILE + : Avr8MemoryType::SRAM); /* * When reading the entire range, we must avoid any attempts to access the OCD data register (OCDDR), as @@ -526,37 +490,40 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr ); } - const auto flatMemoryBuffer = this->readMemory( + const auto flatMemoryData = this->readMemory( memoryType, startAddress, - bufferSize, + readSize, excludedAddresses ); - if (flatMemoryBuffer.size() != bufferSize) { + if (flatMemoryData.size() != readSize) { throw Exception( "Failed to read memory within register type address range (" + std::to_string(startAddress) - + " - " + std::to_string(endAddress) + "). Expected " + std::to_string(bufferSize) - + " bytes, got " + std::to_string(flatMemoryBuffer.size()) + + " - " + std::to_string(endAddress) + "). Expected " + std::to_string(readSize) + + " bytes, got " + std::to_string(flatMemoryData.size()) ); } // Construct our TargetRegister objects directly from the flat memory buffer - for (const auto& descriptor : descriptors) { + for (const auto descriptorId : descriptorIds) { + const auto descriptorIt = this->targetRegisterDescriptorsById.find(descriptorId); + const auto& descriptor = descriptorIt->second; + /* * Multibyte AVR8 registers are stored in LSB form. * - * This is why we use reverse iterators when extracting our data from flatMemoryBuffer. Doing so allows + * This is why we use reverse iterators when extracting our data from flatMemoryData. Doing so allows * us to extract the data in MSB form (as is expected for all register values held in TargetRegister * objects). */ - const auto bufferStartIt = flatMemoryBuffer.rend() - (descriptor->startAddress.value() - startAddress) - - descriptor->size; + const auto bufferStartIt = flatMemoryData.rend() - (descriptor.startAddress.value() - startAddress) + - descriptor.size; output.emplace_back( TargetRegister( - *descriptor, - TargetMemoryBuffer(bufferStartIt, bufferStartIt + descriptor->size) + descriptor.id, + TargetMemoryBuffer(bufferStartIt, bufferStartIt + descriptor.size) ) ); } @@ -567,7 +534,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr void EdbgAvr8Interface::writeRegisters(const Targets::TargetRegisters& registers) { for (const auto& reg : registers) { - const auto& registerDescriptor = reg.descriptor; + const auto& registerDescriptorIt = this->targetRegisterDescriptorsById.find(reg.descriptorId); + assert(registerDescriptorIt != this->targetRegisterDescriptorsById.end()); + + const auto& registerDescriptor = registerDescriptorIt->second; auto registerValue = reg.value; if (registerValue.empty()) { @@ -827,7 +797,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr */ auto eepromSnapshot = std::optional(); - if (this->targetConfig->preserveEeprom) { + if (this->targetConfig.preserveEeprom) { Logger::debug("Capturing EEPROM data, in preparation for chip erase"); eepromSnapshot = this->readMemory( TargetMemoryType::EEPROM, @@ -910,6 +880,43 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr } } + void EdbgAvr8Interface::setTargetParameters() { + if (!this->targetParameters.stackPointerRegisterLowAddress.has_value()) { + throw DeviceInitializationFailure("Failed to find stack pointer register start address"); + } + + if (!this->targetParameters.stackPointerRegisterSize.has_value()) { + throw DeviceInitializationFailure("Failed to find stack pointer register size"); + } + + if (!this->targetParameters.statusRegisterStartAddress.has_value()) { + throw DeviceInitializationFailure("Failed to find status register start address"); + } + + if (!this->targetParameters.statusRegisterSize.has_value()) { + throw DeviceInitializationFailure("Failed to find status register size"); + } + + switch (this->configVariant) { + case Avr8ConfigVariant::DEBUG_WIRE: + case Avr8ConfigVariant::MEGAJTAG: { + this->setDebugWireAndJtagParameters(); + break; + } + case Avr8ConfigVariant::XMEGA: { + this->setPdiParameters(); + break; + } + case Avr8ConfigVariant::UPDI: { + this->setUpdiParameters(); + break; + } + default: { + break; + } + } + } + std::map> EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface() { return std::map>({ @@ -963,49 +970,21 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr }); } - std::optional EdbgAvr8Interface::resolveConfigVariant() { - if (this->family.has_value()) { - const auto configVariantsByFamily = EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface(); - const auto configVariantsByPhysicalInterfaceIt = configVariantsByFamily.find(*(this->family)); + Avr8ConfigVariant EdbgAvr8Interface::resolveConfigVariant( + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface + ) { + const auto configVariantsByFamily = EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface(); + const auto configVariantsByPhysicalInterfaceIt = configVariantsByFamily.find(targetFamily); - if (configVariantsByPhysicalInterfaceIt != configVariantsByFamily.end()) { - const auto& configVariantsByPhysicalInterface = configVariantsByPhysicalInterfaceIt->second; - const auto configVariantIt = configVariantsByPhysicalInterface.find( - this->targetConfig->physicalInterface - ); + assert(configVariantsByPhysicalInterfaceIt != configVariantsByFamily.end()); - if (configVariantIt != configVariantsByPhysicalInterface.end()) { - return configVariantIt->second; - } - } + const auto& configVariantsByPhysicalInterface = configVariantsByPhysicalInterfaceIt->second; + const auto configVariantIt = configVariantsByPhysicalInterface.find(physicalInterface); - } else { - /* - * If there is no family set, we may be able to resort to a simpler mapping of physical interfaces - * to config variants. But this will only work if the selected physical interface is *NOT* JTAG. - * - * This is because JTAG is the only physical interface that could map to two different config - * variants (MEGAJTAG and XMEGA). The only way we can figure out which config variant to use is if we - * know the target family. - * - * This is why we don't allow users to use ambiguous target names (such as the generic "avr8" target - * name), when using the JTAG physical interface. We won't be able to resolve the correct target - * variant. Users are required to specify the exact target name in their config, when using the JTAG - * physical interface. That way, this->family will be set by the time resolveConfigVariant() is called. - */ - static const std::map physicalInterfacesToConfigVariants = { - {PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, - {PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA}, - {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI}, - }; - const auto configVariantIt = physicalInterfacesToConfigVariants.find(this->targetConfig->physicalInterface); + assert(configVariantIt != configVariantsByPhysicalInterface.end()); - if (configVariantIt != physicalInterfacesToConfigVariants.end()) { - return configVariantIt->second; - } - } - - return std::nullopt; + return configVariantIt->second; } void EdbgAvr8Interface::setParameter(const Avr8EdbgParameter& parameter, const std::vector& value) { diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp index 25d45552..ed3f7e87 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp @@ -10,9 +10,10 @@ #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/EdbgInterface.hpp" #include "src/Targets/TargetMemory.hpp" -#include "src/Targets/Microchip/AVR/Target.hpp" +#include "src/Targets/TargetRegister.hpp" #include "src/Targets/Microchip/AVR/AVR8/Family.hpp" #include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp" +#include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp" namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr { @@ -28,7 +29,13 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr class EdbgAvr8Interface: public TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface { public: - explicit EdbgAvr8Interface(EdbgInterface* edbgInterface); + explicit EdbgAvr8Interface( + EdbgInterface* edbgInterface, + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig, + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters, + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById + ); /** * Some EDBG devices don't seem to operate correctly when actioning the masked memory read EDBG command. The @@ -83,32 +90,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * See the comments in that class for more info on the expected behaviour of each method. */ - /** - * As already mentioned in numerous comments above, the EdbgAvr8Interface requires some configuration from - * the user. This is supplied via the user's Bloom configuration. - * - * @param targetConfig - */ - void configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) override; - - /** - * Configures the target family. For some physical interfaces, the target family is required in order - * properly configure the EDBG tool. See EdbgAvr8Interface::resolveConfigVariant() for more. - * - * @param family - */ - void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) override { - this->family = family; - } - - /** - * Accepts target parameters from the AVR8 target instance and sends the necessary target parameters to the - * debug tool. - * - * @param config - */ - void setTargetParameters(const Targets::Microchip::Avr::Avr8Bit::TargetParameters& config) override; - /** * Initialises the AVR8 Generic protocol interface by setting the appropriate parameters on the debug tool. */ @@ -207,10 +188,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr /** * Reads registers from the target. * - * @param descriptors + * @param descriptorIds * @return */ - Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptors& descriptors) override; + Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) override; /** * Writes registers to target. @@ -289,7 +270,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr /** * Project's AVR8 target configuration. */ - std::optional targetConfig; + const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig; /** * The target family is taken into account when configuring the AVR8 Generic protocol on the EDBG device. @@ -297,7 +278,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * We use this to determine which config variant to select. * See EdbgAvr8Interface::resolveConfigVariant() for more. */ - std::optional family; + Targets::Microchip::Avr::Avr8Bit::Family family; /** * The AVR8 Generic protocol provides two functions: Debugging and programming. The desired function must be @@ -320,7 +301,9 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * For the EdbgAvr8Interface, we send the required parameters to the debug tool immediately upon receiving * them. See EdbgAvr8Interface::setTargetParameters(). */ - Targets::Microchip::Avr::Avr8Bit::TargetParameters targetParameters; + const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters; + + const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById; /** * See the comment for EdbgAvr8Interface::setAvoidMaskedMemoryRead(). @@ -357,6 +340,13 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr bool programmingModeEnabled = false; + /** + * Sends the necessary target parameters to the debug tool. + * + * @param config + */ + void setTargetParameters(); + /** * This mapping allows us to determine which config variant to select, based on the target family and the * selected physical interface. @@ -367,11 +357,14 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr > getConfigVariantsByFamilyAndPhysicalInterface(); /** - * Will attempt to resolve the config variant with the information currently held. + * Determines the config variant given a target family and physical interface. * * @return */ - std::optional resolveConfigVariant(); + static Avr8ConfigVariant resolveConfigVariant( + Targets::Microchip::Avr::Avr8Bit::Family targetFamily, + Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface + ); /** * Sets an AVR8 parameter on the debug tool. See the Avr8EdbgParameters class and protocol documentation diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Events/AVR8Generic/BreakEvent.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Events/AVR8Generic/BreakEvent.hpp index 1929077b..ce57a67a 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Events/AVR8Generic/BreakEvent.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Events/AVR8Generic/BreakEvent.hpp @@ -2,8 +2,8 @@ #include -#include "../../AvrEvent.hpp" -#include "src/Targets/Microchip/AVR/Target.hpp" +#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/AvrEvent.hpp" +#include "src/Targets/TargetBreakpoint.hpp" namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr { diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp index 8e4c154c..1632dfcf 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp @@ -41,28 +41,6 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8 Avr8DebugInterface& operator = (const Avr8DebugInterface& other) = default; Avr8DebugInterface& operator = (Avr8DebugInterface&& other) = default; - /** - * Configures the interface. Any debug tool -> target interface specific configuration should take - * place here. - * - * @param targetConfig - */ - virtual void configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) = 0; - - /** - * Sets the target family, independent of other configuration. - * - * @param family - */ - virtual void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) = 0; - - /** - * Should accept Avr8 target parameters for configuration of the interface. - * - * @param config - */ - virtual void setTargetParameters(const Targets::Microchip::Avr::Avr8Bit::TargetParameters& config) = 0; - /** * Should initialise the interface between the debug tool and the AVR8 target. */ @@ -153,12 +131,12 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8 /** * Should read the requested registers from the target. * - * @param descriptors - * A collection of register descriptors, for the registers to be read. + * @param descriptorIds + * A collection of register descriptor IDs, for the registers to be read. * * @return */ - virtual Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptors& descriptors) = 0; + virtual Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) = 0; /** * Should update the value of the given registers. diff --git a/src/Insight/Insight.hpp b/src/Insight/Insight.hpp index 2298319a..487de291 100644 --- a/src/Insight/Insight.hpp +++ b/src/Insight/Insight.hpp @@ -79,19 +79,18 @@ namespace Bloom InsightProjectSettings& insightProjectSettings; EventListener& eventListener; + Services::TargetControllerService targetControllerService = Services::TargetControllerService(); QApplication application; std::map> insightWorkersById; - InsightWindow* mainWindow = new InsightWindow( this->environmentConfig, this->insightConfig, - this->insightProjectSettings + this->insightProjectSettings, + this->targetControllerService.getTargetDescriptor() ); - Services::TargetControllerService targetControllerService = Services::TargetControllerService(); - Targets::TargetState lastTargetState = Targets::TargetState::UNKNOWN; bool targetStepping = false; QTimer* targetResumeTimer = nullptr; diff --git a/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.cpp b/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.cpp index 16a2a750..f2858991 100644 --- a/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.cpp +++ b/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.cpp @@ -5,6 +5,6 @@ namespace Bloom using Services::TargetControllerService; void ReadTargetRegisters::run(TargetControllerService& targetControllerService) { - emit this->targetRegistersRead(targetControllerService.readRegisters(this->descriptors)); + emit this->targetRegistersRead(targetControllerService.readRegisters(this->descriptorIds)); } } diff --git a/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.hpp b/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.hpp index f5d75326..b84bac69 100644 --- a/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.hpp +++ b/src/Insight/InsightWorker/Tasks/ReadTargetRegisters.hpp @@ -10,12 +10,12 @@ namespace Bloom Q_OBJECT public: - explicit ReadTargetRegisters(const Targets::TargetRegisterDescriptors& descriptors) - : descriptors(descriptors) + explicit ReadTargetRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) + : descriptorIds(descriptorIds) {} QString brief() const override { - return "Reading target registers"; + return "Reading " + QString::number(this->descriptorIds.size()) + " target register(s)"; } TaskGroups taskGroups() const override { @@ -31,6 +31,6 @@ namespace Bloom void run(Services::TargetControllerService& targetControllerService) override; private: - Targets::TargetRegisterDescriptors descriptors; + Targets::TargetRegisterDescriptorIds descriptorIds; }; } diff --git a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp index 1d07be02..fe9733c7 100644 --- a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp @@ -36,13 +36,15 @@ namespace Bloom InsightWindow::InsightWindow( const EnvironmentConfig& environmentConfig, const InsightConfig& insightConfig, - InsightProjectSettings& insightProjectSettings + InsightProjectSettings& insightProjectSettings, + const Targets::TargetDescriptor& targetDescriptor ) : QMainWindow(nullptr) , environmentConfig(environmentConfig) , targetConfig(environmentConfig.targetConfig) , insightConfig(insightConfig) , insightProjectSettings(insightProjectSettings) + , targetDescriptor(targetDescriptor) { this->setObjectName("main-window"); this->setWindowTitle("Bloom Insight"); diff --git a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp index b9869bf1..18c87119 100644 --- a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp @@ -34,7 +34,8 @@ namespace Bloom InsightWindow( const EnvironmentConfig& environmentConfig, const InsightConfig& insightConfig, - InsightProjectSettings& insightProjectSettings + InsightProjectSettings& insightProjectSettings, + const Targets::TargetDescriptor& targetDescriptor ); void setEnvironmentConfig(const EnvironmentConfig& environmentConfig) { diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/RegisterHistoryWidget/RegisterHistoryWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/RegisterHistoryWidget/RegisterHistoryWidget.cpp index 3163e8f9..cdd11848 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/RegisterHistoryWidget/RegisterHistoryWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/RegisterHistoryWidget/RegisterHistoryWidget.cpp @@ -140,7 +140,7 @@ namespace Bloom::Widgets const QDateTime& changeDate ) { for (const auto& targetRegister : targetRegisters) { - if (targetRegister.descriptor == this->registerDescriptor) { + if (targetRegister.descriptorId == this->registerDescriptor.id) { this->addItem(targetRegister.value, changeDate); this->updateCurrentItemValue(targetRegister.value); } diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/TargetRegisterInspectorWindow.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/TargetRegisterInspectorWindow.cpp index 020f54ae..91b27dc3 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/TargetRegisterInspectorWindow.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegisterInspector/TargetRegisterInspectorWindow.cpp @@ -118,7 +118,7 @@ namespace Bloom::Widgets QString::fromStdString(this->registerDescriptor.description.value_or("")) ); - if (!this->registerDescriptor.writable) { + if (!this->registerDescriptor.access.writable) { this->registerValueTextInput->setDisabled(true); this->applyButton->setVisible(false); @@ -146,7 +146,7 @@ namespace Bloom::Widgets auto* bitsetWidget = new BitsetWidget( byteNumber, this->registerValue.at(registerByteIndex), - !this->registerDescriptor.writable, + !this->registerDescriptor.access.writable, this ); @@ -273,7 +273,7 @@ namespace Bloom::Widgets this->registerValueBitsetWidgetContainer->setDisabled(false); this->refreshValueButton->setDisabled(false); - if (this->registerDescriptor.writable) { + if (this->registerDescriptor.access.writable) { this->registerValueTextInput->setDisabled(false); this->applyButton->setDisabled(false); } @@ -319,7 +319,7 @@ namespace Bloom::Widgets void TargetRegisterInspectorWindow::refreshRegisterValue() { this->registerValueContainer->setDisabled(true); const auto readTargetRegisterTask = QSharedPointer( - new ReadTargetRegisters({this->registerDescriptor}), + new ReadTargetRegisters({this->registerDescriptor.id}), &QObject::deleteLater ); @@ -331,7 +331,7 @@ namespace Bloom::Widgets this->registerValueContainer->setDisabled(false); for (const auto& targetRegister : targetRegisters) { - if (targetRegister.descriptor == this->registerDescriptor) { + if (targetRegister.descriptorId == this->registerDescriptor.id) { this->setValue(targetRegister.value); } } @@ -353,7 +353,7 @@ namespace Bloom::Widgets void TargetRegisterInspectorWindow::applyChanges() { this->registerValueContainer->setDisabled(true); const auto targetRegister = Targets::TargetRegister( - this->registerDescriptor, + this->registerDescriptor.id, this->registerValue ); const auto writeRegisterTask = QSharedPointer( diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.cpp index c5a133f6..317fa930 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.cpp @@ -15,7 +15,7 @@ namespace Bloom::Widgets RegisterGroupItem::RegisterGroupItem( QString name, const std::set& registerDescriptors, - std::unordered_map& registerItemsByDescriptor + std::unordered_map& registerItemsByDescriptorIds ) : groupName(name) { @@ -25,7 +25,7 @@ namespace Bloom::Widgets registerItem->setVisible(this->isExpanded()); this->registerItems.push_back(registerItem); - registerItemsByDescriptor.insert(std::pair(registerDescriptor, registerItem)); + registerItemsByDescriptorIds.insert(std::pair(registerDescriptor.id, registerItem)); } if (!RegisterGroupItem::registerGroupIconPixmap.has_value()) { diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.hpp index d143e019..70d4c494 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/RegisterGroupItem.hpp @@ -22,7 +22,7 @@ namespace Bloom::Widgets explicit RegisterGroupItem( QString name, const std::set& registerDescriptors, - std::unordered_map& registerItemsByDescriptor + std::unordered_map& registerItemsByDescriptorIds ); bool isExpanded() const { diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.cpp index 91c637bc..0fd91719 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include "src/Insight/UserInterfaces/InsightWindow/UiLoader.hpp" #include "src/Insight/InsightSignals.hpp" @@ -72,36 +73,36 @@ namespace Bloom::Widgets this->filterRegisters(this->searchInput->text()); }); - const auto& registerDescriptors = targetDescriptor.registerDescriptorsByType; + const auto& registerDescriptors = targetDescriptor.registerDescriptorsById; - auto registerDescriptorsByGroupName = std::map>({ - { - "CPU General Purpose", - std::set( - registerDescriptors.at(TargetRegisterType::GENERAL_PURPOSE_REGISTER).begin(), - registerDescriptors.at(TargetRegisterType::GENERAL_PURPOSE_REGISTER).end() - ) + auto registerDescriptorsByGroupName = std::map>(); + + for (const auto& [descriptorId, descriptor] : registerDescriptors) { + if ( + descriptor.type != TargetRegisterType::GENERAL_PURPOSE_REGISTER + && descriptor.type != TargetRegisterType::PORT_REGISTER + && descriptor.type != TargetRegisterType::OTHER + ) { + continue; } - }); - for (const auto& registerDescriptor : registerDescriptors.at(TargetRegisterType::OTHER)) { - const auto groupName = QString::fromStdString(registerDescriptor.groupName.value_or("other")).toUpper(); - registerDescriptorsByGroupName[groupName].insert(registerDescriptor); - } + const auto groupName = descriptor.type == TargetRegisterType::GENERAL_PURPOSE_REGISTER + ? "CPU General Purpose" + : QString::fromStdString(descriptor.groupName.value_or("other")).toUpper(); + + registerDescriptorsByGroupName[groupName].insert(descriptor); + this->registerDescriptors.insert(descriptor); - for (const auto& registerDescriptor : registerDescriptors.at(TargetRegisterType::PORT_REGISTER)) { - const auto groupName = QString::fromStdString(registerDescriptor.groupName.value_or("other")).toUpper(); - registerDescriptorsByGroupName[groupName].insert(registerDescriptor); } for (const auto& [groupName, registerDescriptors] : registerDescriptorsByGroupName) { - this->registerGroupItems.emplace_back(new RegisterGroupItem( - groupName, - registerDescriptors, - this->registerItemsByDescriptor - )); - - this->registerDescriptors.insert(registerDescriptors.begin(), registerDescriptors.end()); + this->registerGroupItems.emplace_back( + new RegisterGroupItem( + groupName, + registerDescriptors, + this->registerItemsByDescriptorId + ) + ); } this->registerListView = new ListView( @@ -144,7 +145,7 @@ namespace Bloom::Widgets this, [this] { if (this->contextMenuRegisterItem != nullptr) { - this->refreshRegisterValues(this->contextMenuRegisterItem->registerDescriptor, std::nullopt); + this->refreshRegisterValues(this->contextMenuRegisterItem->registerDescriptor.id, std::nullopt); } } ); @@ -259,19 +260,31 @@ namespace Bloom::Widgets } void TargetRegistersPaneWidget::refreshRegisterValues( - std::optional registerDescriptor, + std::optional registerDescriptorId, std::optional> callback ) { - if (!registerDescriptor.has_value() && this->registerDescriptors.empty()) { + if (!registerDescriptorId.has_value() && this->registerDescriptors.empty()) { return; } + auto descriptorIds = Targets::TargetRegisterDescriptorIds(); + + if (registerDescriptorId.has_value()) { + descriptorIds.insert(*registerDescriptorId); + + } else { + std::transform( + this->registerDescriptors.begin(), + this->registerDescriptors.end(), + std::inserter(descriptorIds, descriptorIds.end()), + [] (const Targets::TargetRegisterDescriptor& descriptor) { + return descriptor.id; + } + ); + } + const auto readRegisterTask = QSharedPointer( - new ReadTargetRegisters( - registerDescriptor.has_value() - ? Targets::TargetRegisterDescriptors({*registerDescriptor}) - : this->registerDescriptors - ), + new ReadTargetRegisters(descriptorIds), &QObject::deleteLater ); @@ -353,7 +366,7 @@ namespace Bloom::Widgets const auto targetStopped = this->targetState == Targets::TargetState::STOPPED; const auto targetStoppedAndValuePresent = targetStopped - && this->currentRegisterValues.contains(this->contextMenuRegisterItem->registerDescriptor); + && this->currentRegisterValuesByDescriptorId.contains(this->contextMenuRegisterItem->registerDescriptor.id); this->refreshValueAction->setEnabled(targetStopped); this->copyValueDecimalAction->setEnabled(targetStoppedAndValuePresent); @@ -377,27 +390,26 @@ namespace Bloom::Widgets void TargetRegistersPaneWidget::onRegistersRead(const Targets::TargetRegisters& registers) { for (const auto& targetRegister : registers) { - const auto& descriptor = targetRegister.descriptor; - const auto& previousValueIt = this->currentRegisterValues.find(descriptor); - const auto& registerItemIt = this->registerItemsByDescriptor.find(descriptor); + const auto& previousValueIt = this->currentRegisterValuesByDescriptorId.find(targetRegister.descriptorId); + const auto& registerItemIt = this->registerItemsByDescriptorId.find(targetRegister.descriptorId); - if (registerItemIt != this->registerItemsByDescriptor.end()) { + if (registerItemIt != this->registerItemsByDescriptorId.end()) { auto& registerItem = registerItemIt->second; registerItem->setValue(targetRegister.value); - registerItem->valueChanged = previousValueIt != this->currentRegisterValues.end() + registerItem->valueChanged = previousValueIt != this->currentRegisterValuesByDescriptorId.end() ? previousValueIt->second != targetRegister.value : false; } - this->currentRegisterValues[descriptor] = targetRegister.value; + this->currentRegisterValuesByDescriptorId[targetRegister.descriptorId] = targetRegister.value; } this->registerListScene->update(); } void TargetRegistersPaneWidget::clearInlineRegisterValues() { - for (auto& [registerDescriptor, registerItem] : this->registerItemsByDescriptor) { + for (auto& [registerDescriptorId, registerItem] : this->registerItemsByDescriptorId) { registerItem->clearValue(); } @@ -411,10 +423,10 @@ namespace Bloom::Widgets TargetRegisterInspectorWindow* inspectionWindow = nullptr; - const auto& currentValueIt = this->currentRegisterValues.find(registerDescriptor); - const auto& inspectionWindowIt = this->inspectionWindowsByDescriptor.find(registerDescriptor); + const auto& currentValueIt = this->currentRegisterValuesByDescriptorId.find(registerDescriptor.id); + const auto& inspectionWindowIt = this->inspectionWindowsByDescriptorId.find(registerDescriptor.id); - if (inspectionWindowIt != this->inspectionWindowsByDescriptor.end()) { + if (inspectionWindowIt != this->inspectionWindowsByDescriptorId.end()) { inspectionWindow = inspectionWindowIt->second; } else { @@ -424,13 +436,13 @@ namespace Bloom::Widgets this ); - this->inspectionWindowsByDescriptor.insert(std::pair( - registerDescriptor, + this->inspectionWindowsByDescriptorId.insert(std::pair( + registerDescriptor.id, inspectionWindow )); } - if (currentValueIt != this->currentRegisterValues.end()) { + if (currentValueIt != this->currentRegisterValuesByDescriptorId.end()) { inspectionWindow->setValue(currentValueIt->second); } @@ -443,9 +455,9 @@ namespace Bloom::Widgets } void TargetRegistersPaneWidget::copyRegisterValueHex(const TargetRegisterDescriptor& registerDescriptor) { - const auto& valueIt = this->currentRegisterValues.find(registerDescriptor); + const auto& valueIt = this->currentRegisterValuesByDescriptorId.find(registerDescriptor.id); - if (valueIt == this->currentRegisterValues.end()) { + if (valueIt == this->currentRegisterValuesByDescriptorId.end()) { return; } @@ -459,9 +471,9 @@ namespace Bloom::Widgets } void TargetRegistersPaneWidget::copyRegisterValueDecimal(const TargetRegisterDescriptor& registerDescriptor) { - const auto& valueIt = this->currentRegisterValues.find(registerDescriptor); + const auto& valueIt = this->currentRegisterValuesByDescriptorId.find(registerDescriptor.id); - if (valueIt == this->currentRegisterValues.end()) { + if (valueIt == this->currentRegisterValuesByDescriptorId.end()) { return; } @@ -475,9 +487,9 @@ namespace Bloom::Widgets } void TargetRegistersPaneWidget::copyRegisterValueBinary(const TargetRegisterDescriptor& registerDescriptor) { - const auto& valueIt = this->currentRegisterValues.find(registerDescriptor); + const auto& valueIt = this->currentRegisterValuesByDescriptorId.find(registerDescriptor.id); - if (valueIt == this->currentRegisterValues.end()) { + if (valueIt == this->currentRegisterValuesByDescriptorId.end()) { return; } diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.hpp index 76bb28e9..2eb34b00 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetRegistersPane/TargetRegistersPaneWidget.hpp @@ -40,7 +40,7 @@ namespace Bloom::Widgets void expandAllRegisterGroups(); void refreshRegisterValues( - std::optional registerDescriptor = std::nullopt, + std::optional registerDescriptorId = std::nullopt, std::optional> callback = std::nullopt ); @@ -62,9 +62,9 @@ namespace Bloom::Widgets Targets::TargetRegisterDescriptors registerDescriptors; std::vector registerGroupItems; - std::unordered_map registerItemsByDescriptor; - std::unordered_map inspectionWindowsByDescriptor; - std::unordered_map currentRegisterValues; + std::unordered_map registerItemsByDescriptorId; + std::unordered_map inspectionWindowsByDescriptorId; + std::unordered_map currentRegisterValuesByDescriptorId; Targets::TargetState targetState = Targets::TargetState::UNKNOWN; diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.cpp index 84e3f0c4..1309e37d 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.cpp @@ -26,13 +26,6 @@ namespace Bloom::Widgets::InsightTargetWidgets &TargetPackageWidget::onTargetStateChanged ); - QObject::connect( - insightSignals, - &InsightSignals::targetRegistersWritten, - this, - &TargetPackageWidget::onRegistersWritten - ); - QObject::connect( insightSignals, &InsightSignals::programmingModeEnabled, @@ -106,18 +99,4 @@ namespace Bloom::Widgets::InsightTargetWidgets this->setDisabled(false); } } - - void TargetPackageWidget::onRegistersWritten(Targets::TargetRegisters targetRegisters) { - if (this->targetState != TargetState::STOPPED) { - return; - } - - // If a PORT register was just updated, refresh pin states. - for (const auto& targetRegister : targetRegisters) { - if (targetRegister.descriptor.type == Targets::TargetRegisterType::PORT_REGISTER) { - this->refreshPinStates(); - return; - } - } - } } diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.hpp index 2ecef2b1..b66fc349 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPackageWidget.hpp @@ -46,6 +46,5 @@ namespace Bloom::Widgets::InsightTargetWidgets void onTargetStateChanged(Targets::TargetState newState); void onProgrammingModeEnabled(); void onProgrammingModeDisabled(); - void onRegistersWritten(Targets::TargetRegisters targetRegisters); }; } diff --git a/src/Services/TargetControllerService.cpp b/src/Services/TargetControllerService.cpp index 03bbc62b..c137752d 100644 --- a/src/Services/TargetControllerService.cpp +++ b/src/Services/TargetControllerService.cpp @@ -159,9 +159,11 @@ namespace Bloom::Services ); } - TargetRegisters TargetControllerService::readRegisters(const TargetRegisterDescriptors& descriptors) const { + TargetRegisters TargetControllerService::readRegisters( + const Targets::TargetRegisterDescriptorIds& descriptorIds + ) const { return this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(descriptors), + std::make_unique(descriptorIds), this->defaultTimeout )->registers; } diff --git a/src/Services/TargetControllerService.hpp b/src/Services/TargetControllerService.hpp index 2ab7596e..3c46ffc1 100644 --- a/src/Services/TargetControllerService.hpp +++ b/src/Services/TargetControllerService.hpp @@ -98,12 +98,12 @@ namespace Bloom::Services /** * Requests the TargetController to read register values from the target. * - * @param descriptors - * Descriptors of the registers to read. + * @param descriptorIds + * Descriptor IDs of the registers to read. * * @return */ - Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptors& descriptors) const; + Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) const; /** * Requests the TargetController to write register values to the target. diff --git a/src/TargetController/Commands/ReadTargetRegisters.hpp b/src/TargetController/Commands/ReadTargetRegisters.hpp index 3048484c..df74a6ce 100644 --- a/src/TargetController/Commands/ReadTargetRegisters.hpp +++ b/src/TargetController/Commands/ReadTargetRegisters.hpp @@ -15,10 +15,10 @@ namespace Bloom::TargetController::Commands static constexpr CommandType type = CommandType::READ_TARGET_REGISTERS; static const inline std::string name = "ReadTargetRegisters"; - Targets::TargetRegisterDescriptors descriptors; + std::set descriptorIds; - explicit ReadTargetRegisters(const Targets::TargetRegisterDescriptors& descriptors) - : descriptors(descriptors) + explicit ReadTargetRegisters(const std::set& descriptorIds) + : descriptorIds(descriptorIds) {}; [[nodiscard]] CommandType getType() const override { diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index 305e34fb..decce9fc 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -326,42 +326,26 @@ namespace Bloom::TargetController std::map< std::string, - std::function()> + std::function(const TargetConfig&)> > TargetControllerComponent::getSupportedTargets() { using Avr8TargetDescriptionFile = Targets::Microchip::Avr::Avr8Bit::TargetDescription::TargetDescriptionFile; - auto mapping = std::map()>>({ - { - "avr8", - [] { - return std::make_unique(); - } - }, - }); + auto mapping = std::map(const TargetConfig&)>>(); // Include all targets from AVR8 target description files const auto avr8PdMapping = Avr8TargetDescriptionFile::getTargetDescriptionMapping(); - for (auto mapIt = avr8PdMapping.begin(); mapIt != avr8PdMapping.end(); mapIt++) { - // Each target signature maps to an array of targets, as numerous targets can possess the same signature. - const auto targets = mapIt.value().toArray(); + for (auto mapIt = avr8PdMapping.begin(); mapIt != avr8PdMapping.end(); ++mapIt) { + const auto mappingObject = mapIt.value().toObject(); + const auto targetName = mappingObject.find("name").value().toString().toLower().toStdString(); - for (auto targetIt = targets.begin(); targetIt != targets.end(); targetIt++) { - const auto targetName = targetIt->toObject().find("targetName").value().toString() - .toLower().toStdString(); - const auto targetSignatureHex = mapIt.key().toLower().toStdString(); - - if (!mapping.contains(targetName)) { - mapping.insert({ - targetName, - [targetName, targetSignatureHex] { - return std::make_unique( - targetName, - Targets::Microchip::Avr::TargetSignature(targetSignatureHex) - ); - } - }); - } + if (!mapping.contains(targetName)) { + mapping.insert({ + targetName, + [targetName] (const TargetConfig& targetConfig) { + return std::make_unique(targetConfig); + } + }); } } @@ -467,7 +451,7 @@ namespace Bloom::TargetController this->eventListener->deregisterCallbacksForEventType(); this->lastTargetState = TargetState::UNKNOWN; - this->cachedTargetDescriptor = std::nullopt; + this->targetDescriptor = std::nullopt; this->registerDescriptorsByMemoryType.clear(); this->registerAddressRangeByMemoryType.clear(); @@ -518,7 +502,6 @@ namespace Bloom::TargetController ); } - // Initiate debug tool and target this->debugTool = debugToolIt->second(); Logger::info("Connecting to debug tool"); @@ -528,39 +511,24 @@ namespace Bloom::TargetController Logger::info("Debug tool name: " + this->debugTool->getName()); Logger::info("Debug tool serial: " + this->debugTool->getSerialNumber()); - this->target = targetIt->second(); + this->target = targetIt->second(this->environmentConfig.targetConfig); + const auto& targetDescriptor = this->getTargetDescriptor(); - if (!this->target->isDebugToolSupported(this->debugTool.get())) { + if (!this->target->supportsDebugTool(this->debugTool.get())) { throw Exceptions::InvalidConfig( - "Debug tool (\"" + this->debugTool->getName() + "\") not supported " + - "by target (\"" + this->target->getName() + "\")." + "Debug tool (\"" + this->debugTool->getName() + "\") not supported by target (\"" + + targetDescriptor.name + "\")." ); } this->target->setDebugTool(this->debugTool.get()); - this->target->preActivationConfigure(this->environmentConfig.targetConfig); Logger::info("Activating target"); this->target->activate(); Logger::info("Target activated"); - this->target->postActivationConfigure(); - while (this->target->supportsPromotion()) { - auto promotedTarget = this->target->promote(); - - if ( - promotedTarget == nullptr - || std::type_index(typeid(*promotedTarget)) == std::type_index(typeid(*this->target)) - ) { - break; - } - - this->target = std::move(promotedTarget); - this->target->postPromotionConfigure(); - } - - Logger::info("Target ID: " + this->target->getHumanReadableId()); - Logger::info("Target name: " + this->target->getName()); + Logger::info("Target ID: " + targetDescriptor.id); + Logger::info("Target name: " + targetDescriptor.name); } void TargetControllerComponent::releaseHardware() { @@ -589,34 +557,32 @@ namespace Bloom::TargetController void TargetControllerComponent::loadRegisterDescriptors() { const auto& targetDescriptor = this->getTargetDescriptor(); - for (const auto& [registerType, registerDescriptors] : targetDescriptor.registerDescriptorsByType) { - for (const auto& registerDescriptor : registerDescriptors) { - auto startAddress = registerDescriptor.startAddress.value_or(0); - auto endAddress = startAddress + (registerDescriptor.size - 1); + for (const auto& [registerDescriptorId, registerDescriptor] : targetDescriptor.registerDescriptorsById) { + auto startAddress = registerDescriptor.startAddress.value_or(0); + auto endAddress = startAddress + (registerDescriptor.size - 1); - const auto registerAddressRangeIt = this->registerAddressRangeByMemoryType.find( - registerDescriptor.memoryType + const auto registerAddressRangeIt = this->registerAddressRangeByMemoryType.find( + registerDescriptor.memoryType + ); + + if (registerAddressRangeIt == this->registerAddressRangeByMemoryType.end()) { + this->registerAddressRangeByMemoryType.insert( + std::pair(registerDescriptor.memoryType, TargetMemoryAddressRange(startAddress, endAddress)) ); - if (registerAddressRangeIt == this->registerAddressRangeByMemoryType.end()) { - this->registerAddressRangeByMemoryType.insert( - std::pair(registerDescriptor.memoryType, TargetMemoryAddressRange(startAddress, endAddress)) - ); + } else { + auto& addressRange = registerAddressRangeIt->second; - } else { - auto& addressRange = registerAddressRangeIt->second; - - if (startAddress < addressRange.startAddress) { - addressRange.startAddress = startAddress; - } - - if (endAddress > addressRange.endAddress) { - addressRange.endAddress = endAddress; - } + if (startAddress < addressRange.startAddress) { + addressRange.startAddress = startAddress; } - this->registerDescriptorsByMemoryType[registerDescriptor.memoryType].insert(registerDescriptor); + if (endAddress > addressRange.endAddress) { + addressRange.endAddress = endAddress; + } } + + this->registerDescriptorsByMemoryType[registerDescriptor.memoryType].insert(registerDescriptor); } } @@ -640,7 +606,7 @@ namespace Bloom::TargetController (startAddress <= registersAddressRange.startAddress && endAddress >= registersAddressRange.startAddress) || (startAddress <= registersAddressRange.endAddress && endAddress >= registersAddressRange.startAddress) ) { - const auto& registerDescriptors = this->registerDescriptorsByMemoryType.at(memoryType); + const auto& registerDescriptors = registerDescriptorsIt->second; for (const auto& registerDescriptor : registerDescriptors) { if (!registerDescriptor.startAddress.has_value() || registerDescriptor.size < 1) { @@ -707,11 +673,11 @@ namespace Bloom::TargetController } const Targets::TargetDescriptor& TargetControllerComponent::getTargetDescriptor() { - if (!this->cachedTargetDescriptor.has_value()) { - this->cachedTargetDescriptor.emplace(this->target->getDescriptor()); + if (!this->targetDescriptor.has_value()) { + this->targetDescriptor.emplace(this->target->getDescriptor()); } - return *this->cachedTargetDescriptor; + return *this->targetDescriptor; } void TargetControllerComponent::onShutdownTargetControllerEvent(const Events::ShutdownTargetController&) { @@ -807,14 +773,20 @@ namespace Bloom::TargetController std::unique_ptr TargetControllerComponent::handleReadTargetRegisters( ReadTargetRegisters& command ) { - return std::make_unique(this->target->readRegisters(command.descriptors)); + return std::make_unique( + !command.descriptorIds.empty() + ? this->target->readRegisters(command.descriptorIds) + : Targets::TargetRegisters() + ); } std::unique_ptr TargetControllerComponent::handleWriteTargetRegisters(WriteTargetRegisters& command) { - this->target->writeRegisters(command.registers); + if (!command.registers.empty()) { + this->target->writeRegisters(command.registers); + } auto registersWrittenEvent = std::make_shared(); - registersWrittenEvent->registers = command.registers; + registersWrittenEvent->registers = std::move(command.registers); EventManager::triggerEvent(registersWrittenEvent); @@ -822,12 +794,16 @@ namespace Bloom::TargetController } std::unique_ptr TargetControllerComponent::handleReadTargetMemory(ReadTargetMemory& command) { - return std::make_unique(this->target->readMemory( - command.memoryType, - command.startAddress, - command.bytes, - command.excludedAddressRanges - )); + return std::make_unique( + command.bytes > 0 + ? this->target->readMemory( + command.memoryType, + command.startAddress, + command.bytes, + command.excludedAddressRanges + ) + : Targets::TargetMemoryBuffer() + ); } std::unique_ptr TargetControllerComponent::handleWriteTargetMemory(WriteTargetMemory& command) { @@ -876,7 +852,7 @@ namespace Bloom::TargetController const auto bufferBeginIt = buffer.begin() + (registerStartAddress - bufferStartAddress); registersWrittenEvent->registers.emplace_back(TargetRegister( - registerDescriptor, + registerDescriptor.id, TargetMemoryBuffer(bufferBeginIt, bufferBeginIt + registerSize) )); } @@ -889,8 +865,6 @@ namespace Bloom::TargetController } std::unique_ptr TargetControllerComponent::handleEraseTargetMemory(EraseTargetMemory& command) { - const auto& targetDescriptor = this->getTargetDescriptor(); - if ( command.memoryType == this->getTargetDescriptor().programMemoryType && !this->target->programmingModeEnabled() diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 1b2e9ac6..70d4e60d 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -141,10 +141,7 @@ namespace Bloom::TargetController */ Targets::TargetState lastTargetState = Targets::TargetState::UNKNOWN; - /** - * Obtaining a TargetDescriptor for the connected target can be quite expensive. We cache it here. - */ - std::optional cachedTargetDescriptor; + std::optional targetDescriptor; /** * Target register descriptors mapped by the memory type on which the register is stored. @@ -218,7 +215,7 @@ namespace Bloom::TargetController * * @return */ - std::map()>> getSupportedTargets(); + std::map(const TargetConfig&)>> getSupportedTargets(); /** * Processes any pending commands in the queue. diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index 206adad9..a1b9e9d3 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -16,147 +16,136 @@ #include "src/Targets/Microchip/AVR/Fuse.hpp" -// Derived AVR8 targets -#include "XMega/XMega.hpp" -#include "Mega/Mega.hpp" -#include "Tiny/Tiny.hpp" - namespace Bloom::Targets::Microchip::Avr::Avr8Bit { using namespace Exceptions; - void Avr8::preActivationConfigure(const TargetConfig& targetConfig) { - Target::preActivationConfigure(targetConfig); + Avr8::Avr8(const TargetConfig& targetConfig) + : targetConfig(Avr8TargetConfig(targetConfig)) + , targetDescriptionFile(TargetDescription::TargetDescriptionFile(this->targetConfig.name)) + , name(this->targetDescriptionFile.getTargetName()) + , signature(this->targetDescriptionFile.getTargetSignature()) + , family(this->targetDescriptionFile.getFamily()) + , targetParameters(this->targetDescriptionFile.getTargetParameters()) + , supportedPhysicalInterfaces(this->targetDescriptionFile.getSupportedPhysicalInterfaces()) + , padDescriptorsByName(this->targetDescriptionFile.getPadDescriptorsMappedByName()) + , targetVariantsById(this->targetDescriptionFile.getVariantsMappedById()) + , stackPointerRegisterDescriptor( + TargetRegisterDescriptor( + TargetRegisterType::STACK_POINTER, + this->targetParameters.stackPointerRegisterLowAddress.value(), + this->targetParameters.stackPointerRegisterSize.value(), + TargetMemoryType::OTHER, + "SP", + "CPU", + "Stack Pointer Register", + TargetRegisterAccess(true, true) + ) + ) + , statusRegisterDescriptor( + TargetRegisterDescriptor( + TargetRegisterType::STATUS_REGISTER, + this->targetParameters.statusRegisterStartAddress.value(), + this->targetParameters.statusRegisterSize.value(), + TargetMemoryType::OTHER, + "SREG", + "CPU", + "Status Register", + TargetRegisterAccess(true, true) + ) + ) + { + 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(); - // Extract AVR8 specific target config - this->targetConfig = Avr8TargetConfig(targetConfig); - - if (this->targetConfig->name == "avr8") { - Logger::warning("The \"avr8\" target name is deprecated and will be removed in a later version."); - } - - 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); + const auto 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; } - ); - 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 " - + Services::PathService::homeDomainName() + "/docs/configuration/avr8-physical-interfaces. \n\nIf this " - "information is incorrect, please report this to Bloom developers via " - + Services::PathService::homeDomainName() + "/report-issue.\n" - ); - } + return string + "\n - " + physicalInterfaceNames.at(physicalInterface); + } + ); - } else { - if (this->targetConfig->physicalInterface == PhysicalInterface::JTAG) { - throw InvalidConfig( - "The JTAG physical interface cannot be used with an ambiguous target name" - " - please specify the exact name of the target in your configuration file. " - "See " + Services::PathService::homeDomainName() + "/docs/supported-targets" - ); - } - - if (this->targetConfig->physicalInterface == PhysicalInterface::UPDI) { - throw InvalidConfig( - "The UPDI physical interface cannot be used with an ambiguous target name" - " - please specify the exact name of the target in your configuration file. " - "See " + Services::PathService::homeDomainName() + "/docs/supported-targets" - ); - } - } - - if ( - this->targetConfig->manageDwenFuseBit - && this->avrIspInterface == nullptr - && this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE - ) { Logger::warning( - "The connected debug tool (or associated driver) does not provide any ISP interface. " - "Bloom will be unable to update the DWEN fuse bit in the event of a debugWire activation failure." + "\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 " + + Services::PathService::homeDomainName() + "/docs/configuration/avr8-physical-interfaces. \n\n" + + "If this information is incorrect, please report this to Bloom developers via " + + Services::PathService::homeDomainName() + "/report-issue.\n" ); } if ( - this->targetConfig->manageOcdenFuseBit - && this->targetConfig->physicalInterface != PhysicalInterface::JTAG + this->targetConfig.manageDwenFuseBit + && this->avrIspInterface == nullptr + && this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE + ) { + Logger::warning( + "The connected debug tool (or associated driver) does not provide any ISP interface. " + "Bloom will be unable to manage the DWEN fuse bit." + ); + } + + if ( + this->targetConfig.manageOcdenFuseBit + && this->targetConfig.physicalInterface != PhysicalInterface::JTAG ) { Logger::warning( "The 'manageOcdenFuseBit' parameter only applies to JTAG targets. It will be ignored in this session." ); } - this->avr8DebugInterface->configure(*(this->targetConfig)); + this->loadTargetRegisterDescriptors(); + this->loadTargetMemoryDescriptors(); + } + + bool Avr8::supportsDebugTool(DebugTool* debugTool) { + return debugTool->getAvr8DebugInterface( + this->targetConfig, + this->family, + this->targetParameters, + this->targetRegisterDescriptorsById + ) != nullptr; + } + + void Avr8::setDebugTool(DebugTool* debugTool) { + this->targetPowerManagementInterface = debugTool->getTargetPowerManagementInterface(); + this->avr8DebugInterface = debugTool->getAvr8DebugInterface( + this->targetConfig, + this->family, + this->targetParameters, + this->targetRegisterDescriptorsById + ); + + this->avrIspInterface = debugTool->getAvrIspInterface( + this->targetConfig + ); if (this->avrIspInterface != nullptr) { - this->avrIspInterface->configure(*(this->targetConfig)); + this->avrIspInterface->configure(this->targetConfig); } } - void Avr8::postActivationConfigure() { - if (!this->targetDescriptionFile.has_value()) { - this->initFromTargetDescriptionFile(); - } - - /* - * The signature obtained from the device should match what is in the target description file - * - * We don't use this->getId() here as that could return the ID that was extracted from the target description - * file (which it would, if the user specified the exact target name in their project config - see - * Avr8::getId() and TargetControllerComponent::getSupportedTargets() for more). - */ - const auto targetSignature = this->avr8DebugInterface->getDeviceId(); - const auto tdSignature = this->targetDescriptionFile->getTargetSignature(); - - if (targetSignature != tdSignature) { - throw Exception( - "Failed to validate connected target - target signature mismatch.\nThe target signature" - " (\"" + targetSignature.toHex() + "\") does not match the AVR8 target description signature (\"" - + tdSignature.toHex() + "\"). This will likely be due to an incorrect target name in the " - + "configuration file (bloom.yaml)." - ); - } - } - - void Avr8::postPromotionConfigure() { - if (!this->family.has_value()) { - throw Exception("Failed to resolve AVR8 family"); - } - - this->avr8DebugInterface->setFamily(this->family.value()); - this->avr8DebugInterface->setTargetParameters(this->targetParameters.value()); - } - void Avr8::activate() { if (this->isActivated()) { return; @@ -164,17 +153,13 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit this->avr8DebugInterface->init(); - if (this->targetParameters.has_value()) { - this->avr8DebugInterface->setTargetParameters(this->targetParameters.value()); - } - try { this->avr8DebugInterface->activate(); } catch (const Exceptions::DebugWirePhysicalInterfaceError& debugWireException) { // We failed to activate the debugWire physical interface. DWEN fuse bit may need updating. - if (!this->targetConfig->manageDwenFuseBit) { + if (!this->targetConfig.manageDwenFuseBit) { throw TargetOperationFailure( "Failed to activate debugWire physical interface - check target connection and DWEN fuse " "bit. Bloom can manage the DWEN fuse bit automatically. For instructions on enabling this " @@ -192,7 +177,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit // If the debug tool provides a TargetPowerManagementInterface, attempt to cycle the target power if ( this->targetPowerManagementInterface != nullptr - && this->targetConfig->cycleTargetPowerPostDwenUpdate + && this->targetConfig.cycleTargetPowerPostDwenUpdate ) { Logger::info("Cycling target power"); @@ -200,20 +185,19 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit this->targetPowerManagementInterface->disableTargetPower(); Logger::debug( - "Holding power off for ~" - + std::to_string(this->targetConfig->targetPowerCycleDelay.count()) + "Holding power off for ~" + std::to_string(this->targetConfig.targetPowerCycleDelay.count()) + " ms" ); - std::this_thread::sleep_for(this->targetConfig->targetPowerCycleDelay); + std::this_thread::sleep_for(this->targetConfig.targetPowerCycleDelay); Logger::debug("Enabling target power"); this->targetPowerManagementInterface->enableTargetPower(); Logger::debug( - "Waiting ~" + std::to_string(this->targetConfig->targetPowerCycleDelay.count()) + "Waiting ~" + std::to_string(this->targetConfig.targetPowerCycleDelay.count()) + " ms for target power-up" ); - std::this_thread::sleep_for(this->targetConfig->targetPowerCycleDelay); + std::this_thread::sleep_for(this->targetConfig.targetPowerCycleDelay); } } catch (const Exception& exception) { @@ -227,14 +211,31 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } if ( - this->targetConfig->physicalInterface == PhysicalInterface::JTAG - && this->targetConfig->manageOcdenFuseBit + this->targetConfig.physicalInterface == PhysicalInterface::JTAG + && this->targetConfig.manageOcdenFuseBit ) { Logger::debug("Attempting OCDEN fuse bit management"); this->updateOcdenFuseBit(true); } this->activated = true; + + /* + * Validate the target signature. + * + * The signature obtained from the device should match what we loaded from the target description file. + */ + const auto targetSignature = this->avr8DebugInterface->getDeviceId(); + + if (targetSignature != this->signature) { + throw Exception( + "Failed to validate connected target - target signature mismatch.\nThe target signature" + " (\"" + targetSignature.toHex() + "\") does not match the AVR8 target description signature (\"" + + this->signature.toHex() + "\"). This will likely be due to an incorrect target name in the " + + "configuration file (bloom.yaml)." + ); + } + this->avr8DebugInterface->reset(); } @@ -244,8 +245,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit this->clearAllBreakpoints(); if ( - this->targetConfig->physicalInterface == PhysicalInterface::JTAG - && this->targetConfig->manageOcdenFuseBit + this->targetConfig.physicalInterface == PhysicalInterface::JTAG + && this->targetConfig.manageOcdenFuseBit ) { Logger::debug("Attempting OCDEN fuse bit management"); this->updateOcdenFuseBit(false); @@ -261,44 +262,16 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } } - std::unique_ptr Avr8::promote() { - std::unique_ptr promoted = nullptr; - - if (this->family.has_value()) { - // Promote generic AVR8 target to correct family. - switch (this->family.value()) { - case Family::XMEGA: { - Logger::info("AVR8 target promoted to XMega target"); - promoted = std::make_unique(*this); - break; - } - case Family::MEGA: { - Logger::info("AVR8 target promoted to megaAVR target"); - promoted = std::make_unique(*this); - break; - } - case Family::TINY: { - Logger::info("AVR8 target promoted to tinyAVR target"); - promoted = std::make_unique(*this); - break; - } - default: { - break; - } - } - } - - return promoted; - } - - TargetDescriptor Avr8Bit::Avr8::getDescriptor() { - auto descriptor = TargetDescriptor(); - descriptor.id = this->getHumanReadableId(); - descriptor.name = this->getName(); - descriptor.vendorName = std::string("Microchip"); - descriptor.programMemoryType = Targets::TargetMemoryType::FLASH; - descriptor.registerDescriptorsByType = this->targetRegisterDescriptorsByType; - descriptor.memoryDescriptorsByType = this->targetMemoryDescriptorsByType; + TargetDescriptor Avr8::getDescriptor() { + auto descriptor = TargetDescriptor( + this->signature.toHex(), + this->name, + "Microchip", + this->targetMemoryDescriptorsByType, + this->targetRegisterDescriptorsById, + {}, + Targets::TargetMemoryType::FLASH + ); std::transform( this->targetVariantsById.begin(), @@ -345,56 +318,11 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } void Avr8::writeRegisters(TargetRegisters registers) { - for (auto registerIt = registers.begin(); registerIt != registers.end();) { - if (registerIt->descriptor.type == TargetRegisterType::PROGRAM_COUNTER) { - auto programCounterBytes = registerIt->value; - - if (programCounterBytes.size() < 4) { - // All PC register values should be at least 4 bytes in size - programCounterBytes.insert(programCounterBytes.begin(), 4 - programCounterBytes.size(), 0x00); - } - - this->setProgramCounter(static_cast( - programCounterBytes[0] << 24 - | programCounterBytes[1] << 16 - | programCounterBytes[2] << 8 - | programCounterBytes[3] - )); - - registerIt = registers.erase(registerIt); - - } else { - registerIt++; - } - } - - if (!registers.empty()) { - this->avr8DebugInterface->writeRegisters(registers); - } + this->avr8DebugInterface->writeRegisters(registers); } - TargetRegisters Avr8::readRegisters(TargetRegisterDescriptors descriptors) { - TargetRegisters registers; - - for (auto registerDescriptorIt = descriptors.begin(); registerDescriptorIt != descriptors.end();) { - const auto& descriptor = *registerDescriptorIt; - - if (descriptor.type == TargetRegisterType::PROGRAM_COUNTER) { - registers.push_back(this->getProgramCounterRegister()); - - registerDescriptorIt = descriptors.erase(registerDescriptorIt); - - } else { - registerDescriptorIt++; - } - } - - if (!descriptors.empty()) { - auto otherRegisters = this->avr8DebugInterface->readRegisters(descriptors); - registers.insert(registers.end(), otherRegisters.begin(), otherRegisters.end()); - } - - return registers; + TargetRegisters Avr8::readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) { + return this->avr8DebugInterface->readRegisters(descriptorIds); } TargetMemoryBuffer Avr8::readMemory( @@ -416,7 +344,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit void Avr8::eraseMemory(TargetMemoryType memoryType) { if (memoryType == TargetMemoryType::FLASH) { - if (this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE) { + if (this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE) { // debugWire targets do not need to be erased return; } @@ -431,12 +359,12 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit this->writeMemory( memoryType, memoryType == TargetMemoryType::RAM - ? this->targetParameters->ramStartAddress.value() - : this->targetParameters->eepromStartAddress.value(), + ? this->targetParameters.ramStartAddress.value() + : this->targetParameters.eepromStartAddress.value(), TargetMemoryBuffer( memoryType == TargetMemoryType::RAM - ? this->targetParameters->ramSize.value() - : this->targetParameters->eepromSize.value(), + ? this->targetParameters.ramSize.value() + : this->targetParameters.eepromSize.value(), 0xFF ) ); @@ -450,24 +378,13 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit return this->avr8DebugInterface->getProgramCounter(); } - TargetRegister Avr8::getProgramCounterRegister() { - auto programCounter = this->getProgramCounter(); - - return TargetRegister(TargetRegisterDescriptor(TargetRegisterType::PROGRAM_COUNTER), { - static_cast(programCounter >> 24), - static_cast(programCounter >> 16), - static_cast(programCounter >> 8), - static_cast(programCounter), - }); - } - void Avr8::setProgramCounter(std::uint32_t programCounter) { this->avr8DebugInterface->setProgramCounter(programCounter); } std::uint32_t Avr8::getStackPointer() { const auto stackPointerRegister = this->readRegisters( - {this->targetRegisterDescriptorsByType.at(TargetRegisterType::STACK_POINTER)} + {this->stackPointerRegisterDescriptor.id} ).front(); std::uint32_t stackPointer = 0; @@ -649,108 +566,45 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit return this->progModeEnabled; } - void Avr8::initFromTargetDescriptionFile() { - this->targetDescriptionFile = TargetDescription::TargetDescriptionFile( - this->getId(), - (!this->name.empty()) ? std::optional(this->name) : std::nullopt - ); - - this->name = this->targetDescriptionFile->getTargetName(); - this->family = this->targetDescriptionFile->getFamily(); - this->supportedPhysicalInterfaces = this->targetDescriptionFile->getSupportedPhysicalInterfaces(); - - this->targetParameters = this->targetDescriptionFile->getTargetParameters(); - this->padDescriptorsByName = this->targetDescriptionFile->getPadDescriptorsMappedByName(); - this->targetVariantsById = this->targetDescriptionFile->getVariantsMappedById(); - - if (!this->targetParameters->stackPointerRegisterLowAddress.has_value()) { - throw Exception( - "Failed to load sufficient AVR8 target parameters - missing stack pointer start address" - ); - } - - if (!this->targetParameters->statusRegisterStartAddress.has_value()) { - throw Exception( - "Failed to load sufficient AVR8 target parameters - missing status register start address" - ); - } - - this->loadTargetRegisterDescriptors(); - this->loadTargetMemoryDescriptors(); - } - void Avr8::loadTargetRegisterDescriptors() { - this->targetRegisterDescriptorsByType = this->targetDescriptionFile->getRegisterDescriptorsMappedByType(); + this->targetRegisterDescriptorsById = this->targetDescriptionFile.getRegisterDescriptorsMappedById(); /* * All AVR8 targets possess 32 general purpose CPU registers. These are not described in the TDF, so we * construct the descriptors for them here. */ - auto gpRegisterStartAddress = this->targetParameters->gpRegisterStartAddress.value_or(0); + const auto gpRegisterStartAddress = this->targetParameters.gpRegisterStartAddress.value_or(0); for (std::uint8_t i = 0; i <= 31; i++) { - auto generalPurposeRegisterDescriptor = TargetRegisterDescriptor(); - generalPurposeRegisterDescriptor.startAddress = gpRegisterStartAddress + i; - generalPurposeRegisterDescriptor.size = 1; - generalPurposeRegisterDescriptor.type = TargetRegisterType::GENERAL_PURPOSE_REGISTER; - generalPurposeRegisterDescriptor.name = "r" + std::to_string(i); - generalPurposeRegisterDescriptor.groupName = "general purpose cpu"; - generalPurposeRegisterDescriptor.readable = true; - generalPurposeRegisterDescriptor.writable = true; + auto generalPurposeRegisterDescriptor = TargetRegisterDescriptor( + TargetRegisterType::GENERAL_PURPOSE_REGISTER, + gpRegisterStartAddress + i, + 1, + TargetMemoryType::OTHER, + "r" + std::to_string(i), + "CPU General Purpose", + std::nullopt, + TargetRegisterAccess(true, true) + ); - this->targetRegisterDescriptorsByType[generalPurposeRegisterDescriptor.type].insert( - generalPurposeRegisterDescriptor + this->targetRegisterDescriptorsById.emplace( + generalPurposeRegisterDescriptor.id, + std::move(generalPurposeRegisterDescriptor) ); } - /* - * The SP and SREG registers are described in the TDF, so we could just use the descriptors extracted from the - * TDF. The problem with that is, sometimes the SP register consists of two bytes; an SPL and an SPH. These need - * to be combined into one register descriptor. This is why we just use what we already have in - * this->targetParameters. - */ - auto stackPointerRegisterDescriptor = TargetRegisterDescriptor(); - stackPointerRegisterDescriptor.type = TargetRegisterType::STACK_POINTER; - stackPointerRegisterDescriptor.startAddress = this->targetParameters->stackPointerRegisterLowAddress.value(); - stackPointerRegisterDescriptor.size = this->targetParameters->stackPointerRegisterSize.value(); - stackPointerRegisterDescriptor.name = "SP"; - stackPointerRegisterDescriptor.groupName = "CPU"; - stackPointerRegisterDescriptor.description = "Stack Pointer Register"; - stackPointerRegisterDescriptor.readable = true; - stackPointerRegisterDescriptor.writable = true; - - auto statusRegisterDescriptor = TargetRegisterDescriptor(); - statusRegisterDescriptor.type = TargetRegisterType::STATUS_REGISTER; - statusRegisterDescriptor.startAddress = this->targetParameters->statusRegisterStartAddress.value(); - statusRegisterDescriptor.size = this->targetParameters->statusRegisterSize.value(); - statusRegisterDescriptor.name = "SREG"; - statusRegisterDescriptor.groupName = "CPU"; - statusRegisterDescriptor.description = "Status Register"; - statusRegisterDescriptor.readable = true; - statusRegisterDescriptor.writable = true; - - auto programCounterRegisterDescriptor = TargetRegisterDescriptor(); - programCounterRegisterDescriptor.type = TargetRegisterType::PROGRAM_COUNTER; - programCounterRegisterDescriptor.size = 4; - programCounterRegisterDescriptor.name = "PC"; - programCounterRegisterDescriptor.groupName = "CPU"; - programCounterRegisterDescriptor.description = "Program Counter"; - programCounterRegisterDescriptor.readable = true; - programCounterRegisterDescriptor.writable = true; - - this->targetRegisterDescriptorsByType[stackPointerRegisterDescriptor.type].insert( - stackPointerRegisterDescriptor + this->targetRegisterDescriptorsById.emplace( + this->stackPointerRegisterDescriptor.id, + this->stackPointerRegisterDescriptor ); - this->targetRegisterDescriptorsByType[statusRegisterDescriptor.type].insert( - statusRegisterDescriptor - ); - this->targetRegisterDescriptorsByType[programCounterRegisterDescriptor.type].insert( - programCounterRegisterDescriptor + this->targetRegisterDescriptorsById.emplace( + this->statusRegisterDescriptor.id, + this->statusRegisterDescriptor ); } void Avr8::loadTargetMemoryDescriptors() { - const auto ramStartAddress = this->targetParameters->ramStartAddress.value(); - const auto flashStartAddress = this->targetParameters->flashStartAddress.value(); + const auto ramStartAddress = this->targetParameters.ramStartAddress.value(); + const auto flashStartAddress = this->targetParameters.flashStartAddress.value(); this->targetMemoryDescriptorsByType.insert(std::pair( TargetMemoryType::RAM, @@ -758,7 +612,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit TargetMemoryType::RAM, TargetMemoryAddressRange( ramStartAddress, - ramStartAddress + this->targetParameters->ramSize.value() - 1 + ramStartAddress + this->targetParameters.ramSize.value() - 1 ), TargetMemoryAccess(true, true, true) ) @@ -770,15 +624,15 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit TargetMemoryType::FLASH, TargetMemoryAddressRange( flashStartAddress, - flashStartAddress + this->targetParameters->flashSize.value() - 1 + flashStartAddress + this->targetParameters.flashSize.value() - 1 ), TargetMemoryAccess(true, true, false), - this->targetParameters->flashPageSize + this->targetParameters.flashPageSize ) )); - if (this->targetParameters->eepromStartAddress.has_value() && this->targetParameters->eepromSize.has_value()) { - const auto eepromStartAddress = this->targetParameters->eepromStartAddress.value(); + if (this->targetParameters.eepromStartAddress.has_value() && this->targetParameters.eepromSize.has_value()) { + const auto eepromStartAddress = this->targetParameters.eepromStartAddress.value(); this->targetMemoryDescriptorsByType.insert(std::pair( TargetMemoryType::EEPROM, @@ -786,7 +640,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit TargetMemoryType::EEPROM, TargetMemoryAddressRange( eepromStartAddress, - eepromStartAddress + this->targetParameters->eepromSize.value() - 1 + eepromStartAddress + this->targetParameters.eepromSize.value() - 1 ), TargetMemoryAccess(true, true, true) ) @@ -794,14 +648,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } } - TargetSignature Avr8::getId() { - if (!this->id.has_value()) { - this->id = this->avr8DebugInterface->getDeviceId(); - } - - return this->id.value(); - } - void Avr8::updateDwenFuseBit(bool enable) { if (this->avrIspInterface == nullptr) { throw Exception( @@ -811,13 +657,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit ); } - if (!this->targetDescriptionFile.has_value() || !this->id.has_value()) { - throw Exception( - "Insufficient target information for ISP interface - do not use the generic \"avr8\" " - "target name in conjunction with the ISP interface. Please update your target configuration." - ); - } - if (!this->supportedPhysicalInterfaces.contains(PhysicalInterface::DEBUG_WIRE)) { throw Exception( "Target does not support debugWire physical interface - check target configuration or " @@ -825,8 +664,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit ); } - const auto dwenFuseBitsDescriptor = this->targetDescriptionFile->getDwenFuseBitsDescriptor(); - const auto spienFuseBitsDescriptor = this->targetDescriptionFile->getSpienFuseBitsDescriptor(); + const auto dwenFuseBitsDescriptor = this->targetDescriptionFile.getDwenFuseBitsDescriptor(); + const auto spienFuseBitsDescriptor = this->targetDescriptionFile.getSpienFuseBitsDescriptor(); if (!dwenFuseBitsDescriptor.has_value()) { throw Exception("Could not find DWEN bit field in TDF."); @@ -837,7 +676,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } Logger::debug("Extracting ISP parameters from TDF"); - this->avrIspInterface->setIspParameters(this->targetDescriptionFile->getIspParameters()); + this->avrIspInterface->setIspParameters(this->targetDescriptionFile.getIspParameters()); Logger::info("Initiating ISP interface"); this->avrIspInterface->activate(); @@ -890,16 +729,16 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit try { Logger::info("Reading target signature via ISP"); - const auto ispDeviceId = this->avrIspInterface->getDeviceId(); + const auto ispDeviceSignature = this->avrIspInterface->getDeviceId(); - if (ispDeviceId != this->id) { + if (ispDeviceSignature != this->signature) { throw Exception( - "AVR target signature mismatch - expected signature \"" + this->id->toHex() - + "\" but got \"" + ispDeviceId.toHex() + "\". Please check target configuration." + "AVR target signature mismatch - expected signature \"" + this->signature.toHex() + + "\" but got \"" + ispDeviceSignature.toHex() + "\". Please check target configuration." ); } - Logger::info("Target signature confirmed: " + ispDeviceId.toHex()); + Logger::info("Target signature confirmed: " + ispDeviceSignature.toHex()); const auto dwenFuseByte = this->avrIspInterface->readFuse(dwenFuseBitsDescriptor->fuseType).value; const auto spienFuseByte = (spienFuseBitsDescriptor->fuseType == dwenFuseBitsDescriptor->fuseType) @@ -925,7 +764,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit */ throw Exception( "Invalid SPIEN fuse bit value - suspected inaccuracies in TDF data. Please report this to " - "Bloom developers as a matter of urgency, via " + Services::PathService::homeDomainName() + "/report-issue" + "Bloom developers as a matter of urgency, via " + Services::PathService::homeDomainName() + + "/report-issue" ); } @@ -987,14 +827,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit using Services::PathService; using Services::StringService; - if (!this->targetDescriptionFile.has_value() || !this->id.has_value()) { - throw Exception( - "Insufficient target information for managing OCDEN fuse bit - do not use the generic \"avr8\" " - "target name in conjunction with the \"manageOcdenFuseBit\" function. Please update your target " - "configuration." - ); - } - if (!this->supportedPhysicalInterfaces.contains(PhysicalInterface::JTAG)) { throw Exception( "Target does not support JTAG physical interface - check target configuration or " @@ -1003,7 +835,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } const auto targetSignature = this->avr8DebugInterface->getDeviceId(); - const auto tdSignature = this->targetDescriptionFile->getTargetSignature(); + const auto tdSignature = this->targetDescriptionFile.getTargetSignature(); if (targetSignature != tdSignature) { throw Exception( @@ -1014,8 +846,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit ); } - const auto ocdenFuseBitsDescriptor = this->targetDescriptionFile->getOcdenFuseBitsDescriptor(); - const auto jtagenFuseBitsDescriptor = this->targetDescriptionFile->getJtagenFuseBitsDescriptor(); + const auto ocdenFuseBitsDescriptor = this->targetDescriptionFile.getOcdenFuseBitsDescriptor(); + const auto jtagenFuseBitsDescriptor = this->targetDescriptionFile.getJtagenFuseBitsDescriptor(); if (!ocdenFuseBitsDescriptor.has_value()) { throw Exception("Could not find OCDEN bit field in TDF."); @@ -1100,8 +932,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } ProgramMemorySection Avr8::getProgramMemorySectionFromAddress(std::uint32_t address) { - return this->targetParameters->bootSectionStartAddress.has_value() - && address >= this->targetParameters->bootSectionStartAddress.value() + return this->targetParameters.bootSectionStartAddress.has_value() + && address >= this->targetParameters.bootSectionStartAddress.value() ? ProgramMemorySection::BOOT : ProgramMemorySection::APPLICATION; } diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp index 783f0db1..78b6f5d3 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp @@ -5,7 +5,7 @@ #include #include -#include "src/Targets/Microchip/AVR/Target.hpp" +#include "src/Targets/Target.hpp" #include "src/DebugToolDrivers/DebugTool.hpp" #include "src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp" @@ -26,64 +26,27 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit class Avr8: public Target { public: - explicit Avr8() = default; - Avr8(std::string name, const TargetSignature& signature) - : name(std::move(name)) - , Target(signature) - { - this->initFromTargetDescriptionFile(); - }; + explicit Avr8(const TargetConfig& targetConfig); /* * The functions below implement the Target interface for AVR8 targets. * - * See the Bloom::Targets::Target interface class for documentation on the expected behaviour of + * See the Bloom::Targets::Target abstract class for documentation on the expected behaviour of * each function. */ - void preActivationConfigure(const TargetConfig& targetConfig) override; - void postActivationConfigure() override; - void postPromotionConfigure() override; - - void activate() override; - void deactivate() override; - /** * All AVR8 compatible debug tools must provide a valid Avr8Interface. * * @param debugTool * @return */ - bool isDebugToolSupported(DebugTool* debugTool) override { - return debugTool->getAvr8DebugInterface() != nullptr; - } + bool supportsDebugTool(DebugTool* debugTool) override; - void setDebugTool(DebugTool* debugTool) override { - this->targetPowerManagementInterface = debugTool->getTargetPowerManagementInterface(); - this->avr8DebugInterface = debugTool->getAvr8DebugInterface(); - this->avrIspInterface = debugTool->getAvrIspInterface(); - } + void setDebugTool(DebugTool* debugTool) override; - /** - * Instances to this target class can be promoted. See Avr8::promote() method for more. - * - * @return - */ - bool supportsPromotion() override { - return true; - } - - /** - * Instances of this generic Avr8 target class will be promoted to a family specific class (see the Mega, Xmega - * and Tiny classes for more). - * - * @return - */ - std::unique_ptr promote() override; - - std::string getName() const override { - return this->name; - } + void activate() override; + void deactivate() override; TargetDescriptor getDescriptor() override; @@ -97,7 +60,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit void clearAllBreakpoints() override; void writeRegisters(TargetRegisters registers) override; - TargetRegisters readRegisters(TargetRegisterDescriptors descriptors) override; + TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) override; TargetMemoryBuffer readMemory( TargetMemoryType memoryType, @@ -115,7 +78,6 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit TargetState getState() override; TargetProgramCounter getProgramCounter() override; - TargetRegister getProgramCounterRegister(); void setProgramCounter(TargetProgramCounter programCounter) override; TargetStackPointer getStackPointer() override; @@ -137,44 +99,37 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* avr8DebugInterface = nullptr; DebugToolDrivers::TargetInterfaces::Microchip::Avr::AvrIspInterface* avrIspInterface = nullptr; - std::optional targetConfig; + Avr8TargetConfig targetConfig; + TargetDescription::TargetDescriptionFile targetDescriptionFile; + + TargetSignature signature; std::string name; - std::optional family; - std::optional targetDescriptionFile; + Family family; + + TargetParameters targetParameters; std::set supportedPhysicalInterfaces; - - std::optional targetParameters; std::map padDescriptorsByName; std::map targetVariantsById; - std::map targetRegisterDescriptorsByType; + + TargetRegisterDescriptor stackPointerRegisterDescriptor; + TargetRegisterDescriptor statusRegisterDescriptor; + + std::map targetRegisterDescriptorsById; + std::map targetMemoryDescriptorsByType; bool progModeEnabled = false; /** - * Initiates the AVR8 instance from data extracted from the TDF. - */ - void initFromTargetDescriptionFile(); - - /** - * Populates this->targetRegisterDescriptorsByType with registers extracted from the TDF, as well as general + * Populates this->targetRegisterDescriptorsById with registers extracted from the TDF, as well as general * purpose and other CPU registers. */ void loadTargetRegisterDescriptors(); void loadTargetMemoryDescriptors(); - /** - * Extracts the ID from the target's memory. - * - * This function will cache the ID value and use the cached version for any subsequent calls. - * - * @return - */ - TargetSignature getId() override; - /** * Updates the debugWire enable (DWEN) fuse bit on the AVR target. * diff --git a/src/Targets/Microchip/AVR/AVR8/Mega/Mega.hpp b/src/Targets/Microchip/AVR/AVR8/Mega/Mega.hpp deleted file mode 100644 index 02f34901..00000000 --- a/src/Targets/Microchip/AVR/AVR8/Mega/Mega.hpp +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include "src/Targets/Microchip/AVR/AVR8/Avr8.hpp" - -namespace Bloom::Targets::Microchip::Avr::Avr8Bit -{ - class Mega: public Avr8 - { - public: - explicit Mega(const Avr8& avr8): Avr8(avr8) {}; - - bool supportsPromotion() override { - return false; - } - }; -} diff --git a/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp b/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp index 36f6435c..dd915b27 100644 --- a/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp +++ b/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp @@ -22,70 +22,23 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription using Bloom::Targets::TargetVariant; using Bloom::Targets::TargetRegisterDescriptor; - TargetDescriptionFile::TargetDescriptionFile( - const TargetSignature& targetSignature, - std::optional targetName - ) { - const auto targetSignatureHex = targetSignature.toHex(); + TargetDescriptionFile::TargetDescriptionFile(const std::string& targetName) { const auto mapping = TargetDescriptionFile::getTargetDescriptionMapping(); - const auto descriptionFiles = mapping.find(QString::fromStdString(targetSignatureHex).toLower())->toArray(); + const auto descriptionFileObjectIt = mapping.find(QString::fromStdString(targetName).toLower()); - if (descriptionFiles.empty()) { + if (descriptionFileObjectIt == mapping.end()) { throw Exception( - "Failed to resolve target description file for target \"" + targetSignatureHex - + "\" - unknown target signature." + "Failed to resolve target description file for target \"" + targetName + "\" - unknown target name." ); } - if (descriptionFiles.size() > 1 && !targetName.has_value()) { - /* - * There are numerous target description files mapped to this target signature and we don't have a target - * name to filter by. There's really not much we can do at this point, so we'll just instruct the user to - * provide a specific target name. - */ - auto targetNames = QStringList(); - std::transform( - descriptionFiles.begin(), - descriptionFiles.end(), - std::back_inserter(targetNames), - [] (const QJsonValue& descriptionFile) { - return QString( - "\"" + descriptionFile.toObject().find("targetName")->toString().toLower() + "\"" - ); - } - ); + const auto descriptionFileObject = descriptionFileObjectIt.value().toObject(); + const auto descriptionFilePath = QString::fromStdString( + Services::PathService::applicationDirPath()) + "/" + descriptionFileObject.find("tdfPath")->toString(); - throw Exception( - "Failed to resolve target description file for target \"" + targetSignatureHex - + "\" - ambiguous signature.\nThe signature is mapped to numerous targets: " - + targetNames.join(", ").toStdString() + ".\n\nPlease update the target name in your Bloom " - + "configuration file, to one of the above." - ); - } + Logger::debug("Loading AVR8 target description file: " + descriptionFilePath.toStdString()); - for (const auto& mappingJsonValue : descriptionFiles) { - const auto mappingObject = mappingJsonValue.toObject(); - - if ( - targetName.has_value() - && *targetName != mappingObject.find("targetName")->toString().toLower().toStdString() - ) { - continue; - } - - const auto descriptionFilePath = QString::fromStdString(Services::PathService::applicationDirPath()) + "/" - + mappingObject.find("targetDescriptionFilePath")->toString(); - - Logger::debug("Loading AVR8 target description file: " + descriptionFilePath.toStdString()); - Targets::TargetDescription::TargetDescriptionFile::init(descriptionFilePath); - return; - } - - throw Exception( - "Failed to resolve target description file for target \"" + *targetName - + "\" - target signature \"" + targetSignatureHex + "\" does not belong to target with name \"" - + *targetName + "\". Please review your bloom.yaml configuration." - ); + Targets::TargetDescription::TargetDescriptionFile::init(descriptionFilePath); } void TargetDescriptionFile::init(const QDomDocument& xml) { @@ -634,35 +587,28 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription continue; } - auto registerDescriptor = TargetRegisterDescriptor(); - registerDescriptor.type = moduleName == "port" - ? TargetRegisterType::PORT_REGISTER : TargetRegisterType::OTHER; - registerDescriptor.memoryType = TargetMemoryType::RAM; - registerDescriptor.name = moduleRegisterName; - registerDescriptor.groupName = peripheralRegisterGroup.name; - registerDescriptor.size = moduleRegister.size; - registerDescriptor.startAddress = moduleRegister.offset - + peripheralRegisterGroup.offset.value_or(0); + auto registerDescriptor = TargetRegisterDescriptor( + moduleName == "port" ? TargetRegisterType::PORT_REGISTER : TargetRegisterType::OTHER, + moduleRegister.offset + peripheralRegisterGroup.offset.value_or(0), + moduleRegister.size, + TargetMemoryType::RAM, + moduleRegisterName, + peripheralRegisterGroup.name, + moduleRegister.caption.has_value() && !moduleRegister.caption->empty() + ? moduleRegister.caption + : std::nullopt, + moduleRegister.readWriteAccess.has_value() + ? TargetRegisterAccess( + moduleRegister.readWriteAccess.value().find('r') != std::string::npos, + moduleRegister.readWriteAccess.value().find('w') != std::string::npos + ) + : TargetRegisterAccess(true, true) + ); - if (moduleRegister.caption.has_value() && !moduleRegister.caption->empty()) { - registerDescriptor.description = moduleRegister.caption; - } - - if (moduleRegister.readWriteAccess.has_value()) { - const auto& readWriteAccess = moduleRegister.readWriteAccess.value(); - registerDescriptor.readable = readWriteAccess.find('r') != std::string::npos; - registerDescriptor.writable = readWriteAccess.find('w') != std::string::npos; - - } else { - /* - * If the TDF doesn't specify the OCD read/write access for a register, we assume both - * are permitted. - */ - registerDescriptor.readable = true; - registerDescriptor.writable = true; - } - - this->targetRegisterDescriptorsByType[registerDescriptor.type].insert(registerDescriptor); + this->targetRegisterDescriptorsById.emplace( + registerDescriptor.id, + std::move(registerDescriptor) + ); } } } diff --git a/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp b/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp index da0b66f0..69af022c 100644 --- a/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp +++ b/src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp @@ -34,12 +34,11 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription { public: /** - * Will resolve the target description file using the target description JSON mapping and a given target signature. + * Will resolve the target description file using the target description JSON mapping and a given target name. * - * @param targetSignatureHex * @param targetName */ - TargetDescriptionFile(const TargetSignature& targetSignature, std::optional targetName); + TargetDescriptionFile(const std::string& targetName); /** * Extends TDF initialisation to include the loading of physical interfaces for debugging AVR8 targets, among @@ -148,12 +147,12 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription } /** - * Returns a mapping of all target register descriptors extracted from the TDF, by type. + * Returns a mapping of all target register descriptors extracted from the TDF, by ID. * * @return */ - [[nodiscard]] const auto& getRegisterDescriptorsMappedByType() const { - return this->targetRegisterDescriptorsByType; + [[nodiscard]] const auto& getRegisterDescriptorsMappedById() const { + return this->targetRegisterDescriptorsById; } private: @@ -166,7 +165,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription * @return */ static inline auto getFamilyNameToEnumMapping() { - // All keys should be lower case. + // All keys should be lower-case. return std::map { {"megaavr", Family::MEGA}, {"avr mega", Family::MEGA}, @@ -186,7 +185,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription std::map padDescriptorsByName; std::map targetVariantsById; - std::map targetRegisterDescriptorsByType; + std::map targetRegisterDescriptorsById; /** * Populates this->supportedPhysicalInterfaces with physical interfaces defined in the TDF. @@ -204,7 +203,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit::TargetDescription void loadTargetVariants(); /** - * Loads all register descriptors from the TDF, and populates this->targetRegisterDescriptorsByType. + * Loads all register descriptors from the TDF, and populates this->targetRegisterDescriptorsById. */ void loadTargetRegisterDescriptors(); diff --git a/src/Targets/Microchip/AVR/AVR8/Tiny/Tiny.hpp b/src/Targets/Microchip/AVR/AVR8/Tiny/Tiny.hpp deleted file mode 100644 index 5944023e..00000000 --- a/src/Targets/Microchip/AVR/AVR8/Tiny/Tiny.hpp +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include "src/Targets/Microchip/AVR/AVR8/Avr8.hpp" - -namespace Bloom::Targets::Microchip::Avr::Avr8Bit -{ - class Tiny: public Avr8 - { - public: - explicit Tiny(const Avr8& avr8): Avr8(avr8) {}; - - bool supportsPromotion() override { - return false; - } - }; -} diff --git a/src/Targets/Microchip/AVR/AVR8/XMega/XMega.hpp b/src/Targets/Microchip/AVR/AVR8/XMega/XMega.hpp deleted file mode 100644 index 3423864b..00000000 --- a/src/Targets/Microchip/AVR/AVR8/XMega/XMega.hpp +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include "src/Targets/Microchip/AVR/AVR8/Avr8.hpp" - -namespace Bloom::Targets::Microchip::Avr::Avr8Bit -{ - class XMega: public Avr8 - { - public: - explicit XMega(const Avr8& avr8): Avr8(avr8) {}; - - bool supportsPromotion() override { - return false; - } - }; -} diff --git a/src/Targets/Microchip/AVR/Target.hpp b/src/Targets/Microchip/AVR/Target.hpp deleted file mode 100644 index c11e6513..00000000 --- a/src/Targets/Microchip/AVR/Target.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include -#include - -#include "src/Targets/Target.hpp" - -#include "TargetSignature.hpp" - -namespace Bloom::Targets::Microchip::Avr -{ - class Target: public ::Bloom::Targets::Target - { - public: - explicit Target(std::optional id = std::nullopt) - : id(id) - {}; - - std::string getHumanReadableId() override { - return this->getId().toHex(); - } - - protected: - std::optional id; - - virtual TargetSignature getId() = 0; - }; -} diff --git a/src/Targets/Target.hpp b/src/Targets/Target.hpp index 1b3fb3af..04f90c71 100644 --- a/src/Targets/Target.hpp +++ b/src/Targets/Target.hpp @@ -25,9 +25,6 @@ namespace Bloom::Targets * All targets supported by Bloom must implement this interface. * * A single implementation of this interface can represent a single target, or an entire family of targets. - * For an example, see the Avr8 implementation. The Avr8 target class was written in a way that would allow it to - * work, to *at least* the point of target promotion, for all AVR8 targets. For more on target promotion, see the - * Target::promote() function. */ class Target { @@ -41,38 +38,27 @@ namespace Bloom::Targets } /** - * There are three stages of configuration for targets. + * Should check if the given debugTool is compatible with the target. Returning false in this function will + * prevent Bloom from attempting to use the selected debug tool with the selected target. An InvalidConfig + * exception will be raised and Bloom will shutdown. * - * preActivationConfigure() - The first stage is just before target activation (Target::activate() being - * called). At this point, we will not have interacted with the target in any way. This function should cover - * any configuration that can be done without the target being activated. It should also cover any configuration - * that is required in order for us to successfully activate the target. For an example, we use this function - * in the Avr8 target class to configure the debug tool with the correct physical interface and config variant - * parameters (taken from the user's settings, via the TargetConfig object). Without these being configured, - * the debug tool would not be able to interface with the AVR8 target, and thus target activation would fail. + * @param debugTool * - * postActivationConfigure() - The second stage is right after target activation (successful invocation of - * Target::activate()). At this point, we will have established a connection with the target and so interaction - * with the target is permitted here. We use this function in the Avr8 target class to extract the target - * signature from the target's memory, which we then use to find & load the correct target description file. - * - * postPromotionConfigure() - The final stage of configuration occurs just after the target instance has been - * promoted to a different class. See the Target::promote() function for more on this. - * - * If any of the three configuration functions throw an exception, the exception will be treated as a fatal - * error. In response, the TargetController will shutdown, along with the rest of Bloom. - * - * @param targetConfig + * @return */ - virtual void preActivationConfigure(const TargetConfig& targetConfig) { - this->config = targetConfig; - }; - virtual void postActivationConfigure() = 0; - virtual void postPromotionConfigure() = 0; + virtual bool supportsDebugTool(DebugTool* debugTool) = 0; + + /** + * Assuming the Target::isDebugToolSupported() check passed, this function will be called shortly after, by the + * TargetController. + * + * @param debugTool + */ + virtual void setDebugTool(DebugTool* debugTool) = 0; /** * This function should attempt to establish a connection with the target, and put it in a state where - * debugging can be performed. This function will be called after Target::preActivationConfigure(). + * debugging can be performed. * * If an exception is thrown from this function, the TargetController will treat it as a fatal error, and thus * will shutdown, along with the rest of Bloom. @@ -91,81 +77,6 @@ namespace Bloom::Targets */ virtual void deactivate() = 0; - /** - * Should check if the given debugTool is compatible with the target. Returning false in this function will - * prevent Bloom from attempting to use the selected debug tool with the selected target. An InvalidConfig - * exception will be raised and Bloom will shutdown. - * - * For AVR8 targets, we simply check if the debug tool returns a valid Avr8DebugInterface - * (via DebugTool::getAvr8DebugInterface()). If it fails to do so, it would mean that the debug tool, or more - * so our debug tool driver, does not support AVR8 targets. - * - * @param debugTool - * - * @return - */ - virtual bool isDebugToolSupported(DebugTool* debugTool) = 0; - - /** - * Assuming the Target::isDebugToolSupported() check passed, this function will be called shortly after, by the - * TargetController. - * - * @param debugTool - */ - virtual void setDebugTool(DebugTool* debugTool) = 0; - - /** - * Should indicate whether this target class can be promoted to one that better represents the connected - * target. See Target::promote() for more. - * - * @return - */ - virtual bool supportsPromotion() = 0; - - /** - * Bloom allows users to select generic targets within their configuration, but this doesn't have to mean we - * are limited to the generic target class. In some cases, we may want to switch to a target class that is - * more specific to the connected target. We call this "target promotion". See below for an example. - * - * When a user is debugging an AVR8 target, they may not specify the exact name of the target in their project - * configuration file. Instead, they may select the generic 'avr8' target (which maps to the generic Avr8 target - * class). In cases like this, the data we have on the target, at the point of start up, is very limited; all we - * know about the target is that it's part of the AVR8 family. Nothing else. But this is ok, because, when we - * begin the target configuration and activation process, we are able to learn a lot more about the target. - * For AVR8 targets, we extract the target signature shortly after activation, and with that signature we find - * the appropriate target description file, which has all of the information regarding the target that we could - * possibly need. So, by the time we have activated the target, we will know a lot more about it, and it is at - * this point, where we may want to promote it to a more specific target class (from the generic Avr8 target - * class). The generic AVR8 target class will attempt to promote the target to one that is more specific to - * the target's AVR8 family (ATmega, XMega, Tiny, etc). These classes can then also perform promotion of their - * own, if required, where they could promote to a class that's not only specific to an AVR8 family, but to a - * particular target model (for example, a target class that was written specifically for the ATmega328P target). - * - * This function should attempt to promote the current target class to one that is more specific to the - * connected target, with the information it currently holds on the target. - * - * If this function fails to promote the target, it should return an std::unique_ptr(nullptr). - * - * After activating the target, assuming the first call to Target::supportsPromotion() returns true, the - * TargetController will enter a loop, where it will repeatedly call this function and update the target - * instance, until at least one of the following conditions are met: - * - The call to Target::supportsPromotion() on the current target instance returns false - * - The call to Target::promote() on the current target instance returns an std::unique_ptr(nullptr) - * - The call to Target::promote() on the current target instance returns a target class type that is equal - * to the type of the current target instance (promotion failed). - * - * Once at least one of the above conditions are met, the TargetController will break out of the loop and use - * the last promoted target instance from there onwards. - * - * See TargetControllerComponent::acquireHardware() for more on this. - * - * @return - */ - virtual std::unique_ptr promote() = 0; - - virtual std::string getName() const = 0; - virtual std::string getHumanReadableId() = 0; - /** * Should generate and return a TargetDescriptor for the current target. * @@ -230,11 +141,11 @@ namespace Bloom::Targets /** * Should read register values of the registers described by the given descriptors. * - * @param descriptors + * @param descriptorIds * * @return */ - virtual TargetRegisters readRegisters(TargetRegisterDescriptors descriptors) = 0; + virtual TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) = 0; /** * Should read memory from the target. diff --git a/src/Targets/TargetDescriptor.hpp b/src/Targets/TargetDescriptor.hpp index 543887ef..f8db9de1 100644 --- a/src/Targets/TargetDescriptor.hpp +++ b/src/Targets/TargetDescriptor.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "TargetMemory.hpp" @@ -18,10 +19,40 @@ namespace Bloom::Targets std::string id; std::string vendorName; std::map memoryDescriptorsByType; - std::map registerDescriptorsByType; + std::map registerDescriptorsById; std::vector variants; TargetMemoryType programMemoryType; + + TargetDescriptor( + std::string id, + std::string name, + std::string vendorName, + std::map memoryDescriptorsByType, + std::map registerDescriptorsById, + std::vector variants, + TargetMemoryType programMemoryType + ) + : name(name) + , id(id) + , vendorName(vendorName) + , memoryDescriptorsByType(memoryDescriptorsByType) + , registerDescriptorsById(registerDescriptorsById) + , variants(variants) + , programMemoryType(programMemoryType) + {} + + TargetRegisterDescriptorIds registerDescriptorIdsForType(TargetRegisterType type) { + auto output = TargetRegisterDescriptorIds(); + + for (const auto& [descriptorId, descriptor] : this->registerDescriptorsById) { + if (descriptor.type == type) { + output.insert(descriptorId); + } + } + + return output; + } }; } diff --git a/src/Targets/TargetRegister.hpp b/src/Targets/TargetRegister.hpp index d71b59b7..14506f7a 100644 --- a/src/Targets/TargetRegister.hpp +++ b/src/Targets/TargetRegister.hpp @@ -1,8 +1,9 @@ #pragma once +#include +#include #include #include -#include #include #include #include @@ -12,6 +13,9 @@ namespace Bloom::Targets { + using TargetRegisterDescriptorId = std::uint32_t; + using TargetRegisterDescriptorIds = std::set; + enum class TargetRegisterType: std::uint8_t { GENERAL_PURPOSE_REGISTER, @@ -22,23 +26,55 @@ namespace Bloom::Targets OTHER, }; + struct TargetRegisterAccess + { + bool readable = false; + bool writable = false; + + TargetRegisterAccess( + bool readable, + bool writable + ) + : readable(readable) + , writable(writable) + {} + }; + struct TargetRegisterDescriptor { public: + TargetRegisterDescriptorId id; std::optional startAddress; - TargetMemorySize size = 0; - TargetRegisterType type = TargetRegisterType::OTHER; - TargetMemoryType memoryType = TargetMemoryType::OTHER; + TargetMemorySize size; + TargetRegisterType type; + TargetMemoryType memoryType; std::optional name; std::optional groupName; std::optional description; - bool readable = false; - bool writable = false; + TargetRegisterAccess access; - TargetRegisterDescriptor() = default; - explicit TargetRegisterDescriptor(TargetRegisterType type): type(type) {}; + TargetRegisterDescriptor( + TargetRegisterType type, + std::optional startAddress, + TargetMemorySize size, + TargetMemoryType memoryType, + std::optional name, + std::optional groupName, + std::optional description, + TargetRegisterAccess access + ) + : id(++(TargetRegisterDescriptor::lastRegisterDescriptorId)) + , type(type) + , startAddress(startAddress) + , size(size) + , memoryType(memoryType) + , name(name) + , groupName(groupName) + , description(description) + , access(access) + {}; bool operator == (const TargetRegisterDescriptor& other) const { return this->getHash() == other.getHash(); @@ -58,6 +94,7 @@ namespace Bloom::Targets private: mutable std::optional cachedHash; + static inline std::atomic lastRegisterDescriptorId = 0; std::size_t getHash() const; friend std::hash; @@ -65,12 +102,12 @@ namespace Bloom::Targets struct TargetRegister { - TargetRegisterDescriptor descriptor; + TargetRegisterDescriptorId descriptorId; TargetMemoryBuffer value; - TargetRegister(TargetRegisterDescriptor descriptor, std::vector value) + TargetRegister(TargetRegisterDescriptorId descriptorId, std::vector value) : value(std::move(value)) - , descriptor(std::move(descriptor)) + , descriptorId(descriptorId) {}; [[nodiscard]] std::size_t size() const { @@ -80,6 +117,7 @@ namespace Bloom::Targets using TargetRegisters = std::vector; using TargetRegisterDescriptors = std::set; + using TargetRegisterDescriptorMapping = std::map; } namespace std diff --git a/src/Targets/Targets.hpp b/src/Targets/Targets.hpp index ebf7fd5a..5b92021a 100644 --- a/src/Targets/Targets.hpp +++ b/src/Targets/Targets.hpp @@ -1,5 +1,4 @@ #pragma once #include "src/Targets/Target.hpp" -#include "src/Targets/Microchip/AVR/Target.hpp" #include "src/Targets/Microchip/AVR/AVR8/Avr8.hpp"