Have I got the right idea here? Interupts, Functions and very little in loop??

Hi I am slowly learning with my arduino clone.. its purpose will eventually be controlling several rgb led strings around my home. For now Im trying to get my head around basics using two 3v strings of fairy lights.

The circuitry goes together nice & easily, although maybe taking the mick taking the 3v from the arduino.
The code however has proven to be an uphill slog as I knew it would be. My only code experience worth mentioning is vba within excel which i can write quite competently with very little actual knowledge!

This project is simple
2 x 3v fairy lights behind pn2222 transistors, 2 x momentary buttons+10k resisors, lots of jumper wires.
One button for power on / off
One button for effects.. 1 Both On / 2. Both Fade / 3. Alternate fade etc etc

I managed to put a program together that does most of what i want, simply using if blocks and variables. but the buttons were only read once a loop which didnt work when fading.

My new plan is...

  • Remove one button
  • Write an interupt which listens to the lone button and based on its current state calls functions
  • Write functions for "always on" and off. also write functions to set fade variables
  • Write fade effects in main loop which only run when variables set

Am I on the right track with this approach? Thanks in advance for any guidance

note:
I realised while writing this that looped effects must go in the main loop, or if in a function need to be within an infinate loop?

Welcome to the Arduino forum. I am sure you will be able to have success with your project. However, read the forst posting in this forum:
Demonstration code for several things at the same time.

This will show you there is NO need for interrupts with your project. Just try the examples and you will soon see where and how to do the things you have listed.

Paul

BigglesBerry2:
but the buttons were only read once a loop which didnt work when fading.

That will work fine if you write non-blocking code. The fastest button press you can do will still be tens of milliseconds. Your loop duration will likely be a fraction of a millisecond.

BigglesBerry2:
note:
I realised while writing this that looped effects must go in the main loop, or if in a function need to be within an infinate loop?

You can just call the function from loop().

You want to be able to run many tasks together as if they are all in parallel. The code for each task is separate (not fit into an if-else or whatever of another) but they are all in void loop() that acts as the driving wheel for the lot.

If you want fast response and smooth action (answer is yes) then you have to see to it that no task runs too long in any one pass (of loop()) with 200 micros (one 10 bit analog read takes 105) being a big stretch. --- consider that 100 micros is 1600 cpu cycles, a lot can be done in less time.
If a task does loads, break it down into loads of steps that run 1 step each per loop() instead of 1 big step that makes everything else wait. If you need to loop a process through a big array, set up an index and process 1 element per loop() instead of a for-next that makes everything else wait till the whole array is processed.
When you run loop at 40KHz to 70KHz, processing 500 elements does not take long.

You have a button? You set up a task that watches the button and does the debouncing and maintains a status variable that any other task can check.

Want to blink a led? Set up a task that digitalWrite(led pin, variable) and then another (process) task that changes that at the right times.
Right times? How..... well if you know a start time and how long to wait;

if ( waitMillis > 0 ) // only runs when the wait is > 0, lets you turn this "timer" on and off
{
if ( millis() - startMillis >= waitMillis ) // where startMillis and waitMillis are unsigned long variables
{
// now the wait is up, do the thing
waitMillis = 0; // makes this a one-shot, some task sets the start and wait for this to run
}
}

If you have an interrupt it should do the least like grab a copy of the current time and set a flag variable that --- some task in loop() reads Very Soon and acts upon, clearing the flag along the way.

Point is that tasks set/read variables to pass control.

This is an intro to writing event-driven tasking code on Arduino without an OS to slow it down.
Definitely seek more information/techniques like in the Many Things At Once thread and the three links in my signature space below. The more angles you get and the more practice you get, the better that you will get.

Most automation projects including yours won't tap more than 1% of the 16MHz "attention" that a regular Arduino gives. Put in a loop() counter task and you will see.

Have I got the right idea here? Interupts, Functions and very little in loop??

Using functions and having very little in loop() is sensible as it should make the code much easier to understand and to debug.

The use of interrupts is a different concept entirely and really ought not to be part of the same sentence.

As others have said there is no need to use interrupts to detect humans pressing buttons. Interrupts are invaluable for detecting very short duration events (a few microsecs or less) that would otherwise be missed or events that repeat at a high frequency that might be missed by polling.

...R
Several Things at a Time
Planning and Implementing a Program

You can have loads of code or calls to functions with loads of code inside of loop as long as you don't run too much of it in one pass.

You might code for 800 different conditions knowing that at most only a few can happen at the same time.

You might have a process with 100 steps that only 1 executes at a time.

It's not the size of the code inside loop() that matters. It's how long it takes to run.

Wow thank you all for the insightful replies.
I did download the pinned sketch before posting but never got round to opening it! I think i understand and will start fresh tonight, before that im gonna print a copy of the severalthings sketch (& this thread) to study.

const int ledPinA = 9;
const int ledPinB = 10;
const int buttonPin = 4;
const int flashInterval = 500;
const int fadeInterval = 500;
const int flashDuration = 500;
const int buttonInterval = 300;
int buttonCycle = 0; // 0 = OFF - 1 = BOTH ON - 2 = BOTH FADE - 3 = ALTERNATE FADE
int fadeDirection = 0; // 0 = UP - 1 = DOWN
int fadeStep = 5;
int fadeLevel = 0;
int fadeOldLevel = 0;
byte ledAState = LOW;
byte ledBState = LOW;
unsigned long currentMillis = 0; // stores value of millis in each iteration of loop
unsigned long previousLedAMillis = 0;
unsigned long previousLedBMillis = 0;
unsigned long previousFadeMillis = 0;
unsigned long previousButtonMillis = 0; //Time when button last checked

void setup() {
  pinMode (ledPinA, OUTPUT);
  pinMode (ledPinB, OUTPUT);
  pinMode (buttonPin, INPUT);
}

void loop() {
  currentMillis = millis();
  readButtons();
  if (buttonCycle == 1 ) {
    digitalWrite(ledPinA, HIGH);
    digitalWrite(ledPinB, HIGH);
  }
  if (buttonCycle == 2 ) {
    fadeAllLeds();
    switchLeds();
  }
  if (buttonCycle == 3 ) {
  //  fadeAltLeds();
    switchLeds();
  }
  if (buttonCycle == 4) {
    digitalWrite(ledPinA, LOW);
    digitalWrite(ledPinB, LOW);
    buttonCycle = 0;
  }
}

void switchLeds() {
  if (buttonCycle == 3 ) {
    analogWrite(LedPinA, fadeLevel);
    analogWrite(LedPinB, fadeLevel);
  }
}

void fadeAllLeds() {
  if (ledAState == LOW ) {
    if (currentMillis - previousFadeMillis >= fadeInterval) {
      if (fadeDirection == 0) {
        if (fadeLevel == 255) {
          fadeDirection = 1 ;
        } else {
          fadeLevel += 5 ;
        }
      } else {
        if (fadeLevel == 0) {
          fadeDirection = 0 ;
        } else {
          fadeLevel -= 5 ;
        }
      }
    }
  }
}

void readButtons() {
  if (millis() - previosButtonMillis >= buttonInterval) {
    if (digitalRead(buttonPin) == LOW) {
      buttonCycle += 1 ;
      previousButtonMillis += buttonInterval;
    }
  }
}

Hhmm the code above is far far from finished but I was quite sure it should compile, yet i get an 'LedPinA' was not declared in this scope error on the analogWrite line of the switchLeds function.

Im scratching my head and would like to fix before moving on... any obvious mistakes?

LedPinA is not the same as ledPinA; C/C++ is case sensitive.

sterretje:
LedPinA is not the same as ledPinA; C/C++ is case sensitive.

Ive been staring at that for 20 mins and didn't notice.. Thank you

BigglesBerry2:

const int ledPinA = 9;

const int ledPinB = 10;

..... snip

void switchLeds() {
 if (buttonCycle == 3 ) {
   analogWrite(LedPinA, fadeLevel);
   analogWrite(LedPinB, fadeLevel);
 }
}



Hhmm the code above is far far from finished but I was quite sure it should compile, yet i get an 'LedPinA' was not declared in this scope error on the analogWrite line of the switchLeds function.

Im scratching my head and would like to fix before moving on... any obvious mistakes?

The error should have given you a line number. The IDE can be set up to show line numbers.

One small tip that will save you RAM: don't use an int (16 bit) where a byte (8 bit) will do.

int is good for -32768 to +32767
byte is good for 0 to 255

Arduino pin numbers always fit in bytes.

const byte ledPinA = 9;
const byte ledPinB = 10;
const byte buttonPin = 4;
const byte fadeInterval = 80;
const int flashDuration = 500;
const int buttonInterval = 1200;
byte buttonCycle = 0; // 0 = OFF - 1 = BOTH ON - 2 = BOTH FADE - 3 = ALTERNATE FADE
byte fadeDirection = 0; // 0 = UP - 1 = DOWN
byte fadeLevel = 0;
byte fadeRevLevel = 255;
byte ledAState = LOW;
byte ledBState = LOW;
unsigned long currentMillis = 0; // stores value of millis in each iteration of loop
unsigned long previousLedAMillis = 0;
unsigned long previousLedBMillis = 0;
unsigned long previousFadeMillis = 0;
unsigned long previousButtonMillis = 0; //Time when button last checked


void setup() {
  pinMode (ledPinA, OUTPUT);
  pinMode (ledPinB, OUTPUT);
  pinMode (buttonPin, INPUT);
  Serial.begin(9600);
  Serial.println("Starting Control");
}

void loop() {
  currentMillis = millis();
  readButtons();
  if (buttonCycle == 1 ) {
    digitalWrite(ledPinA, HIGH);
    digitalWrite(ledPinB, HIGH);
  }
  if (buttonCycle == 2 ) {
    fadeAllLeds();
    analogWrite(ledPinA, fadeLevel);
    analogWrite(ledPinB, fadeLevel);
  }
  if (buttonCycle == 3 ) {
    fadeAltLeds();
    analogWrite(ledPinA, fadeLevel);
    analogWrite(ledPinB, fadeRevLevel);
  }
  if (buttonCycle == 4) {
    digitalWrite(ledPinA, LOW);
    digitalWrite(ledPinB, LOW);
    buttonCycle = 0;
  }
  Serial.println(buttonCycle);
}

void fadeAllLeds() {
  if (currentMillis - previousFadeMillis >= fadeInterval) {
    if (fadeDirection == 0) {
      if (fadeLevel == 255) {
        fadeDirection = 1 ;
      } else {
        fadeLevel += 5 ;
        previousFadeMillis += fadeInterval;
      }
    } else {
      if (fadeLevel == 0) {
        fadeDirection = 0 ;
      } else {
        fadeLevel -= 5 ;
        previousFadeMillis += fadeInterval;
      }
    }
  }
}

void fadeAltLeds() {
  if (currentMillis - previousFadeMillis >= fadeInterval) {
    if (fadeDirection == 0) {
      if (fadeLevel == 255) {
        fadeDirection = 1 ;
      } else {
        fadeLevel += 5 ;
        fadeRevLevel -= 5 ;
        previousFadeMillis += fadeInterval;
      }
    } else {
      if (fadeLevel == 0) {
        fadeDirection = 0 ;
      } else {
        fadeLevel -= 5 ;
        fadeRevLevel += 5;
        previousFadeMillis += fadeInterval;
      }
    }
  }
}

void readButtons() {
  if (currentMillis - previousButtonMillis >= buttonInterval) {
    if (digitalRead(buttonPin) == HIGH) {
      buttonCycle += 1 ;
      fadeLevel = 0;
      fadeDirection = 0;
      fadeRevLevel = 255;
      previousButtonMillis = millis();
    }
  }
}

seems to be working well except for when fading both, it spasm's a bit before fading first time (alternate fading works fine).. no idea why.
Is it put together well ie am i starting to think along the right lines?

edit: spasms alot before fading if left doing something else for any length of time... not as finished as i thought then

Can you please define "spasm's".

const int buttonInterval = 1200;
void readButtons() {
  if (currentMillis - previousButtonMillis >= buttonInterval) {
    if (digitalRead(buttonPin) == HIGH) {
      buttonCycle += 1 ;
      fadeLevel = 0;
      fadeDirection = 0;
      fadeRevLevel = 255;
      previousButtonMillis = millis();
    }
  }
}

The button interval will lock out a new reading for over a second. It is way too long for normal debounce lockout. You will be more responsive it is less than 50 ms.

Possibly you need to init fadeDirection before calling the fade routines.

Also, variables have 'type', byte, char, word, int are all variable types.
When you mix types in your math sometimes the compiler won't do what you think it should.
I don't see any obvious problems but that doesn't mean everything.

(currentMillis - previousFadeMillis >= fadeInterval)

uses 2 unsigned longs and a byte. You can force the compiler to promote fadeInterval to unsigned long by casting it as unsigned long, there is no Arduino conversion function for unsigned long.

(currentMillis - previousFadeMillis >= (unsigned long) fadeInterval)

Mixed types math is one source 'dint see that' bugs.

PS -- it also means that you don't always put small numbers in small variables as a hard rule.

const byte ledPinA = 9;
const byte ledPinB = 10;
const byte ledR = 3;
const byte ledG = 5;
const byte ledB = 6;
const byte buttonPin = 4;
const byte buttonPin2 = 2;
const int buttonInterval = 50;
const int activInterval = 1000;
byte buttonCycle = 0; 
byte fadeDirection = 0; 
byte fadeLevel = 0;
byte fadeRevLevel = 255;
byte fadeInterval = 25;
unsigned long currentMillis = 0; 
unsigned long previousFadeMillis = 0;
unsigned long previousButtonMillis = 0; 
unsigned long previousActivationMillis = 0;
void setup() {
  pinMode (ledPinA, OUTPUT);
  pinMode (ledPinB, OUTPUT);
  pinMode (buttonPin, INPUT);
  pinMode (buttonPin2, INPUT);
  Serial.begin(9600);
  Serial.println("Starting Control");
}
void loop() {
  currentMillis = millis();
  readButtons();
  switch(buttonCycle) {
    case 0:
      powerDown();   
    break;
    case 1:
      alwaysOn();
    break;
    case 2:
      fadeLeds(0);
    break;
    case 3:
      fadeLeds(1);
    break;
    default:
      buttonCycle = 0;
    break;
  }
}
void readButtons() {
  if (currentMillis - previousButtonMillis >= buttonInterval) {
    if (currentMillis - previousActivationMillis >= activInterval) {
      if (digitalRead(buttonPin) == HIGH) {
        resetLeds();
        buttonCycle += 1 ;
        resetFade();
        previousActivationMillis = currentMillis;     
      }
      if (digitalRead(buttonPin2) == HIGH) {
        if (buttonCycle == 2 || buttonCycle == 3) {
          switch (fadeInterval) {
            case 100:
              fadeInterval = 25;
            break;
            default:
              fadeInterval = fadeInterval += 25;
            break;
          }
          previousActivationMillis = currentMillis;  
        }
      }
    }
    previousButtonMillis = currentMillis;
  }
}
void alwaysOn() {
  digitalWrite(ledPinA, HIGH);
  digitalWrite(ledPinB, HIGH);
  setStatusColor(255,255,255);
}
void powerDown() {
  resetLeds();
  setStatusColor(255,0,0);  
}
void fadeLeds(int alt) {
  if (currentMillis - previousFadeMillis >= fadeInterval) {
    if (fadeDirection == 0) {
      if (fadeLevel == 255) {
        fadeDirection = 1 ;
      } else {
        fadeLevel += 5 ;
        fadeRevLevel -= 5 ;
        previousFadeMillis += fadeInterval;
      }
    } else {
      if (fadeLevel == 50) {
        fadeDirection = 0 ;
      } else {
        fadeLevel -= 5 ;
        fadeRevLevel += 5 ;
        previousFadeMillis += fadeInterval;
      }
    }
    switch (fadeInterval) {
      case 25:
        setStatusColor(255,255,0); 
      break;
      case 50:
        setStatusColor(169,59,155);
      break;
      case 75:
        setStatusColor(59,144,169); 
      break;
      case 100:
        setStatusColor(236,169,54);
      break;
    }
    switch (alt) {
      case 1:
        analogWrite(ledPinA, fadeLevel);
        analogWrite(ledPinB, fadeRevLevel);
      break;
      default:
        analogWrite(ledPinA, fadeLevel);
        analogWrite(ledPinB, fadeLevel);
      break;
    }      
  }
}
void resetLeds() {
  digitalWrite(ledPinA, LOW);
  digitalWrite(ledPinB, LOW);
}
void resetFade() {
  fadeInterval = 25;
  fadeLevel = 0;
  fadeDirection = 0;
  fadeRevLevel = 255;
  previousFadeMillis = millis() - fadeInterval;
}
void setStatusColor(int red, int green, int blue) {
  analogWrite(ledR, red);
  analogWrite(ledG, green);
  analogWrite(ledB, blue);  
}

Cleaned up abit and also added a button to alter fade speed and a status rgb led.

With regards to debouncing.. whether its cheap buttons, my fat fingers or a complete lack of understanding but locking the button read out for any less than 700ms just didn't work for me. I have added another variable so that the button is read every 50ms but locks out actions for a full second when clicked. This actually works very well and the interaction feels very refined.

Haven't had chance to properly read and re-read all posts so might see some some obvious changes / mistakes when I do & please feel free to point out but think ive got the hang of very very basics.

now when i find time will see how much of a headache it is to turn this into an actual thing.. i like it that much

Thanks again for all replies, still to re-read and properly take em all in

Here's an example of button debouncing you can try. My button is a jumper stuck in the button pin hole. When I ground it on the USB port housing (have to hold the board steady when I do it) that is a button press. You can use an actual button/switch, if you have the button wired to 5V and the pin side wired to ground through a pulldown resistor it will see button up as down and down as up but no damage.

All 3 of these files need to go in the sketch folder.

/* buttontimerclass example -- by GoForSmoke 2017 for public domain use

  This uses a button or switch or jumper from pin 2 to GND.
  Depending on your switch, the debounce value may need tuning.

  Button output is a state value for use in the main sketch code.
  -1, Undecided, pin state changed at least debounce-time ago.
  0, button currently still not pressed.
  1, button currently still pressed.
  2, button released since last checked by sketch.
  3, button pressed since last checked by sketch.

  The main sketch may need to track previous output state for real dirty
  switches. A clean switch can work with 3 ms debounce. I use 5 here.
*/

#include "Arduino.h"
#include "buttonlib.h"

//   THIS IS WHERE YOU SET SWITCH PIN NUMBER AND DEBOUNCE MILLIS
buttonclass  user( 2, 5 ); // pin # and debounce ms get compiled to flash

const byte ledpin = 13;

void setup()
{
  Serial.begin( 115200 );
  Serial.println( F( "\n  Startup\n" ));

  pinMode( ledpin, OUTPUT ); // default is INPUT LOW, now is OUTPUT LOW
}

void loop()
{
  static byte buttonRead; // allocated once, used often

  user.runButton(); // turns the crank once, make sure it runs often!

  buttonRead = user.readOutput();

  if ( buttonRead == 3 ) // 1st read since pressed
  {
    digitalWrite( ledpin, HIGH );
    Serial.print( F( "button pressed millis() == " ));
    Serial.println( millis());
  }
  else if ( buttonRead == 2 )
  {
    digitalWrite( ledpin, LOW );
    Serial.print( F( "button released millis() == " ));
    Serial.println( millis());
    Serial.println( );
  }
}
/*
  button.h for public domain use by GoForSmoke May 29 2015
  Revised Sept 2016  
  To use:
  make a buttonclass object in your sketch
  run that object every time through loop(), it is quickly done
  when you want to know the status of the button, you read the object
  it returns state;  bit 0 = current, bit 1 = previous, bit 2 = bounce
*/

// button library, Sept 25th, 2016 by GoForSmoke

#ifndef button_h
#define button_h

#include "Arduino.h"

#define CURRENT_1 1
#define PREVIOUS_2 2
#define BOUNCE_4 4

typedef class button
{
private:
  byte arduPin;
  byte buttonState; // bit 0 = current, bit 1 = previous, bit 2 = bounce
  byte startMs;
  byte debounceMs; 

public:
  button(); // default constructor
  void setButton( byte, byte ); // pin, debounce millis
  button( byte, byte ); // pin, debounce millis
  void setUpButton( void );
  byte runButton( void ); // returns buttonState as below
  // buttonState: bit 0 = current, bit 1 = previous, bit 2 = bounce
  byte buttonRead();  // returns buttonState as above
};

#endif
// button library, Sept 25th, 2016 by GoForSmoke

#include "Arduino.h"
#include "button.h"

//    button ================================================

button::button() // default constructor for arrays, needs the full setup
{
  buttonState = CURRENT_1;
}

button::button( byte ap, byte dbm )
{
  arduPin = ap;
  debounceMs = dbm;
  buttonState = CURRENT_1;
};

void button::setButton( byte ap, byte dbm ) // pin, debounce millis
{
  arduPin = ap;
  debounceMs = dbm;
  pinMode( arduPin, INPUT_PULLUP );
};


void button::setUpButton( void )
{
  pinMode( arduPin, INPUT_PULLUP );
};

byte button::buttonRead()
{
  return buttonState;
};


byte button::runButton( void )
{
//  static byte msNow;

  buttonState &= BOUNCE_4 | CURRENT_1; // clears previous state bit
  buttonState |= ( buttonState & CURRENT_1 ) << 1; // copy current state to previous
  buttonState &= BOUNCE_4 | PREVIOUS_2; // clears current state bit
  buttonState += digitalRead( arduPin ); // current state loaded into bit 0

//  msNow = (byte) millis(); // gets the low byte of millis

  if ( buttonState & 3 == CURRENT_1 || buttonState & 3 == PREVIOUS_2 )  // state change detected
  {
    buttonState ^= BOUNCE_4;      // toggles debounce bit
    // on 1st and odd # changes since last stable state, debounce is on
    // on bounces back to original state, debounce is off 
    if ( buttonState & BOUNCE_4 )
    {
//      startMs = msNow;    // starts/restarts the bounce clock on any change.
      startMs = (byte) millis();    // starts/restarts the bounce clock on any change.
    }
  }
  else if ( buttonState & BOUNCE_4 ) // then wait to clear debounce bit
  {
//    if ( msNow - startMs >= debounceMs ) // then stable button state achieved
    if ( (byte)((byte)millis()) - startMs >= debounceMs ) // then stable button state achieved
    {
      //   understand that stable state means no change for debounceMs. When time
      //   is over the state bits are manipulated to show a state change.
      buttonState &= CURRENT_1; // clear all but the current state bit
      if ( buttonState == 0 )  buttonState = PREVIOUS_2;  // HIGH->LOW
      else                     buttonState = CURRENT_1;  // LOW->HIGH
      //   buttonState now appears as a debounced state change in bits 0 and 1
    }
  }

  return buttonState;
};

//    end button ============================================

I have another example (that uses the same library) showing how to make and use an array of buttons.