Issue reproducing commands stored in vectors

Hi everyone,

I hope you can help me on this task. I'm using a PS2 joypad to control the outputs of an Arduino UNO. The general working flow is as follows:

  • The directional buttons are used to control three leds (up = yellow led, left = red led, green = right led);
  • When any of these buttons are pressed a delay time counter is started to register for how long it has been hold;
  • When the button is released the delay time and an indicator of which button was activated are stored in memory vectors (up to 20 memory positions each).

The recording memory mode is toggled ON/OFF by the SELECT button. A playback mode is activated by the START button and only works when the recording mode is OFF. The playback mode should reproduce the directions and the delay times recorded in the memory vectors backwards, and here is where my code fails. The memory vectors are being stored correctly, but sometimes after executing the playback mode -- playMemory() function -- the program automatically toggles ON the recording mode and stores all three directions in the memory with a very short delay time, which is related to the delay() command at the end of the loop() function. However, when the while() loop is removed from the playMemory() function this behaviour does not happen.

Can any of you understand why this is happening? I guess that it may be cause of misusing of the millis() function as timer somehow, but I don't see how to correct it. Probably there is another way to do this using the timer library, by I'll also include servos and a buzzer in the final project. According to some documentation I've been reading it seems that I can't use this library if the project already includes servos, buzzers and millis() or delay() functions, am I right?

This is the whole code:


#include <PS2X_lib.h>

#define BR 10 // white led - indicator for recording mode ON/OFF
#define VM 11 // red led - left button
#define AM 12 // yellow led - up button
#define VD 13 // green led - right button

int memTimes[20]; // delay time for each command
char memDirections[20]; // records which button is pressed
int memPosition = 0; // position in memory vector
unsigned long delayTime = 0; // variable for delay time counting
bool memoryModeCurr = LOW; // current state for the memory mode button
bool memoryModePrev = LOW; // previous state for the memory mode button

void playMemory();
void recordMemory();

PS2X CONTROLE;

void setup() {
Serial.begin(9600);

pinMode(BR, OUTPUT);
pinMode(VM, OUTPUT);
pinMode(AM, OUTPUT);
pinMode(VD, OUTPUT);
CONTROLE.config_gamepad(2,4,3,5,false,false); // (clock, command, attention, data, Pressures?, Rumble?)
}

void loop() {
static bool recordingMode; // state of the recording Mode

CONTROLE.read_gamepad();

// ON|OFF for the recording mode

memoryModeCurr = CONTROLE.ButtonPressed(PSB_SELECT);
if (memoryModeCurr && !memoryModePrev) {
recordingMode = !recordingMode; // Sometimes after executing playMemory() this state changes by itself, sometimes it doesn't. Why?
Serial.println("recordingMode state has changed.");
}
else {
digitalWrite(BR, LOW);
}
memoryModePrev = memoryModeCurr;

if (recordingMode && (memPosition < 20)) recordMemory();
if (CONTROLE.ButtonPressed(PSB_START) && (memPosition > 0) && !recordingMode) playMemory();

delay(25);
}

void playMemory() {
Serial.println("Playback mode activated.");
for (int mP=memPosition; mP>=0; mP--){
Serial.println(memDirections[mP]);
delayTime = millis();
while ((millis()-delayTime) <= memTimes[mP]) { // if this loop is removed, the issue does not happen

}
}
delayTime = millis();
}

void recordMemory() { // This function works.
digitalWrite(BR, HIGH);

// UP BUTTON
if (CONTROLE.ButtonPressed(PSB_PAD_UP)) { // gets the starting time
delayTime = millis();
}
if (CONTROLE.Button(PSB_PAD_UP)) { // turns on the led while the button is being pressed
digitalWrite(AM, HIGH);
}
if (CONTROLE.ButtonReleased(PSB_PAD_UP)) { // turns off the led and stores the button pressed and the delay time in the memory vector
digitalWrite(AM, LOW);
memTimes[memPosition] = millis() - delayTime;
memDirections[memPosition] = 'U';
memPosition++;
}

// LEFT BUTTON
if (CONTROLE.ButtonPressed(PSB_PAD_LEFT)) { // gets the starting time
delayTime = millis();
}
if (CONTROLE.Button(PSB_PAD_LEFT)) { // turns on the led while the button is being pressed
digitalWrite(VM, HIGH);
}
if (CONTROLE.ButtonReleased(PSB_PAD_LEFT)) { // turns off the led and stores the button pressed and the delay time in the memory vector
digitalWrite(VM, LOW);
memTimes[memPosition] = millis() - delayTime;
memDirections[memPosition] = 'L';
memPosition++;
}

// RIGHT BUTTON
if (CONTROLE.ButtonPressed(PSB_PAD_RIGHT)) { // gets the starting time
delayTime = millis();
}
if (CONTROLE.Button(PSB_PAD_RIGHT)) { // turns on the led while the button is being pressed
digitalWrite(VD, HIGH);
}
if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT)) { // turns off the led and stores the button pressed and the delay time in the memory vector
digitalWrite(VD, LOW);
memTimes[memPosition] = millis() - delayTime;
memDirections[memPosition] = 'R';
memPosition++;
}

// OUTPUTS
if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT) || CONTROLE.ButtonReleased(PSB_PAD_LEFT) || CONTROLE.ButtonReleased(PSB_PAD_UP)){
Serial.print("Delay Time: ");
Serial.println(memTimes[memPosition-1]);

Serial.print("Button pressed: ");
Serial.println(memDirections[memPosition-1]);

Serial.print("Memory vector position: ");
Serial.println(memPosition-1);

Serial.print("Everything: ");
for (int k=0; k<memPosition; k++){
Serial.print(memTimes[k]);
Serial.print(" -- ");
Serial.println(memDirections[k]);
}
Serial.println();
}
}


And this is an example of the output:

recordingMode state has changed.
Delay Time: 605
Button pressed: U
Memory vector position: 0
Everything: 605 -- U

Delay Time: 526
Button pressed: L
Memory vector position: 1
Everything: 605 -- U
526 -- L

Delay Time: 474
Button pressed: R
Memory vector position: 2
Everything: 605 -- U
526 -- L
474 -- R

recordingMode state has changed.
Playback mode activated.

R
L
U <-- This time it worked correctly.
Playback mode activated.

R
L
U <-- This time it worked correctly.
Playback mode activated.

R
L
U
recordingMode state has changed. <-- This time it didn't! It toggled ON the recording mode by itself.
Delay Time: 27
Button pressed: R
Memory vector position: 5
Everything: 605 -- U
526 -- L
474 -- R
27 -- U <-- Related to delay(25)
27 -- L <-- Related to delay(25)
27 -- R <-- Related to delay(25)

I appreciate any help that you can give me. Comments about code optimizing are also welcome if you have some.

What prevents "memPosition" from incrementing past 20?

up = yellow led, left = red led, green = right led

So, you have a yellow LED, a red LED, and a right LED. You have three switches labeled UP, LEFT, and GREEN.

Really?

  • When any of these buttons are pressed a delay time counter is started to register for how long it has been hold;

A delay time counter? Why on earth are you even THINKING delay?

  • When the button is released the delay time

There's that brain-dead delay thinking again.

int memTimes[20];               // delay time for each command
char memDirections[20];         // records which button is pressed

It's NOT a delay time. It's a HOLD time.

Are any of the hold times EVER going to exceed 32 seconds? Is the time in seconds, hours, weeks? Why are you allowing for negative times?

int memPosition = 0;            // position in memory vector

Usually referred to as an index...

if (memoryModeCurr && !memoryModePrev) {
   recordingMode = !recordingMode;           // Sometimes after executing playMemory() this state changes by itself, sometimes it doesn't. Why?
   Serial.println("recordingMode state has changed.");
 }
 else {
   digitalWrite(BR, LOW);
 }
 memoryModePrev = memoryModeCurr;

Look at the state change detection example. This is NOT how it should be done.

First, you look for a change (memoryModeCurr != memoryModePrev), and THEN you look at whether the change was to pressed or to released.

Don't try to do that all in one if statement.

Serial.println("Playback mode activated.");
 for (int mP=memPosition; mP>=0; mP--){
   Serial.println(memDirections[mP]);

Suppose that you have recorded 20 switch numbers and hold times. What value will be in memPosition? Pay VERY careful attention to when you change memPosition. Will the value in memPostion be a valid array index value?

Reading or writing out of bounds of an array is a VERY bad idea (especially writing).

What prevents "memPosition" from incrementing past 20?

 if (recordingMode && (memPosition < 20)) recordMemory();

you have a yellow LED, a red LED, and a right LED. You have three switches labeled UP, LEFT, and GREEN.

That was a typo; those switches are labeled UP, LEFT, RIGHT.

A delay time counter? Why on earth are you even THINKING delay?

Labeling mistake. I was thinking about it as a substitute for the delay(), so I just kept the word 'delay' in mind. It works as a hold time, indeed.

Reading or writing out of bounds of an array is a VERY bad idea (especially writing).

I'm aware of that. This code is just for testing the recording/playback logic, thus for now I just know I can't use more than 20 indexes and it's ok. However, the final version will have the proper condition to avoid it.

Look at the state change detection example. This is NOT how it should be done.

I'll follow that example for a better code.

Despite that, with this code the white LED state and the recording mode state are switching correctly. How could the recording mode be switched ON if I'm not pressing the correspondent button to do so? In a single run it worked twice and then it didn't, as shown in the output example, but nothing changed between each try except for pressing the START button to execute the playback. Even if there is an issue with the recording Mode ON/OFF state changing, you can see that the memory vector is being written like if the UP, LEFT and RIGHT buttons were held during 27 milliseconds each, and I didn't do that. Moreover, they are recorded in the same order that they appear in the recordMemory() function:

// UP BUTTON
 if (CONTROLE.ButtonPressed(PSB_PAD_UP)) {     // gets the starting time
   delayTime = millis();
 }
 if (CONTROLE.Button(PSB_PAD_UP)) {            // turns on the led while the button is being pressed
   digitalWrite(AM, HIGH);
 }
 if (CONTROLE.ButtonReleased(PSB_PAD_UP)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
   digitalWrite(AM, LOW);
   memTimes[memPosition] = millis() - delayTime;
   memDirections[memPosition] = 'U';
   memPosition++;
 }

 // LEFT BUTTON
 if (CONTROLE.ButtonPressed(PSB_PAD_LEFT)) {     // gets the starting time
   delayTime = millis();
 }
 if (CONTROLE.Button(PSB_PAD_LEFT)) {            // turns on the led while the button is being pressed
   digitalWrite(VM, HIGH);
 }
 if (CONTROLE.ButtonReleased(PSB_PAD_LEFT)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
   digitalWrite(VM, LOW);
   memTimes[memPosition] = millis() - delayTime;
   memDirections[memPosition] = 'L';
   memPosition++;
 }

 // RIGHT BUTTON
 if (CONTROLE.ButtonPressed(PSB_PAD_RIGHT)) {     // gets the starting time
   delayTime = millis();  
 }
 if (CONTROLE.Button(PSB_PAD_RIGHT)) {            // turns on the led while the button is being pressed
   digitalWrite(VD, HIGH);
 }
 if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
   digitalWrite(VD, LOW);
   memTimes[memPosition] = millis() - delayTime;
   memDirections[memPosition] = 'R';
   memPosition++;
 }

which makes me think that somehow the program is ignoring all conditions and also executing the commands corespondent to the directional buttons anyway. How could it be possible?

How could the recording mode be switched ON if I'm not pressing the correspondent button to do so?

Because you ARE reading outside the bounds of your array.

Go back and re-read my comments about paying VERY careful attention.

PaulS:
Because you ARE reading outside the bounds of your array.

Go back and re-read my comments about paying VERY careful attention.

Sorry Paul, but I still don't get it. I found the error in the initial condition in the for loop and changed it to memPosition-1, because it was starting from a non recorded index. Now a simple version of playMemory() is as follows:

void playMemory() {
  Serial.println("Playback mode activated.");
  
  for (int mP=memPosition-1; mP>=0; mP--) {
    Serial.println(memTimes[mP]);
//    delay(memTimes[mP]);
  }
}

Using Serial.println(memTimes[mP]) to show the recorded values in the serial monitor doesn't trigger the issue. However, if I use delay(memTimes[mP]) to freeze the execution that amount of milliseconds the issue appears. Aren't both commands reading from the same indexes in the array? How one of them works correctly and the other reads out of the bounds?

Sorry Paul, but I still don't get it. I found the error in the initial condition in the for loop and changed it to memPosition-1, because it was starting from a non recorded index.

So, you DID get it.

However, if I use delay(memTimes[mP]) to freeze the execution that amount of milliseconds the issue appears. Aren't both commands reading from the same indexes in the array? How one of them works correctly and the other reads out of the bounds?

Might I suggest that the fine folks at http://snippets-r-us.com might be better equipped to help you with snippets?

Sorry, this is the whole code:

#include <PS2X_lib.h>

#define BR 10   // white led - recording mode ON/OFF indicator
#define VM 11   // red led - left button
#define AM 12   // yellow led - up button
#define VD 13   // green led - right button

unsigned long memTimes[20];     // hold time for each command
char memDirections[20];         // records which button is pressed
int memIndex = 0;               // index in memory vector
unsigned long startTime = 0;    // hold time counting
bool memoryModeCurr = LOW;      // current state for the memory mode button
bool memoryModePrev = LOW;      // previous state for the memory mode button

void playMemory();
void recordMemory();

PS2X CONTROLE;

void setup() {
  Serial.begin(9600);
  
  pinMode(BR, OUTPUT);
  pinMode(VM, OUTPUT);
  pinMode(AM, OUTPUT);
  pinMode(VD, OUTPUT);
  CONTROLE.config_gamepad(2,4,3,5,false,false);   // (clock, command, attention, data, Pressures?, Rumble?)
}

void loop() {
  static bool recordingMode;      // state of the recording Mode
  
  CONTROLE.read_gamepad();

  // ON|OFF for the recording mode
  memoryModeCurr = CONTROLE.ButtonPressed(PSB_SELECT);
  if (memoryModeCurr != memoryModePrev) {
    if (memoryModeCurr == HIGH) {
      recordingMode = !recordingMode;
      Serial.println("recordingMode state has changed.");
    }
    delay(50);
  }
  memoryModePrev = memoryModeCurr;

  if (recordingMode && (memIndex < 20)) {
    digitalWrite(BR, HIGH);
    recordMemory();
  }
  else{
    digitalWrite(BR, LOW);
  }
  
  if (CONTROLE.ButtonPressed(PSB_START) && (memIndex > 0) && !recordingMode) playMemory();

  delay(25);
}

void playMemory() {
  Serial.println("Playback mode activated.");
  
  for (int mP=memIndex-1; mP>=0; mP--) {
    Serial.println(memTimes[mP]);
//    delay(memTimes[mP]);
  }
}

void recordMemory() {    //  This function works.
  // UP BUTTON
  if (CONTROLE.ButtonPressed(PSB_PAD_UP)) {     // gets the starting time
    startTime = millis();
  }
  if (CONTROLE.Button(PSB_PAD_UP)) {            // turns on the led while the button is being pressed
    digitalWrite(AM, HIGH);
  }
  if (CONTROLE.ButtonReleased(PSB_PAD_UP)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
    digitalWrite(AM, LOW);
    memTimes[memIndex] = millis() - startTime;
    memDirections[memIndex] = 'U';
    memIndex++;
  }

  // LEFT BUTTON
  if (CONTROLE.ButtonPressed(PSB_PAD_LEFT)) {     // gets the starting time
    startTime = millis();
  }
  if (CONTROLE.Button(PSB_PAD_LEFT)) {            // turns on the led while the button is being pressed
    digitalWrite(VM, HIGH);
  }
  if (CONTROLE.ButtonReleased(PSB_PAD_LEFT)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
    digitalWrite(VM, LOW);
    memTimes[memIndex] = millis() - startTime;
    memDirections[memIndex] = 'L';
    memIndex++;
  }

  // RIGHT BUTTON
  if (CONTROLE.ButtonPressed(PSB_PAD_RIGHT)) {     // gets the starting time
    startTime = millis();  
  }
  if (CONTROLE.Button(PSB_PAD_RIGHT)) {            // turns on the led while the button is being pressed
    digitalWrite(VD, HIGH);
  }
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT)) {    // turns off the led and stores the button pressed and the delay time in the memory vector
    digitalWrite(VD, LOW);
    memTimes[memIndex] = millis() - startTime;
    memDirections[memIndex] = 'R';
    memIndex++;
  }
  
  // OUTPUTS
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT) || CONTROLE.ButtonReleased(PSB_PAD_LEFT) || CONTROLE.ButtonReleased(PSB_PAD_UP)){
    Serial.print("Delay Time: ");
    Serial.println(memTimes[memIndex-1]);

    Serial.print("Button pressed: ");
    Serial.println(memDirections[memIndex-1]);

    Serial.print("Memory vector position: ");
    Serial.println(memIndex-1);

    Serial.print("Everything: ");
    for (int k=0; k<memIndex; k++){
      Serial.print(memTimes[k]);
      Serial.print(" -- ");
      Serial.println(memDirections[k]);
    }
    Serial.println();
  }

}

No changes were done in the recordMemory() function, so I didn't change the indexes where the information is being stored in the memory vector. The piece of code for switching the state of SELECT button was changed according the example you mentioned before. I removed the calculations concerning millis() from the playMemory() function just to check how it was working.

I made the playMemory() function as simple as possible in order to check the access to the memory vector. Now, both Serial.println(memTimes[mP]) and delay(memTimes[mP]) are accessing the same vector positions, but they present different behaviors: the first one doesn't trigger the malfunctioning, while the second one does. Is it still related to vector indexing?

The code would be a lot easier to read if you put EVERY { on a line by itself, and put ONE statement per line.

It is NOT necessary to put long comments at the end of already long lines. I prefer to see comments BEFORE a line of, or a block of, code, explaining what the code is supposed to do.

// If the PAD_UP switch is released, turn off the led and store the number
// of the switch pressed and the time it was held in the appropriate arrays
  if (CONTROLE.ButtonReleased(PSB_PAD_UP))
  {
    digitalWrite(AM, LOW);
    memTimes[memIndex] = millis() - startTime;
    memDirections[memIndex] = 'U';
    memIndex++;
  }

Observe, too, that the proper meaning of the times is noted in the comment, as is the proper term for the storage mechanism you are using. You are NOT using vectors.

  if (CONTROLE.Button(PSB_PAD_RIGHT)) {            // turns on the led while the button is being pressed
    digitalWrite(VD, HIGH);
  }

Why do you need to do this? You have code for when this switch BECOMES pressed. Turn the LED on when the switch BECOMES pressed. Turn it off, as you do now, when the switch BECOMES released.

You have several blocks of code to write to the two arrays and increment the index. You should have a function to do that, called in several places.

Now, I do not like the way you increment memIndex AFTER you right data. memIndex then points to the next place to write, not the last place written. It makes it confusing when you use memIndex as both an index and a count of the number of places used in the array.

What I would do is initialize memIndex to -1, and increment it BEFORE writing to either array. Then, memIndex would always be the last position written to.

Your replay code, with or without the delay() accomplishes nothing. Create a function, used only when debugging, to dump the contents of the array. Call that wherever needed, but NOT in the replay function.

Make the replay function actually do something.

Finally, to be honest, I've lost track of what the problem is. Can you explain/summarize what this code actually does (or the code that you produce that implements my suggestions) and how that differs from what you want it to do?

Ok, here is the new version after your suggestions:

#include <PS2X_lib.h>

#define BR 10   // white led - recording mode ON/OFF indicator
#define VM 11   // red led - left button
#define AM 12   // yellow led - up button
#define VD 13   // green led - right button

unsigned long memTimes[20];     // hold time for each command
char memDirections[20];         // records which button is pressed
int memIndex = -1;              // index in memory array
unsigned long startTime = 0;    // hold time counting
bool memoryModeCurr = LOW;      // current state for the memory mode button
bool memoryModePrev = LOW;      // previous state for the memory mode button

void playMemory();
void recordMemory();

PS2X CONTROLE;

void setup()
{
  Serial.begin(9600);
  
  pinMode(BR, OUTPUT);
  pinMode(VM, OUTPUT);
  pinMode(AM, OUTPUT);
  pinMode(VD, OUTPUT);
  CONTROLE.config_gamepad(2,4,3,5,false,false);   // (clock, command, attention, data, Pressures?, Rumble?)
}

void loop()
{
  static bool recordingMode;  // state of the recording Mode
  
  CONTROLE.read_gamepad();

  // Toggles the recording mode ON or OFF when the button SELECT is pressed
  memoryModeCurr = CONTROLE.ButtonPressed(PSB_SELECT);
  if (memoryModeCurr != memoryModePrev)
  {
    if (memoryModeCurr == HIGH)
    {
      recordingMode = !recordingMode;
      Serial.println("recordingMode state has changed.");
    }
  }
  memoryModePrev = memoryModeCurr;

  // Enter the memory recording mode only if recording mode is ON and
  // if the memory index is less than the length of the memory array.
  // Turn on the LED to indicate that the information is being recorded.
  if (recordingMode && (memIndex <= 20))
  {
    digitalWrite(BR, HIGH);
    recordMemory();
  }
  else
  {
    digitalWrite(BR, LOW);
  }

  // Enter the playback mode if START button is pressed AND if the memory
  // index is higher o equals to zero (which means that some information was
  // previously recorded) AND if the recording mode is OFF
  if (CONTROLE.ButtonPressed(PSB_START) && (memIndex >= 0) && recordingMode==false)
  {
    playMemory();
  }

  delay(25);
}

// This function reads the information recorded in the memory arrays
// and replays it backwards.
void playMemory()
{
  Serial.println("Playback mode activated.");
  
  for (int mI=memIndex; mI>=0; mI--) 
  {
    // Calls a function to switch ON the LEDs according to the direction
    // during the proper time stored in the correspondent arrays.
    activateDirection(memDirections[mI], memTimes[mI]);
  }
}

// This function activates the LED correspondent to each direction
// stored in the memory
void activateDirection(char d, unsigned int t)
{
  // Turn ON the proper LED
  switch (d)
  {
    case 'U':
      digitalWrite(AM, HIGH);
      break;
    case 'L':
      digitalWrite(VM, HIGH);
      break;
    case 'R':
      digitalWrite(VD, HIGH);
      break;
  }

  startTime = millis();
  
  // Wait for the hold time to be reached
  // THE MALFUNCTIONING IS BEING TRIGGERED HERE:
  // Sometimes the recording mode is switched ON by itself after reading
  // the memory arrays.
  // It happens if the while loop is replaced to delay(t) as well.
  // It doesn't happen if the while loop is replaced to Serial.println(t).
  while((unsigned long)(millis()-startTime) <= t)
  {
    // Nothing to be done here but wait...
    // Is there a better way to do this?
  }

  // Turn OFF the proper LED
  switch (d)
  {
    case 'U':
      digitalWrite(AM, LOW);
      break;
    case 'L':
      digitalWrite(VM, LOW);
      break;
    case 'R':
      digitalWrite(VD, LOW);
      break;
  }
}

// This function stores the information about the directional button 
// pressed in the gamepad and the time it was held
void recordMemory() 
{
  // If the UP directional button is pressed, turn on the yellow LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_UP))
  {
    digitalWrite(AM, HIGH);
    startTime = millis();
  }
  
  // If the UP button is released, turn off the yellow led and store 
  // the character 'U' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_UP))
  {
    digitalWrite(AM, LOW);
    writeMemory('U', (millis() - startTime));
  }

  // If the LEFT directional button is pressed, turn on the red LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_LEFT))
  {
    digitalWrite(VM, HIGH);
    startTime = millis();
  }

  // If the LEFT button is released, turn off the red led and store 
  // the character 'L' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_LEFT))
  {
    digitalWrite(VM, LOW);
    writeMemory('L', (millis() - startTime));
  }

  // If the RIGHT directional button is pressed, turn on the green LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_RIGHT))
  {
    digitalWrite(VD, HIGH);
    startTime = millis(); 
  }

  // If the RIGHT button is released, turn off the green led and store 
  // the character 'R' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT))
  {
    digitalWrite(VD, LOW);
    writeMemory('R', (millis() - startTime));
  }
  printMemory();
}


// This function stores the character concerning the directional button
// which was pressed and the time it was held in the appropriate arrays
void writeMemory(char d, unsigned int t)
{
  memIndex++;
  memTimes[memIndex] = t;
  memDirections[memIndex] = d;
}

// This function is just for printing the memory arrays as they are being
// written while in the recording mode
void printMemory()
{
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT) || CONTROLE.ButtonReleased(PSB_PAD_LEFT) || CONTROLE.ButtonReleased(PSB_PAD_UP))
  {
    Serial.print("Delay Time: ");
    Serial.println(memTimes[memIndex]);

    Serial.print("Button pressed: ");
    Serial.println(memDirections[memIndex]);

    Serial.print("Memory array position: ");
    Serial.println(memIndex);

    Serial.print("Everything: ");
    
    for (int k=0; k<=memIndex; k++)
    {
      Serial.print(memTimes[k]);
      Serial.print(" -- ");
      Serial.println(memDirections[k]);
    }
    
    Serial.println();
  }
}

The issue is this: sometimes, after executing the playback mode, the program automatically changes the state of the variable recordingMode and enters the recording mode. It only happens when I use some function related to time counting in the function activateDirection(), e.g., while(millis-startingTime <= t) or delay(t). If I just print the value of t the program works correctly, suggesting that the array indexing may not be the problem.

Moreover, after incorrectly entering the recording mode it stores three new positions in the memory arrays, each one corresponding to one of the possible directions. It also stores a hold time comparable to the delay in the loop() function to each one.

Here is an example of the output. Note that the incorrectly recorded sequence appears in the same order as they are in the piece of code corresponding to the recordMemory(), which makes me think that somehow the program is ignoring all conditions.

recordingMode state has changed.
Delay Time: 1237
Button pressed: U
Memory array position: 0
Everything: 1237 -- U

Delay Time: 1526
Button pressed: L
Memory array position: 1
Everything: 1237 -- U
1526 -- L

Delay Time: 1105
Button pressed: R
Memory array position: 2
Everything: 1237 -- U
1526 -- L
1105 -- R

recordingMode state has changed.
Playback mode activated.
recordingMode state has changed.    <-- I didn't press the button, thus this shouldn't happen!
Delay Time: 27
Button pressed: R
Memory array position: 5
Everything: 1237 -- U
1526 -- L
1105 -- R
27 -- U                <-- check out the hold times and the recorded sequence.
27 -- L
27 -- R

The issue is this: sometimes, after executing the playback mode, the program automatically changes the state of the variable recordingMode and enters the recording mode.

Do you see the Serial.print() statements that say that that is happening?

What I would do, in playMemory(), is print the value of memIndex before the loop, and the value of m1 in the loop (why does the index have a number in the name?), to make certain that you are looping only the expected number of times.

I'd also print the value of recordingMode in the loop, to make sure that activateDirection() isn't stepping on it somehow.

I'd also get an explanation ready for why activateDirection() expects an unsigned int, but you pass it an unsigned long.

,ch,hc,bcxv,

First fo all:

I'd also get an explanation ready for why activateDirection() expects an unsigned int, but you pass it an unsigned long.

That was a mistake when declaring the variables. Now it's expecting a unsigned long.

Now, I did the tests and I got these:

  • The value of memIndex was correct before and after the loop. The value of mI (it's an upper case i, not a number one) was looping the correct number of times as well.

  • The value of recordingMode was not changing in the loop. However, the value of memoryModeCurr was changing at the beginning of function loop() after the execution of playMemory().

Since the hold times of unexpected directions where very short and comparable to the delay time in the function loop(), I was wondering if there was some sort of noise in the gamepad communication that could be causing an unrequested changing in the state of the variable memoryModeCurr. After thinking for a while, I reached the following solution:

#include <PS2X_lib.h>

#define BR 10   // white led - recording mode ON/OFF indicator
#define VM 11   // red led - left button
#define AM 12   // yellow led - up button
#define VD 13   // green led - right button

unsigned long memTimes[20];     // hold time for each command
char memDirections[20];         // records which button is pressed
int memIndex = -1;              // index in memory array
unsigned long startTime = 0;    // hold time counting
bool memoryModeCurr = LOW;      // current state for the memory mode button
bool memoryModePrev = LOW;      // previous state for the memory mode button
static bool recordingMode = LOW;  // state of the recording Mode
unsigned long startSelectTime = 0;

PS2X CONTROLE;

void setup()
{
  Serial.begin(9600);
  
  pinMode(BR, OUTPUT);
  pinMode(VM, OUTPUT);
  pinMode(AM, OUTPUT);
  pinMode(VD, OUTPUT);
  CONTROLE.config_gamepad(2,4,3,5,false,false);   // (clock, command, attention, data, Pressures?, Rumble?)
}

void loop()
{
  CONTROLE.read_gamepad();

  // Toggles the recording mode ON or OFF by activating the button SELECT.
  
  // Start counting a hold time when the SELECT button is pressed.
  if (CONTROLE.ButtonPressed(PSB_SELECT))
  {
    startSelectTime = millis();
  }

  // Change the state of recordingMode if the hold time is higher than
  // 50 milliseconds when the button SELECT is released. This avoids
  // entering the recording mode by some sort of "noise" when reading
  // signal from the gamepad.
  if (CONTROLE.ButtonReleased(PSB_SELECT) && (millis()-startSelectTime) > 50)
  {
  recordingMode = !recordingMode;
  Serial.println("recordingMode state has changed.");
  }

  // Enter the memory recording mode only if recording mode is ON and
  // if the memory index is less than the length of the memory array.
  // Turn on the LED to indicate that the information is being recorded.
  if (recordingMode && (memIndex <= 20))
  {
    digitalWrite(BR, HIGH);
    recordMemory();
  }
  else
  {
    digitalWrite(BR, LOW);
  }

  // Enter the playback mode if START button is pressed AND if the memory
  // index is higher o equals to zero (which means that some information was
  // previously recorded) AND if the recording mode is OFF
  if (CONTROLE.ButtonPressed(PSB_START) && (memIndex >= 0) && recordingMode==false)
  {
    playMemory();
  }

  delay(25);
}

// This function reads the information recorded in the memory arrays
// and replays it backwards.
void playMemory()
{
  Serial.println("Playback mode activated.");

//  Serial.print("memIndex (before loop) = ");
//  Serial.println(memIndex);
  
  for (int mI=memIndex; mI>=0; mI--) 
  {
    // Calls a function to switch ON the LEDs according to the direction
    // during the proper time stored in the correspondent arrays.
    activateDirection(memDirections[mI], memTimes[mI]);
  }
}

// This function activates the LED correspondent to each direction
// stored in the memory
void activateDirection(char d, unsigned long t)
{
  // Turn ON the proper LED
  switch (d)
  {
    case 'U':
      digitalWrite(AM, HIGH);
      break;
    case 'L':
      digitalWrite(VM, HIGH);
      break;
    case 'R':
      digitalWrite(VD, HIGH);
      break;
  }

  startTime = millis();

  while((unsigned long)(millis()-startTime) <= t)
  {
    // Nothing to be done here but wait...
    // Is there a better way to do this?
  }

  // Turn OFF the proper LED
  switch (d)
  {
    case 'U':
      digitalWrite(AM, LOW);
      break;
    case 'L':
      digitalWrite(VM, LOW);
      break;
    case 'R':
      digitalWrite(VD, LOW);
      break;
  }
}

// This function stores the information about the directional button 
// pressed in the gamepad and the time it was held
void recordMemory() 
{
  // If the UP directional button is pressed, turn on the yellow LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_UP))
  {
    digitalWrite(AM, HIGH);
    startTime = millis();
  }
  
  // If the UP button is released, turn off the yellow led and store 
  // the character 'U' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_UP))
  {
    digitalWrite(AM, LOW);
    writeMemory('U', (millis() - startTime));
  }

  // If the LEFT directional button is pressed, turn on the red LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_LEFT))
  {
    digitalWrite(VM, HIGH);
    startTime = millis();
  }

  // If the LEFT button is released, turn off the red led and store 
  // the character 'L' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_LEFT))
  {
    digitalWrite(VM, LOW);
    writeMemory('L', (millis() - startTime));
  }

  // If the RIGHT directional button is pressed, turn on the green LED and
  // start the hold time counting for this button
  if (CONTROLE.ButtonPressed(PSB_PAD_RIGHT))
  {
    digitalWrite(VD, HIGH);
    startTime = millis(); 
  }

  // If the RIGHT button is released, turn off the green led and store 
  // the character 'R' and the time it was held
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT))
  {
    digitalWrite(VD, LOW);
    writeMemory('R', (millis() - startTime));
  }
  printMemory();
}


// This function stores the character concerning the directional button
// which was pressed and the time it was held in the appropriate arrays
void writeMemory(char d, unsigned int t)
{
  memIndex++;
  memTimes[memIndex] = t;
  memDirections[memIndex] = d;
}

// This function is just for printing the memory arrays as they are being
// written while in the recording mode
void printMemory()
{
  if (CONTROLE.ButtonReleased(PSB_PAD_RIGHT) || CONTROLE.ButtonReleased(PSB_PAD_LEFT) || CONTROLE.ButtonReleased(PSB_PAD_UP))
  {
    Serial.print("Delay Time: ");
    Serial.println(memTimes[memIndex]);

    Serial.print("Button pressed: ");
    Serial.println(memDirections[memIndex]);

    Serial.print("Memory array position: ");
    Serial.println(memIndex);

    Serial.print("Everything: ");
    
    for (int k=0; k<=memIndex; k++)
    {
      Serial.print(memTimes[k]);
      Serial.print(" -- ");
      Serial.println(memDirections[k]);
    }
    
    Serial.println();
  }
}

In the tests I've made it seems to work nicely. Please, do you have something else to suggest?

Please, do you have something else to suggest?

If it works, quit f*cking with it. 8)

One thing I would suggest is a #define statement to define the maximum number of values to store/replay (20, currently), and to use that #define'd name everywhere in the code where you currently have 20. This way, one line of code is all you need to change to change the limit to 10. Or 50.

Great. Thanks for your help!