Noob needs help with bit masking

OK so I am trying to break apart an eight bit message in CAN I can read CAN and pass the message no problem but for my application I am trying to send my arduino board CAN messages that it can interpret into setting certain relays on but I get the message but i can’t mask it properly to separate out each position there is a one or zero. I started out with a simple method of shifting left to handle one bit at a time and masking all other bits but for some reason I get a one being set for four adjacent positions from 33 to 37 but everywhere else I get the correct outcome.
I pasted some code encase I didn’t explain it well
I do understand that some comments make no sense, they are from earlier revision and I haven’t changed them also I do know I have a lot of temporary variable but I am just using them for testing purposes

void loop() {

  int relay[50];
  int relayIndex = 0;
  //char* test;
  int test[8];
  int state1 = 0;
  int state2 = 0;
  int temp1 = 0; // the number left over from masking
  int temp2 = 0; //index for the masking
  int index1 = 8; // index in bits
  int index2 = 0;  // the index of where we are in variable buffer's length

    // test = Canbus.set_relay(buffer);    //This is the one we will use to receieve can messages

//      while(temp2 < 8){
//        test[temp2] = 0x01;
//        temp2++; }
                                                test[0] = 0x01;
                                    test[1] = 0x01;
                                    test[2] = 0x01;
                                    test[3] = 0x01;
                                    test[4] = 0x01;
                                    test[5] = 0x01;
                                    test[6] = 0x01;
                                    test[7] = 0x01;

     if(test[0] == 0x01) {
       //temp2 = 0x80;
       for(index1 = 0; index1<= 7; index1++) {
         for(index2 = 7; index2>=0; index2--) {
           temp1 = test[index1];
          temp1 = temp1 >> index2;
           temp1 = temp1 && 0x01;
             if( temp1 == 1){
              relay[relayIndex] = 1;}
             
               relayIndex++;
         }
       }
     }   
       if(relay[50] == 1){
       state2 = 0;
       state1 = 1;     
       }
       
     else { 
       state2 = 1;
       state1 = 0;
       
     }

     digitalWrite(LED3, state1);
     digitalWrite(LED2, state2);
     delay(100);

I have no idea what you're saying, but "relay[50] == 1" is out of bounds.
Could you maybe reframe your question with some punctuation?

void loop() {
relayIndex = -1;
            test[0] = 0xff;
          test[1] = 0x01;
          test[2] = 0x01;
          test[3] = 0x01;
          test[4] = 0x01;
          test[5] = 0x01;
          test[6] = 0x01;
          test[7] = 0x01;

       for(index1 = 0; index1<= 7; index1++) {
         for(index2 = 7; index2>=0; index2--) {
           relayIndex++;
           temp1 = test[index1];

          temp1 = temp1 >> index2;
           temp1 = temp1 && 0x01;
             
             if( temp1 == 1){
              relay[relayIndex] = 1;
            }

           if(relayIndex > 49) {
               relayIndex = -1;
             break; }       
         }

       }
        
       if(relay[0] == 1){
       state2 = 0;
       state1 = 1;     
       }
       
     else { 
       state2 = 1;
       state1 = 0;
       
     }
  
     digitalWrite(LED3, state1);
     digitalWrite(LED2, state2);
     delay(1000); 

}

Ok I have this integer array and I want to be able to break it down to individual bits then see which bits are ones and which bits are zeros. For every bit that is one I want to store it in the relay array in different position based on bit number.

Eight times eight is sixty-four, but "relay" is only 50 bytes long.

void loop() {
relayIndex = -1;
            test[0] = 0xff;
          test[1] = 0x01;
          test[2] = 0x01;
          test[3] = 0x01;
          test[4] = 0x01;
          test[5] = 0x01;
          test[6] = 0x01;
          test[7] = 0x01;

       for(index1 = 0; index1<= 7; index1++) {
         for(index2 = 7; index2>=0; index2--) {
             relayIndex++;
           if(relayIndex > 49) {
                        break; }

           temp1 = test[index1];

          temp1 = temp1 >> index2;
           temp1 = temp1 && 0x01;
            
             if( temp1 == 1){
              relay[relayIndex] = 1;
            }
      
         }

       }
        
       if(relay[0] == 1){
       state2 = 0;
       state1 = 1;    
       }
      
     else {
       state2 = 1;
       state1 = 0;
      
     }
  
     digitalWrite(LED3, state1);
     digitalWrite(LED2, state2);
     delay(1000);

}

Ok I changed it so that it never goes out of bounds for the array but still running into problems

I also tried a more simplified way but I am still getting stuck. I get the first byte to work fine but all the other bytes of test are not working. Am I incrementing index1 correctly.

    while(index1 < 8) {
    temp2 = 0x80;
    
    temp1 = test[index1];

  while(index2 < 8) {
    relayIndex++;
 if(relayIndex == 49)
      break;
    clicker = temp1;
    temp1 &= temp2;
    if(temp1 == temp2){
      relay[relayIndex] = 1;
    }
    temp2 = temp2 / 0x02;
    index2++;

    temp1 = clicker;
        }
    index1++;
    }
       if(relay[8] == 1){
       state2 = 0;
       state1 = 1;     
       }
       
     else { 
       state2 = 1;
       state1 = 0;
       
     }
            test[0] = 0xf1;
          test[1] = 0x81;
          test[2] = 0x81;
          test[3] = 0x01;
          test[4] = 0x01;
          test[5] = 0x01;
          test[6] = 0x01;
          test[7] = 0x01;

  while(index < 8) {
    maskValue = 0x80;
    msgByte = test[index];
    index++;
    counter= 0;
    
    while(counter < 8) {
       relayIndex++;
       if(relayIndex == 49){
          break;  }
                
       clicker = msgByte;
       msgByte &= maskValue;
                
       if(msgByte == maskValue){
          relay[relayIndex] = 1;
          }
                
        maskValue = maskValue / 0x02;
        counter++;
        msgByte = clicker;
        }
  
      }

I found out what I did wrong I forgot to put the counter back to zero after going to eight

No, the “for” loop version is much easier to read, once you get your indentation correct.

I’d put the assignment of temp1 into the outer loop, or get rid of it altogether - the compiler should be doing the optimisation, not you.

    for(index1 = 0; index1 < 8; index1++) {
         for(byte bitmask = 0x80; bitmask; bitmask>>=1, relayIndex++) {
             if(relayIndex > 49) {
                        break; 
             }
             relay[relayIndex] = !!( test [index1] & bitmask);
        }
    }

Untested code fragment -

#include <limits>

int value_at_bit(unsigned char array[], int bit_index)
{
    return ((array[bit_index / CHAR_BIT] & (b00000001 << (bit_index % CHAR_BIT))) ? 1 : 0);
}


unsigned char   array[50];

for ( int i = 0; i < sizeof(array) / sizeof(array[0]); i++ )
{
    int bit_value = value_at_bit(array, i);
    ...
}

No, the “for” loop version is much easier to read, once you get your indentation correct.

I’d put the assignment of temp1 into the outer loop, or get rid of it altogether - the compiler should be doing the optimisation, not you.

Code:

for(index1 = 0; index1 < 8; index1++) {
        for(byte bitmask = 0x80; bitmask; bitmask>>=1, relayIndex++) {
            if(relayIndex > 49) {
                       break;
            }
            relay[relayIndex] = !!( test [index1] & bitmask);
       }
   }

Thanks I used that method it also helped me with another problem so double thanks!