Breaking a DO..while loop with an interrupt

What I'm trying to do;
Exit an animation based on EITHER a user input OR a timer countdown.

How I'm trying to do it;
I'm using all millis() for the timer (no delay()), setting a "start_time" variable at the start of the animation, and then checking

run_time=millis()-start_time;

For the user input, I have a variable called "pattern" which is passed to the separate animation routines. IT can be changed by an interrupt attached to a button. Each animation has another variable, "this_pattern", so the loop test on my do..while loop is;

while (run_time <= change_time && pattern == this_pattern);

The timer loop is working...but changing "pattern" doesn't exit the loop immediately - it simply waits until the timer part is completed.

What am I doing wrong? I've declared "pattern" as volatile, and it's being returned from the ISRs...but doesn't seem to be having any effect on the do..while loop...or at least not instantly, which I need, as the loop length can be some 30 seconds and that's too long for a user to wait.

as the loop length can be some 30 seconds and that's too long for a user to wait.

Without seeing the code, I can only guess the clue is in the above statement. You have to find the parts in the loop which take so long and condition them also with this expression:

pattern == this_pattern

interrupt attached to a button

That statement alone indicates that we need to be looking somewhere else. Interrupts are for very fast things. Humans pressing buttons are really really slow. If you feel you need an interrupt for a human button press it usually indicates other design issues with the code.

A human pressing a button takes at a minimum a couple hundred milliseconds. That's the blink of an eye. If your loop function can't get around to checking a button at least once every 200ms then we really need to look at that and rewrite that.

I really would have liked to have a look at the code. Too bad that my crystal ball did not work!

Sorry for code excerpts...it's because it's spread over five or six tabs so is a pain to copy and paste (but much easier to edit). I will try to post in entirety today, if my health and time permit.

The loop is only 30 seconds long if it runs to the timer because it's got change_time set to 30000ms. I don't know how many times it evaluates it during that period - I was really hoping the && statement in it would exit it when "pattern" changes, even if run_time was still less than change_time. Apparently not.

Well, might as well start with the main tab I guess. The code is a mess with lots of variables I have discarded, but not cleaned up yet....

//Illumination
//Tabbed version for easier editing
//Assumes 20 LEDs total
//Linear animation only

//animation control array controls anims on/off
//comment out any line below to prevent the animation from being compiled
#include "01_shared_vars.h"  //3940 bytes
#include "BLINK.h"
#include "BLANK_ALL.h"
#include "DEBUG.h"           //4252 bytes
#include "NEWSHIMMER.h"          //7362 and 530
#include "RGBSINE.h"
#include "SWEEP.h"          //7362 and 530
#include "SPARKLE.h"
#include "CTHULHU.h"
#include "TBPRESS.h"
#include "BBPRESS.h"

void setup() {
  pinMode (PIN, OUTPUT);
  pinMode(TBpin, INPUT_PULLUP);
  pinMode(BBpin, INPUT_PULLUP);
  attachPCINT(digitalPinToPCINT(TBpin), TBPRESS, FALLING);
  attachPCINT(digitalPinToPCINT(BBpin), BBPRESS, FALLING);

  Serial.begin(9600);
  randomSeed(analogRead(0));
  strip.setBrightness(maxBRT); //reduced for battery work
  strip.begin();

}

void loop() {
  BLANK_ALL(1, 0);
      switch (pattern) {
    case 0:
      DEBUG(50,50,5000);
    case 1:
      start_time=millis();
      do{
        run_time=millis()-start_time;
        }while (run_time<=5000);
      sub_pattern = random(2, pattern_max + 1);
      switch (sub_pattern) {
        case 2:
          BLINK(1,100,100);
          SWEEP(pattern,sub_pattern,6000); 
          break;
        case 3:
          BLINK(2,100,100);
          SPARKLE(pattern,sub_pattern,5000); 
          break;
        case 4:
          BLINK(3,100,100);
          RGBSINE(pattern,sub_pattern,5000);
          break;
        default:
          break;
      }
      break;

    case 2:
      sub_pattern=1;
      BLINK(1,100,100);
      SWEEP(pattern,sub_pattern,20000); //runs pattern two with no increment (button exit only)
      break;
    case 3:
      sub_pattern=1;
      BLINK(2,100,100);
      SPARKLE(pattern,sub_pattern,20000); //runs pattern three with no increment (button exit only)
      break;
    case 4:
      sub_pattern=1;
      BLINK(3,100,100);
      RGBSINE(pattern,sub_pattern,20000); //runs pattern four with no increment (button exit only)
      break;
    default:
      break;
  }
}
[code/]

shared.vars tab

This is a REAL mess...

#include <Adafruit_NeoPixel.h>
#include <avr/interrupt.h>
const byte blinkPin = 13;

#define D_LEDS        0   //Debug LEDS
#define N_LEDS       20-D_LEDS   //LEDS in strip (temporary for testing)
#define PIN          11   //output pin
#define worms         1   //max worms in animations
//#define TBpin      3   //testing button switches
//#define BBpin      4   

const byte TBpin = 3;
const byte BBpin = 4;

const float piconv = 0.0175;
float Sindir[4]; // independent values for RGB and brightness sine waves
static float SinMod[4] = {random(0, 360), random(0, 360), random(0, 360), random(0, 360)};  //angular multipliers for each of R G B and Bright
//Cthulhu versions
float CSindir[4] = {0.5, 0.5, 1, 49}; // independent values for RGB and brightness sine waves
static float CSinMod[4] = {180,0,0,0};  //angular multipliers for each of R G B and Bright
int rebelstar;

byte LEDcolour[3]; //(RGB)
//a factor to manage high load. Set in each animation
float Loadfactor = 1;

Adafruit_NeoPixel strip = Adafruit_NeoPixel(N_LEDS, PIN, NEO_GRB + NEO_KHZ800);

#include <PinChangeInterrupt.h>

byte colourpairs [5][2][3][2] = {
  { { {  0,   0}, { 50,  75}, {255, 255} }, { {200, 225}, {  0, 150}, {  0,   0} } },  //Colour Pair One,  C1 : RMin, RMax, GMin, GMax, Bmin, Bmax , C2 : RMin, RMax, GMin, GMax, Bmin, Bmax
  { { {  0,   0}, {  0,  25}, {175, 255} }, { {225, 255}, {150, 175}, {  0,   0} } },  //Colour Pair Two
  { { {255, 255}, {  0,  55}, {  0,  55} }, { {  0,  55}, {  0,  55}, {  0, 255} } },  //Colour Pair Three
  { { {  0, 255}, {  0, 255}, {  0, 255} }, { {  0,   0}, {  0, 255}, {  0, 255} } },  //Colour Pair Four
  { { {  0,   0}, {150, 255}, {  0,  55} }, { {200, 255}, {  0,   0}, {200, 255} } },  //Colour Pair Five
};
float LEDSine[N_LEDS];       //used in sine wave anims but bad for memory 
float LEDSinespeed[N_LEDS];       //used in sine wave anims but bad for memory 
float LEDR[N_LEDS];
float LEDG[N_LEDS];
float LEDB[N_LEDS];
byte LEDONOFF[N_LEDS];

int debug = 1;
int bias[3] = {1, 1, 1};              //used in SPARKLE
int colA[3] = {0, 0, 0};              //current RGB
int colB[3] = {255, 255, 255};        //target RGB
int colfade[3]={0,0,0};
//Current limiter
float MasterLoadfactor = 1;         //overall load factor applied to all animations

//Crossfader
int fadestep;
//Structural
long getcol;
float DS = 1;                         //speed of sine wave
float Master = 1;                     //controls overall brightness, 0 = totally off, 1 = full brightness! Has side effect of speeding up the shimmer as well.
long changecol = 5000;                //time (ms) before colour change (0=off)
boolean sparkles = 1;                 //adds random colour sparkles, 1= on, 0 = off
boolean sparklim = 0;                 //limit sparkles to colour parameters
int chance;
int ms_delay;

int p;                                //pixel number primary;
int q;                                //pixel number secondary;
int i;                                //general increment
int j;                                //general increment
int jmax;                             //j maximum limit
int k;                                //general increment
int l;                                //general increment
float z;

//time variables
int pattern_max = 4;                    //highest pattern number
volatile int pattern;                        
int target_pattern;
int sub_pattern;
int this_pattern;
long TBpressed=millis();
long BBpressed=millis();
volatile byte state = LOW;
int maxBRT=255;
long blink_start;
long blink_on_for;
long blink_end;
long blink_off_for;
long ms_on;
long ms_off;
long start_time;                      //start of animation
long run_time;                        //duration of animation
long change_time = 10000;             //time to change animation
long lastdel;                         //used by BLANK_ALL
float fadelength = 360;
static float fader = 0;
static float fade = fader / fadelength;
float DStot = 1;                      //experimental = an overall fade that colour and overall fade share.
float colDS = 1 * DStot;              //speed of cosine wave for colour
float randomcolDS = 0;
float fadeDS = 0.50 * DStot;          //speed of cosine wave for fade
static boolean showme = 0;            //will display colour ranges at startup. Caution! Slow if wide range set!
int bugLED = 0;                       //testing - first (bugLED) LEDS can be used to indicate sketch state.
boolean sinONOFF = 1;                 //0 = sine wave off, 1 = sine wave on (effectively shimmer ON/OFF
boolean fadeONOFF = 1;                //0 = fade off, 1 = fade on (time still runs but LEDs don't dim)
int maxcycles = 2;
static int cycle = 1;                 //cycle 0 = Blue, cycle 1 = yellow
static int colpair = random(0, 5);
static int phase = 1;                 //phase 0 = main, phase -1 = fade to black, phase 1 = fade in new colour *NOW STARTS ON FADE IN*
static long coltime;                  //start time of current cycle
static long runtime = 0;              //run time of current cycle

BBPress tab

void BBPRESS() {
      pattern = pattern - 1;
      if (pattern < 1) pattern = pattern_max;
      strip.setPixelColor(pattern, 0, 255, 0);
      strip.show();
  }

and TBPress tab

void TBPRESS() {
      pattern = pattern + 1;
      if (pattern > pattern_max) pattern = 1;
      strip.setPixelColor(pattern, 255, 0, 0);
      strip.show();
      }

The three animations - SWEEP, SPARKLE and RGBSINE

SWEEP;

void SWEEP(int pattern,int sub_pattern, long change_time){

  //REwritten, name kept only for convenience
  //this is now a "knight rider" type animation
  
  this_pattern = 2;
  start_time = millis();
  //change_time = 30000;
  coltime = millis();
  float maxfadesteps = 16;
  for (i = 0; i < 3; i++) {
    colA[i] = random(0, 255);
    colB[i] = 0;//random(0, 255);
  }

  /*
     j controls the loop. it is incremented from 0 to 360.
     Across this range the sine of j will vary from -1 to +1
     using the "map" command this can be overlaid onto a string of "N" leds
     but "map" only uses integers so we need to ensure the value of "j"
     is multiplied by a number big enough to ensure most output is >1
  */
  z = 1;
  int tail = 7;
  do {
    p = map(20 * sin(z * PI / 180), -20, 20, D_LEDS, D_LEDS+N_LEDS);
    q = map(20 * sin((z - (9 * tail)) * PI / 180), -20, 20, D_LEDS, D_LEDS+N_LEDS);
    strip.setPixelColor(p, 0, 0, 255);
    getcol = strip.getPixelColor(q);
    strip.setPixelColor(q, 55, 55, 55);
    strip.show();

    //delaystart = millis();

    //  do {
    //  delayend = millis() - delaystart;
    //} while (delayend <= 10);

    z = z + .1;
    fadestep = fadestep + 1;
    if (z >= 360) z = 0;
    if (fadestep >= maxfadesteps) fadestep = 0;

    run_time = millis() - start_time;
    target_pattern=pattern*sub_pattern;   
  } while (target_pattern==this_pattern && run_time <= change_time);
}

SPARKLE;

void SPARKLE(int pattern,int sub_pattern, long change_time) {
 target_pattern=pattern*sub_pattern;
 this_pattern = 3;
  //change_time = 30000;
  start_time = millis();
  Loadfactor = 1;
  for (p = 0; p < N_LEDS; p++) {
    LEDONOFF[p] = 0;
    LEDSine[p] = 0;
    LEDR[p] = random(0, 50);
    LEDG[p] = LEDR[p];
    LEDB[p] = random(100, 256);
  }

  do {
    for (p = D_LEDS; p < D_LEDS+N_LEDS; p++) {
      strip.setPixelColor(p, LEDONOFF[p] * LEDR[p] * sin(LEDSine[p] * PI / 180), LEDONOFF[p] * LEDG[p] * sin(LEDSine[p] * PI / 180), LEDONOFF[p] * LEDB[p] * sin(LEDSine[p] * PI / 180));
      if (LEDONOFF[p] == 0) {
        chance = random(0, 3600);

        if (chance >= 3580) {
          LEDONOFF[p] = 1;
          LEDSine[p] = 0;
        }

      }
      else
      {

        LEDSine[p] = LEDSine[p] + 1;

        if (LEDSine[p] >= 180) {
          //LEDR[p] = random(0, 255);
          //LEDG[p] = random(0, 255);
          //LEDB[p] = random(0, 255);
          LEDONOFF[p] = 0;
          LEDSine[p] = 0;
        }

      }
    }
    strip.show();

    run_time = millis() - start_time;
    target_pattern=pattern*sub_pattern;
  } while (target_pattern==this_pattern && run_time <= change_time);

RGBSINE

void RGBSINE(int pattern, int sub_pattern, long change_time ) {

  //Uses offset sine waves applied to the RGB components of the LEDs
  //All colour values are maxBRT. Let the sine wave do the work!
  //New August 24th - fourth channel for brightness

  for (j = 0; j < 3; j++) {
    Sindir[j] = random(1, 10); //direction and magnitude of travel for Bright and R G B.
  }
  target_pattern = pattern * sub_pattern;
  this_pattern = 4;
  //change_time = 30000;
  start_time = millis();
  do {
    boolean brightpulse = 1;
    boolean colourpulse = 1;
    for (p = D_LEDS; p < D_LEDS+N_LEDS; p++) {
      for (j = 0; j < 3; j++) {
        colA[j] = maxBRT;
        if (colourpulse) {
          colA[j] = colA[j] * (1 + sin(SinMod[j] * PI / 180)) / 2;
        }
        if (brightpulse) {
          colA[j] = colA[j] * (1 + sin(SinMod[3] * PI / 180)) / 2;
        }
      }
      strip.setPixelColor(p, colA[0], colA[1], colA[2]);
      SinMod[3] = SinMod[3] + (17.95 * 3);  // or Sindir[3] brightness adjusts within loop

      /*Above depends on N_LEDS
         unsure of relationship
         for N_LEDS=20, stationary equates to +18,
         +17.99 being very slow down,
         +18.01 being very slow up,
         Multiples of 17.99 keep the same speed but split the LEDs into smaller groups
         Multiply by (N_LEDS + n) for values of n from 1 to N_LEDS/2 to split the lights into n segments travelling down.
         values of N from N/LEDS/2 go in the opposite direction.
      */

      if (SinMod[3] > 359) SinMod[3] = SinMod[3] - ((int)(SinMod[3] / 360) * 360);
    }
    strip.show();
    for (j = 0; j < 3; j++) {
      SinMod[j] = SinMod[j] + Sindir[j];                                               // colour adjust after each loop
      if (SinMod[j] < 0) SinMod[j] = SinMod[j] + 360;
      if (SinMod[j] > 359) SinMod[j] = SinMod[j] - 360;
    }

    run_time = millis() - start_time;
    target_pattern = pattern * sub_pattern;
  } while (target_pattern == this_pattern && run_time <= change_time);
}

Well...you asked.

The only missing parts are debug (which just turns the lights on to prove the connections are sound) and blankall, which blanks the LEDs between patterns.

Oh, hell with it here they are;

DEBUG;

void DEBUG(long ms_on, long ms_off, long change_time) {
  //runs only once at startup
  Loadfactor = 1;
  //change_time=5000;
  start_time = millis();

  do {
    for (p = D_LEDS; p < N_LEDS; p++) {
      strip.setPixelColor(p, maxBRT, maxBRT, maxBRT);
      strip.show();
      blink_start = millis();
      do {
        blink_on_for = millis() - blink_start;
      } while (blink_on_for <= ms_on);

      strip.setPixelColor(p, 0, 0, 0);
      strip.show();
    }
    blink_end = millis();
    do {
      blink_off_for = millis() - blink_end;
    } while (blink_off_for <= ms_off); run_time = millis() - start_time;
  } while (run_time <= change_time);
  pattern=1;
}

BLANk_ALL;

void BLANK_ALL(boolean showme, long ms_delay) {
  for (p = 0; p < N_LEDS; p++) {
    strip.setPixelColor(p, 0, 0, 0);
    lastdel = millis();
    do {
      if (showme) strip.show();
    } while (millis() - lastdel < ms_delay);
  }
  if (!showme) {
    strip.show();
  }
}

Really hope someone can help - even if advising a way to do it entirely differently...as long as the animations run the same, can be timed if a button is NOT pressed and change immediately when one is.

Thanks!

Correction - text comment in sweep that starts "j controls the loop" should have all instances of j replaced by z. Sorry, in my defence I told you it was a mess!

Sorry, I'm lazy and it take too much time to try to make sense of several chunks of code. It's much better to ask for help with a short program that illustrates the problem.

Anyway, in general if you need to be able to stop a WHILE part way through then you should not be using WHILE. Just use IF and allow loop() to do the repetition - that's what it is designed for.

Have a look at how the code is organized in Several Things at a Time

Note how each function runs very briefly and returns to loop() so the next one can be called. None of the functions tries to complete a task in one call. And there may be dozens of calls to a function before it is actually time for it to do anything.

...R

exactly what Robin said.
You are not using delay() but you have found a construct which does exactly the same thing.

 do {
     if (showme) strip.show();
   } while (millis() - lastdel < ms_delay);

These will be unaffected by your attempts to break out of your main loop.

I wrote this as an example for a tutorial thread that I am writing and it may provide some inspiration for you

The sketch runs 3 LED patterns and you can change between them using a button input. To see it working you don't even need LEDs as there is also an output to the Serial monitor. All you need to do to see it running is to change the definition of the changePin variable to suit yourself. Note that the pattern changes when the changePin is taken LOW

Note that the current LED pattern may be "interrupted" at any time, even in the middle of a pattern, and that there is not an interrupt, while or for loop in sight anywhere

//move between LED patterns using an input
//visualisation via Serial monitor as well as LEDs
//button state change and debounce included

enum states
{
  LEFT_TO_RIGHT,  //meaningful names for the LED states
  RIGHT_TO_LEFT,
  FLASH_ALL
};

unsigned long currentTime = millis;  //the current value of millis()
unsigned long patternChangeTime; //time of the last change of pattern occured
unsigned long ledChangeTime = currentTime;  //time of the last LED change
unsigned long ledPeriod = 500;  //period of each LED change
byte currentState = LEFT_TO_RIGHT; //start in this state
const byte changePin = A3;  //change this to suit your system
boolean changePattern = false;
const byte ledPins[] = {3, 5, 6, 9};
const byte NUMBER_OF_LEDS = sizeof(ledPins) / sizeof(ledPins[0]);
int currentLedIndex = 0;  //this is an int because we need to test < 0
byte flashLedsState = LOW;

void setup()
{
  Serial.begin(115200); //start Serial in case it is needed for debugging/reporting
  for (int led = 0; led < NUMBER_OF_LEDS; led++)
  {
    pinMode(ledPins[led], OUTPUT);
    digitalWrite(ledPins[led], HIGH);  //start with all LEDs off
  }
  allLeds(HIGH); //start with all LEDs off
  pinMode(changePin, INPUT_PULLUP);
}

void loop()
{
  currentTime = millis(); //get the current time
  if (checkButtonPress() == true)
  {
    changePattern = true;
  }
  switch (currentState) //execute code for the current state
  {
    case LEFT_TO_RIGHT:
      if (changePattern == true)
      {
        changePattern = false;
        currentState = RIGHT_TO_LEFT;
        allLeds(HIGH);  //turn off all LEDs
        currentLedIndex = NUMBER_OF_LEDS - 1; //start at the right in next state
        ledChangeTime = currentTime;
      }
      if (currentTime - ledChangeTime >= ledPeriod)  //time to change ?
      {
        serialDisplay();
        digitalWrite(ledPins[currentLedIndex], !digitalRead(ledPins[currentLedIndex]));
        ledChangeTime = currentTime;
        currentLedIndex++;
        if (currentLedIndex >= NUMBER_OF_LEDS)
        {
          currentLedIndex = 0;
        }
      }
      break;  //end of code for LEFT_TO_RIGHT
    case RIGHT_TO_LEFT:
      if (changePattern == true)
      {
        changePattern = false;
        currentState = FLASH_ALL;
        allLeds(HIGH);  //turn off all LEDs
        ledChangeTime = currentTime;
      }
      if (currentTime - ledChangeTime >= ledPeriod)  //time to change ?
      {
        serialDisplay();
        digitalWrite(ledPins[currentLedIndex], !digitalRead(ledPins[currentLedIndex]));
        ledChangeTime = currentTime;
        currentLedIndex--;
        if (currentLedIndex < 0)
        {
          currentLedIndex = NUMBER_OF_LEDS - 1;
        }
      }
      break;  //end of code for RIGHT_TO_LEFT
    case FLASH_ALL:
      if (changePattern == true)
      {
        changePattern = false;
        currentState = LEFT_TO_RIGHT;;
        allLeds(HIGH);  //turn off all LEDs
        currentLedIndex = 0;  //start next state at first LED
        ledChangeTime = currentTime;
      }
      if (currentTime - ledChangeTime >= ledPeriod)  //time to change ?
      {
        serialDisplay();
        allLeds(flashLedsState);
        ledChangeTime = currentTime;
        flashLedsState = !flashLedsState;
      }
      break;  //end of code for FLASH_ALL
  }
}

void allLeds(byte state)
{
  for (int led = 0; led < NUMBER_OF_LEDS; led++)
  {
    digitalWrite(ledPins[led], state);  //set state of all LEDs
  }
}

boolean checkButtonPress()
{
  static byte currentButtonState = HIGH;
  static byte previousButtonState = HIGH;
  static unsigned long buttonPressTime;
  static boolean hasBeenPressed = false;
  const byte DEBOUNCE_PERIOD = 10;
  previousButtonState = currentButtonState;
  currentButtonState = digitalRead(changePin);
  if (currentButtonState == LOW && previousButtonState == HIGH)  //button has become pressed
  {
    buttonPressTime = currentTime;
    hasBeenPressed = true;
  }
  if (currentTime - buttonPressTime >= DEBOUNCE_PERIOD && currentButtonState == LOW && hasBeenPressed)
  {
    hasBeenPressed = false;
    return true;
  }
  previousButtonState = currentButtonState;
  return false;
}

void serialDisplay()
{
  for (int led = 0; led < NUMBER_OF_LEDS; led++)
  {
    Serial.print(digitalRead(ledPins[led]) == HIGH ? "O " : "  ");
  }
  Serial.println();
}

Robin2 - teehee, I normally get told not posting my code is unhelpful...now you see why I don't...ALL my stuff is like this and it overfaces people.

6v6gt - that particular loop is one that doesn't need to be interrupted; it's a leftover from one I was using as part of the debugging process...I never set show.me to true any more and it won't be part of the final product.

I THINK I have fixed it and it now works...the issue was a need to have "pattern" declared as static (so that the user's changes are not "lost" when the loop reruns) and volatile (so that the interrupt can change it's value) at the same time...I don't know if that was possible, so I've split it in two; pattern is now static, and instead of the interrupt making a change to pattern directly, the buttons instead set either a "pattern_up" or "pattern_down" variable to 1.

The do_while loop checks for any change to these volatile variables with the following statement;

if (pattern_up == 1 || pattern_down == 1) {
      pattern = pattern + pattern_up - pattern_down;
      pattern_up = 0;
      pattern_down = 0; break;
    }

The break returns it to the (now much simplified) select (pattern) loop.

If I get chance to upload a video of it working I'll post a link to it here later.

GreyArea:
Robin2 - teehee, I normally get told not posting my code is unhelpful...now you see why I don't...ALL my stuff is like this and it overfaces people.

Not posting your code IS bad. So is posting 6 tabs of code. In this case, a simple example would have sufficed.

vaj4088:
Not posting your code IS bad. So is posting 6 tabs of code. In this case, a simple example would have sufficed.

Posting code is always the best. If it's long, convoluted, or just plain ugly (lots of that on the forum) then go with an MCVE.

Posting unused code, unused variables, and comments that are wrong can also be bad. The helpers here are unpaid and yet very helpful. Their time should be put to good use.

vaj4088:
Not posting your code IS bad. So is posting 6 tabs of code. In this case, a simple example would have sufficed.

I've done that before and subsequently been told to post all of it. The risk in writing a simple example is that I miss out the bit that's causing the problem. I get the best results from this forum from people who've been prepared to discuss the general approach, given an outline of what I'm trying to achieve.

They're in a minority its true, but for me they are the best part of this site.

By the way item is now finished and fully functional. I have video of it but as I'm giving it to someone over the weekend I'll share the video after that once I've checked they are okay with it. If not I've a similar personal piece I'm working on, but that won't be ready for some time.

Thanks all again for your help.

GreyArea:
I've done that before and subsequently been told to post all of it. The risk in writing a simple example is that I miss out the bit that's causing the problem. I get the best results from this forum from people who've been prepared to discuss the general approach, given an outline of what I'm trying to achieve.

They're in a minority its true, but for me they are the best part of this site.

By the way item is now finished and fully functional. I have video of it but as I'm giving it to someone over the weekend I'll share the video after that once I've checked they are okay with it. If not I've a similar personal piece I'm working on, but that won't be ready for some time.

Thanks all again for your help.

I think you miss the point. The deal is that you shouldn't normally have a problem that only exists in a 6 tab piece of code. Usually you start out by testing it all out in a short code and get it working there and then add it to your huge code once it is working. If something doesn't work in the larger code then you start adding things into the test code until you find the issue.

That's the way development is done. If you try to write a whole bunch of pages of code at once and then finally try to test it all together at the end then you're just setting yourself up for failure. If you look at like the code for my robot, there are a couple of dozen files full of code that all go together on it. But there's also a test folder with hundreds of little sketches where I put together one or two functions at a time working on how to make things work right.

So really what @Robin meant wasn't that you shouldn't post all your code. It was that if you have code this large with these sorts of problems it usually means that you've got an issue with your development style.

Really, you're just going to have more and more and more issues until you realize that all those neat demo animations that they do with delays and for loops aren't useful in real world code. They're just there to show you how the commands for the leds work.

do-while loops aren't for doing actual things. They're for burning through a string of text or something like that. They're not for waiting on users. You have the loop function. Let it be the loop. Learn to embrace the idea of non-blocking code if you want to be able to do anything cool with an Arduino. Code shouldn't tell a story. The loop code shouldn't be a list of things that need to be done in order. The loop code should be like a high speed checklist that is being constantly run over and over thousands of times a second, mostly doing nothing but occasionally seeing that it is time to take some small step.

GreyArea:
The risk in writing a simple example is that I miss out the bit that's causing the problem.

I want to add to the good advice by @Delta_G in Reply #18.

When I suggested posting a short program that illustrates the problem I had assumed that you would be able to do that, but I recognise that it is not always possible. The problem may in some way actually be due to the size of a large program.

And that is where @Delta_G's advice matters. If you develop code in small functions and test after every few lines are added then you will be able to pinpoint a problem to the most recent few lines that you added. There will be no need to examine any of the functions that you had already tested and proved to work correctly.

...R