Skip to content

Commit 627e7aa

Browse files
adamgreenadamgreen
committed
Simplify DebugSerial singleton implementation
This commit simplifies the singleton implementation of the kernel mode DebugSerial class to be more like ThreadMRI's. The old implementation was too cumbersome, even requiring some of the mriPlatform APIs to be marked as "friend" functions.
1 parent 22dfcae commit 627e7aa

File tree

2 files changed

+88
-120
lines changed

2 files changed

+88
-120
lines changed

‎libraries/MRI/src/boards/portenta-h7/DebugSerial.cpp

+88-87
Original file line numberDiff line numberDiff line change
@@ -24,73 +24,80 @@ extern "C" {
2424
}
2525

2626

27-
// The number of milliseconds to pause at the beginning of setup() to give time for host to enumerate USB device.
28-
#define STARTUP_DELAY_MSEC 250
27+
// Globals that describe the DebugSerial singleton.
28+
static mbed::UnbufferedSerial* g_pSerial;
29+
static IRQn_Type g_irq;
30+
static bool g_breakInSetup;
2931

30-
static const char g_memoryMapXml[] = "<?xml version=\"1.0\"?>"
31-
"<!DOCTYPE memory-map PUBLIC \"+//IDN gnu.org//DTD GDB Memory Map V1.0//EN\" \"http://sourceware.org/gdb/gdb-memory-map.dtd\">"
32-
"<memory-map>"
33-
"<memory type=\"ram\" start=\"0x00000000\" length=\"0x10000\"> </memory>"
34-
"<memory type=\"flash\" start=\"0x08000000\" length=\"0x200000\"> <property name=\"blocksize\">0x20000</property></memory>"
35-
"<memory type=\"ram\" start=\"0x10000000\" length=\"0x48000\"> </memory>"
36-
"<memory type=\"ram\" start=\"0x1ff00000\" length=\"0x20000\"> </memory>"
37-
"<memory type=\"ram\" start=\"0x20000000\" length=\"0x20000\"> </memory>"
38-
"<memory type=\"ram\" start=\"0x24000000\" length=\"0x80000\"> </memory>"
39-
"<memory type=\"ram\" start=\"0x30000000\" length=\"0x48000\"> </memory>"
40-
"<memory type=\"ram\" start=\"0x38000000\" length=\"0x10000\"> </memory>"
41-
"<memory type=\"ram\" start=\"0x38800000\" length=\"0x1000\"> </memory>"
42-
"<memory type=\"ram\" start=\"0x58020000\" length=\"0x2c00\"> </memory>"
43-
"<memory type=\"ram\" start=\"0x58024400\" length=\"0xc00\"> </memory>"
44-
"<memory type=\"ram\" start=\"0x58025400\" length=\"0x800\"> </memory>"
45-
"<memory type=\"ram\" start=\"0x58026000\" length=\"0x800\"> </memory>"
46-
"<memory type=\"ram\" start=\"0x58027000\" length=\"0x400\"> </memory>"
47-
"<memory type=\"flash\" start=\"0x90000000\" length=\"0x10000000\"> <property name=\"blocksize\">0x200</property></memory>"
48-
"<memory type=\"ram\" start=\"0xc0000000\" length=\"0x800000\"> </memory>"
49-
"</memory-map>";
5032

33+
// Run the DebugMonitor and UART interrupts at this priority.
34+
#define DEBUG_ISR_PRIORITY 2
5135

52-
// The singleton through which all of the Platform* APIs redirect their calls.
53-
static DebugSerial* g_pDebugSerial = NULL;
5436

55-
// Will be setting initial breakpoint on setup() routine.
56-
void setup();
37+
struct SystemHandlerPriorities {
38+
uint8_t svcallPriority;
39+
uint8_t pendsvPriority;
40+
uint8_t systickPriority;
41+
};
5742

43+
44+
// Forward Function Declarations
45+
static void setupStopInSetup();
46+
static int justEnteredSetupCallback(void* pv);
47+
static void initSerial();
48+
static SystemHandlerPriorities getSystemHandlerPrioritiesBeforeMriModifiesThem();
49+
static void restoreSystemHandlerPriorities(const SystemHandlerPriorities* pPriorities);
50+
static void switchFaultHandlersToDebugger();
51+
52+
53+
// Forward declaration of external functions used by DebugSerial.
54+
// Will be setting initial breakpoint on setup() routine.
55+
extern "C" void setup();
5856
// The debugger uses this handler to catch faults, debug events, etc.
5957
extern "C" void mriExceptionHandler(void);
6058

6159

62-
DebugSerial::DebugSerial(PinName txPin, PinName rxPin, IRQn_Type irq, uint32_t baudRate, bool breakInSetup /*=true*/) :
63-
_serial(txPin, rxPin, baudRate), _irq(irq), _breakInSetup(breakInSetup)
60+
arduino::DebugSerial::DebugSerial(PinName txPin, PinName rxPin, IRQn_Type irq, uint32_t baudRate, bool breakInSetup /*=true*/) :
61+
_serial(txPin, rxPin, baudRate)
6462
{
6563
// Just return without doing anything if the singleton has already been initialized.
6664
// This ends up using the first initialized DebugSerial object.
67-
if (g_pDebugSerial != NULL) {
65+
if (g_pSerial != NULL) {
6866
return;
6967
}
70-
g_pDebugSerial = this;
68+
g_irq = irq;
69+
g_breakInSetup = breakInSetup;
70+
g_pSerial = &_serial;
7171

7272
mriInit("");
7373

7474
setupStopInSetup();
7575
}
7676

77-
void DebugSerial::setupStopInSetup() {
77+
static void setupStopInSetup()
78+
{
7879
mriCore_SetTempBreakpoint((uint32_t)setup, justEnteredSetupCallback, NULL);
7980
}
8081

81-
int DebugSerial::justEnteredSetupCallback(void* pv){
82-
return g_pDebugSerial->justEnteredSetup();
83-
}
84-
85-
int DebugSerial::justEnteredSetup() {
82+
static int justEnteredSetupCallback(void* pv)
83+
{
8684
initSerial();
8785

8886
// Return 0 to indicate that we want to halt execution at the beginning of setup() or 1 to not force a halt.
89-
return _breakInSetup ? 0 : 1;
87+
return g_breakInSetup ? 0 : 1;
88+
}
89+
90+
static void initSerial()
91+
{
92+
// Hook communication port ISR to allow debug monitor to be awakened when GDB sends a command.
93+
g_pSerial->attach(mriExceptionHandler);
94+
mriCortexMSetPriority(g_irq, DEBUG_ISR_PRIORITY, 0);
95+
NVIC_SetVector(g_irq, (uint32_t)mriExceptionHandler);
9096
}
9197

9298

93-
DebugSerial::~DebugSerial() {
99+
arduino::DebugSerial::~DebugSerial()
100+
{
94101
// IMPORTANT NOTE: You are attempting to destroy the connection to GDB which isn't allowed.
95102
// Don't allow your DebugSerial object to go out of scope like this.
96103
__debugbreak();
@@ -99,59 +106,19 @@ DebugSerial::~DebugSerial() {
99106
}
100107
}
101108

102-
void DebugSerial::setSerialPriority(uint8_t priority) {
103-
mriCortexMSetPriority(_irq, priority, 0);
104-
}
105-
106-
void DebugSerial::initSerial() {
107-
108-
// Hook communication port ISR to allow debug monitor to be awakened when GDB sends a command.
109-
_serial.attach(mriExceptionHandler);
110-
setSerialPriority(2);
111-
NVIC_SetVector(_irq, (uint32_t)mriExceptionHandler);
112-
}
113-
114-
uint32_t DebugSerial::hasReceiveData(void) {
115-
return _serial.readable();
116-
}
117-
118-
int DebugSerial::receiveChar(void) {
119-
while (!hasReceiveData()) {
120-
}
121-
uint8_t byte;
122-
_serial.read(&byte, 1);
123-
return byte;
124-
}
125-
126-
void DebugSerial::sendChar(int character) {
127-
_serial.write(&character, 1);
128-
}
129-
130109

131110

132111

133112
// ---------------------------------------------------------------------------------------------------------------------
134113
// Global Platform_* functions needed by MRI to initialize and communicate with MRI.
135114
// These functions will perform most of their work through the DebugSerial singleton.
136115
// ---------------------------------------------------------------------------------------------------------------------
137-
struct SystemHandlerPriorities {
138-
uint8_t svcallPriority;
139-
uint8_t pendsvPriority;
140-
uint8_t systickPriority;
141-
};
142-
143-
// Forward Function Declarations
144-
static SystemHandlerPriorities getSystemHandlerPrioritiesBeforeMriModifiesThem();
145-
static void restoreSystemHandlerPriorities(const SystemHandlerPriorities* pPriorities);
146-
static void switchFaultHandlersToDebugger();
147-
148-
149-
extern "C" void Platform_Init(Token* pParameterTokens) {
116+
void Platform_Init(Token* pParameterTokens)
117+
{
150118
SystemHandlerPriorities origPriorities = getSystemHandlerPrioritiesBeforeMriModifiesThem();
151-
uint8_t debugMonPriority = 2;
152119

153120
__try
154-
mriCortexMInit((Token*)pParameterTokens, debugMonPriority, WAKEUP_PIN_IRQn);
121+
mriCortexMInit((Token*)pParameterTokens, DEBUG_ISR_PRIORITY, WAKEUP_PIN_IRQn);
155122
__catch
156123
__rethrow;
157124

@@ -188,19 +155,51 @@ static void switchFaultHandlersToDebugger(void) {
188155
}
189156

190157

191-
extern "C" uint32_t Platform_CommHasReceiveData(void) {
192-
return g_pDebugSerial->hasReceiveData();
158+
159+
160+
uint32_t Platform_CommHasReceiveData(void)
161+
{
162+
return g_pSerial->readable();
193163
}
194164

195-
extern "C" int Platform_CommReceiveChar(void) {
196-
return g_pDebugSerial->receiveChar();
165+
int Platform_CommReceiveChar(void)
166+
{
167+
while (!Platform_CommHasReceiveData()) {
168+
}
169+
uint8_t byte;
170+
g_pSerial->read(&byte, 1);
171+
return byte;
197172
}
198173

199-
extern "C" void Platform_CommSendChar(int character) {
200-
g_pDebugSerial->sendChar(character);
174+
void Platform_CommSendChar(int character)
175+
{
176+
g_pSerial->write(&character, 1);
201177
}
202178

203179

180+
181+
182+
static const char g_memoryMapXml[] = "<?xml version=\"1.0\"?>"
183+
"<!DOCTYPE memory-map PUBLIC \"+//IDN gnu.org//DTD GDB Memory Map V1.0//EN\" \"http://sourceware.org/gdb/gdb-memory-map.dtd\">"
184+
"<memory-map>"
185+
"<memory type=\"ram\" start=\"0x00000000\" length=\"0x10000\"> </memory>"
186+
"<memory type=\"flash\" start=\"0x08000000\" length=\"0x200000\"> <property name=\"blocksize\">0x20000</property></memory>"
187+
"<memory type=\"ram\" start=\"0x10000000\" length=\"0x48000\"> </memory>"
188+
"<memory type=\"ram\" start=\"0x1ff00000\" length=\"0x20000\"> </memory>"
189+
"<memory type=\"ram\" start=\"0x20000000\" length=\"0x20000\"> </memory>"
190+
"<memory type=\"ram\" start=\"0x24000000\" length=\"0x80000\"> </memory>"
191+
"<memory type=\"ram\" start=\"0x30000000\" length=\"0x48000\"> </memory>"
192+
"<memory type=\"ram\" start=\"0x38000000\" length=\"0x10000\"> </memory>"
193+
"<memory type=\"ram\" start=\"0x38800000\" length=\"0x1000\"> </memory>"
194+
"<memory type=\"ram\" start=\"0x58020000\" length=\"0x2c00\"> </memory>"
195+
"<memory type=\"ram\" start=\"0x58024400\" length=\"0xc00\"> </memory>"
196+
"<memory type=\"ram\" start=\"0x58025400\" length=\"0x800\"> </memory>"
197+
"<memory type=\"ram\" start=\"0x58026000\" length=\"0x800\"> </memory>"
198+
"<memory type=\"ram\" start=\"0x58027000\" length=\"0x400\"> </memory>"
199+
"<memory type=\"flash\" start=\"0x90000000\" length=\"0x10000000\"> <property name=\"blocksize\">0x200</property></memory>"
200+
"<memory type=\"ram\" start=\"0xc0000000\" length=\"0x800000\"> </memory>"
201+
"</memory-map>";
202+
204203
extern "C" uint32_t Platform_GetDeviceMemoryMapXmlSize(void) {
205204
return sizeof(g_memoryMapXml) - 1;
206205
}
@@ -210,6 +209,8 @@ extern "C" const char* Platform_GetDeviceMemoryMapXml(void) {
210209
}
211210

212211

212+
213+
213214
extern "C" const uint8_t* Platform_GetUid(void) {
214215
return NULL;
215216
}

‎libraries/MRI/src/boards/portenta-h7/DebugSerial.h

-33
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,9 @@
1515
// GDB Kernel debugging of the Arduino Portenta-H7 over a serial connection.
1616
#pragma once
1717

18-
#include <Arduino.h>
1918
#include <mbed.h>
2019

21-
extern "C" {
22-
typedef struct Token Token;
23-
void mriPlatform_Init(Token* pParameterTokens);
24-
uint32_t mriPlatform_CommHasReceiveData(void);
25-
int mriPlatform_CommReceiveChar(void);
26-
void mriPlatform_CommSendChar(int character);
27-
}
2820

29-
30-
// UNDONE: Should I put the debugger objects into the Arduino namespace?
3121
namespace arduino {
3222

3323
class DebugSerial {
@@ -42,30 +32,7 @@ class DebugSerial {
4232
~DebugSerial();
4333

4434
protected:
45-
void construct();
46-
void setupStopInSetup();
47-
48-
// These protected methods are called from global Platform* routines via singleton.
49-
void setSerialPriority(uint8_t priority);
50-
uint32_t hasReceiveData(void);
51-
int receiveChar(void);
52-
void sendChar(int character);
53-
54-
void initSerial();
55-
56-
static int justEnteredSetupCallback(void* pvContext);
57-
int justEnteredSetup();
58-
59-
typedef void(*IsrFunctionPtr)(void);
60-
6135
mbed::UnbufferedSerial _serial;
62-
IRQn_Type _irq;
63-
bool _breakInSetup;
64-
65-
friend void ::mriPlatform_Init(Token* pParameterTokens);
66-
friend uint32_t ::mriPlatform_CommHasReceiveData(void);
67-
friend int ::mriPlatform_CommReceiveChar(void);
68-
friend void ::mriPlatform_CommSendChar(int character);
6936
};
7037

7138
} // namespace arduino

0 commit comments

Comments
 (0)