polyphonic led drumpad

Together with some friends we are building a drumpad based on piezo sensors. When a knock is detected, a led is lit for 200ms. This is working ok using the delay() function, but it kills polyphony. We tried to use millis() instead of delay(), but we don’t really know how to manage it…

i’ll post our code to make it clear:

int ledPin = 2;
int ledPin2 = 3;
int THRESHOLD = 30;  
int t = 0;         
int previousMillis=0;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  }

void press(int n){
  int val = analogRead(n-1); //used n-1 because we have connected piezos to analog input 0 and forth
    if( val >THRESHOLD ) {
      t = 0;
      
      while(analogRead(n-1) > THRESHOLD) {
            t++;
       } 
     
       if(t>60) { 
         for(int i=0; i<1; i++){    //when knock is detected, assign previousMillis to millis, just once (?¿?¿?)
           previousMillis = millis(); // i don't really think this is well done...
         }
         digitalWrite(n+1, HIGH);
         if (millis() - previousMillis > 200){  //it worked ok using delay(200)... :_(
           digitalWrite(n+1, LOW);
         }
       }
    }
}

void loop() {
  press(1);
  press(2);
}

Have been messing around for hours without finding a solution for this problem. I tried lots of combinations, and couldn’t make it work propperly…which is frustrating!
i’m not an experienced programmer (as you can see), and i would appreciate some help…

thanx!

for(int i=0; i<1; i++){ //when knock is detected, assign previousMillis to millis, just once (?¿?¿?)
previousMillis = millis(); // i don’t really think this is well done…
}

No, it’s not. You have a for loop that is explicitly defined to loop once. The loop is unnecessary. Just set previousMillis to millis().

The very next thing you do is see:
if (millis() - previousMillis > 200)

Well, of course it isn’t, because you just set previousMillis to millis(), so at most previousMillis - millis() would be 1.

You could do this:

while(millis() - previousMillis < 200)
{
   // Do nothing...
}

But, then, all you would have done is successfully reproduce the millis function.

I’m not clear on what you are trying to do. If you want to light either LED for 200 ms when either piezo is triggered:

unsigned long onTimes[piezoCount];
void press() // No arguments. Need to check every pin
{
   for(int i=0; i<piezoCount; i++)
   {
      int val = analogRead(i);
      if(val > THRESHOLD)
      {
         while(analogRead(i) > THRESHOLD) t++; // No idea what this is for
         if(t > 60) // Or this
         {
             onTimes[i] = millis(); // Record when the LED is turned on
             digitalWrite(i, HIGH);
         }
       }
    }

    // Now, see if it's time to turn an LED off
    unsigned long now = millis();
    for(int i=0; i<piezoCount; i++)
    {
        // If LED is on, see how long it has been on
        if(onTimes[i] > 0 && now - onTimes[i] > 200)
        {
           // Led has been on long enough
           digitalWrite(i, LOW);
           onTimes[i] = 0;
        }
    }
}

Thanks for your help Paul! :D

I'm just testing the code and it works perfectly, i will soon post documentation of the results

s