Trying to write non blocking code, failing

Hi
I have been trying to get out of using the old faithful but dreaded "delay()" in my code.
I thought I was wrapping my head around the idea of non blocking code but i cant seem to get this to work.
Basically the code is running on an uno
stepper is wired through a setp,dir driver, all inputs and outputs work as expected but the timing doesn't seem to work reliably.
I want to code to once it receives the go signal from the sensor to
wait a specified period
set 'ram' HIGH
wait a period
start moving the stepper and turn on Coder
after a set period turn coder off
run until the label gap sensor sees a gap
keep running until motor has moved more than the value of the pot on A1 then stop
wait a specified period
set ram LOW
not allow another process to start for a specified period.

all the tiling variables are declared and set before setup

What actually happens is the process works but without any of the delays/wait times. the weird thing is that it sometimes it seems to work too.
I put the Serial print in of the label finish and current millis.
sometimes the difference is the amount specified by labelFinish, and sometimes theyre the exact same value.
Any help would be appreciated
Thanks
Fulstop

#include <AccelStepper.h>
#include <MultiStepper.h>
int coder = 12;
int coderActive = 0;
int potPin = A1;
int potValue = 0;
unsigned long coderDwell = 100; //time coder is on/HIGH
int labelGap = 9; // label gap sensor
int go = 8; //jar sensor
int onOff = 7; // on off button pin
int ram = 6; // ram pin
int labelActive = 0;// label is currently feeding, stepper running
int inProcess = 0; //variable to record if the process is running
int ramVar = 0; //variable to record ram position
int labelMaxLength = 10000;
int labelDeadSpace = 50; //steps to ignore label gap
unsigned long ramInStart; //time stamp for ram moving in
const unsigned long ramInToLabelStartDelay = 500;//delay time from ram moving in to label starting
const unsigned long ramInDelay = 100;// delay from go signal to ram moving in
const unsigned long postLabelDelay = 300;//delay after stepper has stopped till the ram moves out
unsigned long labelFinish;//time stamp for when the stepper has stopped
const unsigned long betweenLabelDelay = 1200; //delay from the ram going out to when the next process can start
unsigned long currentMillis;//millis() is checked each run through loop and becomes currentMillis
unsigned long startMillis ; //timestamp for when the process starts/ go sensor triggered
unsigned long postLabel; //timestamp for when stepper stops
unsigned long coderMillis;//timestamp for when the coder output goes HIGH
int stepsBeforeEnd = 0;//variable for recording the steps at labelgap trigger to compare for overrun
int labelOverrun = 0; //variable to adjust how much overrun with potentiometer

AccelStepper labelStepper(AccelStepper::DRIVER, 11, 10);

void setup() {
  pinMode(go, INPUT_PULLUP);
  pinMode(labelGap, INPUT_PULLUP);
  pinMode(onOff, INPUT_PULLUP);
  pinMode(ram, OUTPUT);
  pinMode(coder, OUTPUT);
  labelStepper.setMaxSpeed(1040);
  labelStepper.setAcceleration(5000);//was 12000
  labelStepper.setCurrentPosition(0);

  Serial.begin(9600);
}

void loop() {
  currentMillis = millis();
  potValue = analogRead(potPin);
  labelOverrun = map(potValue, 0, 1024, 800, 0);
  int buttonState = digitalRead(go);
  int labelState = digitalRead(labelGap);
  int onOffState = digitalRead(onOff);



  if ((digitalRead(labelGap) == LOW) && (labelStepper.currentPosition() >= labelDeadSpace) && (inProcess == 3)) //SETS STOP POSITION ONCE GAP SEEN
  {
    //labelActive = 0;
    inProcess = 4;
    //Serial.println("label Gap");
    stepsBeforeEnd = labelStepper.currentPosition();

    //labelStepper.setCurrentPosition(0);
    //postLabel = millis();

  }

  if (((labelStepper.currentPosition() - stepsBeforeEnd) >= labelOverrun) && (inProcess == 4)) //TURNS STEPPER OFF AFTER OVERRUN AMOUNT
  {
    labelActive = 0;
    inProcess = 5;
    //Serial.println("has Overrun enough");
    labelStepper.setCurrentPosition(0);
    postLabel = millis();
  }
  if  ((buttonState == LOW) && (inProcess == 0) && (onOffState == LOW)) //START THE PROCESS
  {
    inProcess = 1;
    //Serial.println("start the process");
    startMillis = millis();
    labelStepper.moveTo(labelMaxLength);
    // labelStepper.setSpeed(labelSpeed);
  }
  if ((inProcess == 1) && (ramVar == 0)) //&& ((currentMillis - startMillis) >= ramInDelay))//RAM IN
  {
    // Serial.println("ram in");
    digitalWrite(ram, HIGH);
    ramVar = 1;
    inProcess = 2;
    ramInStart = millis();
  }

  if ((ramVar == 1) && (currentMillis - ramInStart >= ramInToLabelStartDelay) && (inProcess == 2)) // CODER ON, SET LABELLING ACTIVE
  {
    // Serial.println("coder on, labelling active");
    labelActive = 1;
    digitalWrite(coder, HIGH);
    inProcess = 3;
    coderActive = 1;
    coderMillis = millis();
  }

  if ((coderActive == 1) && (currentMillis - coderMillis >= coderDwell) && (inProcess == 3)) //CODER SIGNAL OFF
  {
    // Serial.println("coder off");
    digitalWrite(coder, LOW);
    coderActive = 3;
  }



  if ((inProcess == 5) && (ramVar == 1) && (currentMillis - postLabel >= postLabelDelay)) //RAM OUT
  {
    //Serial.println("ram OUT");
    digitalWrite(ram, LOW);
    ramVar = 0;
    // inProcess = 0;
    labelFinish = millis();

    inProcess = 6;
  }

  if ((inProcess == 6) && (buttonState == HIGH))
  {
    inProcess = 7;
    // Serial.println("I have seen the empty space");
  }
  if ( inProcess == 7)// && (currentMillis - labelFinish > betweenLabelDelay)) //DELAY TO STOP DOUBLE LABEL
  {

    // Serial.println("delay to stop double label");
    Serial.print("currentMillis ");
    Serial.println(currentMillis);
    Serial.print(" labelFinish ");
    Serial.println(labelFinish);
    inProcess = 0;
    coderActive = 0;
  }

  if (labelStepper.currentPosition() >= labelMaxLength) //TO STOP IT LABELLING FOREVER IF SENSOR ISSUE
  {
    //Serial.println("label gone too long");
    labelActive = 0;
    // labelStepper.setCurrentPosition(0);
    inProcess = 5;
    coderActive = 0;
    labelStepper.setCurrentPosition(0);
    postLabel = millis();
  }
  if (labelActive == 1)//MOVE MOTOR
  {
    labelStepper.run();
    // Serial.print("label Position ");
    //Serial.println(labelStepper.currentPosition());

  }
}

Have a look at TaskMacros. They allow to "unblock" your code without heavy rewrite.

Thanks, iv had a quick look at taskMacros and it looks pretty handy, will Investigate and have a play.
thing is I thought I had the whole (currentMillis- lastMillis >Period ) thing sorted and am confused about what iv done wrong, am I not even close?

Here:

you are not updating the timestamp.

If you write functions instead of putting everything in loop(), you can make static variables all named the same, instead of coming up with a thousand names for global variables. Remember to declare constants as const, it'll save some memory.

all the multi-element conditional statements makes the code hard to read and difficult to know what happens when

looks like you have a state machine (e.g. inProcess) that moves sequentially thru a set of states -- a "sequencer"

i believe the code would be easier to read/understand/debug/enhance if the various processing for each state were under a "switch (inProcess)" statement.
within each state, there can be conditional code for the various stimuli determining when to advance to the next state and what actions to take

detecting the stimuli should be outside the state machine blocks. i believe the various timing code can also be improved, possibly just a single piece of code that generates a single stim indicating the timer has expired

1 Like

Echo the above.

Also suggest:

Using an "enum" type to give each state a name like "waitingForGap" rather than a non-intuitive number like 3. Use this new type to make your "inProcess" variable, instead of making it an int. Another variable, perhaps called "previousProcess" could be updated at the end of loop() so that it's easy to see when the state has changed since the previous execution of loop(), so that any actions that need to be done when entering the new state can be triggered.

When a variable like "coderActive" only ever has 2 values: 0 or 1, use a "bool" variable instead of an int. "bool" variables can only have 2 values: true or false. This makes code a little more readable because instead of saying if (coderActive == 1) you can simply say if (coderActive). When using bool variables, always give them names that reflect the meaning of the variable when the value is true. "coderActive" is a good example. But "onOff" would not be such a good example because it's not 100% obvious that true is meant to mean on and false means off. "systemOn" would be a better name.

consider

#undef MyHW    // changes for my hardware & simulated libraries
#ifdef MyHW
char s [80];

struct AccelStepper  {
    int position;

    enum { DRIVER };
    AccelStepper (int m, int a, int n)  { };

    int  currentPosition (void)     { return position; };
    void moveTo          (int pos)  {
        position = pos;
        sprintf (s, " %s: pos %6d", __func__, position);
        Serial.println (s);
    }
    void run             (void)     {
        position++;
        sprintf (s, " %s: pos %6d", __func__, position);
        if (! (position % 50))
            Serial.println (s);
        delay (100);
    };
    void setAcceleration (int a)  { 
        sprintf (s, " %s: %6d", __func__, a);
        Serial.println (s);
    }
    void setCurrentPosition (int a) {
        position = a;
        sprintf (s, " %s: pos %6d", __func__, position);
        Serial.println (s);
    };
    void setMaxSpeed     (int a) {
        sprintf (s, " %s: %6d", __func__, a);
        Serial.println (s);
    }
};

const int labelGap  = A1;
const int go        = A2;
const int onOff     = A3;

const int coder     = 13;
const int ram       = 12;
const int potPin    = A0;

#else
# include <AccelStepper.h>
# include <MultiStepper.h>

const tint labelGap = 9; // label gap sensor
const int go        = 8; //jar sensor
const int onOff     = 7; // on off button pin

const int coder     = 12;
const int ram       = 6; // ram pin
const int potPin    = A1;
#endif

// -----------------------------------------------------------------------------
int coderActive          = 0;
int potValue             = 0;
unsigned long coderDwell = 100;     // time coder is on/HIGH
int labelActive          = 0;       // label is currently feeding, stepper running
int inProcess            = 0;       // variable to record if the process is running
int ramVar               = 0;       // variable to record ram position
int labelMaxLength       = 10000;
int labelDeadSpace       = 50;      // steps to ignore label gap

unsigned long       ramInStart;                     // time stamp for ram moving in
const unsigned long ramInToLabelStartDelay = 500;   // delay time from ram moving in to label starting
const unsigned long ramInDelay             = 100;   // delay from go signal to ram moving in
const unsigned long postLabelDelay         = 300;   // delay after stepper has stopped till the ram moves out
unsigned long       labelFinish;                    // time stamp for when the stepper has stopped
const unsigned long betweenLabelDelay      = 1200;  // delay from the ram going out to when the next process can start

unsigned long startMillis ;      // timestamp for when the process starts/ go sensor triggered
unsigned long postLabel;         // timestamp for when stepper stops
unsigned long coderMillis;       // timestamp for when the coder output goes HIGH
int          stepsBeforeEnd = 0; // variable for recording the steps at labelgap trigger to compare for overrun
int          labelOverrun   = 0; // variable to adjust how much overrun with potentiometer

AccelStepper labelStepper (AccelStepper::DRIVER, 11, 10);

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

    pinMode (go,         INPUT_PULLUP);
    pinMode (labelGap,   INPUT_PULLUP);
    pinMode (onOff,      INPUT_PULLUP);
    pinMode (ram,        OUTPUT);
    pinMode (coder,      OUTPUT);

    labelStepper.setMaxSpeed (1040);
    labelStepper.setAcceleration (5000);//was 12000
    labelStepper.setCurrentPosition (0);

    Serial.println ("ready");
}

// -----------------------------------------------------------------------------
void loop ()
{
    unsigned long msec  = millis ();
    potValue            = analogRead (potPin);
    labelOverrun        = map (potValue, 0, 1024, 800, 0);
    int buttonState     = digitalRead (go);
    int labelState      = digitalRead (labelGap);
    int onOffState      = digitalRead (onOff);

    switch (inProcess)  {
    case 0:
    default:
        if (LOW == buttonState  && LOW == onOffState)  {
            inProcess   = 1;
            startMillis = msec;
 //         labelStepper.moveTo (labelMaxLength);
            Serial.println ("0: start");
        }
        break;

    case 1:
        if (0 == ramVar) {
            inProcess  = 2;
            ramVar     = 1;
            ramInStart = msec;
            digitalWrite (ram, HIGH);
            Serial.println ("0: ram on");
        }
        break;

    case 2:
        if (1 == ramVar && (msec - ramInStart) >= ramInToLabelStartDelay) {
            inProcess   = 3;
            labelActive = 1;
            coderActive = 1;
            coderMillis = msec;
            digitalWrite (coder, HIGH);
            Serial.println ("2: coder on, labelling active");
        }
        break;

    case 3:
        if (coderActive == 1 && (msec - coderMillis) >= coderDwell) {
            coderActive = 3;
            digitalWrite (coder, LOW);
            Serial.println ("3: coder off");
        }

        if (labelState == LOW && labelStepper.currentPosition () >= labelDeadSpace) {
            inProcess      = 4;
            stepsBeforeEnd = labelStepper.currentPosition ();
            Serial.println ("3: label Gap");
        }
        break;

    case 4:
        sprintf (s, " 4: %d %d %d",
            labelStepper.currentPosition (), stepsBeforeEnd, labelOverrun);
        Serial.println (s);

        if ( (labelStepper.currentPosition () - stepsBeforeEnd) >= labelOverrun) {
            inProcess   = 5;
            labelActive = 0;
            postLabel   = msec;
            labelStepper.setCurrentPosition (0);
            Serial.println ("4: has Overrun enough");
        }
        break;

    case 5:
        if (ramVar == 1 && (msec - postLabel >= postLabelDelay)) {
            inProcess   = 6;
            ramVar      = 0;
            labelFinish = msec;
            digitalWrite (ram, LOW);
            Serial.println ("5: ram OUT");
        }
        break;

    case 6:
        if (buttonState == HIGH) {
            inProcess = 7;
            Serial.println ("6: I have seen the empty space");
        }
        break;

    case 7:
        inProcess   = 0;
        coderActive = 0;
        Serial.print ("currentMillis ");
        Serial.print (msec);
        Serial.print (", labelFinish ");
        Serial.println (labelFinish);
        Serial.println ("7: delay to stop double label");
        break;
    }

    //TO STOP IT LABELLING FOREVER IF SENSOR ISSUE
    if (labelStepper.currentPosition () >= labelMaxLength) {
        inProcess   = 5;
        labelActive = 0;
        coderActive = 0;
        postLabel   = msec;
        Serial.print   ("label Position ");
        Serial.print   (labelStepper.currentPosition ());
        Serial.println ("  label gone too long");
        labelStepper.setCurrentPosition (0);
    }

    //MOVE MOTOR
    if (labelActive == 1) {
        labelStepper.run ();
#if 0
        Serial.print ("label Position ");
        Serial.println (labelStepper.currentPosition ());
#endif
    }
}

Do you mean ALib0.h?

That's my version of the task macros. @Combie provides another library.

I don't want to hijack the thread, so I will start a new one with a question.