Made the EDBG CMSIS-DAP command delay optional for all debug tools, and disabled it by default.
The command delay was really choking Bloom's EDBG driver, causing a very noticeable drag on Bloom's performance. It's much faster with the command delay disabled. There was a good reason for why I introduced this some time ago. Without it, some EDBG debug tools were misbehaving - I remember that for certain. But now, I cannot seem to reproduce these issues. Very odd. If the issues do reappear, I may have to enable the command delay by default, again, for some debug tools. For now, if any users experience issues, I'll just suggest they manually enable the command delay via their project config. Also, I'm not going to document this new config option, as I would prefer the user to approach me if they experience issues as a result of this, so that I'll know if it needs revisiting.
This commit is contained in:
@@ -32,6 +32,7 @@ target_sources(
|
||||
|
||||
# Microchip EDBG debug tools
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/Microchip/EdbgDevice.cpp
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/Microchip/EdbgToolConfig.cpp
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/Microchip/AtmelICE/AtmelIce.cpp
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/Microchip/PowerDebugger/PowerDebugger.cpp
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/Microchip/MplabSnap/MplabSnap.cpp
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
AtmelIce::AtmelIce()
|
||||
AtmelIce::AtmelIce(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
AtmelIce::USB_VENDOR_ID,
|
||||
AtmelIce::USB_PRODUCT_ID,
|
||||
AtmelIce::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -22,7 +22,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint8_t USB_CONFIGURATION_INDEX = 0;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
AtmelIce();
|
||||
AtmelIce(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Atmel-ICE";
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
CuriosityNano::CuriosityNano()
|
||||
CuriosityNano::CuriosityNano(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
CuriosityNano::USB_VENDOR_ID,
|
||||
CuriosityNano::USB_PRODUCT_ID,
|
||||
CuriosityNano::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -21,7 +21,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t USB_PRODUCT_ID = 0x2175;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
CuriosityNano();
|
||||
CuriosityNano(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Curiosity Nano";
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
#include "EdbgDevice.hpp"
|
||||
|
||||
#include "src/DebugToolDrivers/USB/HID/HidInterface.hpp"
|
||||
#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/CmsisDapInterface.hpp"
|
||||
#include "src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/CommandFrames/AvrCommandFrames.hpp"
|
||||
|
||||
#include "src/Exceptions/InvalidConfig.hpp"
|
||||
#include "src/TargetController/Exceptions/DeviceFailure.hpp"
|
||||
#include "src/TargetController/Exceptions/DeviceInitializationFailure.hpp"
|
||||
|
||||
@@ -14,6 +16,7 @@ namespace DebugToolDrivers::Microchip
|
||||
using Exceptions::DeviceInitializationFailure;
|
||||
|
||||
EdbgDevice::EdbgDevice(
|
||||
const DebugToolConfig& debugToolConfig,
|
||||
std::uint16_t vendorId,
|
||||
std::uint16_t productId,
|
||||
std::uint8_t cmsisHidInterfaceNumber,
|
||||
@@ -21,12 +24,14 @@ namespace DebugToolDrivers::Microchip
|
||||
std::optional<std::uint8_t> configurationIndex
|
||||
)
|
||||
: UsbDevice(vendorId, productId)
|
||||
, toolConfig(EdbgToolConfig{debugToolConfig})
|
||||
, cmsisHidInterfaceNumber(cmsisHidInterfaceNumber)
|
||||
, supportsTargetPowerManagement(supportsTargetPowerManagement)
|
||||
, configurationIndex(configurationIndex)
|
||||
{}
|
||||
|
||||
void EdbgDevice::init() {
|
||||
using ::DebugToolDrivers::Protocols::CmsisDap::CmsisDapInterface;
|
||||
using Microchip::Protocols::Edbg::EdbgInterface;
|
||||
using Microchip::Protocols::Edbg::EdbgTargetPowerManagementInterface;
|
||||
|
||||
@@ -52,12 +57,29 @@ namespace DebugToolDrivers::Microchip
|
||||
this->edbgInterface = std::make_unique<EdbgInterface>(std::move(cmsisHidInterface));
|
||||
|
||||
/*
|
||||
* The EDBG/CMSIS-DAP interface doesn't operate properly when sending commands too quickly.
|
||||
* Sometimes, some EDBG tools misbehave when we send commands too quickly, even though we wait for a response
|
||||
* for each command we send...
|
||||
*
|
||||
* Because of this, we have to enforce a minimum time gap between commands. See comment
|
||||
* in CmsisDapInterface class declaration for more info.
|
||||
* Because of this, we make available the option to enforce a minimum time gap between commands. This prevents
|
||||
* the tool from misbehaving, but effectively chokes the EDBG driver, causing a drag on Bloom's performance.
|
||||
*
|
||||
* The command delay is disabled by default for all EDBG tools, but the user can enable it via their project
|
||||
* config.
|
||||
*/
|
||||
this->edbgInterface->setMinimumCommandTimeGap(std::chrono::milliseconds{35});
|
||||
if (this->toolConfig.cmsisCommandDelay.has_value()) {
|
||||
const auto& cmsisCommandDelay = *(this->toolConfig.cmsisCommandDelay);
|
||||
|
||||
if (cmsisCommandDelay > CmsisDapInterface::CMSIS_COMMAND_DELAY_MAX) {
|
||||
throw Exceptions::InvalidConfig{
|
||||
"CMSIS command delay value (" + std::to_string(cmsisCommandDelay.count())
|
||||
+ " ms) is too high. Maximum value: " + std::to_string(
|
||||
CmsisDapInterface::CMSIS_COMMAND_DELAY_MAX.count()
|
||||
) + " ms"
|
||||
};
|
||||
}
|
||||
|
||||
this->edbgInterface->setCommandDelay(cmsisCommandDelay);
|
||||
}
|
||||
|
||||
// We don't need to claim the CMSISDAP interface here as the HIDAPI will have already done so.
|
||||
if (!this->sessionStarted) {
|
||||
|
||||
@@ -7,6 +7,9 @@
|
||||
#include "src/DebugToolDrivers/DebugTool.hpp"
|
||||
#include "src/DebugToolDrivers/USB/UsbDevice.hpp"
|
||||
|
||||
#include "EdbgToolConfig.hpp"
|
||||
#include "src/ProjectConfig.hpp"
|
||||
|
||||
#include "Protocols/EDBG/EdbgInterface.hpp"
|
||||
#include "Protocols/EDBG/AVR/EdbgAvr8Interface.hpp"
|
||||
#include "Protocols/EDBG/AVR/EdbgAvrIspInterface.hpp"
|
||||
@@ -30,6 +33,7 @@ namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
public:
|
||||
EdbgDevice(
|
||||
const DebugToolConfig& debugToolConfig,
|
||||
std::uint16_t vendorId,
|
||||
std::uint16_t productId,
|
||||
std::uint8_t cmsisHidInterfaceNumber,
|
||||
@@ -85,6 +89,7 @@ namespace DebugToolDrivers::Microchip
|
||||
void endSession();
|
||||
|
||||
protected:
|
||||
EdbgToolConfig toolConfig;
|
||||
bool initialised = false;
|
||||
|
||||
/**
|
||||
|
||||
30
src/DebugToolDrivers/Microchip/EdbgToolConfig.cpp
Normal file
30
src/DebugToolDrivers/Microchip/EdbgToolConfig.cpp
Normal file
@@ -0,0 +1,30 @@
|
||||
#include "EdbgToolConfig.hpp"
|
||||
|
||||
#include "src/Helpers/YamlUtilities.hpp"
|
||||
#include "src/Logger/Logger.hpp"
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
EdbgToolConfig::EdbgToolConfig(const DebugToolConfig& toolConfig)
|
||||
: DebugToolConfig(toolConfig)
|
||||
{
|
||||
const auto& toolNode = toolConfig.toolNode;
|
||||
|
||||
const auto edbgDriverNode = toolNode["edbgDriver"];
|
||||
if (edbgDriverNode) {
|
||||
if (edbgDriverNode["cmsisCommandDelay"]) {
|
||||
if (YamlUtilities::isCastable<std::uint16_t>(edbgDriverNode["cmsisCommandDelay"])) {
|
||||
this->cmsisCommandDelay = std::chrono::milliseconds{
|
||||
edbgDriverNode["cmsisCommandDelay"].as<std::uint16_t>()
|
||||
};
|
||||
|
||||
} else {
|
||||
Logger::error(
|
||||
"Invalid EDBG driver config parameter ('cmsisCommandDelay') provided - must be a 16-bit "
|
||||
"unsigned integer. The parameter will be ignored."
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
21
src/DebugToolDrivers/Microchip/EdbgToolConfig.hpp
Normal file
21
src/DebugToolDrivers/Microchip/EdbgToolConfig.hpp
Normal file
@@ -0,0 +1,21 @@
|
||||
#pragma once
|
||||
|
||||
#include <optional>
|
||||
#include <chrono>
|
||||
#include <yaml-cpp/yaml.h>
|
||||
|
||||
#include "src/ProjectConfig.hpp"
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
/**
|
||||
* Extending the generic DebugToolConfig struct to accommodate EDBG configuration parameters.
|
||||
*/
|
||||
struct EdbgToolConfig: public DebugToolConfig
|
||||
{
|
||||
public:
|
||||
std::optional<std::chrono::milliseconds> cmsisCommandDelay = std::nullopt;
|
||||
|
||||
explicit EdbgToolConfig(const DebugToolConfig& toolConfig);
|
||||
};
|
||||
}
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
JtagIce3::JtagIce3()
|
||||
JtagIce3::JtagIce3(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
JtagIce3::USB_VENDOR_ID,
|
||||
JtagIce3::USB_PRODUCT_ID,
|
||||
JtagIce3::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -22,7 +22,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint8_t USB_CONFIGURATION_INDEX = 0;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
JtagIce3();
|
||||
JtagIce3(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "JTAGICE3";
|
||||
|
||||
@@ -5,8 +5,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
MplabPickit4::MplabPickit4()
|
||||
MplabPickit4::MplabPickit4(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
MplabPickit4::USB_VENDOR_ID,
|
||||
MplabPickit4::USB_PRODUCT_ID,
|
||||
MplabPickit4::CMSIS_HID_INTERFACE_NUMBER
|
||||
|
||||
@@ -29,7 +29,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t NON_EDBG_USB_VENDOR_ID = 0x04d8;
|
||||
static const inline std::uint16_t NON_EDBG_USB_PRODUCT_ID = 0x9012;
|
||||
|
||||
MplabPickit4();
|
||||
MplabPickit4(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "MPLAB PICkit 4";
|
||||
|
||||
@@ -5,8 +5,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
MplabSnap::MplabSnap()
|
||||
MplabSnap::MplabSnap(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
MplabSnap::USB_VENDOR_ID,
|
||||
MplabSnap::USB_PRODUCT_ID,
|
||||
MplabSnap::CMSIS_HID_INTERFACE_NUMBER
|
||||
|
||||
@@ -32,7 +32,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t NON_EDBG_USB_PRODUCT_ID = 0x9018;
|
||||
static const inline std::uint16_t NON_EDBG_USB_PRODUCT_ID_ALTERNATIVE = 0x9017;
|
||||
|
||||
MplabSnap();
|
||||
MplabSnap(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "MPLAB Snap";
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
PowerDebugger::PowerDebugger()
|
||||
PowerDebugger::PowerDebugger(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
PowerDebugger::USB_VENDOR_ID,
|
||||
PowerDebugger::USB_PRODUCT_ID,
|
||||
PowerDebugger::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -22,7 +22,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint8_t USB_CONFIGURATION_INDEX = 0;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
PowerDebugger();
|
||||
PowerDebugger(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Power Debugger";
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
XplainedMini::XplainedMini()
|
||||
XplainedMini::XplainedMini(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
XplainedMini::USB_VENDOR_ID,
|
||||
XplainedMini::USB_PRODUCT_ID,
|
||||
XplainedMini::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -21,7 +21,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t USB_PRODUCT_ID = 0x2145;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
XplainedMini();
|
||||
XplainedMini(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Xplained Mini";
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
XplainedNano::XplainedNano()
|
||||
XplainedNano::XplainedNano(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
XplainedNano::USB_VENDOR_ID,
|
||||
XplainedNano::USB_PRODUCT_ID,
|
||||
XplainedNano::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -21,7 +21,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t USB_PRODUCT_ID = 0x2145;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
XplainedNano();
|
||||
XplainedNano(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Xplained Nano";
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
|
||||
namespace DebugToolDrivers::Microchip
|
||||
{
|
||||
XplainedPro::XplainedPro()
|
||||
XplainedPro::XplainedPro(const DebugToolConfig& debugToolConfig)
|
||||
: EdbgDevice(
|
||||
debugToolConfig,
|
||||
XplainedPro::USB_VENDOR_ID,
|
||||
XplainedPro::USB_PRODUCT_ID,
|
||||
XplainedPro::CMSIS_HID_INTERFACE_NUMBER,
|
||||
|
||||
@@ -21,7 +21,7 @@ namespace DebugToolDrivers::Microchip
|
||||
static const inline std::uint16_t USB_PRODUCT_ID = 0x2111;
|
||||
static const inline std::uint8_t CMSIS_HID_INTERFACE_NUMBER = 0;
|
||||
|
||||
XplainedPro();
|
||||
XplainedPro(const DebugToolConfig& debugToolConfig);
|
||||
|
||||
std::string getName() override {
|
||||
return "Xplained Pro";
|
||||
|
||||
@@ -21,6 +21,8 @@ namespace DebugToolDrivers::Protocols::CmsisDap
|
||||
class CmsisDapInterface
|
||||
{
|
||||
public:
|
||||
static constexpr auto CMSIS_COMMAND_DELAY_MAX = std::chrono::milliseconds{200};
|
||||
|
||||
explicit CmsisDapInterface(Usb::HidInterface&& usbHidInterface);
|
||||
|
||||
virtual ~CmsisDapInterface() = default;
|
||||
@@ -38,8 +40,8 @@ namespace DebugToolDrivers::Protocols::CmsisDap
|
||||
return this->usbHidInterface.inputReportSize;
|
||||
}
|
||||
|
||||
void setMinimumCommandTimeGap(std::chrono::milliseconds commandTimeGap) {
|
||||
this->commandDelay = commandTimeGap;
|
||||
void setCommandDelay(std::chrono::milliseconds commandDelay) {
|
||||
this->commandDelay = commandDelay;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -315,66 +315,69 @@ namespace TargetController
|
||||
std::string,
|
||||
std::function<std::unique_ptr<DebugTool>()>
|
||||
> TargetControllerComponent::getSupportedDebugTools() {
|
||||
using namespace DebugToolDrivers::Microchip;
|
||||
using namespace DebugToolDrivers::Wch;
|
||||
|
||||
// The debug tool names in this mapping should always be lower-case.
|
||||
return std::map<std::string, std::function<std::unique_ptr<DebugTool>()>> {
|
||||
{
|
||||
"atmel-ice",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::AtmelIce>();
|
||||
[this] {
|
||||
return std::make_unique<AtmelIce>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"power-debugger",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::PowerDebugger>();
|
||||
[this] {
|
||||
return std::make_unique<PowerDebugger>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"snap",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::MplabSnap>();
|
||||
[this] {
|
||||
return std::make_unique<MplabSnap>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"pickit-4",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::MplabPickit4>();
|
||||
[this] {
|
||||
return std::make_unique<MplabPickit4>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"xplained-pro",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::XplainedPro>();
|
||||
[this] {
|
||||
return std::make_unique<XplainedPro>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"xplained-mini",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::XplainedMini>();
|
||||
[this] {
|
||||
return std::make_unique<XplainedMini>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"xplained-nano",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::XplainedNano>();
|
||||
[this] {
|
||||
return std::make_unique<XplainedNano>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"curiosity-nano",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::CuriosityNano>();
|
||||
[this] {
|
||||
return std::make_unique<CuriosityNano>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"jtagice3",
|
||||
[] {
|
||||
return std::make_unique<DebugToolDrivers::Microchip::JtagIce3>();
|
||||
[this] {
|
||||
return std::make_unique<JtagIce3>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
{
|
||||
"wch-link-e",
|
||||
[this] {
|
||||
return std::make_unique<DebugToolDrivers::Wch::WchLinkE>(this->environmentConfig.debugToolConfig);
|
||||
return std::make_unique<WchLinkE>(this->environmentConfig.debugToolConfig);
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user