[SOLVED] Elimnating delay(), one at a time

Hi All,

i’ve been using delay() for my code and was thinking of eliminating them and try state machine, AKA milli().

int WeldStep()          // Map POT value from A2 to increments of 10 from 10 to 500
{
  int PrevVal;
  PotValue = ((analogRead(Pot) / 20) * 10);
  if (PrevVal != PotValue)
  {
    if (PotValue < 10)
    {
      WeldStepValue = 10;
    }
    if (PotValue >= 500)
    {
      WeldStepValue = 500;
    }
    else
    {
      WeldStepValue = PotValue;
    }
  }
  return WeldStepValue;
  PrevVal = PotValue;
}

void Weld()  //Actual weld
{
  currentMillis = millis();
  int triacState = LOW;
  unsigned long prevMillis = 0;
  lcd.backlight();
  myServo.attach(servoPin);
  digitalWrite(rdy, LOW);
  ServoStart();
  delay(400);
  buttonState = digitalRead(buttonPin);
  zeroCrossingFlag = false; //set flag false and wait for next zero crossing
  while (!zeroCrossingFlag) {};
  delayMicroseconds(tgr_dly);
  triacState = HIGH;
  digitalWrite(TRIAC, triacState);
  Serial.println(" ");
  Serial.print("TRIAC Digital state: ");
  Serial.println(digitalRead(TRIAC));
  Serial.print("Start Time: ");
  Serial.println(currentMillis);
  int buttonState = digitalRead(buttonPin);
  while (buttonState == HIGH)
  {
    if (triacState == HIGH)
    {
      prevMillis = currentMillis;
      triacState = LOW;
      buttonState = HIGH;
      digitalWrite(TRIAC, triacState);
      Serial.print("TRIAC Digital state: ");
      Serial.println(digitalRead(TRIAC));
      Serial.print("End Time: ");
      Serial.println(prevMillis);
      Serial.print("TRIAC off at: ");
      Serial.println(currentMillis - prevMillis);
      Serial.println(" ");
    }
    if (currentMillis - prevMillis >= 450)
    {
      break;
    }
  }

This function is called when a button is pressed. So every time it is run an LED on TRIAC barely binks. but when i use delay above the below line, like delay(10).

while (buttonState == HIGH)
  {

it blinks more brighter meaning its seems more like it has blinked the serial monitor looks like this

TRIAC Digital state: 1
Start Time: 39873
TRIAC Digital state: 0
End Time: 39873
TRIAC off at: 0

So i understand is the startTime and EndTime is the same but where else do i move them?

Any help wouldb appriciated.

  return WeldStepValue;
  PrevVal = PotValue;

Any code after a return statement isn't executed. The compiler should give you a warning about "unreachable code."

currentMillis never gets updated inside the while() loop. How did you expect it to exit this loop? currentMillis is your variable that you control. It doesn't automatically get any new data from the millis() function unless you put that in your code.

oh sorry that was the wrong bit, i was half sleeping while doing this. here is the one

void Weld()  //Actual weld
{
  currentMillis = millis();
  int triacState = LOW;
  unsigned long prevMillis = 0;
  lcd.backlight();
  myServo.attach(servoPin);
  digitalWrite(rdy, LOW);
  ServoStart();
  delay(400);
  buttonState = digitalRead(buttonPin);
  zeroCrossingFlag = false; //set flag false and wait for next zero crossing
  while (!zeroCrossingFlag) {};
  delayMicroseconds(tgr_dly);
  triacState = HIGH;
  digitalWrite(TRIAC, triacState);
  Serial.println(" ");
  Serial.print("TRIAC Digital state: ");
  Serial.println(digitalRead(TRIAC));
  Serial.print("Start Time: ");
  Serial.println(currentMillis);
  int buttonState = digitalRead(buttonPin);
  while ((buttonState == HIGH) && (triacState == HIGH))
    {
      triacState = LOW;
      buttonState = HIGH;
      digitalWrite(TRIAC, triacState);
      Serial.print("TRIAC Digital state: ");
      Serial.println(digitalRead(TRIAC));
      Serial.print("End Time: ");
      Serial.println(prevMillis);
      Serial.print("TRIAC off at: ");
      Serial.println(millis() - currentMillis);
      Serial.println(" ");
    if (millis() - currentMillis>= 450)
    {
      break;
    }
  }
  /*digitalWrite(TRIAC, HIGH);
    delay(pre);               //preWeld time
    digitalWrite(TRIAC, LOW);
    delay(pause);
    zeroCrossingFlag = false; //set flag false and wait for next zero crossing
    while (!zeroCrossingFlag) {};
    delayMicroseconds(tgr_dly);
    digitalWrite(TRIAC, HIGH);
    delay(WeldStep());
    digitalWrite(TRIAC, LOW);*/
  delay(800);
  ServoReturn();
  myServo.detach();
  delay(250);
  digitalWrite(bzr, HIGH);
  delay(100);
  digitalWrite(bzr, LOW);
  backLightStart = 0;
}

So now i get this in the output window

TRIAC Digital state: 1 Start Time: 7309 TRIAC Digital state: 0 End Time: 0 TRIAC off at: 1482

I guess better than the earlier output. so i change the 450 to something like 1000ms but the i don't see any change in the blinking time.

Well, now currentMillis isn't really the current millisecond, is it? Why not give this a more useful name like startOfWeldCycle?

  while (!zeroCrossingFlag) {};

This will wait FOREVER if something gets unplugged or damaged. Since you're already timing with millis(), why not add a timeout and have it print an error message like "Triac didn't detect mains power, check wiring!"

  buttonState = digitalRead(buttonPin);
  ---snip 11 lines---
  int buttonState = digitalRead(buttonPin);

You do realise that you have two variables now? Your first statement updated the global variable. The second creates a new local one.

Even once you fix that, look carefully how you're using this button. Is this button a press-and-release to start welding or should the weld cycle abort if the button isn't held down? If the second case, read it closer to where you use the value. There's a whole bunch of delays between the first read and where you examine the button state.

  while ((buttonState == HIGH) && (triacState == HIGH))
    {
      triacState = LOW;

This will only execute the body of the loop once. It won't run for 450ms like you intended.

Sketch attached. I knew it would just run once when a button is pressed. but was totally lost when trying to use State machine approach.

Should we be looking at buttonState as well ? because that would always be HIGH when the weld() is called.

code.txt (13.2 KB)

All works well with the delay() itself but to break the welding loop takes time. Like i will have to press+hold the button for than 10seconds to exit the second after holding down the button for 2 seconds.

Just keep in mind that millis() is not related to 'state machines' at all. The only interaction may be when a timed event bumps the 'state' and other linked consequences like that. They are separate dictionary entries!

anishkgt: Sketch attached. I knew it would just run once when a button is pressed. but was totally lost when trying to use State machine approach.

No, it is worse than just running once. It turns the triac off IMMEDIATELY. The on-time that it reports is the time since the whole weld function started. How long was it supposed to be on for a single weld?

anishkgt: All works well with the delay() itself but to break the welding loop takes time. Like i will have to press+hold the button for than 10seconds to exit the second after holding down the button for 2 seconds.

Well, now we are back to asking functional questions. What does your code do? What would you like it to do differently?

I think you might be referring to this part of continuousWeld():

    buttonState = digitalRead(buttonPin);
    backLightStart = 0;
    Weld();
    counter++;
    delay(200);
    if (buttonState == LOW) // press and hold buttonPin for 5 seconds to end loop
    {
      break;
    }

You have once again read the button a LONG time before you actually needed to look at the value. In this case, buttonState is a global variable and Weld() might have modified it. So if(buttonState==LOW) won't be checking what the button state was before you started the weld.

All works well with the delay() itself but to break the welding loop takes time. Like i will have to press+hold the button for than 10seconds to exit the second after holding down the button for 2 seconds.

I'm not sure I understand. You have three button press responses in switch (checkButton()). What response(welding loop?) are you trying to break from? The two second press gets you "longer press" and puts you into setWeldHead() with a bunch of servo moves.

You say that the program is not responsive to a button press. What case are you running when the program feels unresponsive to the button? Why do you want to break from a case before it is completed?

it is the continuousWeld() function that i need to break when called. Normally after the button is held down after this function is called, it takes about 5 to 10 secs to break that loop. it is because the weld() function pauses are done via delay().

All works well with the delay() itself but to break the welding loop takes time.

it is the continuousWeld() function that i need to break when called. Normally after the button is held down after this function is called, it takes about 5 to 10 secs to break that loop. it is because the weld() function pauses are done via delay().

I am confused as to what is stable working code, and what is your attempt to revise it to make it more responsive.

Can you please post the "all works well" code which uses delay().

Attached the working code.

code.txt (12.6 KB)

Let’s breakdown the situation, and see why it takes the time to break from the while loop in continuousWeld(). I’m not sure you can do better than what you are doing.

First, examine weld() which is “blocking” but probably OK as you are not doing anything else or really want to interrupt the weld process.

void Weld()  //Actual weld
{
  currentMillis = millis();
  int triacState = LOW;
  unsigned long prevMillis = 0;
  lcd.backlight();
  myServo.attach(servoPin);
  digitalWrite(rdy, LOW);
  ServoStart();
  delay(400);
  buttonState = digitalRead(buttonPin);
  zeroCrossingFlag = false; //set flag false and wait for next zero crossing
  while (!zeroCrossingFlag) {};
  delayMicroseconds(tgr_dly);
  triacState = HIGH;
  digitalWrite(TRIAC, HIGH);
  delay(pre);               //preWeld time
  digitalWrite(TRIAC, LOW);
  delay(pause);
  zeroCrossingFlag = false; //set flag false and wait for next zero crossing
  while (!zeroCrossingFlag) {};
  delayMicroseconds(tgr_dly);
  digitalWrite(TRIAC, HIGH);
  delay(WeldStep());
  digitalWrite(TRIAC, LOW);
  delay(800);
  ServoReturn();
  myServo.detach();
  delay(250);
  digitalWrite(bzr, HIGH);
  delay(100);
  digitalWrite(bzr, LOW);
  backLightStart = 0;
}

Have you bench marked this section, both with and without the Servo. How long does it take? It’s not clear to me that the pre, pause, and weldStep delays are big drivers compared to the servo moves. Are you certain that the three calls to delay(400) and delay(800) and delay(250) are required?

I think that what you are doing with the digitalRead() to break from the while loop in continuousWeld is OK. You really should not be blocked by more than Weld(). The conditional statements with break are not useful, as the scope of the break is limited to the conditional. EDIT: This is not correct. They will break from the while loop, but they are useless because of where the digitalRead() takes place, and how they bracket the while condition which will not execute if the button was pressed and buttonState is LOW. during the while loop

Why do you have delay(100) and delay(200) in the while loop?

void continousWeld()
{
  int bzrCount;
  while (bzrCount < 2)
  {
    digitalWrite(bzr, HIGH);
    delay(60);
    digitalWrite(bzr, LOW);
    delay(60);
    bzrCount++;
  }
  //bzrCount = 0;
  buttonState = digitalRead(buttonPin);
  while (buttonState == HIGH)
  {
   // if (buttonState == LOW) // press and hold buttonPin for 5 seconds to end loop
   // {
   //   break;
  //  }
    int counter;
    lcd.backlight();
    lcd.setCursor(0, 1);
    lcd.print("Weld Time: ");
    lcd.print(WeldStep());
    lcd.print("ms");
    lcd.setCursor(1, 2);
    lcd.print("Check Temperature");
    lcd.setCursor(0, 3);
    lcd.print("Weld Cycle(s): ");
    lcd.print(counter);
    delay(100);
    buttonState = digitalRead(buttonPin);
    backLightStart = 0;
    Weld();
    counter++;
    delay(200);
  //  if (buttonState == LOW) // press and hold buttonPin for 5 seconds to end loop
  //  {
   //   break;
  //  }
  }
  delay(500);
  digitalWrite(bzr, HIGH);
  Display();
  delay(500);
  digitalWrite(bzr, LOW);
  delay(1000);
}

I've tried removing the delay's 200, 400 and 800 and tested them out. Seems to more responsive when breaking the continuous loop.

The 400 was to give time for the servo to reach its position before the weld begins, which i will be tweaking once i have all the casing and assembly done. The 800 was to keep the pressure on the nickel strips so as to cool down the weld nugget there by making a good weld.

The 400 was to give time for the servo to reach its position before the weld begins,

The servo move to welding position certainly has delay() in its move, and the 400 may not be necessary.

void ServoStart() // Increments to start position from Finish
{
  for (i = SrvStartSet(); i <= SrvEndSet(); i += 1)
  {
    myServo.attach(servoPin);
    myServo.write(i);
    delay(25);
  }
}

The delay after servo detach seems unnecessary.

myServo.detach();
  delay(250);

Why do you have delay(100) and delay(200) in the while loop?

Why do you have delay(100) and delay(200) in the while loop?

Unnecessarily added them, removed. :)

The delay with in the void ServoStart() function is the default delay to reach the next position. In mine the servo just returns back the moment the welding is completed.

Sine you say its not necessary i will have to complete the remaining project and test/tweak things a bit. Till then i will just stick to what i had earlier.

One thing you can do to tighten up the while() loop is to remove the unchanging parts of the lcd display and print then at the start only. I may have the cursor positions incorrect, but here’s the idea

void continousWeld()
{
   //print unchanging info only once
   lcd.backlight();
   lcd.setCursor(0, 1);
   lcd.print("Weld Time:    ms");
   lcd.setCursor(1, 2);
   lcd.print("Check Temperature");
   lcd.setCursor(0, 3);
   lcd.print("Weld Cycle(s): ");
  
  int bzrCount;
  while (bzrCount < 2)
  {
    digitalWrite(bzr, HIGH);
    delay(60);
    digitalWrite(bzr, LOW);
    delay(60);
    bzrCount++;
  }
  //bzrCount = 0;
  buttonState = digitalRead(buttonPin);
  while (buttonState == HIGH)
  {
     int counter;
    //lcd.backlight();
    //lcd.setCursor(0, 1);
    //lcd.print("Weld Time: ");
    lcd.setCursor(11,1);
    lcd.print(WeldStep());
    //lcd.print("ms");
    //lcd.setCursor(1, 2);
    //lcd.print("Check Temperature");
    //lcd.setCursor(0, 3);
    //lcd.print("Weld Cycle(s): ");
    lcd.setCursor(14,3);
    lcd.print(counter);
   // delay(100);
    backLightStart = 0;
    Weld();
    counter++;
    buttonState = digitalRead(buttonPin);
   // delay(200);
  }
  delay(500);
  digitalWrite(bzr, HIGH);
  Display();
  delay(500);
  digitalWrite(bzr, LOW);
  delay(1000);
}

One thing you can do to tighten up the while() loop is to remove the unchanging parts of the lcd display and print then at the start only. I may have the cursor positions incorrect, but here's the idea

Thank you for pointing it out to me. i've just done that.