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"
}
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
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
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.
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.
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.
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