The 'stuck values between functions' mystery

Projects Programming
1

Hi all,

I haven't posted here before but have lurked for many years. I have an unusual behavior with some code I have written that I am unable to solve.

The code is 1500 lines long and I can't really post it all here, so I though I'd try describing the problem to see if anyone has a "yep it's this" type reaction and if code is needed I'll post as much of the code as I think will be necessary.

The problem in words

Each time I press a button I cycle through a series of modes. (as expected)

After a short delay the mode begins which involves calling a while loop containing functions with an integer argument in each which sets the number of seconds each function will last (as expected)

After this loop exits (via a button or a timer) the code returns to a standby state (as expected)

When you use the button to select a different mode the new functions all last for the same length as time as that first run through - despite having different arguments passed to them for that new mode (ISSUE)

What I have checked

I have used Serial to print out the variables set from the arguments and these are not "stuck" - i.e. they reflect the expected value.

I'm hoping this is something my tired brain is missing that's obvious but if it's not - I will post my code once I can.

Hopeful for some light bulb moments,

MrCrin

2

Please post your complete sketch, using code tags when you do

3

Then post a striped-down MRE. This is the smallest possible complete code that demonstrates the problem at hand (and only that problem).

2 Likes
4

I was worried you'd say that - I can't post the whole code here, would sections just relevant to this part be acceptable?

5

See Post #3.

6

Hi yes just seen this, give a few minutes to do this - thanks for fast response.

7

A wild guess without seeing your code would be that you have variables with the same name but with different scopes, possibly one local and one global, such that you don't know which of them the code is accessing

8

I thought this as well and have searched my code for this and tried switching all the variable involved in this section to global and back to see if that made a difference... it didn't.

It has me properly confused :laughing:

I'm currently preparing the MRE gfvalvo requested.

9

No, you posted a couple of years ago.

10

I stand corrected

11

Click on your username just above your post. A pop up will appear with a larger pic and your username in larger text. Click on your username again. This will take you to your activity page. Click on "topics created".

Don't worry, were not checking you out or spying on you. When you posted the topic, a message appears at the top that only others see, saying you have not posted for 2 years.

Screenshot_20220616-125810
Screenshot_20220616-1258101080×2520 386 KB

1 Like
12

Mr Crin,
If this is the same code you posted about having lost way back then, I have to ask - did you have a backup? Do you have a backup technique now?

Your description of "several modes' that you step through after a button press sounds suspiciously like a state machine; if that's what it is, let's call it that, shall we?

That aside, it definitely sounds like you've got a variable scope problem. This often results from the generic use of cryptic variable names like i, ii, iii, etc. If you start substituting names for these items that actually mean something, two things will happen. Your code will become much more comprehensible to both you and others, and your ability to understand and efficiently make changes that are actually correct will improve.
Of course, I could be completely off base here, maybe we'll know better if you post some code...
Cheers
C

13

State machine - yes.

The code I'm preparing uses cryptic names but that's because I'm redacting the code - it's not really something I should/can post unredacted online.

Code incoming - it's just taking me some time to get the best compromise between what you need and what I'm happy to post :grinning:

14

Ok so this is a succinct as I can get it and show the problem. It's obviously a butchered version of the full code but it does the same broken thing.

#include <SPI.h>
#include <Wire.h>
#include <avr/sleep.h>      // For low power sleep mode
#include <movingAvg.h>      // https://github.com/JChristensen/movingAvg
#include <FastLED.h>        // https://github.com/FastLED/FastLED
#include <FastLED_GFX.h>    // https://github.com/Jorgen-VikingGod/FastLED-GFX
#include <LEDMatrix.h>      // https://github.com/Jorgen-VikingGod/LEDMatrix
#include <kxtj3-1057.h>
#include <Coordinates.h>

//Timers
volatile unsigned long last_motion = 0;     // Holds millis value and is set every time new motion is detected
volatile unsigned long last_rest = 0;       // Holds millis value and is set every time no motion is detected
volatile unsigned long last_press = 0;      // Holds millis value and is set every time button is pressed
volatile unsigned long last_release = 0;    // Holds millis value and is set every time button is released
int q_mode_start_delay = 3000;         // Duration in milliseconds to wait before starting a q mode after selection.
int mode_exit_delay = 1500;                 // Duration in milliseconds to wait before starting a q mode after selection.
int hold_select_delay = 3000;               // Duration in milliseconds to wait before switching between press and hold modes.
unsigned long mode_start_time = 0;              // Holds millis value set when a mode starts
unsigned long q_mode_duration = 300000;    // How long a q cycle will run for

// Control flow flags
volatile bool button_pressed = 0;
volatile bool button_latch = 0;
volatile bool in_motion = 0;
volatile bool run_q_mode_wait = 0;
volatile bool mode_active = 0;
volatile bool no_motion_wake = 0;
int mode = 0;                                   // Used as the main logic switch for modes

// UI
// Colours
static int s_hue = 128;
static int c_hue = 20;
static int f_hue = 50;
static int tHue = 200;

// LED Patterns and Colour Maps
static int edge_chase[] = {0,1,2,3,4,5,6,7,15,23,31,39,47,55,63,62,61,60,59,58,57,56,48,40,32,24,16,8};
static int inner1_chase[] = {9,10,11,12,13,14,22,30,38,46,54,53,52,51,50,49,41,33,25,17,9};
static int colourWheelMap[] = {0,9,18,27,36,46,55,64,246,0,13,26,38,51,64,73,237,242,0,21,43,64,77,82,228,230,234,0,64,85,89,91,219,217,213,191,128,106,102,100,209,204,191,170,149,128,115,109,200,191,179,166,153,140,128,118,191,182,173,164,155,146,137,128};

// Define pins
#define SWINPUT     2       
#define LEDENABLE   4                 
#define LEDTX       7       

void setupPins()
{
    pinMode(SWINPUT, INPUT);
    attachInterrupt(digitalPinToInterrupt(SWINPUT), ISR_button, CHANGE);
    pinMode(LEDENABLE, OUTPUT);
    pinMode(LEDTX, OUTPUT);
}

#define COLOR_ORDER         GRB
#define CHIPSET             WS2812B
#define MATRIX_WIDTH        8
#define MATRIX_HEIGHT       8
#define MATRIX_TYPE         HORIZONTAL_MATRIX
#define MATRIX_SIZE         (MATRIX_WIDTH*MATRIX_HEIGHT)
#define NUMPIXELS           MATRIX_SIZE

cLEDMatrix<MATRIX_WIDTH, MATRIX_HEIGHT, MATRIX_TYPE> matrix;
CRGB *leds = matrix[0];

int     MASTER_BRIGHT = 255;                // A master value for controlling brightness throughout the code
int     edge_brightness = MASTER_BRIGHT/2;  // Edge brightness in certain modes is always half the master
int     ambientLight = 0;                   // 0 means low light - 1 means bright light

void setupMatrix(){
    FastLED.addLeds<CHIPSET, LEDTX, COLOR_ORDER>(matrix[0], matrix.Size()).setCorrection(TypicalSMD5050);
    FastLED.setCorrection(TypicalSMD5050);
    FastLED.setTemperature(OvercastSky); // This colour corrects for the warm offwhite in the plastic shells
    FastLED.setBrightness(MASTER_BRIGHT);
    FastLED.setDither(1);
    FastLED.clear(true);
}

void ISR_button(){
    if (digitalRead(SWINPUT)==HIGH)
        {
            button_pressed = 1;
            button_latch = 1;
            last_press = millis();
        } else {
            button_pressed = 0;
            last_release = millis();
        }
}

void buttonHandler(){
    if (button_pressed == 1 && button_latch == 1) { // Every time there's a new button press
        while(button_pressed==1){   // While the button is being held
            
            if(!mode_active){ // If not in a q mode then show the hold light
                
                while (millis() - last_press < hold_select_delay){
                    /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                    holdButton();
                }
                while(millis() - last_press > hold_select_delay && millis() - last_press < hold_select_delay*2) //Bedtime mode
                {
                    /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                    float hueShift = 7;
                    int hueStartPoint = 230;
                    float hueMidpoint = hueStartPoint + (14*hueShift);
                    for(int LED = 0; LED < 28; LED++){
                        /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                        if(LED<14){
                            int hNow = hueStartPoint+(LED*hueShift);
                            leds[edge_chase[LED]].setHSV(hNow, 255, 255);
                            FastLED.show();
                            fadeToBlackBy(leds,NUMPIXELS,30);
                            FastLED.delay(25); 
                        } else {
                            int hNow = hueMidpoint-((LED-14)*hueShift);
                            leds[edge_chase[LED]].setHSV(hNow, 255, 255);
                            FastLED.show();
                            fadeToBlackBy(leds,NUMPIXELS,30);
                            FastLED.delay(25);
                        }
                    }
                    if(millis() - last_press > hold_select_delay && millis() - last_press < hold_select_delay*2){
                        run_q_mode_wait = 1;
                    } else {
                        //Do this once to make the transition between the two modes easier to spot
                        if(millis() - last_press > hold_select_delay){run_q_mode_wait = 0;} // Conditional reset of flag
                        for(int LED = 0; LED < 28; LED++){
                            /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                            leds[edge_chase[LED]].setHSV(0, 0, 255);
                            FastLED.show();
                            fadeToBlackBy(leds,NUMPIXELS,30);
                            FastLED.delay(15);
                        }
                    }
                    mode = 8;
                }
                while(millis() - last_press > hold_select_delay*2) 
                {
                    /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                    
                    while(button_pressed){
                        for(int LED = 0; LED < 28; LED++){
                            /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
                            int hNow = (255/27)*LED;
                            leds[edge_chase[LED]].setHSV(hNow, 255, 255);
                            FastLED.show();
                            fadeToBlackBy(leds,NUMPIXELS,30);
                            FastLED.delay(25);
                        } 
                    }
                    mode = 7;
                }
            } else {  
                
                if (millis() - last_press < mode_exit_delay)
                    {
                        
                    } else {
                        
                        while(button_pressed){ // Pause here until released - Indicates an exit has been triggered
                            matrix.DrawRectangle(0, 0, 7, 7, CHSV(160, 255, 255));
                            matrix.DrawFilledRectangle(2, 2, 5, 5, CHSV(180, 200, 150));
                            FastLED.show(); 
                        }
                        mode_active = 0; // Flag as no active q mode
                        mode = 1; // Move to standby mode
                        button_latch = 0; // Skip the normal button release behaviour
                    }
            }       
        } 
    }  
    if (button_latch == 1 && !mode_active){ // Normal button release behaviour (I.e. not when in an active mode or after a press and hold)
        
        if (mode == 6)                      {mode = 3;} 
        else if (mode > 2 && mode < 7)      {mode++;}
        else if (mode < 3)                  {mode = 3;}
        button_latch = 0;
    } 
}

void holdButton(){
    FastLED.clear();
    matrix.DrawRectangle(0, 0, 7, 7, CHSV(0, 0, MASTER_BRIGHT/2));
    FastLED.show();
}

void selectWait(int h){
    while(millis() - last_release < q_mode_start_delay && !mode_active){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        FastLED.clear();
        matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, MASTER_BRIGHT/2));
        FastLED.show();
    }
    if(millis() - last_release > q_mode_start_delay){
        run_q_mode_wait = 1;
    }
}

void qModeReady(int h){
    
    run_q_mode_wait = 0; // Reset flag
    while(!button_pressed){
        for(int LED = 0; LED < 20; LED++){
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            leds[inner1_chase[LED]].setHSV(h, 255, 255);
            fadeToBlackBy(leds,NUMPIXELS,30);
            FastLED.show();
            FastLED.delay(35);
        } 
    }
    
    mode_active = 1; // Flag as actively in a q mode
}

void celebrate(){
}

static int low_offset = 30; 
static int smooth_offset = 5;

void edgeFadeIn(int h){
    int i = 0;
    while(i < edge_brightness){
        fadeToBlackBy(leds,NUMPIXELS,1);
        matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, i));
        FastLED.show();
        FastLED.delay(4);
        i++;
    }
}

void qeIn(int h, int d) { //Duration is in seconds
    Serial.println ("~ In q");
    int targetVal = low_offset;
    float full_seconds = d;
    float updateDelaySpeed = (full_seconds*1000)/(MASTER_BRIGHT - low_offset);
    matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, edge_brightness));
    unsigned long temp = millis();
    Serial.print ("~ Secs = ");
    Serial.println (full_seconds);
    Serial.print ("~ Speed millis = ");
    Serial.println (updateDelaySpeed);
    Serial.print ("~ Target val = ");
    Serial.println (targetVal);
    while(targetVal<MASTER_BRIGHT && !button_pressed){
        EVERY_N_MILLISECONDS(updateDelaySpeed){ 
            matrix.DrawFilledRectangle(3, 3, 4, 4, CHSV(h, 255, constrain(targetVal, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            matrix.DrawRectangle(2, 2, 5, 5, CHSV(h, 255, constrain(targetVal+smooth_offset, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            matrix.DrawRectangle(1, 1, 6, 6, CHSV(h, 255, constrain(targetVal+smooth_offset+smooth_offset, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            targetVal++;
        }    
    }
    Serial.print ("~ In End = ");
    Serial.println (millis()-temp);
}

void qeOut(int h, int d) { //Duration is in seconds
    Serial.println ("~ Out q");
    int targetVal = MASTER_BRIGHT;
    float full_seconds = d;
    float updateDelaySpeed = (full_seconds*1000)/(MASTER_BRIGHT - low_offset);
    matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, edge_brightness));
    unsigned long temp = millis();
    Serial.print ("~ Secs = ");
    Serial.println (full_seconds);
    Serial.print ("~ Speed millis = ");
    Serial.println (updateDelaySpeed);
    Serial.print ("~ Target val = ");
    Serial.println (targetVal);
    while(targetVal>low_offset && !button_pressed){
        EVERY_N_MILLISECONDS(updateDelaySpeed){
            matrix.DrawFilledRectangle(3, 3, 4, 4, CHSV(h, 255, constrain(targetVal, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            matrix.DrawRectangle(2, 2, 5, 5, CHSV(h, 255, constrain(targetVal+smooth_offset, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            matrix.DrawRectangle(1, 1, 6, 6, CHSV(h, 255, constrain(targetVal+smooth_offset+smooth_offset, low_offset, MASTER_BRIGHT)));
            FastLED.show();
            targetVal--;
        }    
    }
    Serial.print ("~ Out End = ");
    Serial.println (millis()-temp);
}

void holdIn(int h, int d) { //Duration is in seconds
    matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, MASTER_BRIGHT/2));
    for (float j = 0; j < d; j++) {
        for (float i = MASTER_BRIGHT; i > (MASTER_BRIGHT-70); i--) {
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            matrix.DrawFilledRectangle(1, 1, 6, 6, CHSV(h, i, i));
            FastLED.show();
            FastLED.delay(1);
        }
        for (float i = (MASTER_BRIGHT-70); i < MASTER_BRIGHT; i++) {
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            matrix.DrawFilledRectangle(1, 1, 6, 6, CHSV(h, i, i));
            FastLED.show();
            FastLED.delay(1);
        }
    }
}

void holdOut(int h, int d) { //Duration is in seconds
    matrix.DrawRectangle(0, 0, 7, 7, CHSV(h, 255, MASTER_BRIGHT/2));
    for (float j = 0; j < d; j++) {
        for (float i = low_offset; i < (low_offset+70); i++) {
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            matrix.DrawFilledRectangle(1, 1, 6, 6, CHSV(h, 255, i));
            FastLED.show();
            FastLED.delay(1);
        }
        for (float i = (low_offset+70); i > low_offset; i--) {
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            matrix.DrawFilledRectangle(1, 1, 6, 6, CHSV(h, 255, i));
            FastLED.show();
            FastLED.delay(1);
        }
    }
}

void s(){
    Serial.println ("~  s");
    while(mode == 3){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        selectWait(s_hue);
        if(run_q_mode_wait){qModeReady(s_hue);}
        mode_start_time = millis();
        if (mode_active) edgeFadeIn(s_hue);
        while(mode_active && (millis() - mode_start_time) < q_mode_duration){
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            qeIn(s_hue, 4);
            holdIn(s_hue, 4);
            qeOut(s_hue, 4);
            holdOut(s_hue, 4);
        }
        if((millis() - mode_start_time) > q_mode_duration){
            celebrate();
            mode_active = 0;
            mode = 2;
            button_latch = 0;          
        }
    }
}

void c(){
    Serial.println ("~  c");
    while(mode == 4){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        selectWait(c_hue);
        if(run_q_mode_wait){qModeReady(c_hue);}
        mode_start_time = millis();
        if (mode_active) edgeFadeIn(c_hue);
        while(mode_active && (millis() - mode_start_time) < q_mode_duration){
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            qeIn(c_hue, 5);
            safeDelay(500);
            qeOut(c_hue, 5);
            safeDelay(500);
        }
        if((millis() - mode_start_time) > q_mode_duration){
            celebrate();
            mode_active = 0;
            mode = 2;
            button_latch = 0;          
        }
    }
}

void f(){
    Serial.println ("~  f");
    while(mode == 5){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        selectWait(f_hue);
        if(run_q_mode_wait){qModeReady(f_hue);}
        mode_start_time = millis();
        if (mode_active) edgeFadeIn(f_hue);
        while(mode_active && (millis() - mode_start_time) < q_mode_duration){
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            qeIn(f_hue, 4);
            holdIn(f_hue, 7);
            qeOut(f_hue, 8);
            safeDelay(200);
        }
        if((millis() - mode_start_time) > q_mode_duration){
            celebrate();
            mode_active = 0;
            mode = 2;
            button_latch = 0;          
        }
    }
}

void t(){
    Serial.println ("~  t");
    while(mode == 6){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        selectWait(tHue);
        if(run_q_mode_wait){qModeReady(tHue);}
        mode_start_time = millis();
        if (mode_active) edgeFadeIn(tHue);
        while(mode_active && (millis() - mode_start_time) < q_mode_duration){
            /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
            qeIn(tHue, 4);
            holdIn(tHue, 4);
            qeOut(tHue, 4);
            safeDelay(200);
        }
        if((millis() - mode_start_time) > q_mode_duration){
            celebrate();
            mode_active = 0;
            mode = 2;
            button_latch = 0;          
        }
    }
}

void l(){
}

void standby(){
}

void pwrSave(){
}

void safeDelay(int delay){
    long unsigned start = millis();
    while((millis() - start) < delay){
        /**BAILOUT**/ if(in_motion || button_pressed) break; /**BAILOUT**/
        FastLED.delay(5);
    }
}

void safeDelayNoMotion(int delay){
    long unsigned start = millis();
    while((millis() - start) < delay){
        /**BAILOUT**/ if(button_pressed) break; /**BAILOUT**/
        FastLED.delay(5);
    }
}

void safeDelayButton(int delay){
    long unsigned start = millis();
    while((millis() - start) < delay){
        /**BAILOUT**/ if(!button_pressed) break; /**BAILOUT**/
        FastLED.delay(5);
    }
}


void setup()
{
    digitalWrite(LEDENABLE, 1); // Enable LED Matrix Power - this might NEED MOVING to wake and sleep functions but is here for now
    FastLED.clear();
    Serial.begin(19200);
    setupPins();
    setupMatrix();
    delay(100);
    mode = 2; 
    button_latch = 0; 
}

void loop()
{
        buttonHandler(); // Manages mode switches from button presses
        switch (mode) {
            case 0:                         
                pwrSave();
                break;
            case 1:                         
                standby();
                if(in_motion) {mode = 2;}
                break;
            case 2:                         
                l();
                mode = 1;
                break;
            case 3:                         
                s();
                break;
            case 4:                         
                c();
                break;
            case 5:                         
                f();
                break;
            case 6:                         
                t();
                break;
            case 7:                         
                break;
            case 8:                         
                break;
            default:
              FastLED.delay(10);
        }
}

I have to be away from my computer for a few hours so I'll pick this back up when I get back...

Fingers crossed for something obvious

15

Use of delay as a variable name seems odd.

16

So... nothing then? That's a shame. Thanks to those who took a stab without the code.

17

If you want people to pinpoint a problem in 500 lines of undocumented code based on a very vague description of the error, you ought to at least give them a bit more time than 5 hours.

You say that every function is supposed to have a different length, but from my cursory understanding, they all basically run for q_mode_duration and that variable never gets changed. I'd suspect that has to do with the problem.

18

The code you posted is still relatively complicated and I have found it difficult to follow in relation to your original problem description. For instance, where does the change of parameters take place for second and subsequent calls to functions after a button press ?

19

Bloated I would say. The problem might be debugged by continuing to remove stuff until it goes away.

20

The button handing is way too complex. Too much flag checking and while loops. This is what happens when you don't let loop() do the looping. If it were me I would take another look at the requirements and and restate them as one or more state machines. Your logic would then be much easier to represent graphically and as code.

I think you will find once you restructure your code it will be easy to eliminate the button ISR and give you more control and make your code extensible.

It is not unusual to restructure code so don't take that too critically. In some cases a couple of flags are sufficient in place of a state machine. In other cases a state machine yields much more understandable and maintainable code. If experienced programmers are telling you it is too complex then it is highly probable it is actually too complex.