Tidying low-level debug tool driver code:

- Use automatic objects for libusb/hidapi resources, where possible (to reduce manual resource management)
- Removed unused/redundant code
- Tidied HidInterface class
- Tidied debug tool initialisation code
- Other bits of tidying
This commit is contained in:
Nav
2022-10-01 16:50:57 +01:00
parent ef4eb4f768
commit a5b0097036
36 changed files with 448 additions and 727 deletions

View File

@@ -8,6 +8,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap
{
using namespace Bloom::Exceptions;
CmsisDapInterface::CmsisDapInterface(Usb::HidInterface&& usbHidInterface)
: usbHidInterface(std::move(usbHidInterface))
{}
void CmsisDapInterface::sendCommand(const Command& cmsisDapCommand) {
if (this->msSendCommandDelay.count() > 0) {
using namespace std::chrono;

View File

@@ -21,14 +21,14 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap
class CmsisDapInterface
{
public:
explicit CmsisDapInterface() = default;
explicit CmsisDapInterface(Usb::HidInterface&& usbHidInterface);
virtual ~CmsisDapInterface() = default;
CmsisDapInterface(const CmsisDapInterface& other) = default;
CmsisDapInterface(CmsisDapInterface&& other) = default;
CmsisDapInterface& operator = (const CmsisDapInterface& other) = default;
CmsisDapInterface& operator = (CmsisDapInterface&& other) = default;
CmsisDapInterface(const CmsisDapInterface& other) = delete;
CmsisDapInterface(CmsisDapInterface&& other) = delete;
CmsisDapInterface& operator = (const CmsisDapInterface& other) = delete;
CmsisDapInterface& operator = (CmsisDapInterface&& other) = delete;
Usb::HidInterface& getUsbHidInterface() {
return this->usbHidInterface;
@@ -65,7 +65,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap
"CMSIS Response type must be derived from the Response class."
);
const auto rawResponse = this->getUsbHidInterface().read(15000);
const auto rawResponse = this->getUsbHidInterface().read(std::chrono::milliseconds(15000));
if (rawResponse.empty()) {
throw Exceptions::DeviceCommunicationFailure("Empty CMSIS-DAP response received");
@@ -113,7 +113,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap
* amongst devices, so we'll need to be able to preActivationConfigure the CMSISDAPInterface from a
* higher level. For an example, see the constructor of the AtmelIce device class.
*/
Usb::HidInterface usbHidInterface = Usb::HidInterface();
Usb::HidInterface usbHidInterface;
/**
* Some CMSIS-DAP debug tools fail to operate properly when we send commands too quickly. Even if we've

View File

@@ -1,6 +1,7 @@
#pragma once
#include <cstdint>
#include <cassert>
#include "EdbgControlCommandFrame.hpp"

View File

@@ -82,7 +82,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
using Bloom::Targets::TargetRegisterType;
using Bloom::Targets::TargetRegisters;
EdbgAvr8Interface::EdbgAvr8Interface(EdbgInterface& edbgInterface)
EdbgAvr8Interface::EdbgAvr8Interface(EdbgInterface* edbgInterface)
: edbgInterface(edbgInterface)
{}
@@ -195,7 +195,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::stop() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Stop()
);
@@ -210,7 +210,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
void EdbgAvr8Interface::run() {
this->clearEvents();
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Run()
);
@@ -223,7 +223,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
void EdbgAvr8Interface::runTo(TargetProgramCounter address) {
this->clearEvents();
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
RunTo(address)
);
@@ -235,7 +235,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::step() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Step()
);
@@ -247,7 +247,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::reset() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Reset()
);
@@ -335,7 +335,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
this->stop();
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
GetProgramCounter()
);
@@ -355,7 +355,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* The program counter will be given in byte address form, but the EDBG tool will be expecting it in word
* address (16-bit) form. This is why we divide it by 2.
*/
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
SetProgramCounter(programCounter / 2)
);
@@ -390,7 +390,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
return TargetSignature(signatureMemory[0], signatureMemory[1], signatureMemory[2]);
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
GetDeviceId()
);
@@ -402,7 +402,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::setBreakpoint(TargetMemoryAddress address) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
SetSoftwareBreakpoints({address})
);
@@ -412,7 +412,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::clearBreakpoint(TargetMemoryAddress address) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ClearSoftwareBreakpoints({address})
);
@@ -422,7 +422,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::clearAllBreakpoints() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ClearAllSoftwareBreakpoints()
);
@@ -750,7 +750,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
throw Exception("AVR8 erase command not supported for debugWire config variant.");
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
EraseMemory(
section.has_value()
? section == ProgramMemorySection::BOOT
@@ -785,7 +785,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
return;
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
EnterProgrammingMode()
);
@@ -801,7 +801,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
return;
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
LeaveProgrammingMode()
);
@@ -909,7 +909,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::setParameter(const Avr8EdbgParameter& parameter, const std::vector<unsigned char>& value) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
SetParameter(parameter, value)
);
@@ -919,7 +919,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
std::vector<unsigned char> EdbgAvr8Interface::getParameter(const Avr8EdbgParameter& parameter, std::uint8_t size) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
GetParameter(parameter, size)
);
@@ -1371,7 +1371,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::activatePhysical(bool applyExternalReset) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ActivatePhysical(applyExternalReset)
);
@@ -1390,7 +1390,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::deactivatePhysical() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
DeactivatePhysical()
);
@@ -1410,7 +1410,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* value of the breakAfterAttach flag. So we still expect a stop event to be received shortly after issuing
* the attach command.
*/
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Attach(
this->configVariant != Avr8ConfigVariant::MEGAJTAG
)
@@ -1434,7 +1434,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::detach() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
Detach()
);
@@ -1684,7 +1684,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* that isn't actually the memory data (like the command ID, version bytes, etc). I could have sought the
* actual value but who has the time. It won't exceed 20 bytes. Bite me.
*/
auto singlePacketSize = static_cast<std::uint32_t>(this->edbgInterface.getUsbHidInputReportSize() - 20);
auto singlePacketSize = static_cast<std::uint32_t>(this->edbgInterface->getUsbHidInputReportSize() - 20);
auto totalResponsePackets = std::ceil(static_cast<float>(bytes) / static_cast<float>(singlePacketSize));
auto totalReadsRequired = std::ceil(static_cast<float>(totalResponsePackets) / 2);
@@ -1713,7 +1713,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ReadMemory(
type,
startAddress,
@@ -1780,7 +1780,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
}
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
WriteMemory(
type,
startAddress,
@@ -1811,7 +1811,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvr8Interface::disableDebugWire() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
DisableDebugWire()
);

View File

@@ -27,7 +27,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
class EdbgAvr8Interface: public TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface
{
public:
explicit EdbgAvr8Interface(EdbgInterface& edbgInterface);
explicit EdbgAvr8Interface(EdbgInterface* edbgInterface);
/**
* Some EDBG devices don't seem to operate correctly when actioning the masked memory read EDBG command. The
@@ -276,7 +276,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
*
* Every EDBG based debug tool that utilises this implementation must provide access to its EDBG interface.
*/
EdbgInterface& edbgInterface;
EdbgInterface* edbgInterface = nullptr;
/**
* Project's AVR8 target configuration.

View File

@@ -26,12 +26,16 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
using Exceptions::TargetOperationFailure;
EdbgAvrIspInterface::EdbgAvrIspInterface(EdbgInterface* edbgInterface)
: edbgInterface(edbgInterface)
{}
void EdbgAvrIspInterface::setIspParameters(const Targets::Microchip::Avr::IspParameters& ispParameters) {
this->ispParameters = ispParameters;
}
void EdbgAvrIspInterface::activate() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
EnterProgrammingMode(
this->ispParameters.programModeTimeout,
this->ispParameters.programModeStabilizationDelay,
@@ -52,7 +56,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvrIspInterface::deactivate() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
LeaveProgrammingMode(
this->ispParameters.programModePreDelay,
this->ispParameters.programModePostDelay
@@ -74,7 +78,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
Fuse EdbgAvrIspInterface::readFuse(FuseType fuseType) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ReadFuse(fuseType, this->ispParameters.readFusePollIndex)
);
@@ -93,7 +97,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
unsigned char EdbgAvrIspInterface::readLockBitByte() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ReadLock(this->ispParameters.readLockPollIndex)
);
@@ -112,7 +116,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
void EdbgAvrIspInterface::programFuse(Targets::Microchip::Avr::Fuse fuse) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ProgramFuse(fuse.type, fuse.value)
);
@@ -125,7 +129,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
unsigned char EdbgAvrIspInterface::readSignatureByte(std::uint8_t signatureByteAddress) {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
ReadSignature(signatureByteAddress, this->ispParameters.readSignaturePollIndex)
);

View File

@@ -22,9 +22,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
class EdbgAvrIspInterface: public TargetInterfaces::Microchip::Avr::AvrIspInterface
{
public:
explicit EdbgAvrIspInterface(EdbgInterface& edbgInterface)
: edbgInterface(edbgInterface)
{};
explicit EdbgAvrIspInterface(EdbgInterface* edbgInterface);
/**
* The EdbgAvrIspInterface doesn't actually require any config from the user, at this point in time. So this
@@ -89,7 +87,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
*
* Every EDBG based debug tool that utilises this implementation must provide access to its EDBG interface.
*/
EdbgInterface& edbgInterface;
EdbgInterface* edbgInterface;
Targets::Microchip::Avr::IspParameters ispParameters;

View File

@@ -8,6 +8,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
{
using namespace Bloom::Exceptions;
EdbgInterface::EdbgInterface(Usb::HidInterface&& usbHidInterface)
: CmsisDapInterface(std::move(usbHidInterface))
{}
Protocols::CmsisDap::Response EdbgInterface::sendAvrCommandsAndWaitForResponse(
const std::vector<Avr::AvrCommand>& avrCommands
) {

View File

@@ -22,7 +22,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
class EdbgInterface: public CmsisDapInterface
{
public:
explicit EdbgInterface() = default;
explicit EdbgInterface(Usb::HidInterface&& usbHidInterface);
/**
* Send an AvrCommandFrame to the debug tool and wait for a response.

View File

@@ -12,8 +12,12 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
using Protocols::CmsisDap::Edbg::Avr::CommandFrames::EdbgControl::GetParameter;
using Protocols::CmsisDap::Edbg::Avr::CommandFrames::EdbgControl::SetParameter;
EdbgTargetPowerManagementInterface::EdbgTargetPowerManagementInterface(EdbgInterface* edbgInterface)
: edbgInterface(edbgInterface)
{}
void EdbgTargetPowerManagementInterface::enableTargetPower() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
SetParameter(EdbgParameters::CONTROL_TARGET_POWER, 0x01)
);
@@ -23,7 +27,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
}
void EdbgTargetPowerManagementInterface::disableTargetPower() {
auto response = this->edbgInterface.sendAvrCommandFrameAndWaitForResponseFrame(
auto response = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame(
SetParameter(EdbgParameters::CONTROL_TARGET_POWER, 0x00)
);

View File

@@ -10,8 +10,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
class EdbgTargetPowerManagementInterface: public TargetInterfaces::TargetPowerManagementInterface
{
public:
explicit EdbgTargetPowerManagementInterface(EdbgInterface& edbgInterface)
: edbgInterface(edbgInterface) {};
explicit EdbgTargetPowerManagementInterface(EdbgInterface* edbgInterface);
/**
* Issues a Set Parameter command to the EDBG tool, to enable the target power.
@@ -24,6 +23,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg
void disableTargetPower() override;
private:
EdbgInterface& edbgInterface;
EdbgInterface* edbgInterface;
};
}