microbit: Minor bug fixes and refinements

- Add maximum depth for event queues, to prevent buggy scripts causing total memory exhaustion.
  - Suppress generation of A/B click events when A+B click is generated
  - preservation of event ordering on messagebus for resursive event generation cases.
  - bugfix of message bus processing to prevent occasional dual processing of events
  - bugfix MicroBitDisplay to behave correctly when delay parameter is zero.

inc/MicroBitButton.h

@@ -31,6 +31,13 @@
 #define MICROBIT_BUTTON_SIGMA_THRESH_LO         2
 #define MICROBIT_BUTTON_DOUBLE_CLICK_THRESH     50
 
+enum MicroBitButtonEventConfiguration
+{
+    MICROBIT_BUTTON_SIMPLE_EVENTS,
+    MICROBIT_BUTTON_ALL_EVENTS
+};
+
+
 /**
   * Class definition for MicroBit Button.
   *
@@ -38,12 +45,12 @@
   */
 class MicroBitButton : public MicroBitComponent
 {
-    PinName name;                   // mBed pin name of this pin.
-    DigitalIn pin;                  // The mBed object looking after this pin at any point in time (may change!).
+    PinName name;                                           // mbed pin name of this pin.
+    DigitalIn pin;                                          // The mbed object looking after this pin at any point in time (may change!).
     
-    unsigned long downStartTime;    // used to store the current system clock when a button down event occurs
-    uint8_t sigma;                  // integration of samples over time.
-    uint8_t doubleClickTimer;       // double click timer (ticks).
+    unsigned long downStartTime;                            // used to store the current system clock when a button down event occurs
+    uint8_t sigma;                                          // integration of samples over time. We use this for debouncing, and noise tolerance for touch sensing
+    MicroBitButtonEventConfiguration eventConfiguration;    // Do we want to generate high level event (clicks), or defer this to another service.
     
     public:
 
@@ -69,7 +76,7 @@ class MicroBitButton : public MicroBitComponent
       * MICROBIT_BUTTON_EVT_HOLD
       * @endcode
       */
-    MicroBitButton(uint16_t id, PinName name, PinMode mode = PullNone);
+    MicroBitButton(uint16_t id, PinName name, MicroBitButtonEventConfiguration eventConfiguration = MICROBIT_BUTTON_ALL_EVENTS, PinMode mode = PullNone);
     
     /**
       * Tests if this Button is currently pressed.

inc/MicroBitConfig.h

@@ -91,6 +91,13 @@
 #define MESSAGE_BUS_LISTENER_DEFAULT_FLAGS          MESSAGE_BUS_LISTENER_QUEUE_IF_BUSY
 #endif
 
+//
+// Maximum event queue depth. If a queue exceeds this depth, further events will be dropped.
+// Used to prevent message queues growing uncontrollably due to badly behaved user code and causing panic conditions.
+// 
+#ifndef MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH
+#define MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH        20
+#endif
 //
 // Core micro:bit services
 //

inc/MicroBitMessageBus.h

@@ -64,10 +64,11 @@ class MicroBitMessageBus : public MicroBitComponent
       * or the constructors provided by MicroBitEvent.
 	  *
 	  * @param evt The event to send. 
-      * @param mask The type of listeners to process (optional). Matches MicroBitListener flags. If not defined, all standard listeners will be processed.
-      * @return The 1 if all matching listeners were processed, 0 if further processing is required.
+      * @param urgent The type of listeners to process (optional). If set to true, only listeners defined as urgent and non-blocking will be processed
+      * otherwise, all other (standard) listeners will be processed.
+      * @return 1 if all matching listeners were processed, 0 if further processing is required.
 	  */
-	int process(MicroBitEvent &evt, uint32_t mask = MESSAGE_BUS_LISTENER_REENTRANT | MESSAGE_BUS_LISTENER_QUEUE_IF_BUSY |  MESSAGE_BUS_LISTENER_DROP_IF_BUSY |  MESSAGE_BUS_LISTENER_NONBLOCKING);
+	int process(MicroBitEvent &evt, bool urgent = false);
 
 	/**
 	  * Register a listener function.
@@ -232,6 +233,7 @@ class MicroBitMessageBus : public MicroBitComponent
     MicroBitEventQueueItem      *evt_queue_head;    // Head of queued events to be processed.
     MicroBitEventQueueItem      *evt_queue_tail;    // Tail of queued events to be processed.
     uint16_t                    nonce_val;          // The last nonce issued.
+    uint16_t                    queueLength;        // The number of events currently waiting to be processed.
             
     void queueEvent(MicroBitEvent &evt);
     MicroBitEventQueueItem* dequeueEvent();

inc/MicroBitMultiButton.h

@@ -3,10 +3,12 @@
 
 #include "MicroBit.h"
 
-#define MICROBIT_MULTI_BUTTON_STATE_1               1
-#define MICROBIT_MULTI_BUTTON_STATE_2               2
-#define MICROBIT_MULTI_BUTTON_HOLD_TRIGGERED_1      4
-#define MICROBIT_MULTI_BUTTON_HOLD_TRIGGERED_2      8
+#define MICROBIT_MULTI_BUTTON_STATE_1               0x01
+#define MICROBIT_MULTI_BUTTON_STATE_2               0x02
+#define MICROBIT_MULTI_BUTTON_HOLD_TRIGGERED_1      0x04
+#define MICROBIT_MULTI_BUTTON_HOLD_TRIGGERED_2      0x08
+#define MICROBIT_MULTI_BUTTON_SUPRESSED_1           0X10 
+#define MICROBIT_MULTI_BUTTON_SUPRESSED_2           0x20
 
 /**
   * Class definition for MicroBitMultiButton.
@@ -22,8 +24,10 @@ class MicroBitMultiButton : public MicroBitComponent
     uint16_t    otherSubButton(uint16_t b);
     int         isSubButtonPressed(uint16_t button);
     int         isSubButtonHeld(uint16_t button);
+    int         isSubButtonSupressed(uint16_t button);
     void        setButtonState(uint16_t button, int value);
     void        setHoldState(uint16_t button, int value);
+    void        setSupressedState(uint16_t button, int value);
 
     public:
 

source/MicroBit.cpp

@@ -63,8 +63,8 @@ MicroBit::MicroBit() :
 #endif    
     MessageBus(),
     display(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_WIDTH, MICROBIT_DISPLAY_HEIGHT),
-    buttonA(MICROBIT_ID_BUTTON_A,MICROBIT_PIN_BUTTON_A),
-    buttonB(MICROBIT_ID_BUTTON_B,MICROBIT_PIN_BUTTON_B),
+    buttonA(MICROBIT_ID_BUTTON_A,MICROBIT_PIN_BUTTON_A, MICROBIT_BUTTON_SIMPLE_EVENTS),
+    buttonB(MICROBIT_ID_BUTTON_B,MICROBIT_PIN_BUTTON_B, MICROBIT_BUTTON_SIMPLE_EVENTS),
     buttonAB(MICROBIT_ID_BUTTON_AB,MICROBIT_ID_BUTTON_A,MICROBIT_ID_BUTTON_B), 
     accelerometer(MICROBIT_ID_ACCELEROMETER, MMA8653_DEFAULT_ADDR),
     compass(MICROBIT_ID_COMPASS, MAG3110_DEFAULT_ADDR),

source/MicroBitButton.cpp

@@ -18,17 +18,16 @@
   * MICROBIT_BUTTON_EVT_UP
   * MICROBIT_BUTTON_EVT_CLICK
   * MICROBIT_BUTTON_EVT_LONG_CLICK
-  * MICROBIT_BUTTON_EVT_DOUBLE_CLICK
   * MICROBIT_BUTTON_EVT_HOLD
   * @endcode
   */
-MicroBitButton::MicroBitButton(uint16_t id, PinName name, PinMode mode) : pin(name, mode)
+MicroBitButton::MicroBitButton(uint16_t id, PinName name, MicroBitButtonEventConfiguration eventConfiguration, PinMode mode) : pin(name, mode)
 {
     this->id = id;
     this->name = name;
+    this->eventConfiguration = eventConfiguration;
     this->downStartTime = 0;
     this->sigma = 0;
-    this->doubleClickTimer = 0;
     uBit.addSystemComponent(this);
 }
 
@@ -71,12 +70,15 @@ void MicroBitButton::systemTick()
     {
         status = 0;
         MicroBitEvent evt(id,MICROBIT_BUTTON_EVT_UP);
-    
-        //determine if this is a long click or a normal click and send event
-        if((ticks - downStartTime) >= MICROBIT_BUTTON_LONG_CLICK_TIME)
-            MicroBitEvent evt(id,MICROBIT_BUTTON_EVT_LONG_CLICK);    
-        else
-            MicroBitEvent evt(id,MICROBIT_BUTTON_EVT_CLICK);
+   
+       if (eventConfiguration == MICROBIT_BUTTON_ALL_EVENTS)
+       { 
+           //determine if this is a long click or a normal click and send event
+           if((ticks - downStartTime) >= MICROBIT_BUTTON_LONG_CLICK_TIME)
+               MicroBitEvent evt(id,MICROBIT_BUTTON_EVT_LONG_CLICK);    
+           else
+               MicroBitEvent evt(id,MICROBIT_BUTTON_EVT_CLICK);
+       }
     }
 
     //if button is pressed and the hold triggered event state is not triggered AND we are greater than the button debounce value

source/MicroBitDisplay.cpp

@@ -478,7 +478,8 @@ void MicroBitDisplay::print(char c, int delay)
     if (animationMode == ANIMATION_MODE_NONE)
     {
         this->printAsync(c, delay);
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 
@@ -509,7 +510,9 @@ void MicroBitDisplay::print(ManagedString s, int delay)
     if (animationMode == ANIMATION_MODE_NONE)
     {
         this->printAsync(s, delay);
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        //TODO: Put this in when we merge tight-validation
+        //if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 
@@ -537,7 +540,8 @@ void MicroBitDisplay::print(MicroBitImage i, int x, int y, int alpha, int delay)
     if (animationMode == ANIMATION_MODE_NONE)
     {
         this->printAsync(i, x, y, alpha, delay);
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 
@@ -632,7 +636,9 @@ void MicroBitDisplay::scroll(ManagedString s, int delay)
         this->scrollAsync(s, delay);
 
         // Wait for completion.
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        //TODO: Put this in when we merge tight-validation
+        //if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 
@@ -663,7 +669,9 @@ void MicroBitDisplay::scroll(MicroBitImage image, int delay, int stride)
         this->scrollAsync(image, delay, stride);
 
         // Wait for completion.
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        //TODO: Put this in when we merge tight-validation
+        //if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 
@@ -740,7 +748,9 @@ void MicroBitDisplay::animate(MicroBitImage image, int delay, int stride, int st
         this->animateAsync(image, delay, stride, startingPosition);
 
         // Wait for completion.
-        fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
+        //TODO: Put this in when we merge tight-validation
+        //if (delay > 0)
+            fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE);
     }
 }
 

source/MicroBitListener.cpp

@@ -60,16 +60,23 @@ MicroBitListener::~MicroBitListener()
   */
 void MicroBitListener::queue(MicroBitEvent e)
 {
-    MicroBitEventQueueItem *q = new MicroBitEventQueueItem(e);
+    int queueDepth;
+
     MicroBitEventQueueItem *p = evt_queue;
 
     if (evt_queue == NULL)
-        evt_queue = q;
+        evt_queue = new MicroBitEventQueueItem(e);
     else
     {
+        queueDepth = 1;
+
         while (p->next != NULL)
+        {
             p = p->next;
+            queueDepth++;
+        }
 
-        p->next = q;
+        if (queueDepth < MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH) 
+            p->next = new MicroBitEventQueueItem(e);
     }
 }

source/MicroBitMessageBus.cpp

@@ -15,6 +15,7 @@ MicroBitMessageBus::MicroBitMessageBus()
 	this->listeners = NULL;
     this->evt_queue_head = NULL;
     this->evt_queue_tail = NULL;
+    this->queueLength = 0;
 }
 
 /**
@@ -86,7 +87,6 @@ void async_callback(void *param)
     listener->flags &= ~MESSAGE_BUS_LISTENER_BUSY;
 }
 
-
 /**
   * Queue the given event for processing at a later time.
   * Add the given event at the tail of our queue.
@@ -97,25 +97,55 @@ void MicroBitMessageBus::queueEvent(MicroBitEvent &evt)
 {
     int processingComplete;
 
-    // Firstly, process all handler regsitered as URGENT. These pre-empt the queue, and are useful for fast, high priority services.
-    processingComplete = this->process(evt, MESSAGE_BUS_LISTENER_URGENT);
+    if (queueLength >= MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH)
+        return;
+
+    MicroBitEventQueueItem *item = new MicroBitEventQueueItem(evt);
+    MicroBitEventQueueItem *prev;
+
+    // Queue this event. We always do this to maintain event ordering...
+    // It costs a a little time and memory, but is worth it.
+    __disable_irq();
+
+    prev = evt_queue_tail;
 
-    if (!processingComplete)
+    if (evt_queue_tail == NULL)
     {
-        // We need to queue this event for later processing...
-        MicroBitEventQueueItem *item = new MicroBitEventQueueItem(evt);
+        evt_queue_head = evt_queue_tail = item;
+    }
+    else 
+    {
+        evt_queue_tail->next = item;
+        evt_queue_tail = item;
+    }
 
-        __disable_irq();
+    queueLength++;
+
+    __enable_irq();
 
-        if (evt_queue_tail == NULL)
-            evt_queue_head = evt_queue_tail = item;
-        else 
-            evt_queue_tail->next = item;
+    // Now process all handler regsitered as URGENT. 
+    // These pre-empt the queue, and are useful for fast, high priority services.
+    processingComplete = this->process(evt, true);
 
-        __enable_irq();
+    if (processingComplete)
+    {
+        // No more processing is required... drop the event from the list to avoid the need for processing later.
+        if (evt_queue_head == item)
+            evt_queue_head = item->next;
+
+        if (evt_queue_tail == item)
+            evt_queue_tail = prev;
+
+        if (prev)
+            prev->next = item->next;
+
+        queueLength--;
+
+        delete item;
     }
 }
 
+
 /**
   * Extract the next event from the front of the event queue (if present).
   * @return 
@@ -135,10 +165,13 @@ MicroBitEventQueueItem* MicroBitMessageBus::dequeueEvent()
 
         if (evt_queue_head == NULL)
             evt_queue_tail = NULL;
+
+        queueLength--;
     }
     
     __enable_irq();
 
+
     return item;
 }
 
@@ -254,20 +287,23 @@ void MicroBitMessageBus::send(MicroBitEvent evt)
  * This will attempt to call the event handler directly, but spawn a fiber should that
  * event handler attempt a blocking operation.
  * @param evt The event to be delivered.
- * @param mask The type of listeners to process (optional). Matches MicroBitListener flags. If not defined, all standard listeners will be processed.
- * @return The 1 if all matching listeners were processed, 0 if further processing is required.
+ * @param urgent The type of listeners to process (optional). If set to true, only listeners defined as urgent and non-blocking will be processed
+ * otherwise, all other (standard) listeners will be processed.
+ * @return 1 if all matching listeners were processed, 0 if further processing is required.
  */
-int MicroBitMessageBus::process(MicroBitEvent &evt, uint32_t mask)
+int MicroBitMessageBus::process(MicroBitEvent &evt, bool urgent)
 {
 	MicroBitListener *l;
     int complete = 1;
+    bool listenerUrgent;
 
     l = listeners;
     while (l != NULL)
     {
 	    if((l->id == evt.source || l->id == MICROBIT_ID_ANY) && (l->value == evt.value || l->value == MICROBIT_EVT_ANY))
         {
-            if(l->flags & mask && !(l->flags & MESSAGE_BUS_LISTENER_DELETING))
+            listenerUrgent = (l->flags & MESSAGE_BUS_LISTENER_IMMEDIATE) == MESSAGE_BUS_LISTENER_IMMEDIATE;
+            if(listenerUrgent == urgent && !(l->flags & MESSAGE_BUS_LISTENER_DELETING))
             {
                 l->evt = evt;
 

source/MicroBitMultiButton.cpp

@@ -66,6 +66,16 @@ int MicroBitMultiButton::isSubButtonHeld(uint16_t button)
     return 0;
 }
 
+int MicroBitMultiButton::isSubButtonSupressed(uint16_t button)
+{
+    if (button == button1)
+        return status & MICROBIT_MULTI_BUTTON_SUPRESSED_1;
+        
+    if (button == button2)
+        return status & MICROBIT_MULTI_BUTTON_SUPRESSED_2;
+        
+    return 0;
+}
 
 void MicroBitMultiButton::setButtonState(uint16_t button, int value)
 {
@@ -105,6 +115,26 @@ void MicroBitMultiButton::setHoldState(uint16_t button, int value)
     }
 }
 
+void MicroBitMultiButton::setSupressedState(uint16_t button, int value)
+{
+    if (button == button1)
+    {
+        if (value)
+            status |= MICROBIT_MULTI_BUTTON_SUPRESSED_1;
+        else
+            status &= ~MICROBIT_MULTI_BUTTON_SUPRESSED_1;
+    }
+        
+    if (button == button2)
+    {
+        if (value)
+            status |= MICROBIT_MULTI_BUTTON_SUPRESSED_2;
+        else
+            status &= ~MICROBIT_MULTI_BUTTON_SUPRESSED_2;
+    }
+}
+
+
 void MicroBitMultiButton::onEvent(MicroBitEvent evt)
 {
     int button = evt.source;
@@ -118,30 +148,40 @@ void MicroBitMultiButton::onEvent(MicroBitEvent evt)
                 MicroBitEvent e(id, MICROBIT_BUTTON_EVT_DOWN);
                 
         break;
+
+        case MICROBIT_BUTTON_EVT_HOLD:
+            setHoldState(button, 1);
+            if(isSubButtonHeld(otherButton))
+                MicroBitEvent e(id, MICROBIT_BUTTON_EVT_HOLD);
+            
+        break;
         
         case MICROBIT_BUTTON_EVT_UP:
-            setButtonState(button, 0);
-            setHoldState(button, 0);
-            
             if(isSubButtonPressed(otherButton))
+            {
                 MicroBitEvent e(id, MICROBIT_BUTTON_EVT_UP);
-        break;
-        
-        case MICROBIT_BUTTON_EVT_CLICK:
-        case MICROBIT_BUTTON_EVT_LONG_CLICK:
+                
+                if (isSubButtonHeld(button) && isSubButtonHeld(otherButton))
+                    MicroBitEvent e(id, MICROBIT_BUTTON_EVT_LONG_CLICK);
+                else
+                    MicroBitEvent e(id, MICROBIT_BUTTON_EVT_CLICK);
+
+                setSupressedState(otherButton, 1);
+            }
+            else if (!isSubButtonSupressed(button))
+            {
+                if (isSubButtonHeld(button))
+                    MicroBitEvent e(button, MICROBIT_BUTTON_EVT_LONG_CLICK);
+                else
+                    MicroBitEvent e(button, MICROBIT_BUTTON_EVT_CLICK);
+            }
+
             setButtonState(button, 0);
             setHoldState(button, 0);
-            if(isSubButtonPressed(otherButton))
-                MicroBitEvent e(id, evt.value);
-            
-        break;
-        
-        case MICROBIT_BUTTON_EVT_HOLD:
-            setHoldState(button, 1);
-            if(isSubButtonHeld(otherButton))
-                MicroBitEvent e(id, MICROBIT_BUTTON_EVT_HOLD);
-            
+            setSupressedState(button, 0);
+
         break;
+
     }
 }