mstimer2 not working

system · July 10, 2011, 6:26pm

Hello,

i got the advice to use an interupt to controll my LED, instead of using a blink/delay combination, so i installed the mstimer2 lib, but when I try to compile i get an error about a wrong usage of the void function(the message is german.). How do I do that right?

This is my code

void playstep(int stepnr) {

   if (currentSwitchState[stepnr] == 1) {
                AnalogValue = analogRead(stepnr);
                note = AnalogValue/16;
                noteOn(0xB0, 0x7B, 0x00);
                noteOn(0x90, note, 0x7F);
                
         MsTimer2::set(50, flash(stepnr));
         MsTimer2::start(); 
              
           //     digitalWrite(LEDpins[stepnr], HIGH);
             //   delay(50);
              // digitalWrite(LEDpins[stepnr], LOW);
             //   delay(currentTime);
}
}

void flash(int led) {
  static boolean output = HIGH;
  
  digitalWrite(LEDpins[led], output);
  output = !output;
}

thnx

johnwasser · July 11, 2011, 1:11am

MsTimer2::set() takes as its second argument the name a a function that takes no argument and returns 'void'.

You are trying to pass to it the result of calling the flash() function. The flash() function does not return a value.

Sorry but you can't provide arguments to the function that you want MsTimer2 to call at a later time.

system · July 11, 2011, 5:13am

How can I then use mstimer2 to flash may LEDs?

I have 8 LEDs right now and each LED has to flash at a certain point.

johnwasser · July 11, 2011, 1:24pm

You have to create functions that take no arguments:

void flash1(void) { flash(1) }
void flash2(void) { flash(2) }
void flash3(void) { flash(3) }

You can put those functions in an array and use them like your do LEDpins:

(*void flashfuncs(void))[] = {flash1,flash2,flash3};

If I got the syntax right, 'flashfuncs' is an array of pointers to functions that return void and take no arguments.

system · July 11, 2011, 2:35pm

nice, is it the best way to use a switch/case to choose which LED should blink? will try that when im home from work....

johnwasser · July 11, 2011, 3:59pm

The switch statement is useful when you need to compare a value against a variety of constants.

If you need to compare more than one value or the values to compare agains aren't constants you should use a series of if statements.

system · July 11, 2011, 4:28pm

mmmh, somehow i cant see any blinking:

void playstep(int stepnr) {

   if (currentSwitchState[stepnr] == 1) {
                AnalogValue = analogRead(stepnr);
                note = AnalogValue/16;
                noteOn(0xB0, 0x7B, 0x00);
                noteOn(0x90, note, 0x7F);
         
               switch (stepnr) {
                  case 0:   
                  MsTimer2::set(50, flash0);
                  MsTimer2::start();
                  break;
                case 1:    
                  MsTimer2::set(50, flash1);
                  MsTimer2::start();
                  break;
                case 2:    
                  MsTimer2::set(50, flash2);
                  MsTimer2::start();
                  break;
                case 3:    
                  MsTimer2::set(50, flash3);
                  MsTimer2::start();
                  break;
                   case 4:    
                  MsTimer2::set(50, flash4);
                  MsTimer2::start();
                  break;
                   case 5:    
                  MsTimer2::set(50, flash5);
                  MsTimer2::start();
                  break;
                   case 6:    
                  MsTimer2::set(50, flash6);
                  MsTimer2::start();
                  break;
                   case 7:    
                  MsTimer2::set(50, flash7);
                  MsTimer2::start();
                  break;
              }       
         
          
              
           //     digitalWrite(LEDpins[stepnr], HIGH);
             //   delay(50);
              // digitalWrite(LEDpins[stepnr], LOW);
             //   delay(currentTime);
}
}

void flash(int led) {
  digitalWrite(LEDpins[led], HIGH);
}

void flash0(void) {
  flash(0);
}
void flash1(void) {
  flash(1);
}
void flash2(void) {
  flash(2);
}
void flash3(void) {
  flash(3);
}
void flash4(void) {
  flash(4);
}
void flash5(void) {
  flash(5);
}
void flash6(void) {
  flash(6);
}
void flash7(void) {
  flash(7);
}

it seems this code is killing my complete programm

johnwasser · July 12, 2011, 5:16pm

Unlike the MsTimer2 example, your "flash()" function just sets the pin HIGH. I don't see the pin ever being set to LOW. After you go through the eight steps of the sequencer all the LED pins will be HIGH. I'm guessing that you wanted some different effect.

To get each LED to flash while the note is playing you will need to toggle the LED on and off. You probably also want to turn the previous light off because you can't be sure if it was left on by the last timer tick. Try this code:

void playstep(int stepnr) {

  if (currentSwitchState[stepnr] == 1) {
    AnalogValue = analogRead(stepnr);
    note = AnalogValue/16;
    noteOn(0xB0, 0x7B, 0x00);
    noteOn(0x90, note, 0x7F);

    switch (stepnr) {
    case 0:
      MsTimer2::stop();
      digitalWrite(LEDpins[7], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash0);
      MsTimer2::start();
      break;
    case 1:    
      MsTimer2::stop();
      digitalWrite(LEDpins[0], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash1);
      MsTimer2::start();
      break;
    case 2:    
      MsTimer2::stop();
      digitalWrite(LEDpins[1], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash2);
      MsTimer2::start();
      break;
    case 3:    
      MsTimer2::stop();
      digitalWrite(LEDpins[2], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash3);
      MsTimer2::start();
      break;
    case 4:    
      MsTimer2::stop();
      digitalWrite(LEDpins[3], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash4);
      MsTimer2::start();
      break;
    case 5:    
      MsTimer2::stop();
      digitalWrite(LEDpins[4], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash5);
      MsTimer2::start();
      break;
    case 6:    
      MsTimer2::stop();
      digitalWrite(LEDpins[5], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash6);
      MsTimer2::start();
      break;
    case 7:    
      MsTimer2::stop();
      digitalWrite(LEDpins[6], LOW);  // Turn off previous LED
      MsTimer2::set(50, flash7);
      MsTimer2::start();
      break;
    }       
  }
}

void flash0(void) { static boolean output = HIGH; digitalWrite(LEDpins[0], output); output = !output; }
void flash1(void) { static boolean output = HIGH; digitalWrite(LEDpins[1], output); output = !output; }
void flash2(void) { static boolean output = HIGH; digitalWrite(LEDpins[2], output); output = !output; }
void flash3(void) { static boolean output = HIGH; digitalWrite(LEDpins[3], output); output = !output; }
void flash4(void) { static boolean output = HIGH; digitalWrite(LEDpins[4], output); output = !output; }
void flash5(void) { static boolean output = HIGH; digitalWrite(LEDpins[5], output); output = !output; }
void flash6(void) { static boolean output = HIGH; digitalWrite(LEDpins[6], output); output = !output; }
void flash7(void) { static boolean output = HIGH; digitalWrite(LEDpins[7], output); output = !output; }
[/code[

system · July 12, 2011, 8:03pm

Hello,

the sequencer is playing but all the LED are LOW. I have a arduino mega is that of matter? The flashX functions do work. i tried that. I even set the mstimer2 to 500ms, no luck :(

johnwasser · July 12, 2011, 8:15pm

Perhaps the problem is a side-effect of something you do elsewhere in the code. If you want more help you should try posting all of your code.

system · July 13, 2011, 5:35pm

Hello,

here is my current code.
I reverted back to my last working version.
Hope this helps

#include <MsTimer2.h>

int switchPins[] = {38, 39, 40, 41, 42, 43, 44, 45};
int LEDpins[] = {22, 23, 24, 25, 26, 27, 28, 29};
int currentSwitchState[] = {0,0,0,0,0,0,0,0};
int AnalogValue = 0;

const int timePin1 = 8;  
const int timePin2 = 9;

const int playPin = 10;

const int modePin = 11;
const int middleC = 60;    // Middle C (MIDI note value 60) is the lowest note we'll play

byte note = 0x2A;
byte kanal = 0xB1;

int currentTimeState = 0;
int currentTimeStateDown = 0;

int playState = 0;
int lastplayState = 0;
int playing = 0;

int aktiv = 0;
int buttonCounter = 0;
int delaypause = 1000;
unsigned long timercount = 0;
unsigned long timer_start = 0;
unsigned long wartezeit = 5000; //1000ms = 1 sekunde
unsigned long currentTime = 250;
unsigned long Timer = 0;
int step = 0;

void setup() {
  //  set the states of the I/O pins:
      for (int x = 0;x<8;x++) {
       pinMode(LEDpins[x], OUTPUT); 
       pinMode(switchPins[x], INPUT);

    } 
                    

  pinMode(timePin1, INPUT);
  pinMode(timePin2, INPUT);
  pinMode(playPin, INPUT);
  //  Set MIDI baud rate:
  Serial.begin(31250);
  // turn all notes off
  noteOn(0xB00, 0x7B, 0x00);
  noteOn(0xB01, 0x7B, 0x00);
  noteOn(0xB02, 0x7B, 0x00);
  noteOn(0xB03, 0x7B, 0x00);
  noteOn(0xB04, 0x7B, 0x00);
  noteOn(0xB05, 0x7B, 0x00);
  noteOn(0xB06, 0x7B, 0x00);
  noteOn(0xB07, 0x7B, 0x00);
  blink(3);
  timer_start = millis();
}

void loop() {


//timing settings  
  currentTimeState = digitalRead(timePin1);
      if (currentTimeState == 1) {
        currentTime = currentTime + 50;
      }

  currentTimeStateDown = digitalRead(timePin2);
      if (currentTimeStateDown == 1) {       
        if (currentTime <= 50) {
          currentTime = 100;
        } else {     
          currentTime = currentTime - 50;
        }
      }
      
//play button action
  playState = digitalRead(playPin);
  
  if (playState != lastplayState) {
    
    // change the state of the led when someone pressed the button
    if (playState == 1) { 
      if(playing==1) {
        playing=0;
        noteOn(0xB0, 0x78, 0x00);
      } else { 
        playing=1;         
      }
    }
    // remember the current state of the button
    lastplayState = playState;
  }

  
 
//Main sequence
  if (playing == 1) {
/*    int x = 0;
    for (x = 0;x<8;x++) {
      currentSwitchState[x] = digitalRead(switchPins[x]);
       playstep(x); 
    } 

*/        
  if ((millis() - Timer) > currentTime) {
  
    Timer = millis();
currentSwitchState[step] = digitalRead(switchPins[step]);
       playstep(step); 
    if (step < 7) {
      
        
       step++; 
     } else {
  
       step = 0;
    }
  }
     
}   
    

}

//****************************************************************
//****************************************************************
// Functions
//****************************************************************
//****************************************************************


void noteOn(byte cmd, byte data1, byte  data2) {
  Serial.print(cmd, BYTE);
  Serial.print(data1, BYTE);
  Serial.print(data2, BYTE);
}

// Blinks an LED 3 times
void blink(int howManyTimes){
  int i;
  for (i=0; i< howManyTimes; i++) {
    digitalWrite(LEDpins[1], HIGH);
    digitalWrite(LEDpins[2], HIGH);
    delay(100);
    digitalWrite(LEDpins[1], LOW);
    digitalWrite(LEDpins[2], LOW);
    delay(100);
  }
}


void playstep(int stepnr) {

  if (currentSwitchState[stepnr] == 1) {
    
    AnalogValue = analogRead(stepnr);
    note = AnalogValue/8;
    noteOn(0xB0, 0x7B, 0x00);
    noteOn(0x90, note, 0x7F);

    digitalWrite(LEDpins[stepnr], HIGH);
    delay(50);
    digitalWrite(LEDpins[stepnr], LOW);
  }
}

void flash(int led) {
  digitalWrite(LEDpins[led], HIGH);
}

void flash0(void) {
  flash(0);
}
void flash1(void) {
  flash(1);
}
void flash2(void) {
  flash(2);
}
void flash3(void) {
  flash(3);
}
void flash4(void) {
  flash(4);
}
void flash5(void) {
  flash(5);
}
void flash6(void) {
  flash(6);
}
void flash7(void) {
  flash(7);
}

greetings

johnwasser · July 14, 2011, 2:48pm

The “working” version you just posted doesn’t use MsTimer2 at all.

It is not clear from the non-working version what you wanted to do with MsTimer2. Perhaps if you explained what you want the code to do it would be possible to suggest a way of doing it.

system · July 14, 2011, 6:53pm

Hello,

what im trying to do is that everytime the function playstep() is called to flash the LED for a couple of ms.

practically to replace these 3 lines with an intterupt call that flashes the LED, removing the delay() in my code

digitalWrite(LEDpins[stepnr], HIGH);
delay(50);
digitalWrite(LEDpins[stepnr], LOW);

hope this makes it clear

johnwasser · July 14, 2011, 10:08pm

Would it be good enough to just turn the LED on for the duration of the step?

for (int i=0; i<8; i++)
   {
   digitalWrite(LEDpins[i], i==stepnr);
   }

This loop goes through all the LED pins, turning off the ones that don’t match the current step number and turning on the one that does.

When you stop the sequence you can do this to make sure the last step doesn’t leave the light on:

for (int i=0; i<8; i++)
   {
   digitalWrite(LEDpins[i], LOW);
   }

system · July 15, 2011, 6:15am

Hello John,

this really works great. Sometimes the solution is a lot easier than I thought.

One questions remains: Will the for() create a slight delay? I had a for() in my programm before and i had to press a button at the right time, sometimes it didnt get the signal.

greetings

johnwasser · July 15, 2011, 2:09pm

Everything you put in a program creates a 'slight delay'. In this case the delay should be only a few microseconds (millionths of a second) so it should not cause a usability problem.

system · July 17, 2011, 9:42am

Hello,

thnx to your help i nearly finished the sequnecer and now started finalizing my looper and I run into the same problem. I have 8 channels, each channel loops one note and gets triggert every few seconds. When the note is retriggert i want the LED to flash, as long as the note is playing the LED should stay lit. Most of the code is the same, only the function looks a bit different, could I use the mstimer here? I tried but with the same result, note is playing but no LED is on.

void loopstep(int stepnr) {
  
if (currentSwitchState[stepnr] == 1) {
  
          AnalogValue[stepnr] = analogRead(stepnr);
          note[stepnr] = AnalogValue[stepnr]/8;
          noteOn(channel[stepnr], 0x7B, 0x00);
          noteOn(channel[stepnr], 64, 127);
          noteOn(channel2[stepnr], note[stepnr], 0x7F);
          lastNotePlayed[stepnr] = note[stepnr];
          lastSwitchState[stepnr] = 1;
          
          digitalWrite(LEDpins[stepnr], LOW);
          delay(30);          
          digitalWrite(LEDpins[stepnr], HIGH);
 } 
   if (currentSwitchState[stepnr] == 0 && lastSwitchState[stepnr] == 1) {
        noteOn(channel[stepnr], 0x7B, 0x00);
        //blink(1);
        digitalWrite(LEDpins[stepnr], LOW); 
        lastSwitchState[stepnr] = 0;
    } 
}

system · May 6, 2021, 2:41am