Revision 5:8bf639bbedb5, committed 2015-05-16

Comitter:

finneyj

Date:

Sat May 16 22:28:56 2015 +0000

Parent:

4:f998ee705a20

Commit message:

Heap corruption bugfix... fixed:; ; - Heap corruption from BLE stack (hangover from BETA online build?); - BLEDevice can't be brought up in static context; - Calling scheduler callback when scheduler not initialised; - Rewrite of MicroBitImage linear bufs

Changed in this revision

--- a/MicroBit.cpp	Fri May 15 22:23:17 2015 +0000
+++ b/MicroBit.cpp	Sat May 16 22:28:56 2015 +0000
@@ -1,8 +1,6 @@
 #include "inc/MicroBit.h"
 #include "URIBeaconConfigService.h"
 
-extern void dfuCallbackFn();
-
 char MICROBIT_BLE_DEVICE_NAME[] = "BBC MicroBit Prototype";
 char MICROBIT_BLE_MANUFACTURER[] = "The Cast of W1A";
 char MICROBIT_BLE_MODEL[] = "Microbit SB2";
@@ -19,14 +17,22 @@
 MicroBit::MicroBit() //:display(MICROBIT_ID_DISPLAY, 5, 5)
   //leftButton(MICROBIT_ID_LEFT_BUTTON,MICROBIT_PIN_LEFT_BUTTON)
   //i2c(MICROBIT_PIN_SDA, MICROBIT_PIN_SCL)
-{
-    display = new MicroBitDisplay(MICROBIT_ID_DISPLAY, 5, 5); 
+{   
+}
 
-    // Set up BLE stack.    
+void MicroBit::initDisplay()
+{
+    // Set up LED Matrix Display.    
+    display = new MicroBitDisplay(MICROBIT_ID_DISPLAY, 5, 5);    
+    display->startDisplay();
+}
 
+void MicroBit::initBLE()
+{
+    // Set up BLE stack.        
     ble = new BLEDevice();
     ble->init();
-    
+ 
     /* Setup auxiliary services. */
     ble_firmware_update_service = new DFUService(*ble);
     ble_device_information_service = new DeviceInformationService(*ble, MICROBIT_BLE_MANUFACTURER, MICROBIT_BLE_MODEL, MICROBIT_BLE_SERIAL, MICROBIT_BLE_HARDWARE_VERSION, MICROBIT_BLE_FIRMWARE_VERSION, MICROBIT_BLE_SOFTWARE_VERSION);
@@ -34,10 +40,7 @@
     /* Setup advertising. */
     ble->accumulateAdvertisingPayload(GapAdvertisingData::BREDR_NOT_SUPPORTED | GapAdvertisingData::LE_GENERAL_DISCOVERABLE);
     ble->accumulateAdvertisingPayload(GapAdvertisingData::COMPLETE_LOCAL_NAME, (uint8_t *)MICROBIT_BLE_DEVICE_NAME, sizeof(MICROBIT_BLE_DEVICE_NAME));
-    //ble.setDeviceName(reinterpret_cast<uint8_t *>(&MICROBIT_BLE_DEVICE_NAME));
     ble->setAdvertisingType(GapAdvertisingParams::ADV_CONNECTABLE_UNDIRECTED);
-
     ble->setAdvertisingInterval(Gap::MSEC_TO_ADVERTISEMENT_DURATION_UNITS(1000));
-    
-    ble->startAdvertising();  
+    ble->startAdvertising();   
 }

--- a/MicroBitDisplay.cpp	Fri May 15 22:23:17 2015 +0000
+++ b/MicroBitDisplay.cpp	Sat May 16 22:28:56 2015 +0000
@@ -3,10 +3,7 @@
   *
   * An MicroBitDisplay represents the LED matrix array on the MicroBit device.
   */
-#include "inc/MicroBitDisplay.h"
-#include "MicroBitFiber.h"
-
-extern Serial pc;
+#include "MicroBit.h"
 
     /**
       * Provides the mapping from Matrix ROW/COL to a linear X/Y buffer. 
@@ -82,7 +79,7 @@
   * @param x the width of the display in pixels.
   * @param y the height of the display in pixels.     
   */
-MicroBitDisplay::MicroBitDisplay(int id, int x, int y) : columnPins(MICROBIT_DISPLAY_COLUMN_PINS), rowPins(MICROBIT_DISPLAY_ROW_PINS), image(x*2,y*2)
+MicroBitDisplay::MicroBitDisplay(int id, int x, int y) : strobe(), columnPins(MICROBIT_DISPLAY_COLUMN_PINS), rowPins(MICROBIT_DISPLAY_ROW_PINS), image(x*2,y)
 {
     this->id = id;
     this->width = x;
@@ -95,17 +92,26 @@
     this->scrollingPosition = 0;
     this->scrollingChar = 0;
     this->scrollingText = NULL;
+}
 
-    this->strobe.attach(this, &MicroBitDisplay::strobeUpdate, MICROBIT_DISPLAY_REFRESH_PERIOD);
+void displayISR()
+{
+    uBit.display->strobeUpdate();
 }
 
+void MicroBitDisplay::startDisplay()
+{
+    this->strobe.attach(displayISR, MICROBIT_DISPLAY_REFRESH_PERIOD);
+}
+
+
 /**
   * Internal frame update method, used to strobe the display.
   * TODO: Write a more efficient, complementary variation of this method for the case where 
   * MICROBIT_DISPLAY_ROW_COUNT > MICROBIT_DISPLAY_COLUMN_COUNT.
   */   
 void MicroBitDisplay::strobeUpdate()
-{
+{   
     // TODO: Cache row data for future use, so we don't recompute so often?
     int rowdata;
     int coldata;
@@ -126,7 +132,7 @@
     columnPins.write(0xffff);    
     rowPins.write(rowdata);
     columnPins.write(~coldata);
-    
+
     // Update Scrolling Text if we need to.
     if (scrollingText != NULL && (++scrollingTick == scrollingDelay))
     {
@@ -136,7 +142,7 @@
     
     // Scheduler callback. We do this here just as a single timer is more efficient. :-)
 #ifdef FIBER_SCHEDULER    
-    scheduler_tick();
+    //scheduler_tick();
 #endif    
 }
 

--- a/MicroBitImage.cpp	Fri May 15 22:23:17 2015 +0000
+++ b/MicroBitImage.cpp	Sat May 16 22:28:56 2015 +0000
@@ -30,11 +30,32 @@
   * 
   * @param x the width of the image.
   * @param y the height of the image.     
+  *
+  * Bitmap buffer is linear, with 8 bits per pixel, row by row, 
+  * top to bottom with no word alignment. Stride is therefore the image width in pixels.
+  * in where w and h are width and height respectively, the layout is therefore:
+  *
+  * |[0,0]...[w,o][1,0]...[w,1]  ...  [[w,h]
+  *
+  * A copy of the image is made in RAM, as images are mutable.
+  *
+  * TODO: Consider an immutable flavour, which might save us RAM for animation spritesheets...
+  * ...as these could be kept in FLASH.
   */
 MicroBitImage::MicroBitImage(int x, int y)
 {
     this->init(x,y,NULL);
+}
 
+/**
+    * Copy Constructor. 
+    * Clone an existing MicroBitImage.
+    * 
+    * @param image The MicroBitImage to clone.
+    */
+MicroBitImage::MicroBitImage(MicroBitImage &image)
+{
+    this->init(image.getWidth(),image.getHeight(),image.bitmap);
 }
 
 /**
@@ -43,37 +64,32 @@
   * 
   * @param x the width of the image.
   * @param y the height of the image.
-  * @param bitmap a 2D array representing the image.
-  *     
+  * @param the bitmap buffer to copy. 
+  *
   */
-MicroBitImage::MicroBitImage(int x, int y, int **bitmap)
+MicroBitImage::MicroBitImage(int x, int y, uint8_t *bitmap)
 {
     this->init(x,y,bitmap);
 }
 
 MicroBitImage::~MicroBitImage()
 {
-    for (int i = 0; i < height; i++)
-            delete[] this->bitmap[i];
-
     delete[] this->bitmap;
 }
 
-void MicroBitImage::init(int x, int y, int **bitmap)
+void MicroBitImage::init(int x, int y, uint8_t *bitmap)
 {
     // Create a copy of the array
     this->width = x;
     this->height = y;
     
-    // create a jagged array to represent the image. We use a jagged rather than 2D array here to 
-    // ease type checking across the myriad of languages that might target us.
-    this->bitmap = new int*[width];
-
-    for (int i = 0; i < width; i++)
-        this->bitmap[i] = new int[height];
+    // create a linear buffer to represent the image. We could use a jagged/2D array here, but experimentation
+    // showed this had a negative effect on memory management (heap fragmentation etc).
+     
+    this->bitmap = new uint8_t[width*height];
 
     if (bitmap)
-        this->printImage(0,0,bitmap);
+        this->printImage(x,y,bitmap);
     else
         this->clear();
 }
@@ -85,9 +101,7 @@
   */
 void MicroBitImage::clear()
 {
-    for (int x = 0; x < width; x++)
-        for (int y = 0; y < height; y++)
-            this->bitmap[x][y] = 0;
+    memclr(this->bitmap, width*height);
 }
  
 
@@ -98,35 +112,54 @@
   * @param y The co-ordinate of the pixel to change w.r.t. top left origin.
   * @param value The new value of the pixel.
   */
-void MicroBitImage::setPixelValue(int x , int y, int value)
+void MicroBitImage::setPixelValue(int x , int y, uint8_t value)
 {
-    this->bitmap[x][y] = value;
+    this->bitmap[y*width+x] = value;
 }
 
 /**
   * Determined the value of a given pixel.
   * @return The value assigned to the givne pixel location
   */
-int MicroBitImage::getPixelValue(int x , int y)
+uint8_t MicroBitImage::getPixelValue(int x , int y)
 {
-    return this->bitmap[x][y];
+    return this->bitmap[y*width+x];
 }
 
 /**
   * Replaces the content of this image with that of a given 
-  * 2D array representing the image.
-  * Origin is in the top left corner of the image.
+  * bitmap representation of an image.
+  *
+  * Data is copied. Any out of range data is safely ignored.
   *
   * @param x the width of the image.
   * @param y the height of the image.
-  * @param bitmap a 2D array representing the image.
+  * @param linear bitmap representing the image.
   *     
   */
-void MicroBitImage::printImage(int x, int y, int **bitmap)
+void MicroBitImage::printImage(int width, int height, uint8_t *bitmap)
 {
-    for (int i = 0; i < min(x,width); i++)
-        for (int j = 0; j < min(y,height); j++)
-            this->bitmap[i][j] = bitmap[i][j];
+    uint8_t *pIn, *pOut;
+    int pixelsToCopyX, pixelsToCopyY;
+
+    // Sanity check.
+    if (width <= 0 || width <= 0)
+        return;
+
+    // Calcualte sane start pointer.
+    pixelsToCopyX = min(width,this->width);
+    pixelsToCopyY = min(height,this->height);
+
+    pIn = bitmap;
+    pOut = this->bitmap;
+    
+    // Copy the image, stride by stride.
+    for (int i=0; i<pixelsToCopyY; i++)
+    {
+        memcpy(pOut, pIn, pixelsToCopyX);
+        pIn += width;
+        pOut += this->width;
+    }
 }
   
 /**
@@ -138,17 +171,55 @@
   * @param y The uppermost Y co-ordinate in this image where the given image should be pasted.
   * @param alpha set to 1 if transparency clear pixels in given image should be treated as transparent. Set to 0 otherwise.
   */
-void MicroBitImage::paste(MicroBitImage *image, int x, int y, int alpha)
+void MicroBitImage::paste(MicroBitImage &image, int x, int y, int alpha)
 {
+    uint8_t *pIn, *pOut;
+    int cx, cy;
+
     // Sanity check.
-    if (x >= width || y >= height)
+    // We permit writes that overlap us, but ones that are clearly out of scope we can filter early.
+    if (x >= width || y >= height || x+image.width <= 0 || y+image.height <= 0)
         return;
+
+    //Calculate the number of byte we need to copy in each dimension.
+    cx = x < 0 ? min(image.width + x, width) : min(image.width, width - x);
+    cy = y < 0 ? min(image.height + y, height) : min(image.height, height - y);
+
+    // Calcualte sane start pointer.
+    pIn = image.bitmap;
+    pOut = bitmap;
+    pOut += (x > 0) ? x : 0;
+    pOut += (y > 0) ? width*y : 0;
+
+    // Copy the image, stride by stride
+    // If we want primitive transparecy, we do this byte by byte.
+    // If we don't, use a more efficient block memory copy instead. Every little helps!
+
+    if (alpha)
+    {
+        for (int i=0; i<cy; i++)
+        {
+            for (int j=0; j<cx; j++)
+            {
+                // Copy this byte if appropriate.
+                if (*(pIn+j) != 0)
+                    *(pOut+j) = *(pIn+j);
+            }
     
-    // Paste.
-    for (int i = 0; i < min(image->width,width-x); i++)
-        for (int j = 0; j < min(image->height,height-y); j++)
-            if (!(alpha && bitmap[i][j] == 0)) 
-                this->bitmap[i+x][j+y] = bitmap[i][j];        
+            pIn += image.width;
+            pOut += width;
+        }
+    }
+    else
+    {
+        for (int i=0; i<cy; i++)
+        {
+            memcpy(pOut, pIn, cx);
+
+            pIn += image.width;
+            pOut += width;
+        }
+    }
 }
 
  /**
@@ -160,8 +231,8 @@
   */
 void MicroBitImage::print(char c, int x, int y)
 {
-    
     unsigned char v;
+    int x1, y1;
     
     // Sanity check. Silently ignore anything out of bounds.
     if (x >= width || y >= height || c < MICROBIT_FONT_ASCII_START || c > MICROBIT_FONT_ASCII_END)
@@ -179,10 +250,17 @@
         else
             offset++;
         
+        // Update our Y co-ord write position
+        y1 = y+row;
+        
         for (int col = 0; col < MICROBIT_FONT_WIDTH; col++)
-            if (x+col < width && y+row < height)
-                this->bitmap[x+col][y+row] = (v & (0x08 >> col)) ? 1 : 0;
-        
+        {
+            // Update our X co-ord write position
+            x1 = x+col;
+            
+            if (x1 < width && y1 < height)
+                this->bitmap[y1*width+x1] = (v & (0x08 >> col)) ? 255 : 0;
+        }
     }  
 }
 
@@ -194,14 +272,20 @@
   */
 void MicroBitImage::shiftLeft(int n)
 {
-    for (int i = 0; i < width-1 ; i++)
-        for (int j = 0; j < height; j++)
-                this->bitmap[i][j] = this->bitmap[i+1][j];         
-                
-    // Blank fill the rightmost column.
-    for (int j = 0; j < height; j++)
-        this->bitmap[width-1][j] = 0;
-        
+    uint8_t *p = bitmap;
+    int pixels = width-n;
+    
+    if (n <= 0 || n > width)
+        return;
+
+
+    for (int y = 0; y < height; y++)
+    {
+        // Copy, and blank fill the rightmost column.
+        memcpy(p, p+n, pixels);
+        memclr(p+pixels, n);
+        p += width;
+    }        
 }
 
 /**
@@ -211,13 +295,19 @@
   */
 void MicroBitImage::shiftRight(int n)
 {
-    for (int i = 0; i < width-1 ; i++)
-        for (int j = 0; j < height; j++)
-                this->bitmap[i+1][j] = this->bitmap[i][j];         
+    uint8_t *p = bitmap;
+    int pixels = width-n;
+    
+    if (n <= 0 || n > width)
+        return;
 
-    // Blank fill the leftmost column.
-    for (int j = 0; j < height; j++)
-        this->bitmap[0][j] = 0;
+    for (int y = 0; y < height; y++)
+    {
+        // Copy, and blank fill the leftmost column.
+        memcpy(p+n, p, pixels);
+        memclr(p, n);
+        p += width;
+    }        
 }
 
 
@@ -228,13 +318,25 @@
   */
 void MicroBitImage::shiftUp(int n)
 {
-    for (int i = 0; i < width ; i++)
-        for (int j = 0; j < height-1; j++)
-            this->bitmap[i][j] = this->bitmap[i][j+1];         
-            
-    // Blank fill the bottommost row.
-    for (int i = 0; i < width; i++)
-        this->bitmap[i][height] = 0;
+    uint8_t *pOut, *pIn;
+   
+    if (n <= 0 || n > height)
+        return;
+    
+    pOut = bitmap;
+    pIn = bitmap+width*n;
+    
+    for (int y = 0; y < height; y++)
+    {
+        // Copy, and blank fill the leftmost column.
+        if (y < height-n)
+            memcpy(pOut, pIn, width);
+        else
+            memclr(pOut, width);
+             
+        pIn += width;
+        pOut += width;
+    }        
 }
 
 
@@ -245,14 +347,25 @@
   */
 void MicroBitImage::shiftDown(int n)
 {
-    for (int i = 0; i < width ; i++)
-        for (int j = 0; j < height-1; j++)
-            this->bitmap[i][j+1] = this->bitmap[i][j];         
-
-    // Blank fill the topmost row.
-    for (int i = 0; i < height; i++)
-        this->bitmap[i][0] = 0;
-
+    uint8_t *pOut, *pIn;
+   
+    if (n <= 0 || n > height)
+        return;
+    
+    pOut = bitmap + width*(height-1);
+    pIn = pOut - width*n;
+    
+    for (int y = 0; y < height; y++)
+    {
+        // Copy, and blank fill the leftmost column.
+        if (y < height-n)
+            memcpy(pOut, pIn, width);
+        else
+            memclr(pOut, width);
+             
+        pIn -= width;
+        pOut -= width;
+    }        
 }
 /**
   * Gets the width of this image.

--- a/inc/MicroBit.h	Fri May 15 22:23:17 2015 +0000
+++ b/inc/MicroBit.h	Sat May 16 22:28:56 2015 +0000
@@ -70,7 +70,10 @@
       * Create a representation of a MicroBit device.
       * @param messageBus callback function to receive MicroBitMessageBus events.
       */
-    MicroBit();    
+    MicroBit();  
+    
+    void initDisplay();
+    void initBLE();  
 };
 
 // Definition of the global instance of the MicroBit class.

--- a/inc/MicroBitCompat.h	Fri May 15 22:23:17 2015 +0000
+++ b/inc/MicroBitCompat.h	Sat May 16 22:28:56 2015 +0000
@@ -7,6 +7,9 @@
 
 #define min(a,b) (a < b ? a : b)
 #define max(a,b) (a > b ? a : b)
+#define memclr(a,b) (memset(a,0,b))
+
 #define PI 3.14159265359
 
+
 #endif
\ No newline at end of file

--- a/inc/MicroBitDisplay.h	Fri May 15 22:23:17 2015 +0000
+++ b/inc/MicroBitDisplay.h	Sat May 16 22:28:56 2015 +0000
@@ -9,7 +9,7 @@
 
 #define MICROBIT_DISPLAY_REFRESH_PERIOD     0.002
 
-#define MICROBIT_SB1
+#define MICROBIT_SB2
 
 #define NO_CONN 0
 
@@ -55,7 +55,6 @@
     MatrixPoint(int x, int y);
 };
 
-
 class MicroBitDisplay
 {
     int id;
@@ -97,6 +96,12 @@
     void strobeUpdate();
 
     /**
+      * Registers the strobeUpdate method with the IRQ handler.
+      * Display begins refreshing after this call.
+      */   
+    void startDisplay();
+
+    /**
       * Prints the given character to the display.
       *
       * @param c The character to display.

--- a/inc/MicroBitImage.h	Fri May 15 22:23:17 2015 +0000
+++ b/inc/MicroBitImage.h	Sat May 16 22:28:56 2015 +0000
@@ -11,17 +11,24 @@
 
 class MicroBitImage
 {
-    int width;          // Width of the bitmap, in pixels.
-    int height;         // Height of the bitmap, in pixels.
-    int **bitmap;       // 2D array representing the bitmap image.    
+    int width;              // Width of the bitmap, in pixels.
+    int height;             // Height of the bitmap, in pixels.
     
     /**
       * Internal constructor support function. 
       */
-    void init(int x, int y, int **bitmap);
+    void init(int x, int y, uint8_t *bitmap);
     
     public:
-
+    uint8_t *bitmap;        // 2D array representing the bitmap image.    
+    
+    /**
+      * Copy Constructor. 
+      * Clone an existing MicroBitImage.
+      * 
+      * @param image The MicroBitImage to clone.
+      */
+    MicroBitImage(MicroBitImage &image);
     
     /**
       * Constructor. 
@@ -41,7 +48,7 @@
       * @param bitmap a 2D array representing the image.
       *     
       */
-    MicroBitImage(int x, int y, int **bitmap);
+    MicroBitImage(int x, int y, uint8_t *bitmap);
 
     /**
       * Destructor. 
@@ -61,13 +68,13 @@
       * @param y The co-ordinate of the pixel to change w.r.t. top left origin.
       * @param value The new value of the pixel.
       */
-    void setPixelValue(int x , int y, int value);
+    void setPixelValue(int x , int y, uint8_t value);
 
     /**
       * Determined the value of a given pixel.
       * @return The value assigned to the givne pixel location
       */
-    int getPixelValue(int x , int y);
+    uint8_t getPixelValue(int x , int y);
 
     /**
       * Replaces the content of this image with that of a given 
@@ -79,7 +86,7 @@
       * @param bitmap a 2D array representing the image.
       *     
       */
-    void printImage(int x, int y, int **bitmap);
+    void printImage(int x, int y, uint8_t *bitmap);
     
     /**
       * Pastes a given bitmap at the given co-ordinates.
@@ -90,7 +97,7 @@
       * @param y The uppermost Y co-ordinate in this image where the given image should be pasted.
       * @param alpha set to 1 if transparency clear pixels in given image should be treated as transparent. Set to 0 otherwise.
       */
-    void paste(MicroBitImage *image, int x, int y, int alpha);
+    void paste(MicroBitImage &image, int x, int y, int alpha);
  
      /**
       * Prints a character to the display at the given location

--- a/test/main_hello_world_test.cpp	Fri May 15 22:23:17 2015 +0000
+++ b/test/main_hello_world_test.cpp	Sat May 16 22:28:56 2015 +0000
@@ -4,10 +4,6 @@
 
 #include "MicroBit.h"
 
-#ifdef MICROBIT_DBG
-Serial pc(USBTX, USBRX);
-#endif
-
 MicroBit uBit;
 
 char *defaultMessage = "HI HOWARD! WANT TO PLAY?";
@@ -23,18 +19,26 @@
     update = 1;
 }
 
-
-void dfuCallbackFn()
-{
-}
-
 int main()
-{    
-    //scheduler_init();
+{   
     char msg[50];
     
+    uBit.initDisplay();
+    uBit.initBLE();
+ 
+ #ifdef DBG
+    uBit.display->scrollString(defaultMessage);
+    
+    while(1)
+    {
+        wait(0.01);
+                    
+        if (uBit.ble)
+            uBit.ble->waitForEvent();
+    }
+#endif
+
     strcpy(msg, defaultMessage);
-     
     while(1)
     {
         if (update)
@@ -44,11 +48,11 @@
         }
         
         uBit.display->scrollString(msg);
-    
+
         for (int i=0; i<150; i++)
         {
             wait(0.1);
-            
+                        
             if (uBit.ble)
                 uBit.ble->waitForEvent();