Any suggestions to make this code work right?

I wrote this code for a gate opener I made up. I cant seem to see why its acting up.

Can anyone see any errors in my code that would affect the opening command?

Upon first power up the open command (remoteA) works fine.

After that the opening command does what its supposed to except it only momentarily fires "digitalWrite (power, LOW)" command. It should be set LOW for the same amount of time as "digitalWrite (openGate, LOW)", but it doesnt, it just blips on the relay on then off immediately.

Strange thing is, the close command code is almost identical and it always works as it should, no problems.

Any suggestions?

/* 
 *  Swing Gate Opener with 2Ch remote, gate lock, motor, buzzer, amp monitor and two independent limit switches
 *  
 *  By Darren Clarke
 *  
 */
int current = A0;  // monitor motor current
int buzzer = 3;  // operation buzzer
int power = 5;  // 24v power on/off
int remoteOpen = 6;  // remote button A to digital pin 6
int remoteClose = 7;  // remote button B to digital pin 7
int openGate = 8;  // open gate to motor on digital pin 8
int closeGate = 9;  // close gate to motor on digital pin 9
int gateLock = 10;  // gate lock connected to digital pin 10

// variables
int openRelay = 0;  // variable to store the motor direction
int closeRelay = 0;  // variable for previous motor direction
int remoteA = 0;  // variable for remote A condition
int remoteB = 0;  // variable for remote B condition
int lock = 0;  // varialbe for gate lock
int mA = 0;  // variable for motor current
unsigned long previousMillisA = 0;
unsigned long previousMillisB = 0;
unsigned long currentMillisA = 0;
unsigned long currentMillisB = 0;

void setup() {
  // put your setup code here, to run once:
pinMode (buzzer, OUTPUT);  // sets the digital pin 4 as output
pinMode (power, OUTPUT);  // sets the digital pin 5 as output
pinMode (remoteOpen, INPUT);  // sets the digital pin 6 as input
pinMode (remoteClose, INPUT);  // sets the digital pin 7 as input
pinMode (openGate, OUTPUT);  // sets the digital pin 8 as output
pinMode (closeGate, OUTPUT);  // sets the digital pin 9 as output
pinMode (gateLock, OUTPUT);  // sets the digital pin 10 as output
digitalWrite (closeGate, HIGH);  // sets relay to open
digitalWrite (openGate, HIGH);  // sets relay to open
digitalWrite (power, HIGH);  // sets relay to open
digitalWrite (gateLock, HIGH);  // sets relay to open
analogWrite (buzzer, 0);  // sets buzzer off
Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:
remoteA = digitalRead (remoteOpen);  // read remote A input
remoteB = digitalRead (remoteClose);  // read remote B input
openRelay = digitalRead (openGate);  // read state of openGate
closeRelay = digitalRead (closeGate);  // read state of closeGate
lock = digitalRead (gateLock);  // read state of gate lock
mA = analogRead (current);  // read value of motor current
currentMillisA = millis();
currentMillisB = millis();
Serial.print ("aIn ");
Serial.print (remoteA);
Serial.print (", bIn ");
Serial.print (remoteB);
Serial.print (", Open ");
Serial.print (openRelay);
Serial.print (", Close ");
Serial.print (closeRelay);
Serial.print (", Lock ");
Serial.print (lock);
Serial.print (", mA ");
Serial.print (mA);
Serial.print (", cur ");
Serial.print (currentMillisA);
Serial.print (", prev ");
Serial.print (previousMillisA);
Serial.println (" ");


// to open the gate
if ((remoteA == HIGH) && (remoteB == LOW)) {
  delay (500);  // time for gate direction reversal
  digitalWrite (power, HIGH);  // stops previous movement
  digitalWrite (closeGate, HIGH);  // Stop gate open command
  analogWrite (buzzer, 100);  // starts buzzer
  digitalWrite (gateLock, LOW);  // energize lock open (unlock state)
  delay (500);  // time for lock to open
  digitalWrite (openGate, LOW);  // actuate gate open
  digitalWrite (power, LOW);  //  supplies 24v power
  previousMillisA = millis();  // set time references
  currentMillisA = millis();  // set time references
  }

// to stop open command
if ((closeRelay == HIGH) && (currentMillisA - previousMillisA) >= 6000){
  digitalWrite (gateLock, HIGH);  // denergize lock (locked state)
  digitalWrite (openGate, HIGH);  // stop open relay to motor after open
  digitalWrite (power, HIGH);  // remove 24v power
  analogWrite (buzzer, 0);
  }

// to close the gate
if ((remoteB == HIGH) && (remoteA == LOW)) {
  delay (500);  // time for gate direction reversal
  digitalWrite (power, HIGH);  // stops previous movement
  digitalWrite (gateLock, HIGH);  // de-energize lock on quick reverse
  digitalWrite (openGate, HIGH);  // Stop gate close command
  analogWrite (buzzer, 100);  // starts buzzer
  digitalWrite (closeGate, LOW);  // actuate gate close
  digitalWrite (power, LOW);  // supplies 24v power
  previousMillisB = millis();  // set time references
  currentMillisB = millis();  // set time references
  }

// to stop close command
if ((openRelay == HIGH) && (currentMillisB - previousMillisB) >= 6000){
  digitalWrite (closeGate, HIGH);  // stop close relay to motor
  digitalWrite (power, HIGH);  // remove 24v power
  analogWrite (buzzer, 0);
  }

delay (100);
}

Well, although delay() and millis() can be used together, usually it is a sign of bad design and likely confusion. I've never seen both previous and current millis updated at the same time, why are you doing this?:

  previousMillisB = millis();  // set time references
  currentMillisB = millis();  // set time references
unsigned long previousMillisA = 0;
unsigned long previousMillisB = 0;
unsigned long currentMillisA = 0;
unsigned long currentMillisB = 0;

These are dumb names. What, exactly, are these times supposed to represent? Why not use names that reflect that?

pinMode (remoteOpen, INPUT);  // sets the digital pin 6 as input
pinMode (remoteClose, INPUT);  // sets the digital pin 7 as input

Do you have external (pullup or pulldown) resistors wired with these pins? Why not use the internal pullup resistors and make wiring easier?

 delay (500);  // time for gate direction reversal

It is silly to use millis() and delay() in the same program. What, EXACTLY, is happening during this delay()? As far as I can tell, nothing is happening, which makes the delay() useless.

I'd strongly recommend that you create some functions. It is easy to tel what closeTheGate() is supposed to do. It has far harder to tell that

 delay (500);  // time for gate direction reversal
  digitalWrite (power, HIGH);  // stops previous movement
  digitalWrite (gateLock, HIGH);  // de-energize lock on quick reverse
  digitalWrite (openGate, HIGH);  // Stop gate close command
  analogWrite (buzzer, 100);  // starts buzzer
  digitalWrite (closeGate, LOW);  // actuate gate close
  digitalWrite (power, LOW);  // supplies 24v power
  previousMillisB = millis();  // set time references
  currentMillisB = millis();  // set time references

is supposed to close the gate.

You can then test each function, and prove that it is working, before you try to build the complete program.

Thanks so much for the replies. Its probably clear I'm not a programmer, thus the dumb names and bad design. I'm an Instrumentation tech / Metrologist of 20 years and only really have experience in ladder logic programming.

So Im doing my best.

The delay is just to protect the motor from fast switching the direction of the motor and burning it up.

I used the millis timing so that the code could still be read for reversing of the motor during its current direction, and also stopping during over amp conditions.

As far as the previous and current millis updated at the same time, I did this to start the counter at that moment. All the other examples of using millis that I could find are just repetitive lights flashing or other simple stuff that started once the arduino was powered up. I wanted a specific trigger and then it stopped, so I felt i needed to create a time stamp at that moment.

My digital inputs are driven by a remote control receiver.

To my actual problem, I have noticed that if I repeatedly hit the remoteA button it does eventually work like it should, but I have to push it 2 or 3 times. Almost like a bouncing problem or timing issue, but only during that one open gate section and only on the "power" digital output.

Again thanks a bunch for your input. I appreciate your time.

Almost like a bouncing problem or timing issue,

Or, perhaps, all those delay()s are getting in the way.

I had removed all the delays. Problem still exists.

For some reason the 24 power on command just doesnt stay engaged. I can see on the relay board that it engages for a split second then turns off. If I hit the remote button once or twice more it engages as it should.

The same output is used for the close command which works flawlessly, so I cant imagine its the output of the arduino.

The code is almost identical. I even removed the gate unlock command to see if that is messing with it, making the open and close code basically the same. Problem still exists.

Very strange.

slickpuss:
I had removed all the delays. Problem still exists.

Post the lastest version of your program.

...R
Planning and Implementing a Program

You haven't answer the question about the external resistors. I suspect a floating pin problem.

Ok, i figured it out.

Thanks all for your help.

It only happens on the A channel because its first in the series of the code. The turn off command after the timer isnt specific enough. The if conditions are met on the B channel so it keeps turning the power off immediately.

I just need be more specific in my if command or make another tweek.

Thanks guys, your suggestions caused me to do much more digging and code adjustments.

Its working like a peach now.

Thanks.