general coding question / advice

dunno, if this is the right place to ask, but this is my first arduino project aside from the blinking led.
maybe some experienced user can have a look and tell me if there are things to consider for optimisation, coding manners and such. i experience a strange behaviour where the rythm changes without any changes to the potis at the inputs. those are currently just between 0...+5V (later i plan to add control voltage inputs as well).

it seems that the rythm sometimes doesn't play along all sixteen notes. maybe there is something wrong in the way i play the loops?

it is a pattern generator for my eurorack to primarily generate drum patterns. the acc outs are not done yet, i am thinking of using them as pwm outs to create controlvoltages.

// pattern generator
const int led = 13;
const int trig1Out = 3;
const int trig2Out = 5;
const int trig3Out = 7;
const int acc1Out = 9;
const int acc2Out = 11;
const int acc3Out = A7;
const int clockIn = 2;
const int resetIn = 4;
const int selectIn = A0;
const int modifyIn = A1;
const int chaosIn = A2;
const int fill1In = A3;
const int fill2In = A4;
const int fill3In = A5;
volatile int clockState = LOW;

const uint16_t patterns[][3] = {
  {
    0b0000000000000000,
    0b0000000000000000,
    0b0000000000000000
  },
  {
    0b1000100010001000,
    0b0000000000000000,
    0b0000000000000000
  },
  {
    0b1000100010001000,
    0b0000100000001000,
    0b1000000010000000
  },
  {
    0b1000100010001010,
    0b0000100000001000,
    0b1010101010101010
  },
  {
    0b1001001010010010,
    0b0000100000001001,
    0b0010001000100010
  },
  {
    0b1000001000100000,
    0b0000100000001000,
    0b0010001000100010
  },
  //rock
  {
    0b1000001010000000,
    0b0000100000001000,
    0b1000100010001000
  },
  {
    0b1000001010100010,
    0b0000100000001000,
    0b1000100010001000
  },
  {
    0b1010010010000001,
    0b0000100000001000,
    0b1111111111111110
  },
  {
    0b1000000000100001,
    0b0000100000001000,
    0b1010101010101010
  },
  //pop
  {
    0b1000000010000010,
    0b0000101000001000,
    0b1010101010101010,
  },
  {
    0b1000001010000010,
    0b0000100000100000,
    0b1010101010101010
  },
  {
    0b1000001000100100,
    0b0001000010001000,
    0b1000100010001000
  },
  {
    0b1000010000001000,
    0b0001000000100100,
    0b1000100010001000
   },
  {
    0b1000101000100000,
    0b0011000000001111,
    0b0000000000000000
  },
  {
    0b1000000010100000,
    0b0000100100001000,
    0b1000100010001000
  },
  {
    0b1010000010110000,
    0b0000100100001000,
    0b1000100010001000
  },
  {
    0b1000000000100000,
    0b0000101010001011,
    0b1000000000000000
  },
  //reggea
  {
    0b0000100000001000,
    0b0000100101001001,
    0b1111111111111111
  },
  {
    0b0000100000001000,
    0b0011001000110010,
    0b1111111111111111
  },
  {
    0b0000000000000010,
    0b0010101000001100,
    0b0000000000000010
  },
  {
    0b1000100010001000,
    0b0000000010000000,
    0b1011111000000000
  },
  {
    0b1000100010001000,
    0b0010001000100011,
    0b1011101110111011
  },
  {
    0b1000100010001000,
    0b0000100000001001,
    0b1100110011001100
  },
  //R&B
  {
    0b1010001000100000,
    0b0000100000001001,
    0b1010101010101010
  },
  {
    0b1010000010110000,
    0b0000100101001001,
    0b1010101010101010
  },
  {
    0b1010010100110100,
    0b1101011101011111,
    0b1010101010101010
  },
  {
    0b1001000100000001,
    0b0010000010110010,
    0b1001101000000000
  },
  {
    0b1010000010110000,
    0b0000100101001001,
    0b1000000000000001
  },
  {
    0b1000000010000000,
    0b0000100000001001,
    0b1101110111011101
  },
  //afro
  {
    0b1000000010000000,
    0b0000001000100010,
    0b1011101010101010
  },
  {
    0b1000000010100010,
    0b0000001000001000,
    0b1011101010101010
  },
  {
    0b1000000010000000,
    0b0001001000000000,
    0b1011101010101010
  },
  {
    0b1000001010000010,
    0b0000100000100000,
    0b1000000100000010
  },
  //funk
  {
    0b1001000000100100,
    0b0000100000001000,
    0b1000100010001000
  },
  {
    0b1100000100100100,
    0b0000100000001000,
    0b1010101010101010
  },
  {
    0b1010010010000000,
    0b0000000000010000,
    0b1011101110111011
  },
  {
    0b1000101010000100,
    0b0001000000111000,
    0b1001010100000100
  },
  {
    0b0010010010010000,
    0b0000100100001000,
    0b1010111010111011
  },
  {
    0b1010010000000000,
    0b0000100000001111,
    0b1010010000000000
  }
};
const uint8_t numPatterns = sizeof(patterns) / sizeof(patterns[0]);

int counter = 0;
// the setup routine runs once when you press reset:
void setup() {   
  Serial.begin(9600);
       
  pinMode(led, OUTPUT);               
  pinMode(trig1Out, OUTPUT);      
  pinMode(trig2Out, OUTPUT);      
  pinMode(trig3Out, OUTPUT);       
  pinMode(acc1Out, OUTPUT);        
  pinMode(acc2Out, OUTPUT);        
  pinMode(acc3Out, OUTPUT);     
  pinMode(clockIn, INPUT_PULLUP); 
  pinMode(resetIn, INPUT_PULLUP); 
  pinMode(selectIn, INPUT);
  pinMode(modifyIn, INPUT);
  pinMode(chaosIn, INPUT);
  pinMode(fill1In, INPUT);
  pinMode(fill2In, INPUT);
  pinMode(fill3In, INPUT);

  delayMicroseconds(50);
  attachInterrupt(digitalPinToInterrupt(clockIn), clockInterrupt, FALLING ); 
  //attachInterrupt(digitalPinToInterrupt(resetIn), resetInterrupt, FALLING );  
  
  //Serial.print(numPatterns);
}
uint8_t select = 0;
uint8_t modify = 0;
uint8_t chaos = 0;
uint8_t fill1 = 0;
uint8_t fill2 = 0;
uint8_t fill3 = 0;

const int steps = 16;
int currentStep = 0;
uint16_t basePattern;
int baseStep = 0;
uint16_t snarePattern;
int snareStep = 0;
uint16_t hihatPattern;
int hihatStep = 0;

void loop() {  
  counter++;
  counter%=1024;    
  if (digitalRead(resetIn)==LOW) {
    currentStep = 0;
  }
  if(clockState == HIGH) {
    select = analogRead(selectIn);
    modify = analogRead(modifyIn)/100;
    chaos = analogRead(chaosIn);
    fill1 = (analogRead(fill1In))/25;
    fill2 = (analogRead(fill2In))/25;
    fill3 = (analogRead(fill3In))/25;
    int patternNumber = select * numPatterns/1024;
 
    digitalWrite(led,HIGH);
    basePattern = patterns[patternNumber][0];
    snarePattern = patterns[patternNumber][1];
    hihatPattern = patterns[patternNumber][2];

    /*fill patterns*/
    int fillStep=0;
    int fillCount = fill1;
    while(fillCount>0) {
        bitWrite(basePattern,fillStep,1);
        fillStep += int(16/(fillCount+1));
        if (fillStep>15) {
            fillStep = 0;
        }
        fillCount--;
    }
    if (fill1>32) basePattern /= fill1;
    fillStep=0;
    fillCount = fill2;
    while(fillCount>0) {
        bitWrite(snarePattern,fillStep,1);
        fillStep += int(16/(fillCount+1));
        if (fillStep>15) {
            fillStep = 0;
        }
        fillCount--;
    }
    if (fill2>32) snarePattern /= fill2;
    fillStep=0;
    fillCount = fill3;
    while(fillCount>0) {
        bitWrite(hihatPattern,fillStep,1);
        fillStep += int(16/(fillCount+1));
        if (fillStep>15) {
            fillStep = 0;
        }
        fillCount--;
    }
    if (fill3>32) hihatPattern /= fill3;
    
    /*chaos*/
    randomSeed(random(0,12345)+counter);
    if (random(100)<map(chaos,0,1023,-2,123)) {
      for (int i=0;i<chaos/64+1;i++) {
        int n = random(16);
        bitWrite(basePattern, n, !bitRead(basePattern,n));
      }
    }
    if (random(100)<map(chaos,0,1023,-2,123)) {
      for (int i=0;i<chaos/64+1;i++) {
        int n = random(16);
        bitWrite(snarePattern, n, !bitRead(snarePattern,n));
      }
    }
    if (random(100)<map(chaos,0,1023,-2,123)) {
      for (int i=0;i<chaos/64+1;i++) {
        int n = random(16);
        bitWrite(hihatPattern, n, !bitRead(hihatPattern,n));
      }
    }
    if (chaos>900) {
      basePattern &= chaos * fill1;
      snarePattern &= chaos * fill2;
      hihatPattern &= chaos * fill3;
    }
        
    baseStep = (steps-1-currentStep) + (modify/32);
    snareStep = (steps-1-currentStep) + (modify/8);
    hihatStep = (steps-1-currentStep) + modify;
    digitalWrite(trig1Out,bitRead(basePattern,baseStep%steps));
    digitalWrite(trig2Out,bitRead(snarePattern,snareStep%steps));
    digitalWrite(trig3Out,bitRead(hihatPattern,hihatStep%steps));
    
    currentStep ++;
    if (currentStep>=steps) {
      currentStep = 0;
    }
    delay(10);
    
    digitalWrite(trig1Out,LOW);
    digitalWrite(trig2Out,LOW);
    digitalWrite(trig3Out,LOW);
    digitalWrite(led,LOW);
    clockState = LOW;
    
  }
  else 
  {
  }
}

void clockInterrupt()
{   
  clockState = HIGH;
}

hopefully someone can share some knowledge

    modify = analogRead(modifyIn)/(1024/100);

I guess this doesn't do what you expect it to do. It will return the digitized value at A1 divided by 10. This is integer arithmetic, it won't convert to floats and back to integer. So if the value of analogRead(A1) is 409, modify will be 40.

Generally it's hard to follow the code without having any clue about the hardware behind it. As you failed to provide a wiring diagram of your setup, it's wild guessing to follow the program flow.

pylon:

    modify = analogRead(modifyIn)/(1024/100);

I guess this doesn't do what you expect it to do. It will return the digitized value at A1 divided by 10. This is integer arithmetic, it won't convert to floats and back to integer. So if the value of analogRead(A1) is 409, modify will be 40.

Generally it's hard to follow the code without having any clue about the hardware behind it. As you failed to provide a wiring diagram of your setup, it's wild guessing to follow the program flow.

thank you. i added the schematic of the module as it currently looks like.

it seems that the rythm sometimes doesn't play along all sixteen notes. maybe there is something wrong in the way i play the loops?

it is a pattern generator for my eurorack to primarily generate drum patterns. the acc outs are not done yet, i am thinking of using them as pwm outs to create controlvoltages.

The attached schematics doesn't show a system your describe in these sentences. What connects to CLOCK? What to RESET? How should we know how notes are played or where the rhythm comes from?

Did you fix the problem I described in my last answer?

pylon:
The attached schematics doesn't show a system your describe in these sentences. What connects to CLOCK? What to RESET? How should we know how notes are played or where the rhythm comes from?

sorry. i guess i am bound to thinking that it was obvious. the CLOCK and RESET inputs are getting voltages between -8 ... +8V from my eurorack system. CLOCK defines the speed the steps are played at. RESET is just to set the counter back to zero if wanted.

pylon:
Did you fix the problem I described in my last answer?

it seems so. at least the arduino is acting like i wanted it to.

by the way, the issue i found with shuffling stuff is due to certain lossy potentiometer positions. i used Serial.print to have a look at the values and some where jumping.

Hi,
OPs schematic.

Tom... :slight_smile:

Hi,
Are all your potentiometers 100R?
6 x 100R in parallel = 100/6 = 16R6.
Current required for the 6 pots;
5v / 16R6 = 0.3A = 300mA.

Where are you getting your 5V supply for the potentiometers from?

10K potentiometers are what is used as standard input for analog input pin impedance.

You have Serial.begin started, so it would be worth placing Serial.println statements at crucial parts of your code to do some troubleshooting.

Tom.. :slight_smile:

thank you Tom. to be honest when i draw the circuit i messed up those potentiometer values. actually they are 10k. sorry for that. i now take the 5 volts directly from my eurorack power bus. there are 1.5A available