48 pot MIDI controller needs to be faster...

Hello - just bought an arduino uno about 2 weeks ago as I have been wanting to build my own MIDI controller with just a bunch of knobs for a while now. So forgive my extreme programming ignorance -- I am just starting to learn. I did take a intro to PHP class a couple semester ago... Anyway my project uses the Arduino UNO and 3 16-channel multiplexers (Texas Instruments CD74HC4067). I am also using the MocoLUFA firmware to send MIDI over USB. The code I have written works, but the output is kinda slow - I'm guessing because it's doing 48 loops for each of the 48 loops. Here's the code, hopefully you understand what I'm getting at -- I know there are probably much better ways to do this - so help me out please:

int input[] = {
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111,
B00000000,
B00010000,
B00100000,
B00110000,
B01000000,
B01010000,
B01100000,
B01110000,
B10000000,
B10010000,
B10100000,
B10110000,
B11000000,
B11010000,
B11100000,
B11110000,
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111,};  //set array of port values, 0-15 are PORTB pins 8-11, 16-31 are PORTD pins 4-7, 32-47 are PORTC pins 0-1 and PORTD pins 2-3

int val0;
int val1;
int val2;  //initialize analogRead value variables
int chanVal[48]; //initialize array to hold all 48 values to compare with old values
int oldChanVal[48];  //initialize array to hold all 48 old values to compare with new values

void setup(){

DDRB = B00001111;  // set PORTB (digital pins 8-11) to output
PORTB = B00000000;  // set PORTB to LOW
DDRD = DDRD | B11111100;  // set PORTD (digital pins 2-7) to output while making sure not to change pins 0 and 1 (TX and RX)
PORTD = B00000000;  // set PORTD to LOW
DDRC = DDRC | B000011;  // set PORTC (analog pins 0-1) to output 
PORTC = B000000;  // set PORTC to LOW
Serial.begin(38400);  //set this to 31250 for MIDI
}

void loop(){

for(int a=0; a < 48; a++){  //loop through all 48 channels
  
  for(int i=0; i < 16; i++){  //loop through first 16 channels to change PORTB value and assign val0 to chanVal array
    val0 = map(analogRead(A2), 0, 1023, 0, 127);  
    PORTB = input[i];
    chanVal[i] = val0;
  }
  
  for(int j=16; j < 32; j++){  //loop through 2nd 16 channels to change PORTD value and assign val1 to chanVal array
    val1 = map(analogRead(A3), 0, 1023, 0, 127);  
    PORTD = input[j];
    chanVal[j] = val1;
  }
  
  for(int k=32; k < 48; k++){  //loop through 3rd 16 channels to change PORTC and PORTD value and assign val2 to chanVal array
    val2 = map(analogRead(A4), 0, 1023, 0, 127);  
    PORTC = input[k];
    PORTD = input[k];
    chanVal[k] = val2;
  }
  
  if(abs(chanVal[a] - oldChanVal[a]) > 1){  //test to see if any of the values in the chanVal array have changed by more than one -- if so, print
    Serial.print(0xB0);
    Serial.print("::");
    Serial.print(a);
    Serial.print("::");
    Serial.println(chanVal[a]);
  }
  
  oldChanVal[a] = chanVal[a]; //assign chanVal values to oldChanVal before starting over
}

delay(4);
}

What's the delay at the end for?

CrossRoads:
What's the delay at the end for?

MIDI messages don't seem to be able to be picked up in Ableton Live (or MIDI-OX when testing) when the delay is anything less than 4ms. Also of course, the Serial.print is changed to Serial.write and the "::" commented out when I am testing it.

Oops. Nevermind on the delay. I just tested it again and there is no reason for it to be there.

Does that help the responsiveness at all? Or just ocurring once per loop it doesn't really matter?

Not really, it made no noticeable difference.

Ha, I think I just figured it out...

int input[] = {
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111,
B00000000,
B00010000,
B00100000,
B00110000,
B01000000,
B01010000,
B01100000,
B01110000,
B10000000,
B10010000,
B10100000,
B10110000,
B11000000,
B11010000,
B11100000,
B11110000,
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111,};  //set array of port values, 0-15 are PORTB pins 8-11, 16-31 are PORTD pins 4-7, 32-47 are PORTC pins 0-1 and PORTD pins 2-3


int chanVal[48]; //initialize array to hold all 48 values to compare with old values
int oldChanVal[48];  //initialize array to hold all 48 old values to compare with new values

void setup(){

DDRB = B00001111;  // set PORTB (digital pins 8-11) to output
PORTB = B00000000;  // set PORTB to LOW
DDRD = DDRD | B11111100;  // set PORTD (digital pins 2-7) to output while making sure not to change pins 0 and 1 (TX and RX)
PORTD = B00000000;  // set PORTD to LOW
DDRC = DDRC | B000011;  // set PORTC (analog pins 0-1) to output 
PORTC = B000000;  // set PORTC to LOW
Serial.begin(31250);  //set this to 31250 for MIDI
}

void loop(){

for(int a=0; a < 48; a++){  //loop through all 48 channels
  
  if((a >= 0) && (a < 16)){  //if first 16 channels change PORTB value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A2), 0, 1023, 0, 127);  
    PORTB = input[a];
   }
  
  else if((a >= 16) && (a < 32)){  //if 2nd 16 channels change PORTD value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A3), 0, 1023, 0, 127);  
    PORTD = input[a];
   }
  
  else{  //if 3rd 16 channels change PORTD and PORTC value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A4), 0, 1023, 0, 127);  
    PORTD = input[a];
    PORTC = input[a];
   }
  
  if(abs(chanVal[a] - oldChanVal[a]) > 1){  //test to see if any of the values in the chanVal array have changed by more than one -- if so, print
    Serial.write(0xB0);
    //Serial.print("::");
    Serial.write(a);
    //Serial.print("::");
    Serial.write(chanVal[a]);
  
}
  
oldChanVal[a] = chanVal[a]; //assign chanVal values to oldChanVal before starting over  
}

delay(48);
}

Now the delay is necessary. XD

You are doing an identical analog read and map each time through each of those inner loops.

Given that the analog input value is not going to change in any significant way over the microsecond timescale this seems very wasteful; better to read each input once, do whatever mapping/conversion you need on the value and then use that as many times as you need.

I'm not sure what your output code is doing, but you are repeatedly updating chanVal without using the values afterwards until you get to the 'has anything changed' code, so all the intermediate updates seem pointless since they will be overwritten. This makes me suspect you may have a logic fault, but since I don't know what the sketch is supposed to do it's impossible to say whether it's faulty or just inefficient. Similarly, you are output values to the port registers at a very high but uncontrolled rate; is this useful?

It looks to me as if the sketch should be able to run substantially faster than your serial port is capable of logging the output. Probably, if the output is intended to do anything useful, you ought to be controlling how frequently this code runs. The 'blink without delay' example sketch shows you how to do that.

monoLab:
Ha, I think I just figured it out...

Yes, the code you posted first set up a loop variable 'a' to go round 48 times, then your code pretty much ignored 'a' and did all the work again in inner loops :slight_smile:

monoLab:
Now the delay is necessary. XD

48ms? Are you sure? So to read 48 inputs that is 2.3 seconds

Also, is the baud rate just ignored by MOCOLUFA? If not, then given you are doing MIDI USB over USB which is a 12Mbit/s channel, your baud rate could go a lot higher.

Your input, ChanVal and oldChanVal all hold 7 or 8 bit values. So declare them as byte not int.

Note that some controller numbers have predefined meanings. You probably want a bit more control over which physical pot controlls which MIDI controller, rather than just writing to controllers 0 .. 47. In particular, controller 0 is bank Select so altering that randomly will give you unpredictable results.

So I suggest adding

bye controller[48] = {
80, 81, 82, 83,
86, 86, 87 .... and so on ....
}

and then

  if(chanVal[a] != oldChanVal[a]){  //test to see if any of the values in the chanVal array have changed
    Serial.write(0xB0);
    Serial.write(controller[a]);
    Serial.write(chanVal[a]);

Nantonos:

monoLab:
Now the delay is necessary. XD

48ms? Are you sure? So to read 48 inputs that is 2.3 seconds

Also, is the baud rate just ignored by MOCOLUFA? If not, then given you are doing MIDI USB over USB which is a 12Mbit/s channel, your baud rate could go a lot higher.

It's actually at the end of the main program loop and not within the for loop so it doesn't happen 48 times -- just once every 48... And I don't think the baud rate is ignored -- I accidentally forgot to switch it to 31250 once before doing the MocoLUFA firmware switch and I got nothing when I tried it out with MIDI-OX.

Nantonos:
Your input, ChanVal and oldChanVal all hold 7 or 8 bit values. So declare them as byte not int.

Note that some controller numbers have predefined meanings. You probably want a bit more control over which physical pot controlls which MIDI controller, rather than just writing to controllers 0 .. 47. In particular, controller 0 is bank Select so altering that randomly will give you unpredictable results.

So I suggest adding

bye controller[48] = {

80, 81, 82, 83,
86, 86, 87 .... and so on ....
}




and then



if(chanVal[a] != oldChanVal[a]){  //test to see if any of the values in the chanVal array have changed
    Serial.write(0xB0);
    Serial.write(controller[a]);
    Serial.write(chanVal[a]);

Thank you. I can't do "!=" though because the pots are just too noisy. A threshold of about one is like the minimum.

Ok, everything is working pretty smoothly now except for one hiccup. Instead of the first pot of each of the 3 groups of 16 pots sending data as controller 0, it is sending as controller 1 and the last pot of the 16 pot group is sending as 0. In the second and third groups of 16 the last pot is not being read at all and the first pot of the second group is sending data as 16 and 17 at the same time. I have a feeling this has something to do with my crazy array of PORT values, but I'm not seeing it.
Here's the code, if anyone else sees what I am probably obviously doing wrong please lemme know. Thank you:

byte input[48] = {
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111,
B00000000,
B00010000,
B00100000,
B00110000,
B01000000,
B01010000,
B01100000,
B01110000,
B10000000,
B10010000,
B10100000,
B10110000,
B11000000,
B11010000,
B11100000,
B11110000,
B00000000,
B00000001,
B00000010,
B00000011,
B00000100,
B00000101,
B00000110,
B00000111,
B00001000,
B00001001,
B00001010,
B00001011,
B00001100,
B00001101,
B00001110,
B00001111};  //set array of port values, 0-15 are PORTB pins 8-11, 16-31 are PORTD pins 4-7, 32-47 are PORTC pins 0-1 and PORTD pins 2-3

//byte controller[48] = {64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111};  //MIDI controller number to send
int chanVal[48]; //initialize array to hold all 48 values to compare with old values
int oldChanVal[48];  //initialize array to hold all 48 old values to compare with new values

void setup(){

DDRB = B00001111;  // set PORTB (digital pins 8-11) to output
PORTB = B00000000;  // set PORTB to LOW
DDRD = DDRD | B11111100;  // set PORTD (digital pins 2-7) to output while making sure not to change pins 0 and 1 (TX and RX)
PORTD = B00000000;  // set PORTD to LOW
DDRC = DDRC | B000011;  // set PORTC (analog pins 0-1) to output 
PORTC = B000000;  // set PORTC to LOW
Serial.begin(38400);  //set this to 31250 for MIDI
}

void loop(){

for(int a=0; a < 48; a++){  //loop through all 48 channels
  
  if((a >= 0) && (a < 16)){  //during first 16 channels change PORTB value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A2), 0, 1023, 0, 127);  
    PORTB = input[a];
   }
  
  else if((a >= 16) && (a < 32)){  //during 2nd 16 channels change PORTD value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A3), 0, 1023, 0, 127);  
    PORTD = input[a];
   }
  
  else if((a >= 32) && (a < 48)){  //during 3rd 16 channels change PORTD and PORTC value and assign analog value to chanVal array
    chanVal[a] = map(analogRead(A4), 0, 1023, 0, 127);  
    PORTD = input[a]; 
    PORTC = input[a];
   }
  
  if(abs(chanVal[a] - oldChanVal[a]) > 1){  //test to see if any of the values in the chanVal array have changed by more than one -- if so, print
    Serial.print(0xB0);
    Serial.print("::");
    Serial.print(a);
    Serial.print("::");
    Serial.println(chanVal[a]);
  
}
  
oldChanVal[a] = chanVal[a]; //assign chanVal values to oldChanVal before starting over  
}

delay(48);  //less than about 48ms causes slow knob movements not to be picked up well and more than 48 causes latency issues
}

And when I isolate any of the if statements by commenting out the other two -- the isolated one will act just like the first group of 16 with the last pot being read as 0.

for(int a=0; a < 48; a++){  //loop through all 48 channels
  if((a >= 0) && (a < 16)){
   }
  else if((a >= 16) && (a < 32)){
   }
  else if((a >= 32) && (a < 48)){

Wouldn't three for loops make more sense?

PaulS:

for(int a=0; a < 48; a++){  //loop through all 48 channels

if((a >= 0) && (a < 16)){
  }
 else if((a >= 16) && (a < 32)){
  }
 else if((a >= 32) && (a < 48)){



Wouldn't three for loops make more sense?

:smiley:

Yea... For my purposes it does the same thing though.