array value changing mysteriously

I have almost finished my most ambitious project yet, with some help from you guys along the way.

It is an inline hockey scoreboard, with 2 teamnames of eight 7x5 LED arrays, plus a whole load of bcd switches for scores, penalties, players number, period etc.

There is also an LCD display and buttons to set the teamnames, or select from a library of previous names. Thats working fine.

When I transmit to the scoreboard, a 22 or 23 byte message is sent with a digit to say if it is a number command ( 35 or # ) or a letter command ( 42 or * ) followed by the appropriate data.

It starts well enough, you can set both the teamnames, and the age group display, but as soon as I send a score update, or set time, the array that I have stored the letters in becomes corrupt, and puts one of the number data into one of my stored letter data. Which gives a strangely spelt new name.
here is the main loop, with a note to show where the problem arises , the buf[2] of the number data, somehow gets written to the buf[21] of the letter array.

( I monitored the free memory and there are 372 bytes spare )

I have attached the whole code, its really messy and needs tidying up when I can get some free time.

I think I have overlooked something stupid as usual, anybody spot it ?

 void loop(){
  
    if ( standby == HIGH ) {  
      blankalpha ();     // clear teamnames
      blanknumbers ();   // clear numbers
    }  
    else  { 
      showpenalties ();  
    }
  
  
    checktime ();  //  clock countdown
    swapdisplay();  // switch alpha displays between teamname and age group
  
    uint8_t buf[VW_MAX_MESSAGE_LEN];  //  get message
    uint8_t buflen = VW_MAX_MESSAGE_LEN;
    if (vw_get_message(buf, &buflen)) // Non-blocking
    {
      Serial.println("Got: ");  // Show on PC for debugging
      Serial.print(" buflen =   ");
      Serial.println( int (buflen));
      for ( int h = 0; h <=buflen-1; h++ ) { Serial.print(" buffer =   ");
        Serial.print(h); Serial.print( "  =  "); Serial.println( int(buf [h]));
      }
      if ( buf [0] == thisPin ) { //  checking device number
        Serial.println(" pins match   ");   
        standby = LOW;  //  wake up board 
        if ( buf [1] == 42 ) {    //  = * ALPHA DATA COMING IN  
          
          for ( int y = 2; y <=22; y++ ) {
            letterstore [y] = buf [y];   // save incoming names as buf will be changing with scores etc
            Serial.print(" letterstore  "); Serial.print(y); 
            Serial.print("  = ");   Serial.println(char ( letterstore [y ]));
          }   //   *****   at this point letterstore 21 is correct as received in the buffer unless
          //  a number button has been pressed, n which case it changes to the new buf[2] value ( key )*****
        
          group = LOW;  // 
          showteamnames ();
         
        } //  end of if ( buf [1] == '*' )
        //***************************************************************************     
        if ( buf [1] == 35 )  // =  # NUMERIC DATA INCOMING
        {  
          key = buf [2];   
          Serial.print(" key = ");
          Serial.println( int (key));   
  
          for ( int s = 1; s<= 19; s++ ) { 
            rxnumb [s] = buf [s+2] ;      
            Serial.print(" rxnumb  ");   
            Serial.print(s);  
            Serial.print(" = ");  
            Serial.println( int ( rxnumb [s]));
            penminT =  rxnumb[7]; 
            penminU = rxnumb[8]; 
  
          }
          sortnumbers ();
          refreshpenalties ();
        }  // end of if ( buf [1] == '35' )  //  NUMERIC DATA INCOMING 
      }
  
      Serial.print("freeMemory()=");
      Serial.println(freeMemory());
    }  //  end of if message
  } // end loop

ap9rxforum.pde (36.4 KB)

That can happen if you write to an array at an index larger than it was initialised for. There are no array boundary checks at run time so this can happen if you are calculating an array index to use.
You can however write your own array index boundary checks before you actually do the storing.

Thanks Mike, I thought we had it there, I saw that I had initialised the buffer array, which is set by buflen, so I removed that and its still the same.

The other buffers I think I have set right ?

I will have to leave it for now, other screaming jobs to get onto, and I will have another look during he night.

Hopefully some one will spot my errors.

This is suspect:

          for ( int y = 2; y <=22; y++ ) {
            letterstore [y] = buf [y];

letterstore is declared to be 22 characters but you are storing a value in letterstore[22] which is off the end of the array.
There may be other similar problems.

Pete

Thanks Pete

let me play with that.

GIVE THAT MAN A BELLS ! ( its an advert we have in South Africa to honour a great job done )

I mistook the definition as the range of variables, when its actually the total addresses ? of them ( I dont know the lingo )

So I changed letterstore to [23] and all is fine, Thanks indeed !

And Grumpy Mike , you were on the right track there, I have bumped your karma up to 499 !

Boffin1:
So I changed letterstore to [23] and all is fine, Thanks indeed !

As Pete suggested did you check the rest of your code?

  [...]
  byte rxnumb [19 ];
  [...]
  
          for ( int s = 1; s<= 19; s++ ) { 
            rxnumb [s] = buf [s+2] ;      
  [...]
            Serial.print(" rxnumb  ");   
  [...]

Just in case you didn’t find it :slight_smile:

Regards,

Brad
KF7FER

It’s very common to use a for loop to process elements in an array, and the conventional way to implement that is using a < operator instead of a <= operator. That means you can compare against the number of elements in the array, and stop correctly at the last element. Any time you see a for loop being used to process an array using a <= operator for the end condition, that should make you stop and check your logic.

You need to understand that in C and C++ and Java, an array with N elements has indexed elements [0] to [N-1].

This is non-obvious to beginners and also non-obvious to people whose first computer language was Cobol or Fortran or Basic, which would have array elements from [1] to [N}.

You should always be careful with loops accessing arrays, for this issue. The preferred style seems to be, using < in for loop termination criteria, rather than <=

int a[10] ;
for ( int i=0 ; i<10 ; i++ )          //  don't put  i <= 10 here
{
    a[i] = i ;
}

[0]

Thanks all, I will go over it all again this morning,

I was using for ( x = 2 , x<=22,x++ ) thinking about it having 20 positions, but of course the array will allocate all 23 .

Perhaps it is best that I always use a x=0 to x < final, and then add 2 afterwards, I can see what is happening in the array then.

I love this learning, and especially all the help available on the forum.

Thanks Brad for spotting the byte rxnumb [19 ];, same error on my part, I have made it 20.

“don’t put i <= 10 here” thats just something I have copied, I couldnt really see the point of it