Extracting nibbles from bytes in a for loop

Question

I'm having some problems with a DMA interrupt handler I've adapted for an LCD display on a MicroChip PIC32 embedded chip.

I have limited internal memory and need a frame buffer for my colour LCD. Have decided to use 16 colours and use a nibble per pixel. I've created an array which looks like this:

unsigned char GraphicsFrame[FRAME_HEIGHT][LINE_LENGTH/2];

In my interrupt handler I am converting the 4-bit nibble into a 16-bit value to send to the LCD via the parallel port and DMA transfer. I use a look up table in the interrupt handler to achieve this but when debugging this I'm going into the General_Exception_Handler and it's pointing toward a problem in the way I extract the 4-bit nibble and convert it to the 16-bit value:

for( i=0,j=0; i<30; i++, lineX++ )
{
    // not particularly elegant, perhaps look at a better way
    if     ((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x0) lineBuffer[j++] = (unsigned short int)RGBBlack;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x1) lineBuffer[j++] = (unsigned short int)RGBBlue;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x2) lineBuffer[j++] = (unsigned short int)RGBRed;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x3) lineBuffer[j++] = (unsigned short int)RGBGreen;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x4) lineBuffer[j++] = (unsigned short int)RGBCyan;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x6) lineBuffer[j++] = (unsigned short int)RGBYellow;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0x7) lineBuffer[j++] = (unsigned short int)RGBKaneGreen;
    else if((((GraphicsFrame[line][i]) >> 4) & 0x0F) == 0xF) lineBuffer[j++] = (unsigned short int)RGBWhite;

    if     ((GraphicsFrame[line][i] & 0x0F) == 0x0) lineBuffer[j++] = (unsigned short int)RGBBlack;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x1) lineBuffer[j++] = (unsigned short int)RGBBlue;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x2) lineBuffer[j++] = (unsigned short int)RGBRed;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x3) lineBuffer[j++] = (unsigned short int)RGBGreen;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x4) lineBuffer[j++] = (unsigned short int)RGBCyan;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x5) lineBuffer[j++] = (unsigned short int)RGBMagenta;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x6) lineBuffer[j++] = (unsigned short int)RGBYellow;
    else if((GraphicsFrame[line][i] & 0x0F) == 0x7) lineBuffer[j++] = (unsigned short int)RGBKaneGreen;
    else if((GraphicsFrame[line][i] & 0x0F) == 0xF) lineBuffer[j++] = (unsigned short int)RGBWhite;
}

I'm trying to set up the DMA to transfer 60 pixels at a time (60 x 16-bits) using another array which contains the 60 pixels:

unsigned short int lineBuffer[60];

Can anyone spot a problem with the way I'm extracting the nibble and converting it? There are no warnings or errors so nothing is jumping out at me!

Any help appreciated, thanks

score 1 · Answer 1 · answered Apr 17 '14 at 11:40

It looks ok as far as I can tell from what you posted. The only thing fishy is the line and lineX variables. They don't seem to be used. Also if you want to iterate through the first dimension of the 2D array, you will likely have to use nested for loops.

The rest of the post is a code review, rather than an answer:

The code posted is not a lookup table. Consider doing it like this instead:

const uint16_t COLOR_TABLE [COLORS_N] = 
{
  RGBBlack,      // 0x0
  RGBBlue,       // 0x1
  RGBRed,        // 0x2
  RGBGreen,      // 0x3
  RGBCyan,       // 0x4
  undefined,     // 0x5
  RGBYellow,     // 0x6
  RGBKaneGreen,  // 0x7
  undefined,     // 0x8
  undefined,     // 0x9
  undefined,     // 0xA
  undefined,     // 0xB
  undefined,     // 0xC
  undefined,     // 0xD
  undefined,     // 0xE
  RGBWhite,      // 0xF
};

...

uint8_t line_iterator=0;

for(uint8_t i=0; i<LINE_LENGTH/2; i++)
{
  uint8_t nibble;

  nibble = (uint8_t) ((GraphicsFrame[line][i]) >> 4) & 0x0F);
  lineBuffer[line_iterator] =  COLOR_TABLE[nibble];

  nibble = (uint8_t) (GraphicsFrame[line][i] & 0x0F);
  lineBuffer[line_iterator + 1] =  COLOR_TABLE[nibble];

  line_iterator += 2;
}

Comments on the above:

This lookup table will significantly improve both readability and program speed. If this is for an ISR you definitely must use a table like this.
Since it is an embedded system, you should use the integer types from stdint.h.
Beware of implicit integer promotions.
Keep for loops simple and avoid obfuscating them. Use self-explaining variable names for the iterators if you need more than one.
Don't use "magic numbers" like 30.

Thanks a lot for your comments. Yes, I hadn't actually implemented a look up table! It was a bit 'work in progress' but I like your solution and have implemented most of it. I'm still getting problems so think I am going out of bounds somewhere in the ISR — mr_Alex_Nok_, Apr 17 '14 at 13:55

Extracting nibbles from bytes in a for loop

1 Answers1