What am I doing wrong here? - Bit Manipulation

I have a very specific issue.

I'm doing a project for school which is simply a Ping Pong game using a 5x7 Led Matrix in Arduino UNO.

The problem is with the part of the ball, I can make it bounce no problem in the X position, but there is something weird going in with the Y position.

Here is the code just for the ball:

long prevMoveTime = millis(), currentMoveTime; // Variables for asynchronous drawBall method.
int ballPosX = B00100, ballPosY = B1110111; // X & Y positions of the ball with their initial values.
bool ballDirX = true, ballDirY = true; // Direction on the ball (true = left/up, false = right/down).

void setup() {
  for (int i = 0; i <= 13; i++) { pinMode(i, OUTPUT); }

  PORTB = ballPosX;
  PORTD = ballPosY;
}

void drawBall() {
  currentMoveTime = millis();

  switch (ballPosX) {
    case B10000: ballDirX = true; break;
    case B00001: ballDirX = false; break;
  }

  switch (ballPosY) {
    case B0111111: ballDirY = true; break;
    case B1111110: ballDirY = false; break;
  }

  if (currentMoveTime - prevMoveTime > 1000) {
    if (ballDirX) { ballPosX >>= 1; }
    else { ballPosX <<= 1; }

    PORTB = ballPosX;

    if (ballDirY) { ballPosY >>= 1; ballPosY |= B1000000; }
    else { ballPosY <<= 1; ballPosY |= B0000001; }

    PORTD = ballPosY;

    prevMoveTime = currentMoveTime;
  }
}

void loop() {
  drawBall();
}

So basically, there is something weird going on where it won't read the binary value B0111111 and thus the ballDirY bool won't change value.

I've tried about everything I could:

Using an if instead of a switch.
Swapping the cases.
Swapping the true/false assignments.
Using a delay instead of the asynchronous trick.
Tried using the serial monitor to debug but what it shows me has nothing to do with what I see in the led matrix.
Tried using decimal numbers instead of binary.
Tried using '0b' instead of 'B' to use the binary values.

The first conclusion I've come to is that something is happening with the B0111111 value, the row in the matrix does receive the signal so clearly ballPosY = B0111111, but the switch either won't read it or ballDirY won't change value.

The second conclusion I've come to is that I'm simply stupid and there is something I haven't noticed lol.

Thanks in advance for any answers and fell free to let me know if I'm just dumb lmao.

Solution

When you shift left, you are not clearing out unused bits (7-15). On Arduino, int is 16 bits, so, for example, when you shift 1000001 << 1 you get 10000010 and not 0000010, as you seem to be expecting.

To clear the bits, make a mask with the bits you want to use set and the extra bits clear. In your code this would be B0000000001111111 Then, & (AND) that with your value.

Here's an example program that DOES NOT clear the bits, to demonstrate the error:

#include <iostream>
#include <bitset>
#include <cstdint>

enum {UP, DOWN};
const uint16_t ALL_ROWS        = 0b1111111;
const uint16_t TOP_ROW_MASK    = 0b0111111;
const uint16_t BOTTOM_ROW_MASK = 0b1111110;
const uint16_t TOP_ROW         = 0b1000000;
const uint16_t BOTTOM_ROW      = 0b0000001;

int main()
{
    uint16_t ballPosY = 0b1110111;
    uint8_t ballDirY = UP;
    
    for (int i = 0; i < 20; i++) {
        if (ballPosY == TOP_ROW_MASK) {
            ballDirY = DOWN;
        }
        else if (ballPosY == BOTTOM_ROW_MASK) {
            ballDirY = UP;
        }
        
        if (ballDirY == DOWN) {
            ballPosY = (ballPosY >> 1) | TOP_ROW;
            std::cout << "DOWN: ";
        }
        else {
            ballPosY = ((ballPosY << 1) | BOTTOM_ROW);
            std::cout << "UP  : ";
        }
        
        // Use a bitset for printing
        std::bitset<16> bits(ballPosY);
        std::cout << bits << '\n';
    }

    return 0;
}

And the output is:

UP  : 0000000011101111
UP  : 0000000111011111
UP  : 0000001110111111
UP  : 0000011101111111
UP  : 0000111011111111
UP  : 0001110111111111
UP  : 0011101111111111
UP  : 0111011111111111
UP  : 1110111111111111
UP  : 1101111111111111
UP  : 1011111111111111
UP  : 0111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111
UP  : 1111111111111111

If you take the code above and replace

ballPosY = ((ballPosY << 1) | BOTTOM_ROW);

With

ballPosY = ((ballPosY << 1) | BOTTOM_ROW) & ALL_ROWS;

the output will be correct:

UP  : 0000000001101111
UP  : 0000000001011111
UP  : 0000000000111111
DOWN: 0000000001011111
DOWN: 0000000001101111
DOWN: 0000000001110111
DOWN: 0000000001111011
DOWN: 0000000001111101
DOWN: 0000000001111110
UP  : 0000000001111101
UP  : 0000000001111011
UP  : 0000000001110111
UP  : 0000000001101111
UP  : 0000000001011111
UP  : 0000000000111111
DOWN: 0000000001011111
DOWN: 0000000001101111
DOWN: 0000000001110111
DOWN: 0000000001111011
DOWN: 0000000001111101

Note: in my code I use a bunch of constants to make the code easier to read. I also use 0b to designate a binary number since my compiler does not support B.

Also, I use uint16_t instead of int to ensure the value is 16 bits, like on the Arduino. It is also unsigned. I recommend always using unsigned for bit masks.