From aca544677ebac90dbf98db6a7a365586d2487c78 Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Sat, 17 Oct 2015 20:35:16 +0100 Subject: [PATCH 1/7] microbit: Updates to enable queing of display animation calls Updates to change the behaviour of the scroll/print/animate faily of function away from being pre-emtive and instead prroviding queing behaviour. Minor updates to provide complete sets of async equivalent operations Updates to the scheduler to provide wait/notify/waitone semantics. --- inc/MicroBitComponent.h | 4 +- inc/MicroBitConfig.h | 2 +- inc/MicroBitDisplay.h | 47 ++++- inc/MicroBitMessageBus.h | 5 - source/MicroBitDFUService.cpp | 4 +- source/MicroBitDisplay.cpp | 345 +++++++++++++++++++++------------- source/MicroBitFiber.cpp | 14 +- source/MicroBitMessageBus.cpp | 15 +- 8 files changed, 281 insertions(+), 155 deletions(-) diff --git a/inc/MicroBitComponent.h b/inc/MicroBitComponent.h index 7779069..ed184cb 100644 --- a/inc/MicroBitComponent.h +++ b/inc/MicroBitComponent.h @@ -41,7 +41,9 @@ #define MICROBIT_ID_IO_P20 25 //SDA #define MICROBIT_ID_BUTTON_AB 26 // Button A+B multibutton -#define MICROBIT_ID_ALERT 27 // Alert channel, used for general purpose condition synchronisation and alerting. + +#define MICROBIT_ID_NOTIFY 1023 // Notfication channel, for general purpose synchronisation +#define MICROBIT_ID_NOTIFY_ONE 1022 // Notfication channel, for general purpose synchronisation class MicroBitComponent { diff --git a/inc/MicroBitConfig.h b/inc/MicroBitConfig.h index 235b90e..a01d2ec 100644 --- a/inc/MicroBitConfig.h +++ b/inc/MicroBitConfig.h @@ -88,7 +88,7 @@ // MESSAGE_BUS_LISTENER_NONBLOCKING #ifndef MESSAGE_BUS_LISTENER_DEFAULT_FLAGS -#define MESSAGE_BUS_LISTENER_DEFAULT_FLAGS MESSAGE_BUS_LISTENER_REENTRANT +#define MESSAGE_BUS_LISTENER_DEFAULT_FLAGS MESSAGE_BUS_LISTENER_QUEUE_IF_BUSY #endif // diff --git a/inc/MicroBitDisplay.h b/inc/MicroBitDisplay.h index b499411..50e80c9 100644 --- a/inc/MicroBitDisplay.h +++ b/inc/MicroBitDisplay.h @@ -19,6 +19,7 @@ * MessageBus Event Codes */ #define MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE 1 +#define MICROBIT_DISPLAY_EVT_FREE 2 /** * I/O configurations for common devices. @@ -70,6 +71,7 @@ enum AnimationMode { ANIMATION_MODE_NONE, + ANIMATION_MODE_STOPPED, ANIMATION_MODE_SCROLL_TEXT, ANIMATION_MODE_PRINT_TEXT, ANIMATION_MODE_SCROLL_IMAGE, @@ -103,7 +105,6 @@ class MicroBitDisplay : public MicroBitComponent uint8_t mode; uint8_t greyscaleBitMsk; uint8_t timingCount; - uint16_t nonce; Timeout renderTimer; MicroBitFont font; @@ -221,6 +222,11 @@ class MicroBitDisplay : public MicroBitComponent */ void sendAnimationCompleteEvent(); + /** + * Blocks the current fiber until the display is available (i.e. not effect is being displayed). + * Animations are queued until their time to display. + */ + void waitForFreeDisplay(); public: // The mutable bitmap buffer being rendered to the LED matrix. @@ -242,16 +248,29 @@ public: MicroBitDisplay(uint16_t id, uint8_t x, uint8_t y); /** - * Resets the current given animation. - * @param delay the delay after which the animation is reset. + * Stops any currently running animation, and any that are waiting to be displayed. */ - void resetAnimation(uint16_t delay); + void stopAnimation(); /** * Frame update method, invoked periodically to strobe the display. */ virtual void systemTick(); + /** + * Prints the given character to the display, if it is not in use. + * + * @param c The character to display. + * @param d Optional parameter - the time for which to show the character. Zero displays the character forever. + * + * Example: + * @code + * uBit.display.printAsync('p'); + * uBit.display.printAsync('p',100); + * @endcode + */ + void printAsync(char c, int delay = 0); + /** * Prints the given string to the display, one character at a time. * Uses the given delay between characters. @@ -267,6 +286,24 @@ public: */ void printAsync(ManagedString s, int delay = MICROBIT_DEFAULT_PRINT_SPEED); + /** + * Prints the given image to the display, if the display is not in use. + * Returns immediately, and executes the animation asynchronously. + * + * @param i The image to display. + * @param x The horizontal position on the screen to display the image (default 0) + * @param y The vertical position on the screen to display the image (default 0) + * @param alpha Treats the brightness level '0' as transparent (default 0) + * @param delay The time to delay between characters, in timer ticks. + * + * Example: + * @code + * MicrobitImage i("1,1,1,1,1\n1,1,1,1,1\n"); + * uBit.display.print(i,400); + * @endcode + */ + void printAsync(MicroBitImage i, int x, int y, int alpha, int delay = 0); + /** * Prints the given character to the display. * @@ -307,7 +344,7 @@ public: * uBit.display.print(i,400); * @endcode */ - void print(MicroBitImage i, int x, int y, int alpha, int delay = MICROBIT_DEFAULT_PRINT_SPEED); + void print(MicroBitImage i, int x, int y, int alpha, int delay = 0); /** * Scrolls the given string to the display, from right to left. diff --git a/inc/MicroBitMessageBus.h b/inc/MicroBitMessageBus.h index a31e7fb..072ff89 100644 --- a/inc/MicroBitMessageBus.h +++ b/inc/MicroBitMessageBus.h @@ -203,11 +203,6 @@ class MicroBitMessageBus : public MicroBitComponent template void ignore(uint16_t id, uint16_t value, T* object, void (T::*handler)(MicroBitEvent)); - /** - * Returns a 'nonce' for use with the NONCE_ID channel of the message bus. - */ - uint16_t nonce(); - private: /** diff --git a/source/MicroBitDFUService.cpp b/source/MicroBitDFUService.cpp index 702249f..cfa3462 100644 --- a/source/MicroBitDFUService.cpp +++ b/source/MicroBitDFUService.cpp @@ -191,7 +191,7 @@ void MicroBitDFUService::onDataWritten(const GattWriteCallbackParams *params) */ void MicroBitDFUService::showTick() { - uBit.display.resetAnimation(0); + uBit.display.stopAnimation(); uBit.display.image.setPixelValue(0,3, 255); uBit.display.image.setPixelValue(1,4, 255); @@ -206,7 +206,7 @@ void MicroBitDFUService::showTick() */ void MicroBitDFUService::showNameHistogram() { - uBit.display.resetAnimation(0); + uBit.display.stopAnimation(); uint32_t n = NRF_FICR->DEVICEID[1]; int ld = 1; diff --git a/source/MicroBitDisplay.cpp b/source/MicroBitDisplay.cpp index 4a27b26..01dd54c 100644 --- a/source/MicroBitDisplay.cpp +++ b/source/MicroBitDisplay.cpp @@ -234,10 +234,10 @@ MicroBitDisplay::animationUpdate() void MicroBitDisplay::sendAnimationCompleteEvent() { // Signal that we've completed an animation. - MicroBitEvent evt1(id,MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + MicroBitEvent(id,MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); - // Wake up any fibers that were blocked on the animation (if any). - MicroBitEvent evt2(MICROBIT_ID_ALERT, nonce); + // Wake up a fiber that was blocked on the animation (if any). + MicroBitEvent(MICROBIT_ID_NOTIFY_ONE, MICROBIT_DISPLAY_EVT_FREE); } /** @@ -304,7 +304,6 @@ void MicroBitDisplay::updateScrollImage() scrollingImageRendered = true; } - /** * Internal animateImage update method. * Paste the stored bitmap at the appropriate point and stop on the last frame. @@ -333,28 +332,70 @@ void MicroBitDisplay::updateAnimateImage() * Resets the current given animation. * @param delay the delay after which the animation is reset. */ -void MicroBitDisplay::resetAnimation(uint16_t delay) +void MicroBitDisplay::stopAnimation() { - //sanitise this value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - // Reset any ongoing animation. if (animationMode != ANIMATION_MODE_NONE) { animationMode = ANIMATION_MODE_NONE; - this->sendAnimationCompleteEvent(); + + // Indicate that we've completed an animation. + MicroBitEvent(id,MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + + // Wake up aall fibers that may blocked on the animation (if any). + MicroBitEvent(MICROBIT_ID_NOTIFY, MICROBIT_DISPLAY_EVT_FREE); } // Clear the display and setup the animation timers. this->image.clear(); - this->animationDelay = delay; - this->animationTick = delay-1; } /** - * Prints the given string to the display, one character at a time. - * Uses the given delay between characters. + * Blocks the current fiber until the display is available (i.e. not effect is being displayed). + * Animations are queued until their time to display. + * + */ +void MicroBitDisplay::waitForFreeDisplay() +{ + // If there's an ongoing animation, wait for our turn to display. + if (animationMode != ANIMATION_MODE_NONE && animationMode != ANIMATION_MODE_STOPPED) + fiber_wait_for_event(MICROBIT_ID_NOTIFY, MICROBIT_DISPLAY_EVT_FREE); +} + + +/** + * Prints the given character to the display, if it is not in use. + * + * @param c The character to display. + * @param d Optional parameter - the time for which to show the character. Zero displays the character forever. + * + * Example: + * @code + * uBit.display.printAsync('p'); + * uBit.display.printAsync('p',100); + * @endcode + */ +void MicroBitDisplay::printAsync(char c, int delay) +{ + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) + { + image.print(c, 0, 0); + + if(delay <= 0) + return; + + animationDelay = delay; + animationTick = 0; + animationMode = ANIMATION_MODE_PRINT_CHARACTER; + } +} + +/** + * Prints the given string to the display, one character at a time, if the display is not in use. * Returns immediately, and executes the animation asynchronously. * * @param s The string to display. @@ -367,41 +408,77 @@ void MicroBitDisplay::resetAnimation(uint16_t delay) */ void MicroBitDisplay::printAsync(ManagedString s, int delay) { - //sanitise this value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - this->resetAnimation(delay); - - this->printingChar = 0; - this->printingText = s; - - animationMode = ANIMATION_MODE_PRINT_TEXT; + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) + { + //sanitise this value + if(delay <= 0 ) + delay = MICROBIT_DEFAULT_SCROLL_SPEED; + + printingChar = 0; + printingText = s; + animationDelay = delay; + animationTick = 0; + + animationMode = ANIMATION_MODE_PRINT_TEXT; + } } /** - * Prints the given character to the display. + * Prints the given image to the display, if the display is not in use. + * Returns immediately, and executes the animation asynchronously. + * + * @param i The image to display. + * @param x The horizontal position on the screen to display the image (default 0) + * @param y The vertical position on the screen to display the image (default 0) + * @param alpha Treats the brightness level '0' as transparent (default 0) + * @param delay The time to delay between characters, in timer ticks. + * + * Example: + * @code + * MicrobitImage i("1,1,1,1,1\n1,1,1,1,1\n"); + * uBit.display.print(i,400); + * @endcode + */ +void MicroBitDisplay::printAsync(MicroBitImage i, int x, int y, int alpha, int delay) +{ + // If the display is free, it's our turn to display. + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) + { + image.paste(i, x, y, alpha); + + if(delay <= 0) + return; + + animationDelay = delay; + animationTick = 0; + + animationMode = ANIMATION_MODE_PRINT_CHARACTER; + } +} + +/** + * Prints the given character to the display, and wait for it to complete. * * @param c The character to display. * * Example: * @code * uBit.display.print('p'); + * uBit.display.print('p',100); * @endcode */ void MicroBitDisplay::print(char c, int delay) { - image.print(c, 0, 0); - - if(delay <= 0) - return; - - this->animationDelay = delay; - animationMode = ANIMATION_MODE_PRINT_CHARACTER; - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + this->printAsync(c, delay); + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } /** @@ -423,12 +500,16 @@ void MicroBitDisplay::print(ManagedString s, int delay) if(delay <= 0 ) delay = MICROBIT_DEFAULT_SCROLL_SPEED; - // Start the effect. - this->printAsync(s, delay); - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + this->printAsync(s, delay); + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } /** @@ -446,17 +527,17 @@ void MicroBitDisplay::print(ManagedString s, int delay) */ void MicroBitDisplay::print(MicroBitImage i, int x, int y, int alpha, int delay) { - image.paste(i, x, y, alpha); - - if(delay <= 0) - return; - - this->animationDelay = delay; - animationMode = ANIMATION_MODE_PRINT_CHARACTER; - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + this->printAsync(i, x, y, alpha, delay); + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } /** @@ -474,17 +555,19 @@ void MicroBitDisplay::print(MicroBitImage i, int x, int y, int alpha, int delay) */ void MicroBitDisplay::scrollAsync(ManagedString s, int delay) { - //sanitise this value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - this->resetAnimation(delay); - - this->scrollingPosition = width-1; - this->scrollingChar = 0; - this->scrollingText = s; - - animationMode = ANIMATION_MODE_SCROLL_TEXT; + // If the display is free, it's our turn to display. + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) + { + //sanitise the delay parameter + if(delay <= 0 ) + delay = MICROBIT_DEFAULT_SCROLL_SPEED; + + scrollingPosition = width-1; + scrollingChar = 0; + scrollingText = s; + + animationMode = ANIMATION_MODE_SCROLL_TEXT; + } } /** @@ -502,18 +585,20 @@ void MicroBitDisplay::scrollAsync(ManagedString s, int delay) */ void MicroBitDisplay::scrollAsync(MicroBitImage image, int delay, int stride) { - //sanitise the delay value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - this->resetAnimation(delay); + // If the display is free, it's our turn to display. + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) + { + //sanitise the delay value + if(delay <= 0 ) + delay = MICROBIT_DEFAULT_SCROLL_SPEED; - this->scrollingImagePosition = stride < 0 ? width : -image.getWidth(); - this->scrollingImageStride = stride; - this->scrollingImage = image; - this->scrollingImageRendered = false; - - animationMode = ANIMATION_MODE_SCROLL_IMAGE; + this->scrollingImagePosition = stride < 0 ? width : -image.getWidth(); + this->scrollingImageStride = stride; + this->scrollingImage = image; + this->scrollingImageRendered = false; + + animationMode = ANIMATION_MODE_SCROLL_IMAGE; + } } /** @@ -531,16 +616,19 @@ void MicroBitDisplay::scrollAsync(MicroBitImage image, int delay, int stride) */ void MicroBitDisplay::scroll(ManagedString s, int delay) { - //sanitise this value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - // Start the effect. - this->scrollAsync(s, delay); - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + // Start the effect. + this->scrollAsync(s, delay); + + // Wait for completion. + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } /** @@ -559,16 +647,19 @@ void MicroBitDisplay::scroll(ManagedString s, int delay) */ void MicroBitDisplay::scroll(MicroBitImage image, int delay, int stride) { - //sanitise the delay value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - // Start the effect. - this->scrollAsync(image, delay, stride); - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + // Start the effect. + this->scrollAsync(image, delay, stride); + + // Wait for completion. + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } /** @@ -591,30 +682,27 @@ void MicroBitDisplay::scroll(MicroBitImage image, int delay, int stride) */ void MicroBitDisplay::animateAsync(MicroBitImage image, int delay, int stride, int startingPosition) { - // Assume right to left functionality, to align with scrollString() - stride = -stride; - - //sanitise the delay value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - // Reset any ongoing animation. - if (animationMode != ANIMATION_MODE_NONE) + // If the display is free, it's our turn to display. + if (animationMode == ANIMATION_MODE_NONE || animationMode == ANIMATION_MODE_STOPPED) { - animationMode = ANIMATION_MODE_NONE; - this->sendAnimationCompleteEvent(); - } - - this->animationDelay = delay; - this->animationTick = delay-1; + // Assume right to left functionality, to align with scrollString() + stride = -stride; - //calculate starting position which is offset by the stride - this->scrollingImagePosition = (startingPosition == MICROBIT_DISPLAY_ANIMATE_DEFAULT_POS)?MICROBIT_DISPLAY_WIDTH + stride:startingPosition; - this->scrollingImageStride = stride; - this->scrollingImage = image; - this->scrollingImageRendered = false; - - animationMode = ANIMATION_MODE_ANIMATE_IMAGE; + //sanitise the delay value + if(delay <= 0 ) + delay = MICROBIT_DEFAULT_SCROLL_SPEED; + + animationDelay = delay; + animationTick = delay-1; + + //calculate starting position which is offset by the stride + scrollingImagePosition = (startingPosition == MICROBIT_DISPLAY_ANIMATE_DEFAULT_POS) ? MICROBIT_DISPLAY_WIDTH + stride : startingPosition; + scrollingImageStride = stride; + scrollingImage = image; + scrollingImageRendered = false; + + animationMode = ANIMATION_MODE_ANIMATE_IMAGE; + } } /** @@ -637,16 +725,19 @@ void MicroBitDisplay::animateAsync(MicroBitImage image, int delay, int stride, i */ void MicroBitDisplay::animate(MicroBitImage image, int delay, int stride, int startingPosition) { - //sanitise the delay value - if(delay <= 0 ) - delay = MICROBIT_DEFAULT_SCROLL_SPEED; - - // Start the effect. - this->animateAsync(image, delay, stride, startingPosition); - - // Wait for completion. - nonce = uBit.MessageBus.nonce(); - fiber_wait_for_event(MICROBIT_ID_ALERT, nonce); + // If there's an ongoing animation, wait for our turn to display. + this->waitForFreeDisplay(); + + // If the display is free, it's our turn to display. + // If someone called stopAnimation(), then we simply skip... + if (animationMode == ANIMATION_MODE_NONE) + { + // Start the effect. + this->animateAsync(image, delay, stride, startingPosition); + + // Wait for completion. + fiber_wait_for_event(MICROBIT_ID_DISPLAY, MICROBIT_DISPLAY_EVT_ANIMATION_COMPLETE); + } } diff --git a/source/MicroBitFiber.cpp b/source/MicroBitFiber.cpp index be6e3a5..b46b581 100644 --- a/source/MicroBitFiber.cpp +++ b/source/MicroBitFiber.cpp @@ -15,7 +15,7 @@ */ Fiber *currentFiber = NULL; // The context in which the current fiber is executing. Fiber *forkedFiber = NULL; // The context in which a newly created child fiber is executing. -Fiber *idleFiber = NULL; // IDLE task - performs a power efficient sleep, and system maintenance tasks. +Fiber *idleFiber = NULL; // IDLE task - performs a power efficient sleep, and system maintenance tasks. /* * Scheduler state. @@ -205,6 +205,7 @@ void scheduler_event(MicroBitEvent evt) { Fiber *f = waitQueue; Fiber *t; + int notifyOneComplete = 0; // Check the wait queue, and wake up any fibers as necessary. while (f != NULL) @@ -214,7 +215,16 @@ void scheduler_event(MicroBitEvent evt) // extract the event data this fiber is blocked on. uint16_t id = f->context & 0xFFFF; uint16_t value = (f->context & 0xFFFF0000) >> 16; - + + // Special case for the NOTIFY_ONE channel... + if ((notifyOneComplete == 0) && (id == MICROBIT_ID_NOTIFY && evt.source == MICROBIT_ID_NOTIFY_ONE) && (value == MICROBIT_EVT_ANY || value == evt.value)) + { + // Wakey wakey! + dequeue_fiber(f); + queue_fiber(f,&runQueue); + notifyOneComplete = 1; + } + if ((id == MICROBIT_ID_ANY || id == evt.source) && (value == MICROBIT_EVT_ANY || value == evt.value)) { // Wakey wakey! diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index 6e02466..d0e0076 100644 --- a/source/MicroBitMessageBus.cpp +++ b/source/MicroBitMessageBus.cpp @@ -15,18 +15,6 @@ MicroBitMessageBus::MicroBitMessageBus() this->listeners = NULL; this->evt_queue_head = NULL; this->evt_queue_tail = NULL; - this->nonce_val = 0; -} - -/** - * Returns a 'nonce' for use with the NONCE_ID channel of the message bus. - */ -uint16_t MicroBitMessageBus::nonce() -{ - // In the global scheme of things, a terrible nonce generator. - // However, for our purposes, this is simple and adequate for local use. - // This would be a bad idea if our events were networked though - can you think why? - return nonce_val++; } /** @@ -86,6 +74,9 @@ void async_callback(void *param) listener->evt = item->evt; listener->evt_queue = listener->evt_queue->next; delete item; + + // We spin the scheduler here, to preven any particular event handler from continuously holding onto resources. + schedule(); } else break; From 72e5a9a6a6c30fa84693b58fca5a2234a0dfce8c Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Sun, 18 Oct 2015 19:09:54 +0100 Subject: [PATCH 2/7] microbit: BUGFIX - post merge fixes - MicroBitDisplay::scroll() timing bug - MicroBitDisplay::print() timing bug - MicroBitFiber wait/notify bug --- source/MicroBitDisplay.cpp | 18 +++++++++++------- source/MicroBitFiber.cpp | 11 +++++++++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/source/MicroBitDisplay.cpp b/source/MicroBitDisplay.cpp index 01dd54c..178501a 100644 --- a/source/MicroBitDisplay.cpp +++ b/source/MicroBitDisplay.cpp @@ -314,6 +314,7 @@ void MicroBitDisplay::updateAnimateImage() if (scrollingImagePosition <= -scrollingImage.getWidth() + (MICROBIT_DISPLAY_WIDTH + scrollingImageStride) && scrollingImageRendered) { animationMode = ANIMATION_MODE_NONE; + this->clear(); this->sendAnimationCompleteEvent(); return; } @@ -566,6 +567,8 @@ void MicroBitDisplay::scrollAsync(ManagedString s, int delay) scrollingChar = 0; scrollingText = s; + animationDelay = delay; + animationTick = 0; animationMode = ANIMATION_MODE_SCROLL_TEXT; } } @@ -592,11 +595,13 @@ void MicroBitDisplay::scrollAsync(MicroBitImage image, int delay, int stride) if(delay <= 0 ) delay = MICROBIT_DEFAULT_SCROLL_SPEED; - this->scrollingImagePosition = stride < 0 ? width : -image.getWidth(); - this->scrollingImageStride = stride; - this->scrollingImage = image; - this->scrollingImageRendered = false; + scrollingImagePosition = stride < 0 ? width : -image.getWidth(); + scrollingImageStride = stride; + scrollingImage = image; + scrollingImageRendered = false; + animationDelay = delay; + animationTick = 0; animationMode = ANIMATION_MODE_SCROLL_IMAGE; } } @@ -692,15 +697,14 @@ void MicroBitDisplay::animateAsync(MicroBitImage image, int delay, int stride, i if(delay <= 0 ) delay = MICROBIT_DEFAULT_SCROLL_SPEED; - animationDelay = delay; - animationTick = delay-1; - //calculate starting position which is offset by the stride scrollingImagePosition = (startingPosition == MICROBIT_DISPLAY_ANIMATE_DEFAULT_POS) ? MICROBIT_DISPLAY_WIDTH + stride : startingPosition; scrollingImageStride = stride; scrollingImage = image; scrollingImageRendered = false; + animationDelay = delay; + animationTick = delay-1; animationMode = ANIMATION_MODE_ANIMATE_IMAGE; } } diff --git a/source/MicroBitFiber.cpp b/source/MicroBitFiber.cpp index 3a5e653..f6f0287 100644 --- a/source/MicroBitFiber.cpp +++ b/source/MicroBitFiber.cpp @@ -163,6 +163,10 @@ void scheduler_init() idleFiber->tcb.SP = CORTEX_M0_STACK_BASE - 0x04; idleFiber->tcb.LR = (uint32_t) &idle_task; + // Register to receive events in the NOTIFY channel - this is used to implement wait-notify semantics + uBit.MessageBus.listen(MICROBIT_ID_NOTIFY, MICROBIT_EVT_ANY, scheduler_event, MESSAGE_BUS_LISTENER_IMMEDIATE); + uBit.MessageBus.listen(MICROBIT_ID_NOTIFY_ONE, MICROBIT_EVT_ANY, scheduler_event, MESSAGE_BUS_LISTENER_IMMEDIATE); + // Flag that we now have a scheduler running uBit.flags |= MICROBIT_FLAG_SCHEDULER_RUNNING; } @@ -236,7 +240,8 @@ void scheduler_event(MicroBitEvent evt) } // Unregister this event, as we've woken up all the fibers with this match. - uBit.MessageBus.ignore(evt.source, evt.value, scheduler_event); + if (evt.source != MICROBIT_ID_NOTIFY && evt.source != MICROBIT_ID_NOTIFY_ONE) + uBit.MessageBus.ignore(evt.source, evt.value, scheduler_event); } @@ -320,7 +325,9 @@ void fiber_wait_for_event(uint16_t id, uint16_t value) queue_fiber(f, &waitQueue); // Register to receive this event, so we can wake up the fiber when it happens. - uBit.MessageBus.listen(id, value, scheduler_event, MESSAGE_BUS_LISTENER_IMMEDIATE); + // Special case for teh notify channel, as we always stay registered for that. + if (id != MICROBIT_ID_NOTIFY && id != MICROBIT_ID_NOTIFY_ONE) + uBit.MessageBus.listen(id, value, scheduler_event, MESSAGE_BUS_LISTENER_IMMEDIATE); // Finally, enter the scheduler. schedule(); From 1eeaeca2c6618341763b923a30fd19cead354178 Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Wed, 28 Oct 2015 14:17:12 +0000 Subject: [PATCH 3/7] microbit: BUGFIX - correct initialization of MicroBitListener code now correctly initiailizes the evt_queue filed needed to prevent lockup during event processing (understandably!). --- source/MicroBitListener.cpp | 2 ++ source/MicroBitMessageBus.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/source/MicroBitListener.cpp b/source/MicroBitListener.cpp index 09f1498..d04c320 100644 --- a/source/MicroBitListener.cpp +++ b/source/MicroBitListener.cpp @@ -23,6 +23,7 @@ MicroBitListener::MicroBitListener(uint16_t id, uint16_t value, void (*handler)( this->cb_arg = NULL; this->flags = flags; this->next = NULL; + this->evt_queue = NULL; } /** @@ -41,6 +42,7 @@ MicroBitListener::MicroBitListener(uint16_t id, uint16_t value, void (*handler)( this->cb_arg = arg; this->flags = flags | MESSAGE_BUS_LISTENER_PARAMETERISED; this->next = NULL; + this->evt_queue = NULL; } /** diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index 12d9a34..1cfba19 100644 --- a/source/MicroBitMessageBus.cpp +++ b/source/MicroBitMessageBus.cpp @@ -66,7 +66,7 @@ void async_callback(void *param) else listener->cb(listener->evt); - // If there are more events to process, dequeue te next one and process it. + // If there are more events to process, dequeue the next one and process it. if ((listener->flags & MESSAGE_BUS_LISTENER_QUEUE_IF_BUSY) && listener->evt_queue) { MicroBitEventQueueItem *item = listener->evt_queue; From 0fa82960488212e20e41944ad40f743277b130dd Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Thu, 29 Oct 2015 00:08:33 +0000 Subject: [PATCH 4/7] microbit: BUGFIX safe deletion of listeners also correction to minor bu in MicroBitFiber that could conceivably send more notifications that it should... --- inc/MicroBitListener.h | 2 ++ inc/MicroBitMessageBus.h | 6 ++++ source/MicroBitFiber.cpp | 16 +++++---- source/MicroBitMessageBus.cpp | 68 ++++++++++++++++++++++++----------- 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/inc/MicroBitListener.h b/inc/MicroBitListener.h index af7bf22..6b327b9 100644 --- a/inc/MicroBitListener.h +++ b/inc/MicroBitListener.h @@ -14,6 +14,8 @@ #define MESSAGE_BUS_LISTENER_DROP_IF_BUSY 0x0020 #define MESSAGE_BUS_LISTENER_NONBLOCKING 0x0040 #define MESSAGE_BUS_LISTENER_URGENT 0x0080 +#define MESSAGE_BUS_LISTENER_DELETING 0x8000 + #define MESSAGE_BUS_LISTENER_IMMEDIATE (MESSAGE_BUS_LISTENER_NONBLOCKING | MESSAGE_BUS_LISTENER_URGENT) diff --git a/inc/MicroBitMessageBus.h b/inc/MicroBitMessageBus.h index 33eb6be..a6b3288 100644 --- a/inc/MicroBitMessageBus.h +++ b/inc/MicroBitMessageBus.h @@ -222,6 +222,12 @@ class MicroBitMessageBus : public MicroBitComponent int add(MicroBitListener *newListener); int remove(MicroBitListener *newListener); + /** + * Cleanup any MicroBitListeners marked for deletion from the list. + * @return The number of listeners removed from the list. + */ + int deleteMarkedListeners(); + MicroBitListener *listeners; // Chain of active listeners. MicroBitEventQueueItem *evt_queue_head; // Head of queued events to be processed. MicroBitEventQueueItem *evt_queue_tail; // Tail of queued events to be processed. diff --git a/source/MicroBitFiber.cpp b/source/MicroBitFiber.cpp index f6f0287..5d72d0c 100644 --- a/source/MicroBitFiber.cpp +++ b/source/MicroBitFiber.cpp @@ -221,15 +221,19 @@ void scheduler_event(MicroBitEvent evt) uint16_t value = (f->context & 0xFFFF0000) >> 16; // Special case for the NOTIFY_ONE channel... - if ((notifyOneComplete == 0) && (id == MICROBIT_ID_NOTIFY && evt.source == MICROBIT_ID_NOTIFY_ONE) && (value == MICROBIT_EVT_ANY || value == evt.value)) + if ((evt.source == MICROBIT_ID_NOTIFY_ONE && id == MICROBIT_ID_NOTIFY) && (value == MICROBIT_EVT_ANY || value == evt.value)) { - // Wakey wakey! - dequeue_fiber(f); - queue_fiber(f,&runQueue); - notifyOneComplete = 1; + if (!notifyOneComplete) + { + // Wakey wakey! + dequeue_fiber(f); + queue_fiber(f,&runQueue); + notifyOneComplete = 1; + } } - if ((id == MICROBIT_ID_ANY || id == evt.source) && (value == MICROBIT_EVT_ANY || value == evt.value)) + // Normal case. + else if ((id == MICROBIT_ID_ANY || id == evt.source) && (value == MICROBIT_EVT_ANY || value == evt.value)) { // Wakey wakey! dequeue_fiber(f); diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index 1cfba19..675f6bb 100644 --- a/source/MicroBitMessageBus.cpp +++ b/source/MicroBitMessageBus.cpp @@ -142,6 +142,45 @@ MicroBitEventQueueItem* MicroBitMessageBus::dequeueEvent() return item; } +/** + * Cleanup any MicroBitListeners marked for deletion from the list. + * @return The number of listeners removed from the list. + */ +int MicroBitMessageBus::deleteMarkedListeners() +{ + MicroBitListener *l, *p; + int removed = 0; + + l = listeners; + p = NULL; + + // Walk this list of event handlers. Delete any that match the given listener. + while (l != NULL) + { + if (l->flags & MESSAGE_BUS_LISTENER_DELETING) + { + if (p == NULL) + listeners = l->next; + else + p->next = l->next; + + // delete the listener. + MicroBitListener *t = l; + l = l->next; + + delete t; + removed++; + + continue; + } + + p = l; + l = l->next; + } + + return removed; +} + /** * Periodic callback from MicroBit. * Process at least one event from the event queue, if it is not empty. @@ -149,6 +188,9 @@ MicroBitEventQueueItem* MicroBitMessageBus::dequeueEvent() */ void MicroBitMessageBus::idleTick() { + // Clear out any listeners marked for deletion + this->deleteMarkedListeners(); + MicroBitEventQueueItem *item = this->dequeueEvent(); // Whilst there are events to process and we have no useful other work to do, pull them off the queue and process them. @@ -225,7 +267,7 @@ int MicroBitMessageBus::process(MicroBitEvent &evt, uint32_t mask) { if((l->id == evt.source || l->id == MICROBIT_ID_ANY) && (l->value == evt.value || l->value == MICROBIT_EVT_ANY)) { - if(l->flags & mask) + if(l->flags & mask && !(l->flags & MESSAGE_BUS_LISTENER_DELETING)) { l->evt = evt; @@ -377,7 +419,7 @@ int MicroBitMessageBus::add(MicroBitListener *newListener) { methodCallback = (newListener->flags & MESSAGE_BUS_LISTENER_METHOD) && (l->flags & MESSAGE_BUS_LISTENER_METHOD); - if (l->id == newListener->id && l->value == newListener->value && (methodCallback ? *l->cb_method == *newListener->cb_method : l->cb == newListener->cb)) + if (l->id == newListener->id && l->value == newListener->value && (methodCallback ? *l->cb_method == *newListener->cb_method : l->cb == newListener->cb) && !(l->flags & MESSAGE_BUS_LISTENER_DELETING)) return 0; l = l->next; @@ -437,7 +479,7 @@ int MicroBitMessageBus::add(MicroBitListener *newListener) */ int MicroBitMessageBus::remove(MicroBitListener *listener) { - MicroBitListener *l, *p; + MicroBitListener *l; int removed = 0; //handler can't be NULL! @@ -445,7 +487,6 @@ int MicroBitMessageBus::remove(MicroBitListener *listener) return 0; l = listeners; - p = NULL; // Walk this list of event handlers. Delete any that match the given listener. while (l != NULL) @@ -453,29 +494,16 @@ int MicroBitMessageBus::remove(MicroBitListener *listener) if ((listener->flags & MESSAGE_BUS_LISTENER_METHOD) == (l->flags & MESSAGE_BUS_LISTENER_METHOD)) { if(((listener->flags & MESSAGE_BUS_LISTENER_METHOD) && (*l->cb_method == *listener->cb_method)) || - ((!(listener->flags & MESSAGE_BUS_LISTENER_METHOD) && l->cb == listener->cb))) + ((!(listener->flags & MESSAGE_BUS_LISTENER_METHOD) && l->cb == listener->cb))) { if ((listener->id == MICROBIT_ID_ANY || listener->id == l->id) && (listener->value == MICROBIT_EVT_ANY || listener->value == l->value)) { - // Found a match. Remove from the list. - if (p == NULL) - listeners = l->next; - else - p->next = l->next; - - // delete the listener. - MicroBitListener *t = l; - l = l->next; - - delete t; - removed++; - - continue; + // Found a match. mark this to be removed from the list. + l->flags |= MESSAGE_BUS_LISTENER_DELETING; } } } - p = l; l = l->next; } From 84da6a4a09fe3ee56667749301ca7bc4c7ea5f34 Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Sat, 31 Oct 2015 10:27:38 +0000 Subject: [PATCH 5/7] 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 | 19 ++++++--- inc/MicroBitConfig.h | 7 ++++ inc/MicroBitMessageBus.h | 8 ++-- inc/MicroBitMultiButton.h | 12 ++++-- source/MicroBit.cpp | 4 +- source/MicroBitButton.cpp | 20 +++++---- source/MicroBitDisplay.cpp | 22 +++++++--- source/MicroBitListener.cpp | 17 +++++--- source/MicroBitMessageBus.cpp | 68 +++++++++++++++++++++++------- source/MicroBitMultiButton.cpp | 76 ++++++++++++++++++++++++++-------- 10 files changed, 184 insertions(+), 69 deletions(-) diff --git a/inc/MicroBitButton.h b/inc/MicroBitButton.h index 231cadd..59370d4 100644 --- a/inc/MicroBitButton.h +++ b/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. diff --git a/inc/MicroBitConfig.h b/inc/MicroBitConfig.h index 31e9416..182a79a 100644 --- a/inc/MicroBitConfig.h +++ b/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 // diff --git a/inc/MicroBitMessageBus.h b/inc/MicroBitMessageBus.h index a6b3288..7ee9141 100644 --- a/inc/MicroBitMessageBus.h +++ b/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(); diff --git a/inc/MicroBitMultiButton.h b/inc/MicroBitMultiButton.h index 1a8b34e..f54702a 100644 --- a/inc/MicroBitMultiButton.h +++ b/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: diff --git a/source/MicroBit.cpp b/source/MicroBit.cpp index 07634d0..c788afa 100644 --- a/source/MicroBit.cpp +++ b/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), diff --git a/source/MicroBitButton.cpp b/source/MicroBitButton.cpp index 1b04303..80e6c1e 100644 --- a/source/MicroBitButton.cpp +++ b/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 diff --git a/source/MicroBitDisplay.cpp b/source/MicroBitDisplay.cpp index 178501a..95deb94 100644 --- a/source/MicroBitDisplay.cpp +++ b/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); } } diff --git a/source/MicroBitListener.cpp b/source/MicroBitListener.cpp index d04c320..f85535b 100644 --- a/source/MicroBitListener.cpp +++ b/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 { - while (p->next != NULL) - p = p->next; + queueDepth = 1; - p->next = q; + while (p->next != NULL) + { + p = p->next; + queueDepth++; + } + + if (queueDepth < MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH) + p->next = new MicroBitEventQueueItem(e); } } diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index 675f6bb..4d64f16 100644 --- a/source/MicroBitMessageBus.cpp +++ b/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; - if (!processingComplete) + 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 (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++; - if (evt_queue_tail == NULL) - evt_queue_head = evt_queue_tail = item; - else - evt_queue_tail->next = item; + __enable_irq(); - __enable_irq(); + // 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); + + 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; diff --git a/source/MicroBitMultiButton.cpp b/source/MicroBitMultiButton.cpp index f477756..3cc1342 100644 --- a/source/MicroBitMultiButton.cpp +++ b/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_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: - 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); break; + + case MICROBIT_BUTTON_EVT_UP: + if(isSubButtonPressed(otherButton)) + { + MicroBitEvent e(id, MICROBIT_BUTTON_EVT_UP); + + 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); + setSupressedState(button, 0); + + break; + } } From b2e9369771b70816b922dc2b8401ec30e59f26f0 Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Sun, 1 Nov 2015 12:59:52 +0000 Subject: [PATCH 6/7] microbit: BUGFIX DoS attacks on the MessageBus - Enures an event listener is not deleted whilst a fiber is activiely processing a queue - Added support for resurrection of event listeners in cases where identical listeners are removed/added repetitively. --- source/MicroBitMessageBus.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index 4d64f16..f630f47 100644 --- a/source/MicroBitMessageBus.cpp +++ b/source/MicroBitMessageBus.cpp @@ -190,7 +190,7 @@ int MicroBitMessageBus::deleteMarkedListeners() // Walk this list of event handlers. Delete any that match the given listener. while (l != NULL) { - if (l->flags & MESSAGE_BUS_LISTENER_DELETING) + if (l->flags & MESSAGE_BUS_LISTENER_DELETING && !l->flags & MESSAGE_BUS_LISTENER_BUSY) { if (p == NULL) listeners = l->next; @@ -455,8 +455,16 @@ int MicroBitMessageBus::add(MicroBitListener *newListener) { methodCallback = (newListener->flags & MESSAGE_BUS_LISTENER_METHOD) && (l->flags & MESSAGE_BUS_LISTENER_METHOD); - if (l->id == newListener->id && l->value == newListener->value && (methodCallback ? *l->cb_method == *newListener->cb_method : l->cb == newListener->cb) && !(l->flags & MESSAGE_BUS_LISTENER_DELETING)) + if (l->id == newListener->id && l->value == newListener->value && (methodCallback ? *l->cb_method == *newListener->cb_method : l->cb == newListener->cb)) + { + // We have a perfect match for this event listener already registered. + // If it's marked for deletion, we simply resurrect the listener, and we're done. + // Either way, we return an error code, as the *new* listener should be released... + if(l->flags & MESSAGE_BUS_LISTENER_DELETING) + l->flags &= ~MESSAGE_BUS_LISTENER_DELETING; + return 0; + } l = l->next; } From 39abf824dcd70005a1c565b61157d466a4caa45d Mon Sep 17 00:00:00 2001 From: Joe Finney Date: Sun, 1 Nov 2015 15:16:27 +0000 Subject: [PATCH 7/7] microbit: More efficient handling of events Removed unnecessary queing of item on the MessageBus, whilst maintaining causal ordering. --- source/MicroBitMessageBus.cpp | 72 ++++++++++++++++------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/source/MicroBitMessageBus.cpp b/source/MicroBitMessageBus.cpp index f630f47..e3f781b 100644 --- a/source/MicroBitMessageBus.cpp +++ b/source/MicroBitMessageBus.cpp @@ -97,54 +97,48 @@ void MicroBitMessageBus::queueEvent(MicroBitEvent &evt) { int processingComplete; - 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 (evt_queue_tail == NULL) - { - evt_queue_head = evt_queue_tail = item; - } - else - { - evt_queue_tail->next = item; - evt_queue_tail = item; - } - - queueLength++; - - __enable_irq(); + MicroBitEventQueueItem *prev = evt_queue_tail; // 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); + // If we've already processed all event handlers, we're all done. + // No need to queue the event. if (processingComplete) + return; + + // If we need to queue, but there is no space, then there's nothg we can do. + if (queueLength >= MESSAGE_BUS_LISTENER_MAX_QUEUE_DEPTH) + return; + + // Otherwise, we need to queue this event for later processing... + // We queue this event at the tail of the queue at the point where we entered queueEvent() + // This is important as the processing above *may* have generated further events, and + // we want to maintain ordering of events. + MicroBitEventQueueItem *item = new MicroBitEventQueueItem(evt); + + // The queue was empty when we entered this function, so queue our event at the start of the queue. + __disable_irq(); + + if (prev == NULL) { - // 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; + item->next = evt_queue_head; + evt_queue_head = item; + } + else + { + item->next = prev->next; + prev->next = item; } -} + if (item->next == NULL) + evt_queue_tail = item; + + queueLength++; + + __enable_irq(); +} /** * Extract the next event from the front of the event queue (if present).