SOLVED Code hanging up

I’ve added several redundancies and serial prints trying to track this problem down. Adding redundant relay off lines helped make the number of occurrences lower, but the code locks up after many “shifts.” Twenty or more, which should never happen in real life, but I’d still like to eliminate.

The serial prints aren’t helping me either, as I get lines like “neutr+” and “rever+” which aren’t even an option. It’s stopping in the middle of a serial print, it would also lock up before I added the print calls.

Hardware: Uno, two relay pack for extend/retract (12V), 5V switching supply to 5V pin on Uno, external relay to switch 12V brake on signal to ground for brakeSafetyPin, linear feedback actuator.

/* Electronic shifter using a feedback linear actuator
   Use pots in circuit to allow end user to calibrate for vehicle
   Add buzzer for no brake with gear selection?
   What to do if brake released while shifter in motion?*/

//shifter buttons
const byte parkPin = 2;
const byte reversePin = 3;
const byte neutralPin = 4;
const byte drivePin = 5;

//actuator relays
const byte extendPin = 6;
const byte retractPin = 7;

//brake pressed in order to switch gears
const byte brakeSafetyPin = 8;

//shifter positon through feedback
const byte actuatorPin = A4;

//current position
const byte drive = 1;
const byte reverse = 2;
const byte neutral = 3;
const byte park = 4;

const byte parkGear = 1;
const byte reverseGear = 2;
const byte neutralGear = 3;
const byte driveGear = 4;
byte stage;
//byte stage = parkGear;

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

  // put your setup code here, to run once:
  pinMode (parkPin, INPUT_PULLUP);
  pinMode (reversePin, INPUT_PULLUP);
  pinMode (neutralPin, INPUT_PULLUP);
  pinMode (drivePin, INPUT_PULLUP);
  pinMode (extendPin, OUTPUT);
  pinMode (retractPin, OUTPUT);
  pinMode (brakeSafetyPin, INPUT_PULLUP);

  //error beep
  pinMode (13, OUTPUT);
  digitalWrite (13, LOW);
}

void loop() {
  // put your main code here, to run repeatedly:
  if (digitalRead(parkPin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Park();
  }
  else if (digitalRead(reversePin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Reverse();
  }
  else if (digitalRead(neutralPin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Neutral();
  }
  else if (digitalRead(drivePin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Drive();
  }
  else {
    digitalWrite (extendPin, HIGH);
    digitalWrite (retractPin, HIGH);
    Serial.println("loop");
  }
}

void Park() { //hookup motion sensor to verify no motion?
  if (analogRead(actuatorPin) > 655) {
    while (analogRead(actuatorPin) > 650) {
      digitalWrite (extendPin, LOW);
      digitalWrite (retractPin, HIGH);
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("Park extend");
  }
  //digitalWrite (extendPin, HIGH);
  //delay (100);

  else if (analogRead(actuatorPin) < 645) {
    while (analogRead(actuatorPin) < 650) {
      digitalWrite (retractPin, LOW);
      digitalWrite (extendPin, HIGH);
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("Park retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = parkGear;
  Serial.println("Park off");
}
void Reverse() { // only allow from park?
  if (analogRead(actuatorPin) > 580) {
    while (analogRead(actuatorPin) > 575) {
      digitalWrite (extendPin, LOW);
      digitalWrite (retractPin, HIGH);
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("reverse extend");
  }
  // digitalWrite (extendPin, HIGH);
  // delay (100);

  else if (analogRead(actuatorPin) < 570) {
    while (analogRead(actuatorPin) < 575) {
      digitalWrite (retractPin, LOW);
      digitalWrite (extendPin, HIGH);
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("reverse retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = reverseGear;
  Serial.println("reverse off");
}

void Neutral() {
  if (analogRead(actuatorPin) > 545) {
    while (analogRead(actuatorPin) > 540) {
      digitalWrite (extendPin, LOW);
      digitalWrite (retractPin, HIGH);
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("neutral extend");
  }
  // digitalWrite (extendPin, HIGH);
  // delay (100);

  else if (analogRead(actuatorPin) < 535) {
    while (analogRead(actuatorPin) < 540) {
      digitalWrite (retractPin, LOW);
      digitalWrite (extendPin, HIGH);
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("neutral retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = neutralGear;
  Serial.println("neutral off");
}

void Drive() { // only allow from park? Better only allow no motion? Best
  if (analogRead(actuatorPin) > 510) {
    while (analogRead(actuatorPin) > 505) {
      digitalWrite (extendPin, LOW);
      digitalWrite (retractPin, HIGH);
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("drive extend");
  }
  //digitalWrite (extendPin, HIGH);
  //delay (100);

  else if (analogRead(actuatorPin) < 500) {
    while (analogRead(actuatorPin) < 505) {
      digitalWrite (retractPin, LOW);
      digitalWrite (extendPin, HIGH);
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("drive retract");
  }
  else {
    beep ();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  stage = driveGear;
  delay (100);
  Serial.println("drive off");
}

void beep() {
  digitalWrite (13, HIGH);
  delay (200);
  digitalWrite (13, LOW);
  delay (200);
  digitalWrite (13, HIGH);
  delay (200);
  digitalWrite (13, LOW);
  delay (200);
  digitalWrite (13, HIGH);
  delay (200);
  digitalWrite (13, LOW);
  Serial.println("beep");
}

I really have to wonder why you want to shift into park over and over again. It seems to me that you’d want to shift into park when the parkPin BECAME LOW, not when it IS LOW.

The same applies to the other switches. Action should, it seems to me, happen only when the switch BECOMES pressed.

Perhaps you should start with explaining why the code is written the way it is. Then, perhaps it will be easier to help you determine why it locks up.

I'm not sure if you're saying the confusion is in my "while" loops or if you're saying I should use a state change for the switches? I'll try to explain what's happening.

There are four buttons for park, reverse, neutral and drive. The appropriate button only has to be pressed one time and the loop function determines what button was pressed and sends it to the corresponding function. At this point the button doesn't need to be held down. The call is made when the button becomes LOW, it does not need to stay LOW.

The "while" loops are so the button does not need to be held down. The analog pin is constantly read to move the actuator, once the actuator reaches the proper position it stops.

It's not in the car right now, but it works perfectly and did so even while in the car. It just hangs up after a bit, my last run was over 100 shifts. In real life this many shifts should never be reached before the car is turned off, which resets the Uno, I just don't want to take the chance.

By the serial prints it's definitely getting hung up in one of the gears, I just don't know where within the gears.

The call is made when the button becomes LOW, it does not need to stay LOW.

That is not true the way the code is written. The Arduino may read the state of the Park switch, for instance, many times, when the switch is pressed, and try to move the solenoid to the park position many times before the user releases the switch.

The state change detection example shows how to move the solenoid only once when the switch BECOMES pressed.

    while (analogRead(actuatorPin) > 650) {
      digitalWrite (extendPin, LOW);
      digitalWrite (retractPin, HIGH);
    }

Why do you need to keep diddling with the pins while the solenoid is moving? You should diddle with the pins BEFORE the while loop starts. You do not NEED to have something in the body of the while statement (although a comment that acknowledges that you are just waiting for the exit criteria to happen is usually placed there).

I would put more Serial.print() statements in the code, to see what the actuator pin reading is, prior to each block of code.

I ran some tests after renaming the prints to 1drive, 2drive, 3drive, etc. I had to failures both after a large number of shifts, I did not count. One failure in 2neutral and one failure in 2park.

The 2neutral is in this part of the code.

else if (analogRead(actuatorPin) < 535) {
    while (analogRead(actuatorPin) < 540) {
      digitalWrite (retractPin, LOW);
      digitalWrite (extendPin, HIGH);
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("2neutral retract");
  }

Your reply came in while I was typing, I like what you’re saying. I am diddling with things more than need be, but that made my hangups go from every 10 shifts to every 100 shifts. I’ll try setting the pins before the loop as suggested and also look into the state change. Thank you!

Thanks Paul! I think we have it!

I made the changes you suggested and was able to get several hundred shifts in short order with no hangups. I’ll leave on the test bench a few days for a bunch more testing before I install in the car, this looks promising.

Revised code

/* Electronic shifter using a feedback linear actuator
   Use pots in circuit to allow end user to calibrate for vehicle
   Add buzzer for no brake with gear selection?
   What to do if brake released while shifter in motion?*/

//shifter buttons
const byte parkPin = 2;
const byte reversePin = 3;
const byte neutralPin = 4;
const byte drivePin = 5;

//actuator relays
const byte extendPin = 6;
const byte retractPin = 7;

//brake pressed in order to switch gears
const byte brakeSafetyPin = 8;

//shifter positon through feedback
const byte actuatorPin = A4;

//current position
const byte drive = 1;
const byte reverse = 2;
const byte neutral = 3;
const byte park = 4;

const byte parkGear = 1;
const byte reverseGear = 2;
const byte neutralGear = 3;
const byte driveGear = 4;
byte stage;
//byte stage = parkGear;

byte PbuttonState = 0;
byte PlastButtonState = 0;
byte RbuttonState = 0;
byte RlastButtonState = 0;
byte NbuttonState = 0;
byte NlastButtonState = 0;
byte DbuttonState = 0;
byte DlastButtonState = 0;

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

  // put your setup code here, to run once:
  pinMode (parkPin, INPUT_PULLUP);
  pinMode (reversePin, INPUT_PULLUP);
  pinMode (neutralPin, INPUT_PULLUP);
  pinMode (drivePin, INPUT_PULLUP);
  pinMode (extendPin, OUTPUT);
  pinMode (retractPin, OUTPUT);
  pinMode (brakeSafetyPin, INPUT_PULLUP);

  //error beep
  pinMode (13, OUTPUT);
  digitalWrite (13, LOW);
}

void loop() {
  // put your main code here, to run repeatedly:
  PbuttonState = digitalRead(parkPin);
  if (PbuttonState != PlastButtonState) {
    if (digitalRead(parkPin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
      delay (100);
      Park();
    }
    PlastButtonState = 1;
  }

  RbuttonState = digitalRead(reversePin);
  if (RbuttonState != RlastButtonState) {
   if (digitalRead(reversePin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Reverse();
  }
  RlastButtonState = 1;
  }

  NbuttonState = digitalRead(neutralPin);
  if (NbuttonState != NlastButtonState) {
   if (digitalRead(neutralPin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Neutral();
  }
  NlastButtonState = 1;
  }

  DbuttonState = digitalRead(drivePin);
  if (DbuttonState != DlastButtonState) {
   if (digitalRead(drivePin) == LOW && digitalRead(brakeSafetyPin) == LOW) {
    delay (100);
    Drive();
  }
  DlastButtonState = 1;
  }
  
    digitalWrite (extendPin, HIGH);
    digitalWrite (retractPin, HIGH);
    Serial.println("loop");
  
}

void Park() { //hookup motion sensor to verify no motion?
  if (analogRead(actuatorPin) > 655) {
    digitalWrite (extendPin, LOW);
    digitalWrite (retractPin, HIGH);
    while (analogRead(actuatorPin) > 650) {
      //wait to reach park position
      Serial.println("1PWhile");
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("1Park extend");
  }

  else if (analogRead(actuatorPin) < 645) {
    digitalWrite (retractPin, LOW);
    digitalWrite (extendPin, HIGH);
    while (analogRead(actuatorPin) < 650) {
      //wait to reach park position
      Serial.println("2PWhile");
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("2Park retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = parkGear;
  Serial.println("3Park off");
}

void Reverse() { // only allow from park?
  if (analogRead(actuatorPin) > 580) {
    digitalWrite (extendPin, LOW);
    digitalWrite (retractPin, HIGH);
    while (analogRead(actuatorPin) > 575) {
      //wait to reach reverse position
      Serial.println("1RWhile");
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("1reverse extend");
  }

  else if (analogRead(actuatorPin) < 570) {
    digitalWrite (retractPin, LOW);
    digitalWrite (extendPin, HIGH);
    while (analogRead(actuatorPin) < 575) {
      //wait to reach reverse position
      Serial.println("2RWhile");
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("2reverse retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = reverseGear;
  Serial.println("3reverse off");
}

void Neutral() {
  if (analogRead(actuatorPin) > 545) {
    digitalWrite (extendPin, LOW);
    digitalWrite (retractPin, HIGH);
    while (analogRead(actuatorPin) > 540) {
      //wait to reach neutral position
      Serial.println("1NWhile");
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("1neutral extend");
  }
  
  else if (analogRead(actuatorPin) < 535) {
    digitalWrite (retractPin, LOW);
    digitalWrite (extendPin, HIGH);
    while (analogRead(actuatorPin) < 540) {
      //wait to reach neutral position
      Serial.println("2NWhile");
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("2neutral retract");
  }
  else {
    beep();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  delay (100);
  stage = neutralGear;
  Serial.println("3neutral off");
}

void Drive() { // only allow from park? Better only allow no motion? Best
  if (analogRead(actuatorPin) > 510) {
    digitalWrite (extendPin, LOW);
    digitalWrite (retractPin, HIGH);
    while (analogRead(actuatorPin) > 505) {
      //wait to reach drive position
      Serial.println("1DWhile");
    }
    digitalWrite (extendPin, HIGH);
    Serial.println("1drive extend");
  }
 
  else if (analogRead(actuatorPin) < 500) {
    digitalWrite (retractPin, LOW);
    digitalWrite (extendPin, HIGH);
    while (analogRead(actuatorPin) < 505) {
      //wait to reach drive position
      Serial.println("2DWhile");
    }
    digitalWrite (retractPin, HIGH);
    Serial.println("2drive retract");
  }
  else {
    beep ();
  }
  digitalWrite (extendPin, HIGH);
  digitalWrite (retractPin, HIGH);
  stage = driveGear;
  delay (100);
  Serial.println("3drive off");
}

void beep() {
  digitalWrite (13, HIGH);
  delay (100);
  digitalWrite (13, LOW);
  delay (100);
  digitalWrite (13, HIGH);
  delay (100);
  digitalWrite (13, LOW);
  delay (100);
  digitalWrite (13, HIGH);
  delay (100);
  digitalWrite (13, LOW);
  Serial.println("beep");
}