Switch ThreadMRI to use mbed serial objects

This commit enables ThreadMRI to use mbed's lower level serial objects
instead of the currently used Arduino ones. The ThreadMRI constructor
now takes a DebugCommInterface pointer to either a
UartDebugCommInterface or UsbDebugCommInterface instantiation.

This change allows ThreadMRI to use the attach() method on both UART
and USB based connections to support breaking into running programs
via CTRL+C. Before it had to insert a shim into the device's vector
table.

NOTE: I had to buffer the bytes as they were received in the attached
      routine for UART based communications since if it didn't read out
      the most recently received byte, the UART receive interrupt line
      wouldn't be cleared and it would just keep launching the UART
      ISR.

Author: adamgreen

libraries/ThreadMRI/examples/ThreadMRI/ThreadMRI.ino

@@ -4,9 +4,11 @@
 
 #include <ThreadMRI.h>
 
-//ThreadMRI g_debugger(Serial1, USART1_IRQn, 115200, THREADMRI_BREAK_ON_SETUP);
-ThreadMRI g_debugger(Serial, OTG_HS_IRQn, 115200, THREADMRI_BREAK_ON_SETUP);
-//ThreadMRI g_debugger(Serial, false);
+//UartDebugCommInterface g_comm(SERIAL1_TX, SERIAL1_RX, 230400);
+//ThreadMRI              g_debugSerial(&g_comm, THREADMRI_BREAK_ON_SETUP);
+
+UsbDebugCommInterface  g_comm(&SerialUSB);
+ThreadMRI              g_debugSerial(&g_comm, THREADMRI_BREAK_ON_SETUP);
 
 extern "C" void testContext(void);
 

libraries/ThreadMRI/src/ThreadMRI.cpp

@@ -73,9 +73,7 @@ extern "C"
 
 
 // Globals that describe the ThreadMRI singleton.
-static HardwareSerial*          g_pSerial;
-static IRQn_Type                g_irq;
-static uint32_t                 g_baudRate;
+static DebugCommInterface*      g_pComm;
 static bool                     g_breakInSetup;
 
 // The ID of the halted thread being debugged.
@@ -110,8 +108,6 @@ volatile uint32_t               mriThreadOrigHardFault;
 volatile uint32_t               mriThreadOrigMemManagement;
 volatile uint32_t               mriThreadOrigBusFault;
 volatile uint32_t               mriThreadOrigUsageFault;
-// Address of the original Serial peripheral ISR to be hooked.
-void                            (*g_origSerialISR)(void);
 
 // Entries to track the chunks of the context in a scatter list.
 #if MRI_DEVICE_HAS_FPU
@@ -151,9 +147,8 @@ static void resumeApplicationThreads();
 static void readThreadContext(osThreadId_t thread);
 static void switchRtxHandlersToDebugStubsForSingleStepping();
 static void restoreRtxHandlers();
-static void callSerialBeginFromSetup();
+static void callAttachFromSetup();
 static int justEnteredSetupCallback(void* pv);
-static void hookSerialISR();
 static bool isThreadMode(uint32_t excReturn);
 static bool hasEncounteredDebugEvent();
 static bool hasControlCBeenDetected();
@@ -174,35 +169,16 @@ static void setControlCFlag();
 
 
 
-
-ThreadMRI::ThreadMRI(HardwareSerial& serial, IRQn_Type IRQn, uint32_t baudRate, bool breakInSetup /* =true */)
-{
-    init(serial, IRQn, baudRate, breakInSetup);
-}
-
-
-ThreadMRI::ThreadMRI(HardwareSerial& serial, bool breakInSetup /* =true */)
+ThreadMRI::ThreadMRI(DebugCommInterface* pCommInterface, bool breakInSetup /*=true*/)
 {
-    init(serial, SysTick_IRQn, 115200, breakInSetup);
-}
-
-
-ThreadMRI::ThreadMRI(HardwareSerial& serial, IRQn_Type IRQn)
-{
-    init(serial, IRQn, 0, false);
-}
-
-void ThreadMRI::init(HardwareSerial& serial, IRQn_Type IRQn, uint32_t baudRate, bool breakInSetup)
-{
-    if (g_pSerial != NULL) {
+    if (g_pComm != NULL) {
         // Only allow 1 ThreadMRI object to be initialized.
         return;
     }
 
     // Setup the singleton.
-    g_irq = IRQn;
-    g_baudRate = baudRate;
     g_breakInSetup = breakInSetup;
+    g_pComm = pCommInterface;
 
     // Initialize the MRI core.
     mriInit("");
@@ -225,10 +201,7 @@ void ThreadMRI::init(HardwareSerial& serial, IRQn_Type IRQn, uint32_t baudRate,
         return;
     }
 
-    g_pSerial = &serial;
-    if (g_baudRate != 0) {
-        callSerialBeginFromSetup();
-    }
+    callAttachFromSetup();
 }
 
 static __NO_RETURN void mriMain(void *pv)
@@ -364,32 +337,19 @@ static void restoreRtxHandlers()
     NVIC_SetVector(SysTick_IRQn, mriThreadOrigSysTick);
 }
 
-static void callSerialBeginFromSetup()
+static void callAttachFromSetup()
 {
     mriCore_SetTempBreakpoint((uint32_t)setup, justEnteredSetupCallback, NULL);
 }
 
 static int justEnteredSetupCallback(void* pv)
 {
-    g_pSerial->begin(g_baudRate);
-    //while (!g_pSerial) {}
-
-    if (g_irq != SysTick_IRQn) {
-        hookSerialISR();
-    } else {
-        (static_cast<USBSerial*>(g_pSerial))->attach(serialISRHook);
-    }
+    g_pComm->attach(serialISRHook);
 
     // Return 0 to indicate that we want to halt execution at the beginning of setup() or 1 to not force a halt.
     return g_breakInSetup ? 0 : 1;
 }
 
-static void hookSerialISR()
-{
-    g_origSerialISR = (void(*)(void))NVIC_GetVector(g_irq);
-    NVIC_SetVector(g_irq, (uint32_t)serialISRHook);
-}
-
 
 ThreadMRI::~ThreadMRI() {
     // IMPORTANT NOTE: You are attempting to destroy the connection to GDB which isn't allowed.
@@ -441,20 +401,20 @@ static void recordAndSwitchFaultHandlersToDebugger()
 
 uint32_t Platform_CommHasReceiveData(void)
 {
-    return g_pSerial->available();
+    return g_pComm->available();
 }
 
 int Platform_CommReceiveChar(void)
 {
-    while (!g_pSerial->available()) {
+    while (!g_pComm->available()) {
         // Busy wait.
     }
-    return g_pSerial->read();
+    return g_pComm->read();
 }
 
 void Platform_CommSendChar(int character)
 {
-    g_pSerial->write(character);
+    g_pComm->write(character);
 }
 
 
@@ -693,10 +653,7 @@ static void clearFaultStatusBits()
 
 static void serialISRHook()
 {
-    if (g_origSerialISR) {
-        g_origSerialISR();
-    }
-    if (!isDebuggerActive() && g_pSerial->available() > 0) {
+    if (!isDebuggerActive() && g_pComm->available()) {
         // Pend a halt into the debug monitor now that there is data from GDB ready to be read by it.
         setControlCFlag();
         setMonitorPending();
@@ -712,3 +669,97 @@ static void setControlCFlag()
 {
     mriCortexMFlags |= CORTEXM_FLAGS_CTRL_C;
 }
+
+
+
+
+DebugCommInterface::~DebugCommInterface()
+{
+}
+
+UartDebugCommInterface::UartDebugCommInterface(PinName txPin, PinName rxPin, uint32_t baudRate) :
+    _pCallback(NULL), _serial(txPin, rxPin, baudRate), _read(0), _write(0)
+{
+}
+
+UartDebugCommInterface::~UartDebugCommInterface()
+{
+}
+
+bool UartDebugCommInterface::available()
+{
+    return _read != _write;
+}
+
+uint8_t UartDebugCommInterface::read()
+{
+    // This read should never block since Platform_CommReceiveChar() always checks available() first.
+    ASSERT ( available() );
+
+    uint8_t byte = _queue[_read];
+    _read = wrappingIncrement(_read);
+    return byte;
+}
+
+uint32_t UartDebugCommInterface::wrappingIncrement(uint32_t val)
+{
+    return (val + 1) & (sizeof(_queue) - 1);
+}
+
+void UartDebugCommInterface::write(uint8_t c)
+{
+    _serial.write(&c, 1);
+}
+
+void UartDebugCommInterface::attach(void (*pCallback)())
+{
+    _pCallback = pCallback;
+    _serial.attach(mbed::callback(this, &UartDebugCommInterface::onReceivedData));
+}
+
+void UartDebugCommInterface::onReceivedData()
+{
+    while (_serial.readable()) {
+        uint8_t byte;
+        _serial.read(&byte, 1);
+        if (wrappingIncrement(_write) != _read) {
+            // _queue isn't full so we can add this byte to it.
+            _queue[_write] = byte;
+            _write = wrappingIncrement(_write);
+        }
+    }
+
+    ASSERT ( _pCallback != NULL );
+    _pCallback();
+}
+
+
+
+UsbDebugCommInterface::UsbDebugCommInterface(arduino::USBSerial* pSerial) :
+    _pSerial(pSerial)
+{
+}
+
+UsbDebugCommInterface::~UsbDebugCommInterface()
+{
+}
+
+bool UsbDebugCommInterface::available()
+{
+    return _pSerial->available() > 0;
+}
+
+uint8_t UsbDebugCommInterface::read()
+{
+    return _pSerial->read();
+}
+
+void UsbDebugCommInterface::write(uint8_t c)
+{
+    _pSerial->write(c);
+}
+
+void UsbDebugCommInterface::attach(void (*pCallback)())
+{
+    _pSerial->attach(pCallback);
+}

libraries/ThreadMRI/src/ThreadMRI.h

@@ -16,46 +16,78 @@
 // the USB serial connection.
 #pragma once
 
+#include <mbed.h>
+
+
+class DebugCommInterface {
+public:
+    virtual ~DebugCommInterface() = 0;
+
+    virtual bool available() = 0;
+	virtual uint8_t read() = 0;
+	virtual void write(uint8_t c) = 0;
+    virtual void attach(void (*pCallback)()) = 0;
+};
+
+class UartDebugCommInterface : public DebugCommInterface {
+public:
+    UartDebugCommInterface(PinName txPin, PinName rxPin, uint32_t baudRate);
+    virtual ~UartDebugCommInterface();
+
+    virtual bool available();
+	virtual uint8_t read();
+	virtual void write(uint8_t c);
+    virtual void attach(void (*pCallback)());
+
+protected:
+    uint32_t wrappingIncrement(uint32_t val);
+    void onReceivedData();
+
+    void                    (* _pCallback)();
+    mbed::UnbufferedSerial  _serial;
+    volatile uint32_t       _read;
+    volatile uint32_t       _write;
+    uint8_t                 _queue[8];
+};
+
+class UsbDebugCommInterface : public DebugCommInterface {
+public:
+    UsbDebugCommInterface(arduino::USBSerial*);
+    virtual ~UsbDebugCommInterface();
+
+    virtual bool available();
+	virtual uint8_t read();
+	virtual void write(uint8_t c);
+    virtual void attach(void (*pCallback)());
+
+protected:
+    arduino::USBSerial*  _pSerial;
+};
+
 
 // Pass THREADMRI_BREAK_ON_SETUP as breakInSetup parameter of ThreadMRI constructors to halt in setup().
-#define THREADMRI_BREAK_ON_SETUP true
+#define THREADMRI_BREAK_ON_SETUP    true
+// Pass THREADMRI_NO_BREAK_ON_SETUP as breakInSetup parameter of ThreadMRI constructors to NOT halt in setup().
+#define THREADMRI_NO_BREAK_ON_SETUP false
 
 
 class ThreadMRI {
 public:
-    // You must declare your ThreadMRI object as a global or function scoped static so that it doesn't get
-    // destroyed. Which constructor you use, depends on where you declare it.
-
-    // Use this constructor when declaring ThreadMRI object as a global outside of any functions.
-    // You must specify the baudrate so that ThreadMRI can call begin() on the specified serial object.
-    //  breakInSetup - should it break at beginning of setup().
-    //
-    // Global Example:
-    //      ThreadMRI threadMRI(Serial1, USART1_IRQn, 115200);
-    ThreadMRI(HardwareSerial& serial, IRQn_Type IRQn, uint32_t baudRate, bool breakInSetup=true);
-
-    // Use this constructor when declaring ThreadMRI object as a function scoped static. You must call begin() on
-    // the serial object before constructing the ThreadMRI object.
-    //
-    // Function Scoped Static Example:
-    //      #include <MRI.h>
-    //      void setup() {
-    //          Serial1.begin(115200);
-    //          static ThreadMRI threadMRI(Serial1, USART1_IRQn);
-    //          __debugbreak();
-    //      }
-    ThreadMRI(HardwareSerial& serial, IRQn_Type IRQn);
-
-    ThreadMRI(HardwareSerial& serial, bool breakInSetup=true);
-
-    // You should never let your ThreadMRI object go out of scope. Make it global or static. To warn you if you do
-    // let it go out of scope by mistake, this destructor will break into GDB and then enter an infinite loop.
-    ~ThreadMRI();
+    // You must declare your ThreadMRI object as a global.
+    // Examples:
+    //      UartDebugCommInterface g_comm(SERIAL1_TX, SERIAL1_RX, 230400);
+    //      ThreadMRI              g_debugSerial(&g_comm, THREADMRI_BREAK_ON_SETUP);
+    //              -- OR --
+    //      UsbDebugCommInterface  g_comm(&SerialUSB);
+    //      ThreadMRI              g_debugSerial(&g_comm, THREADMRI_BREAK_ON_SETUP);
+    ThreadMRI(DebugCommInterface* pCommInterface, bool breakInSetup=THREADMRI_BREAK_ON_SETUP);
 
-protected:
-    void init(HardwareSerial& serial, IRQn_Type IRQn, uint32_t baudRate, bool breakInSetup);
+    // Your ThreadMRI object should never go out of scope. To warn you if you do let it go out of scope by mistake,
+    // this destructor will break into GDB and then enter an infinite loop.
+    ~ThreadMRI();
 };
 
+
 // Use to insert a hardcoded breakpoint into your code.
 #ifndef __debugbreak
     #define __debugbreak()  { __asm volatile ("bkpt #0"); }