Rework Wire Protocol

src/CLR/Debugger/Debugger.cpp

@@ -59,6 +59,8 @@ void CLR_DBG_Debugger::Debugger_Discovery()
 
     CLR_INT32 wait_sec = 5;
 
+    uint8_t loopCounter = 1;
+
     CLR_INT64 expire = HAL_Time_CurrentTime() + (wait_sec * TIME_CONVERSION__TO_SECONDS);
 
     // Send "presence" ping.
@@ -69,7 +71,7 @@ void CLR_DBG_Debugger::Debugger_Discovery()
     {
         CLR_EE_DBG_EVENT_BROADCAST(
             CLR_DBG_Commands::c_Monitor_Ping,
-            sizeof(cmd),
+            0,
             &cmd,
             WP_Flags_c_NoCaching | WP_Flags_c_NonCritical);
 
@@ -96,6 +98,11 @@ void CLR_DBG_Debugger::Debugger_Discovery()
             CLR_Debug::Printf("No debugger found...\r\n");
             break;
         }
+
+        // pause for 250ms so this is not flooding the channel with PING packets
+        Events_WaitForEvents(0, loopCounter * 500);
+
+        loopCounter++;
     }
 
     g_CLR_RT_ExecutionEngine.WaitForDebugger();
@@ -1005,30 +1012,27 @@ bool CLR_DBG_Debugger::Monitor_Reboot(WP_Message *msg)
 
     CLR_DBG_Commands::Monitor_Reboot *cmd = (CLR_DBG_Commands::Monitor_Reboot *)msg->m_payload;
 
-    if (NULL != cmd)
+    if (CLR_DBG_Commands::Monitor_Reboot::c_EnterNanoBooter ==
+        (cmd->m_flags & CLR_DBG_Commands::Monitor_Reboot::c_EnterNanoBooter))
     {
-        if (CLR_DBG_Commands::Monitor_Reboot::c_EnterNanoBooter ==
-            (cmd->m_flags & CLR_DBG_Commands::Monitor_Reboot::c_EnterNanoBooter))
-        {
-            success = RequestToLaunchNanoBooter();
-        }
-        else if (
-            CLR_DBG_Commands::Monitor_Reboot::c_EnterProprietaryBooter ==
-            (cmd->m_flags & CLR_DBG_Commands::Monitor_Reboot::c_EnterProprietaryBooter))
-        {
-            success = RequestToLaunchProprietaryBootloader();
-        }
-
-        g_CLR_RT_ExecutionEngine.m_iReboot_Options = cmd->m_flags;
+        success = RequestToLaunchNanoBooter();
+    }
+    else if (
+        CLR_DBG_Commands::Monitor_Reboot::c_EnterProprietaryBooter ==
+        (cmd->m_flags & CLR_DBG_Commands::Monitor_Reboot::c_EnterProprietaryBooter))
+    {
+        success = RequestToLaunchProprietaryBootloader();
     }
-
-    Events_WaitForEvents(0, 100); // give message a little time to be flushed
 
     WP_ReplyToCommand(msg, success, false, NULL, 0);
 
-    Events_WaitForEvents(0, 100); // give message a little time to be flushed
+    // on success, apply reboot options and set reboot pending flag
+    if (success)
+    {
+        g_CLR_RT_ExecutionEngine.m_iReboot_Options = cmd->m_flags;
 
-    CLR_EE_DBG_SET(RebootPending);
+        CLR_EE_DBG_SET(RebootPending);
+    }
 
     return true;
 }
@@ -1264,22 +1268,27 @@ bool CLR_DBG_Debugger::Debugging_Execution_ChangeConditions(WP_Message *msg)
     CLR_DBG_Commands::Debugging_Execution_ChangeConditions *cmd =
         (CLR_DBG_Commands::Debugging_Execution_ChangeConditions *)msg->m_payload;
 
-    g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions |= cmd->FlagsToSet;
-    g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions &= ~cmd->FlagsToReset;
+    // save current value
+    int32_t conditionsCopy = g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions;
 
-    // updating the debugging execution conditions requires sometime to propagate
-    // make sure we allow enough time for that to happen
-    OS_DELAY(100);
+    // apply received flags
+    conditionsCopy |= cmd->FlagsToSet;
+    conditionsCopy &= ~cmd->FlagsToReset;
 
+    // send confirmation reply
     if ((msg->m_header.m_flags & WP_Flags_c_NonCritical) == 0)
     {
         CLR_DBG_Commands::Debugging_Execution_ChangeConditions::Reply cmdReply;
 
-        cmdReply.CurrentState = g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions;
+        cmdReply.CurrentState = conditionsCopy;
 
         WP_ReplyToCommand(msg, true, false, &cmdReply, sizeof(cmdReply));
     }
 
+    // set & reset new conditions
+    g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions |= cmd->FlagsToSet;
+    g_CLR_RT_ExecutionEngine.m_iDebugger_Conditions &= ~cmd->FlagsToReset;
+
     return true;
 }
 

src/CLR/Diagnostics/Info.cpp

@@ -226,7 +226,7 @@ void CLR_Debug::Emit(const char *text, int len)
             HAL_Windows_Debug_Print(s_buffer);
 #endif
 
-            if (CLR_EE_DBG_IS(Enabled) && !CLR_EE_DBG_IS(Quiet))
+            if (CLR_EE_DBG_IS(Enabled) && CLR_EE_DBG_IS_NOT(Quiet))
             {
                 CLR_EE_DBG_EVENT_BROADCAST(
                     CLR_DBG_Commands_c_Monitor_Message,
@@ -235,8 +235,9 @@ void CLR_Debug::Emit(const char *text, int len)
                     WP_Flags_c_NonCritical | WP_Flags_c_NoCaching);
             }
 
-            if (HalSystemConfig.DebugTextPort != HalSystemConfig.DebuggerPort)
+            if (CLR_EE_DBG_IS_NOT(Enabled) || HalSystemConfig.DebugTextPort != HalSystemConfig.DebuggerPort)
             {
+
 #if !defined(_WIN32)
                 DebuggerPort_Write(
                     HalSystemConfig.DebugTextPort,

src/CLR/Include/WireProtocol.h

@@ -58,13 +58,13 @@ typedef enum WP_Flags
 // enum with machine states for Wire Procotol receiver
 typedef enum ReceiveState
 {
-    ReceiveState_Idle             = (1 << 0),
-    ReceiveState_Initialize       = (2 << 0),
-    ReceiveState_WaitingForHeader = (3 << 0),
-    ReceiveState_ReadingHeader    = (4 << 0),
-    ReceiveState_CompleteHeader   = (5 << 0),
-    ReceiveState_ReadingPayload   = (6 << 0),
-    ReceiveState_CompletePayload  = (7 << 0),
+    ReceiveState_Idle             = 1,
+    ReceiveState_Initialize       = 2,
+    ReceiveState_WaitingForHeader = 3,
+    ReceiveState_ReadingHeader    = 4,
+    ReceiveState_CompleteHeader   = 5,
+    ReceiveState_ReadingPayload   = 6,
+    ReceiveState_CompletePayload  = 7,
 }ReceiveState;
 
 // enum with CLR monitor commands
@@ -124,24 +124,6 @@ typedef struct WP_Message
 {
     WP_Packet      m_header;
     uint8_t*       m_payload;
-
-    uint8_t*       m_pos;
-    uint16_t       m_size;
-    uint64_t       m_payloadTicks;
-    int            m_rxState;
-
-    void (*Initialize)(WP_Controller* parent);
-    void (*PrepareReception)(void);
-    void (*PrepareRequest)(unsigned int cmd, unsigned int flags, unsigned int payloadSize, unsigned char* payload);
-    void (*PrepareReply)(const void** req, unsigned int flags, unsigned int payloadSize, unsigned char* payload);
-    void (*SetPayload)(unsigned char* payload);
-    void (*Release)(void);
-    bool (*Process)(void);
-
-    bool (*VerifyHeader)(void);
-    bool (*VerifyPayload)(void);
-    void (*ReplyBadPacket)(unsigned int flags);
-
 }WP_Message;
 
 

src/CLR/Include/WireProtocol_App_Interface.h

@@ -9,12 +9,10 @@
 #include "WireProtocol.h"
 
 //////////////////////////////////////////
-extern uint8_t receptionBuffer[sizeof(WP_Packet) + WP_PACKET_SIZE];
-extern void ReplyToCommand(WP_Message* message, int fSuccess, int fCritical, void* ptr, int size);
+extern void ReplyToCommand(WP_Message *message, int fSuccess, int fCritical, void *ptr, int size);
 
 //////////////////////////////////////////
-int WP_App_ProcessHeader(WP_Message* message);
-int WP_App_ProcessPayload(WP_Message* message);
+uint8_t WP_App_ProcessHeader(WP_Message *message);
+uint8_t WP_App_ProcessPayload(WP_Message *message);
 
 #endif // _WIREPROTOCOL_APP_INTERFACE_H_
-

src/CLR/Include/WireProtocol_HAL_Interface.h

@@ -9,8 +9,24 @@
 #include "WireProtocol.h"
 
 //////////////////////////////////////////
-int WP_ReceiveBytes(uint8_t *ptr, unsigned short *size);
-int WP_TransmitMessage(WP_Message *message);
+
+///
+/// @brief Receives n bytes from the Wire Protocol channel.
+///
+/// @param ptr Pointer to the buffer that will hold the received bytes.
+/// @param size Number of bytes to read. On return it will have the number of bytes actually received.
+/// @return bool true if any bytes where received, false otherwise.
+///
+uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size);
+
+///
+/// @brief Sends a message through the Wire Protocol channel.
+///
+/// @param message Message to send
+/// @return bool true for transmition succesfull, false otherwise.
+///
+uint8_t WP_TransmitMessage(WP_Message *message);
+
 void WP_CheckAvailableIncomingData();
 
 #endif // _WIREPROTOCOL_HAL_INTERFACE_H_

src/CLR/Include/WireProtocol_Message.h

@@ -37,8 +37,8 @@ extern "C"
 {
 #endif
 
+    void WP_Message_PrepareReception();
     void WP_Message_Initialize(WP_Message *message);
-    void WP_Message_PrepareReception(WP_Message *message);
     void WP_Message_PrepareRequest(
         WP_Message *message,
         uint32_t cmd,
@@ -51,12 +51,11 @@ extern "C"
         uint32_t flags,
         uint32_t payloadSize,
         uint8_t *payload);
-    void WP_Message_SetPayload(WP_Message *message, uint8_t *payload);
-    void WP_Message_Release(WP_Message *message);
-    int WP_Message_VerifyHeader(WP_Message *message);
-    int WP_Message_VerifyPayload(WP_Message *message);
-    void WP_Message_ReplyBadPacket(uint32_t flags);
-    int WP_Message_Process(WP_Message *message);
+    uint8_t WP_Message_VerifyHeader(WP_Message *message);
+    uint8_t WP_Message_VerifyPayload(WP_Message *message);
+    void WP_Message_Process();
+
+    void WP_Message_PrepareReception_Platform();
 
 #ifdef __cplusplus
 }

src/CLR/Include/nanoCLR_Runtime.h

@@ -3392,7 +3392,8 @@ struct CLR_RT_ExecutionEngine
     static const int c_fDebugger_StateResolutionFailed = 0x00000001;
     static const int c_fDebugger_StateProgramRunning = 0x00000400;
     static const int c_fDebugger_StateProgramExited = 0x00000800;
-    static const int c_fDebugger_StateMask = c_fDebugger_StateProgramRunning + c_fDebugger_StateProgramExited;
+    static const int c_fDebugger_StateMask =
+        c_fDebugger_StateProgramRunning + c_fDebugger_StateProgramExited + c_fDebugger_StateResolutionFailed;
     //
     static const int c_fDebugger_BreakpointsDisabled = 0x00001000;
     //

src/CLR/Messaging/Messaging.cpp

@@ -386,7 +386,7 @@ bool CLR_Messaging::ProcessPayload(WP_Message *msg)
 }
 
 // wrapper function for CLR_Messaging::ProcessPayload(
-extern "C" int Messaging_ProcessPayload(WP_Message *msg)
+extern "C" bool Messaging_ProcessPayload(WP_Message *msg)
 {
     if (g_CLR_DBG_Debugger == NULL)
     {

src/CLR/WireProtocol/WireProtocol_App_Interface.c

@@ -6,21 +6,23 @@
 #include <nanoWeak.h>
 #include "WireProtocol_App_Interface.h"
 
-extern int Messaging_ProcessPayload(WP_Message *message);
+extern uint8_t Messaging_ProcessPayload(WP_Message *message);
+static uint8_t receptionBuffer[sizeof(WP_Packet) + WP_PACKET_SIZE];
 
-int WP_App_ProcessHeader(WP_Message *message)
+uint8_t WP_App_ProcessHeader(WP_Message *message)
 {
     // check for reception buffer overflow
-    if (message->m_header.m_size > sizeof(receptionBuffer))
+    if (message->m_header.m_size > WP_PACKET_SIZE)
     {
         return false;
     }
 
     message->m_payload = receptionBuffer;
+
     return true;
 }
 
-int WP_App_ProcessPayload(WP_Message *message)
+uint8_t WP_App_ProcessPayload(WP_Message *message)
 {
     return Messaging_ProcessPayload(message);
 }

src/CLR/WireProtocol/WireProtocol_HAL_Interface.c

@@ -16,7 +16,7 @@
 /////////////////////////////////////////////////////////////////////////////////////////////////
 
 // provided as weak to be replaced by actual implementation by HAL interface
-__nfweak int WP_ReceiveBytes(uint8_t *ptr, unsigned short *size)
+__nfweak uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
 {
     (void)(ptr);
     (void)(size);
@@ -26,7 +26,7 @@ __nfweak int WP_ReceiveBytes(uint8_t *ptr, unsigned short *size)
 }
 
 // provided as weak to be replaced by actual implementation by HAL interface
-__nfweak int WP_TransmitMessage(WP_Message *message)
+__nfweak uint8_t WP_TransmitMessage(WP_Message *message)
 {
     (void)(message);