Using an ISR to start over on a Nano

I am backlighting a piece of glass, and using a BCD switch to choose the color/effect. Some of the effects are simple fades, but they are very long fades. I want to use an ISR so that when in the middle of a long fade and the BCD is switched to a different color, it happens immediately. Without the ISR, if the BCD is changed, it only would take effect after the long fade is complete, and I donʻt want the ISR to return to where it left off in the fade - I want it to go back to the top of the main loop.
My thinking is that an ISR attached to bit1 of the switch (pin 3 on the Nano) would go to "void change", and inside that void is only one command: goto (label). The label in this case is "changeSelect". This is where I get the "expected identifier before ʻ;ʻ token" error.
I also was getting an error saying "label 'changeSelect' used but not defined", so I tried #defining it, but am missing a parameter I think.

Yes, I know many people are opposed to using ʻgotoʻ statements. Iʻm open to suggestions, but this seemed like a pretty easy way to do it.....

Thoughts?

//USING NANO AND BCD SWITCH TO CHANGE COLORS 
//BCD SWITCH: DIGIKEY 2449-RD10RA16RTT  16position Rotary BCD Switch
//IDE version:  1.8.16 

#include <Adafruit_NeoPixel.h>

#define changeSelect  //  <<<< What do I define this as?
#define NUM_LEDS 105  // 62, 105, 114, 156, 
#define PIN 7
const int bit1 = 11; // D11 is attached to D3 for ISR since bit1 changes state at every selection change
const int bit2 = 9;
const int bit4 = 12;
const int bit8 = 10;
int prevselect = 0;

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


void setup()
{
  pinMode (9, INPUT_PULLUP);
  pinMode (10, INPUT_PULLUP);
  pinMode (11, INPUT_PULLUP);
  pinMode (12, INPUT_PULLUP);

  strip.begin();
  strip.clear();  // Initialize all pixels to 'off'
  strip.show();

  attachInterrupt(digitalPinToInterrupt(3), change, CHANGE);  // if the BCD switch CHANGEs, run the ISR in "void change"
}

void loop() {

changeSelect:  // THIS IS WHERE I WANT TO GO AS SOON AS THE BCD SWITCH IS CHANGED

  // Read the BCD switch
  int select;
  if (digitalRead(bit1) == 0)
  {
    select = select + 1;
  }
  if (digitalRead(bit2) == 0)
  {
    select = select + 2;
  }
  if (digitalRead(bit4) == 0)
  {
    select = select + 4;
  }
  if (digitalRead(bit8) == 0)
  {
    select = select + 8;
  }

  if (select != prevselect)  //if the selection has changed, clear the strip.  If I get the ISR working, I wonʻt need this.
  { strip.clear();
    strip.show();
  }
  prevselect = select;

  switch (select)

  { // Turn on the appropriate mix of LEDs as per the switch setting
    case 0  : //RED
      strip.fill(0xff0000);
      strip.show();
      break;

    case 1  : //ORANGE
      strip.fill(0xff4400);
      strip.show();
      break;

    case 2  : //YELLOW
      strip.fill(0xddff00);
      strip.show();
      break;

    case 3  : // GREEN
      strip.fill(0x00ff00);
      strip.show();
      break;
      break;

    case 4  : //TEAL
      strip.fill(0x00aa88);
      strip.show();
      break;

    case 5  : //BLUE
      strip.fill(0x0000ff);
      strip.show();
      break;

    case 6  : //PURPLE
      strip.fill(0x330077);
      strip.show();
      break;

    case 7  : //WHITE
      strip.fill(0x66ffff);
      strip.show();
      break;

  }
}

void change() {
  goto changeSelect; // Yes, I know you all hate ʻgotoʻ statements...
  //THIS IS WHERE I GET AN ERROR "expected identifier before ʻ;ʻ token"
}

Why not just set a global flag "changed = 1" in the ISR, and check (and reset) that in your loop{} ?

What is changeselect ?

Start by simplifying the sketch. The switch/case is unnecessary and could be replaced by an array of values indexed using the value of the select variable

An observation. At one point in the sketch you add 8 to the value of select but the case values only go up to 7

You get an error because you are goto'ing outside the current scope (the function change()) and you would not be returning correctly from the ISR. You may need to learn about how the stack works and how functions are called and return.

As mentioned, change() could just set a global variable and you check (and reset) that variable in your code and then do whatever you want.

I would also comment that your code seems to set the colors of the strip EVERY time through loop(). This is unnecessary (just do it once) and may be the cause why you perceive the change request to be unresponsive. You really should not need an ISR to detect the change in the switch in this situation.

to code you have posted seems not to be the code you described here

The code you have posted has no fades at all.
I assume that your fadings use for-loops or while-loops.

Using a reset to abort a function is like tearing apart a sheet of paper and writing the text on a new sheet of paper.

Simply setting a flag in the interrupt does not re-start the microcontroller.
Not sure if setting the hardware watch-dog-timer will work inside an interrupt

best regards Stefan

again
If your fading is using a for-loop or a while-loop there are too options:

Option 1:
adding code into each and every for-loop / while-loop if button is pressed abort for-loop/while-loop immidiately.
.
Option 2:
not using a single for-loop / while-loop
but
replacing the for-loops / while-loops by software that realises the "looping" only based on

void loop()

I have written a short tutorial about the basic principle

best regards Stefan

1 Like

That's a totally unnecessary and inappropriate use of interrupts. You need to learn how to write non-blocking code. Here's an Adafruit guide for doing just that. As an added bonus, the guide is about your exact problem ... writing non-blocking code for NeoPixel displays.

There are many ways to handle this issue.

Here's one off the wall. It uses the setjmp/longjmp mechanism to switch by pushbutton on an interrupt, between as many loop() -like functions as you have room for. Try it out, the demo switches between three loop-like functions.


Wokwi_badge setjmp/longjmp demo


// https://wokwi.com/projects/373311239978862593
// https://forum.arduino.cc/t/using-an-isr-to-start-over-on-a-nano/1158993

// also see post 30, and the thread that this came from
// https://forum.arduino.cc/t/question-that-probably-has-a-really-obvious-answer-but-im-just-too-dumb-to-find-it/887654/30


/* setjmp example */

# include <setjmp.h>

# define numLoops	3
# define myButton	3
# define theLED	LED_BUILTIN

static jmp_buf buf;

int whichLoop = 0;

unsigned char once = 0;

volatile unsigned char ignore = 0;

volatile int ignored = 0;

void myISR()
{
	if (ignore) {
		ignored++;
		return;
	}

	detachInterrupt(digitalPinToInterrupt(myButton));
	ignore = 1;
	
	/* and  yank the big lever */
	longjmp(buf, 1);
}

void setup() {

	Serial.begin(115200);
	Serial.println("hello setjmp/longjmp world!\n");

	pinMode(myButton, INPUT_PULLUP);
	pinMode(theLED, OUTPUT);
}

void loop()
{
	for (; ; ) {
		while (1) {
			if (!setjmp(buf)) {

				EIFR |= (1 << digitalPinToInterrupt(myButton));		// clear flag on button interrupt
				attachInterrupt(digitalPinToInterrupt(myButton), myISR, FALLING);
//				delay(20);
				ignore = 0;
				break;						// normal return, just start looping
			}
			else {							// big lever return, do whatever, then start mostly all over
				whichLoop++;
				if (whichLoop >= numLoops)
					whichLoop = 0;
	
				Serial.println("");
				if (ignored) { Serial.print(" ignored "); Serial.print(ignored); }
				Serial.print("switching to loop "); Serial.print(whichLoop);
				Serial.println("");
				delay(500);					// very poor man's debounce - get off that switch
			}
		}
		for (; ; )
			loopDispatch();
	}
}

void loopDispatch() {
	switch (whichLoop) {
	case 0 :
		loop0();
		break;

	case 1 :
		loop1();
		break;

	case 2 :
		loop2();
		break;
	}
}

void loop0()
{
	static unsigned char toggle;

	digitalWrite(LED_BUILTIN, ++toggle & 0x1);

	delay(333);
}

void loop1()
{
	int counter = 0;

	for (; ; ) {
		Serial.print("loop 1 : ");
		Serial.println(counter);

		counter++;

		delay(33);
	}
}

void loop2()
{
	static int counter = 0;

	Serial.print("loop 2 : ");
	Serial.println(counter);

	counter++;

	delay(33);
}

There is only the need to get off the button in less than 500 milliseconds, otherwise you'll skip ahead in the cycle of loops.

It's a bit delicate, so don't get too creative with the mechanism.

Close readers will wonder about this

void loop()
{
	for (; ; ) {

captivated loop with a infinite loop... read here:


a7

1 Like

Thank you all for your help - I have coded for non-blocking and various flags and millis before, but somehow I just didn’t think about using it. (Got so deep in the project that I couldn’t back out far enough to get perspective…)
A few answers to your queries:
@StefanL38: I did not include the “fading” cases as I was trying to follow forum guidelines by not including hundreds of lines of cases you didn’t need to see to understand my issue.
I did not want to reset the microcontroller - I just wanted to return to the top of the main loop.
I will probably use your Option 1 suggestion.
@UKHeliBob: Yes, my 4th bit said “+8”, while the last case is number 7. Case 0 is the first case. (I coded 15 cases, but…. See note above).

Again, thank you all for your help and your time.
Dave

But as pointed out previously, in the code that you did post you don't need any of them

I know of no such rule. If anything the reverse is true and experience has shown that problems are often caused by sections of code that are not posted because the user decides that they are not relevant

Another idea, even lower rent, is to write your own delay() function.

Write a function myDelay(delayTime) that not only wastes time, but watches a pushbutton and returns prematurely if it sees it pressed.

Then where you call delay() substitute a call for this new function, and if it says it was prematurely exited, do a return right out of whatever mess you were in.

See post #13 and this hole thread:

For more, search these fora for mydelay. It's a crude but effective hack.

a7

Although a short MRE that exhibits the same problem is much preferred over hundreds of lines of irrelevant, messy, ugly code.

Agreed, but in this case what was posted was not an MRE as it contained no LED fades despite them being the main problem for which a solution was being sought

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.