How to compare a memory bits in C++?

Question

I need help with a memory bit comparison function.

I bought a LED Matrix here with 4 x HT1632C chips and I'm using it on my Arduino Mega2560.

There're no code available for this chipset (it's not the same as HT1632) and I'm writing on my own. I have a plot function that gets x,y coordinates and a color and that pixel turn on. Only this is working perfectly.

But I need more performance on my display, so I tried to make a shadowRam variable that is a "copy" of my device memory. Before I plot anything on display it checks on shadowRam to see if it's really necessary to change that pixel. When I enabled this (getShadowRam) on the plot function my display has some, just SOME (like 3 or 4 on entire display) ghost pixels (pixels that are not supposed to be turned on).

If I just comment the prev_color if's on my plot function it works perfectly.

Also, I'm cleaning my shadowRam array setting all matrix to zero.

Variables:

#define BLACK  0
#define GREEN  1
#define RED    2
#define ORANGE 3
#define CHIP_MAX 8
byte shadowRam[63][CHIP_MAX-1] = {0};

getShadowRam function:

byte HT1632C::getShadowRam(byte x, byte y) {
    byte addr, bitval, nChip;

    if (x>=32) {
        nChip = 3 + x/16 + (y>7?2:0);
    } else {
        nChip = 1 + x/16 + (y>7?2:0);
    }

    bitval = 8>>(y&3);

    x = x % 16;
    y = y % 8;
    addr = (x<<1) + (y>>2);

    if ((shadowRam[addr][nChip-1] & bitval) && (shadowRam[addr+32][nChip-1] & bitval)) {
      return ORANGE;
    } else if (shadowRam[addr][nChip-1] & bitval) {
        return GREEN;
    } else if (shadowRam[addr+32][nChip-1] & bitval) {
        return RED;
    } else {
        return BLACK;
    }
}

Plot function:

void HT1632C::plot (int x, int y, int color)
{
    if (x<0 || x>X_MAX || y<0 || y>Y_MAX)
        return;

    if (color != BLACK && color != GREEN && color != RED && color != ORANGE)
        return;

    char addr, bitval;
    byte nChip;

    byte prev_color = HT1632C::getShadowRam(x,y);

    bitval = 8>>(y&3);

    if (x>=32) {
        nChip = 3 + x/16 + (y>7?2:0);
    } else {
        nChip = 1 + x/16 + (y>7?2:0);
    }

    x = x % 16;
    y = y % 8;
    addr = (x<<1) + (y>>2);

    switch(color) {
        case BLACK:
            if (prev_color != BLACK) { // compare with memory to only set if pixel is other color
                // clear the bit in both planes;
                shadowRam[addr][nChip-1] &= ~bitval;
                HT1632C::sendData(nChip, addr, shadowRam[addr][nChip-1]);
                shadowRam[addr+32][nChip-1] &= ~bitval;
                HT1632C::sendData(nChip, addr+32, shadowRam[addr+32][nChip-1]);
            }
            break;

        case GREEN:
            if (prev_color != GREEN) { // compare with memory to only set if pixel is other color
             // set the bit in the green plane and clear the bit in the red plane;
             shadowRam[addr][nChip-1] |= bitval;
             HT1632C::sendData(nChip, addr, shadowRam[addr][nChip-1]);
             shadowRam[addr+32][nChip-1] &= ~bitval;
             HT1632C::sendData(nChip, addr+32, shadowRam[addr+32][nChip-1]);
            }
            break;

        case RED:
            if (prev_color != RED) { // compare with memory to only set if pixel is other color
                // clear the bit in green plane and set the bit in the red plane;
                shadowRam[addr][nChip-1] &= ~bitval;
                HT1632C::sendData(nChip, addr, shadowRam[addr][nChip-1]);
                shadowRam[addr+32][nChip-1] |= bitval;
                HT1632C::sendData(nChip, addr+32, shadowRam[addr+32][nChip-1]);
            }
            break;

        case ORANGE:
            if (prev_color != ORANGE) { // compare with memory to only set if pixel is other color
                // set the bit in both the green and red planes;
                shadowRam[addr][nChip-1] |= bitval;
                HT1632C::sendData(nChip, addr, shadowRam[addr][nChip-1]);
                shadowRam[addr+32][nChip-1] |= bitval;
                HT1632C::sendData(nChip, addr+32, shadowRam[addr+32][nChip-1]);
            }
            break;
    }
}

If it helps: The datasheet of board I'm using. Page 7 has the memory mapping I'm using.

Also, I have a video of display working.

X_MAX and Y_MAX are undefined. Also, the definition of sendData might prove useful. — Merlyn Morgan-Graham, Dec 24 '10 at 02:59
Is byte an unsigned char? Is the problem that you're using char in some places, and byte in others? — Merlyn Morgan-Graham, Dec 24 '10 at 03:22
X_MAX=63 && Y_MAX=31 and I don't know if char/byte mix is a problem. — Trunet, Dec 24 '10 at 03:37
Things would be a lot simpler if you didn't combine the two shadow-RAM arrays into a single color, but just split the new color into two bits and then compared desired-value against shadow-old-value for each bit separately. — Ben Voigt, Dec 24 '10 at 04:22
Also, no one said you have to organize your shadow RAM the same way as the real device. You'd be much better off having two separate arrays for `shadowGreen` and `shadowRed`. — Ben Voigt, Dec 24 '10 at 04:54

Merlyn Morgan-Graham · Answer 1 · 2010-12-24T06:19:05.803

This isn't a real answer, but I think it might be a step towards figuring this out. Since there is so much code duplication and confusing conditional code, you should start with a refactor. It will then be much easier to understand the algorithm. I've taken a stab at it, though no promises that it will be bug free.

Get rid of getShadowRam, and modify plot to look like this:

void HT1632C::plot (int x, int y, byte color)
{
  if (x < 0 || x > X_MAX || y < 0 || y > Y_MAX)
    return;

  if (color != BLACK && color != GREEN && color != RED && color != ORANGE)
    return;

  // using local struct to allow local function definitions
  struct shadowRamAccessor {
    shadowRamAccessor(byte x, byte y) {
      nChip = (x >= 32 ? 3 : 1)
        + x / 16
        + (y > 7 ? 2 : 0);
      bitval = 8 >> (y & 3);
      addr = ((x % 16) << 1) + ((y % 8) >> 2);
      highAddr = addr + 32;
    }

    byte& getShadowRam(byte addr) {
      return shadowRam[addr][nChip-1];
    }

    byte getPreviousColor() {
      byte greenComponent = getShadowRam(addr) & bitval ? GREEN : BLACK;
      byte redComponent = getShadowRam(highAddr) & bitval ? RED : BLACK;
      return greenComponent | redComponent;
    }

    void setValue(byte newColor) {
      byte prev_color = getPreviousColor();
      if(newColor != prev_color)
        setValue(newColor & GREEN, newColor & RED);
    }

    void setValue(bool greenBit, bool redBit)
    {
      HT1632C::sendData(nChip, addr,
        greenBit
          ? getShadowRam(addr) |= bitval
          : getShadowRam(addr) &= !bitval
        );
      HT1632C::sendData(nChip, highAddr,
        redBit
          ? getShadowRam(highAddr) |= bitval
          : getShadowRam(highAddr) &= ~bitval
        );
    }

    byte nChip, bitval, addr, highAddr;
  };

  shadowRamAccessor(x, y).setValue(color);
}

I think the point is to avoid overwriting bytes that didn't change. Maybe there's an `if` statement missing in `setValue(byte)`, currently `prev_color` is assigned and never read. — Ben Voigt, Dec 24 '10 at 04:24
C++ is not my beach! the code didn't compile :( and I don't know why. sorry. — Trunet, Dec 24 '10 at 04:49
@Trunet: I'm just doing this to help make the memory mapping logic more obvious. Out of curiosity, what error(s) did you get? — Merlyn Morgan-Graham, Dec 24 '10 at 06:20

score 1 · Answer 2 · answered Dec 24 '10 at 01:36

1

Uhm.. Probably I'm missing something here, but why don't you change that big switch to:


if(color != prev_color)
{
    shadowRam[addr][nChip-1] |= bitval;
    HT1632C::sendData(nChip, addr, shadowRam[addr][nChip-1]);
    shadowRam[addr+32][nChip-1] &= ~bitval;
    HT1632C::sendData(nChip, addr+32, shadowRam[addr+32][nChip-1]);
}

answered Dec 24 '10 at 01:36

yurymik

2,194
20
14

You are missing something, the `case` s have different combinations of `|=` and `&=` – caf Dec 24 '10 at 02:03
Oh I see. Anyway, I don't quite understand the logic behind this code. Why do we set/clear `bitval` that's based on value of `y&3`, but not `color`? Does it have some additional information encoded into? – yurymik Dec 24 '10 at 02:45
1

the chipset I'm using uses a memory mapping like this: 32 addresses, each one has 4 bits. the first 16 addresses are GREEN, the last 16 are RED. each bit of each address is a COLUMN on my display. if I enable GREEN and RED I got ORANGE. if I turn off everything I get BLACK that is no LED turned on. – Trunet Dec 24 '10 at 03:41

How to compare a memory bits in C++?

2 Answers2