Help on making my code more efficient | MSGEQ7 & Cascaded 4017's

Hello everyone,

I am trying to make a led equalizer with 7x3 cascaded 4017’s and an msgeq7 chip. Ive got everything working hardware wise, but the code ive been using is too slow too keep the leds ‘‘on’’, which results in a flikker because the loop can’t rotate fast enough.

I am using one resetpin and one clockpin for each one of the 7 msgeq7’s output values.

There must be some way i can improve my beginner programming, so i was hoping some of you would be kind enough to help me out to get the loop to rotate faster.

Cheers,

Nick

int analogPin = A0; // read from multiplexer using analog input 0
int strobePin = 8; // strobe is attached to digital pin 2
int MSGResetPin = 9; // reset is attached to digital pin 3
int spectrumValue; // to hold a2d values
int clockPin = 52;
int clockPin1 = 50;
int clockPin2 = 48;
int clockPin3 = 46;
int clockPin4 = 44;
int clockPin5 = 42;
int clockPin6 = 40;
int resetPin = 53;
int resetPin1 = 51;
int resetPin2 = 49;
int resetPin3 = 47;
int resetPin4 = 45;
int resetPin5 = 43;
int resetPin6 = 41;

void setup()
{
  pinMode(analogPin, INPUT);
  pinMode(strobePin, OUTPUT);
  pinMode(MSGResetPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(clockPin1, OUTPUT);
  pinMode(clockPin2, OUTPUT);
  pinMode(clockPin3, OUTPUT);
  pinMode(clockPin4, OUTPUT);
  pinMode(clockPin5, OUTPUT);
  pinMode(clockPin6, OUTPUT);
  pinMode(resetPin, OUTPUT);
  pinMode(resetPin1, OUTPUT);
  pinMode(resetPin2, OUTPUT);
  pinMode(resetPin3, OUTPUT);
  pinMode(resetPin4, OUTPUT);
  pinMode(resetPin5, OUTPUT);
  pinMode(resetPin6, OUTPUT);
  analogReference(DEFAULT);
  digitalWrite(resetPin, LOW);
  digitalWrite(strobePin, HIGH);
}

void loop()
{
  digitalWrite(MSGResetPin, HIGH);
  digitalWrite(MSGResetPin, LOW);


  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue = spectrumValue / 40; // scale analogRead's value to Write's 255 max
  for (int var1 = 0; var1 < PWMvalue; var1 = var1 + 1) {
    clock();
  }
  reset();

  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue1 = spectrumValue / 40; // scale analogRead's value to Write's 255 max
  for (int var2 = 0; var2 < PWMvalue1; var2 = var2 + 1) {
    clock1();
  }

  reset1();
  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue2 = spectrumValue / 40; // scale analogRead's value to Write's 255 max
  for (int var3 = 0; var3 < PWMvalue2; var3 = var3 + 1) {
    clock2();
  }
  reset2();

  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue3 = spectrumValue / 40;  // scale analogRead's value to Write's 255 max
  for (int var4 = 0; var4 < PWMvalue3; var4 = var4 + 1) {
    clock3();
  }
  reset3();

  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue4 = spectrumValue / 40;  // scale analogRead's value to Write's 255 max
  for (int var5 = 0; var5 < PWMvalue4; var5 = var5 + 1) {
    clock4();
  }
  reset4();

  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue5 = spectrumValue / 40; // scale analogRead's value to Write's 255 max
  for (int var6 = 0; var6 < PWMvalue5; var6 = var6 + 1) {
    clock5();
  }
  reset5();

  digitalWrite(strobePin, HIGH);
  digitalWrite(strobePin, LOW);
  spectrumValue = analogRead(analogPin);
  int PWMvalue6 = spectrumValue / 40; // scale analogRead's value to Write's 255 max
  for (int var7 = 0; var7 < PWMvalue6; var7 = var7 + 1) {
    clock6();
  }

  digitalWrite(strobePin, HIGH);




  reset6();
}

void clock() {
  digitalWrite(clockPin, HIGH);
  delay(1);
  digitalWrite(clockPin, LOW);
}
void clock1() {
  digitalWrite(clockPin1, HIGH);
  delay(1);
  digitalWrite(clockPin1, LOW);
}
void clock2() {
  digitalWrite(clockPin2, HIGH);
  delay(1);
  digitalWrite(clockPin2, LOW);
}
void clock3() {
  digitalWrite(clockPin3, HIGH);
  delay(1);
  digitalWrite(clockPin3, LOW);
}
void clock4() {
  digitalWrite(clockPin4, HIGH);
  delay(1);
  digitalWrite(clockPin4, LOW);
}
void clock5() {
  digitalWrite(clockPin5, HIGH);
  delay(1);
  digitalWrite(clockPin5, LOW);
}
void clock6() {
  digitalWrite(clockPin6, HIGH);
  delay(1);
  digitalWrite(clockPin6, LOW);
}



void reset() {
  digitalWrite(resetPin, HIGH);
  digitalWrite(resetPin, LOW);
}
void reset1() {
  digitalWrite(resetPin1, HIGH);
  digitalWrite(resetPin1, LOW);
}
void reset2() {
  digitalWrite(resetPin2, HIGH);
  digitalWrite(resetPin2, LOW);
}
void reset3() {
  digitalWrite(resetPin3, HIGH);
  digitalWrite(resetPin3, LOW);
}
void reset4() {
  digitalWrite(resetPin4, HIGH);
  digitalWrite(resetPin4, LOW);
}
void reset5() {
  digitalWrite(resetPin5, HIGH);
  digitalWrite(resetPin5, LOW);
}
void reset6() {
  digitalWrite(resetPin6, HIGH);
  digitalWrite(resetPin6, LOW);
}

I don't understand what you mean by reset pin and why there are so many. Can you draw a schematic of how you have wired things up. Is this the latch pin?
The LEDs attached to a shift register should not go out by themselves, you just strobe them when you are ready to change them.
You should get all the bits into one array and then output them all at the same time. Your code looks way over complex and I can't follow what is going on. It is far too verbose.
Have you got decoupling capacitors on the power supply pins of all chips?

Hi Nick. Try removing all the delay() calls, and change all your loops from this

  for (int var6 = 0; var6 < PWMvalue5; var6 = var6 + 1) {
    clock5();
  }
  reset5();

To this

  reset5();
  for (int var6 = 0; var6 < PWMvalue5; var6 = var6 + 1) {
    clock5();
  }

Hello everyone,

With some help from another forum and some research ive got the code working.
Ive found out about some techniques that make the code a lot quicker!
If you’re wondering what i have changed, here is the code:

int analogPin = A0; // read from multiplexer using analog input 0
int strobePin = 8; // strobe is attached to digital pin 2
int MSGResetPin = 9; // reset is attached to digital pin 3
int spectrumValue; // to hold a2d values
int clockPins[7] = {52, 50, 48, 46, 44, 42, 40 };
int resetPins[7] = {53, 51, 49, 47, 45, 43, 41 };
int band[7];

void setup()
{
  pinMode(analogPin, INPUT);
  pinMode(strobePin, OUTPUT);
  pinMode(MSGResetPin, OUTPUT);
  pinMode(clockPins[7], OUTPUT);
  pinMode(resetPins[7], OUTPUT);
  digitalWrite(MSGResetPin, LOW);
  digitalWrite(strobePin, HIGH);
}

void loop()
{
  for ( int j = 0; j < 7; j++)
  {
    pinMode(clockPins[j], OUTPUT);
    pinMode(resetPins[j], OUTPUT);
    digitalWrite(MSGResetPin, HIGH);
    digitalWrite(MSGResetPin, LOW);
    digitalWrite(strobePin, LOW);
    spectrumValue = analogRead(analogPin);
    int PWMvalue = spectrumValue / 40; // scale analogRead's value to Write's 25 max
    band[j] = PWMvalue;
    for (int var1 = 0; var1 < band[j]; var1 = var1 + 1) {
      Pulse(clockPins[j]);
    }
    Pulse(resetPins[j]);
  }
  digitalWrite(strobePin, HIGH);
}

void Pulse(int pin) {
  digitalWrite(pin, HIGH);
  digitalWrite(pin, LOW);
}

Yes, considerably better. You removed the delay() calls, which will have helped reduce the flickering. But there is still my other suggestion. Have you tried that? It should make the display much brighter

PaulRB:
Yes, considerably better. You removed the delay() calls, which will have helped reduce the flickering. But there is still my other suggestion. Have you tried that? It should make the display much brighter

I have actually, but for some reason the flikkering is gone when i reset the 4017's after the for loop, but once i put the reset before the for loop the led's are brighter but flikkering like crazy.

Njick:
I have actually, but for some reason the flikkering is gone when i reset the 4017's after the for loop, but once i put the reset before the for loop the led's are brighter but flikkering like crazy.

I can't explain why that happens, it makes no sense to me. Resetting after the loop means that the leds will be on only for a tiny fraction of a second before the reset switches them off again. Resetting before the loop should mean that the leds are on all the time except for a tiny fraction of a second. Where the reset is done does not affect the speed that the code runs, so should not either increase or decrease the flickering, except that the flickering might appear less noticeable if the leds are dim.

Can you post that schematic? And the code with the resets done before the loops? Perhaps even a video showing the flickering of the two versions of code, although flickering can be difficult to show on a video.

PaulRB:
I can't explain why that happens, it makes no sense to me. Resetting after the loop means that the leds will be on only for a tiny fraction of a second before the reset switches them off again. Resetting before the loop should mean that the leds are on all the time except for a tiny fraction of a second. Where the reset is done does not affect the speed that the code runs, so should not either increase or decrease the flickering, except that the flickering might appear less noticeable if the leds are dim.

Can you post that schematic? And the code with the resets done before the loops? Perhaps even a video showing the flickering of the two versions of code, although flickering can be difficult to show on a video.

Hello,

I have made some schematics with eagle, you can find them here:

this basicly repeats 7 times:

Ive also made a video like you requested, the first one showing the reset BEFORE the for loop.
Second one being the the one with the reset AFTER the for loop.

Hmm... I don't see any series resistors in the video or on your schematic. And no bypass caps either, as Mike suspected. Could these problems be causing your flickering problems? It does not look how I expected either before or after the code change. With 4017, only 1 output should be high at any time, but it looks like they are all on at the same time, as if the count being sent to them is random every time. I expected to see a single led lit, moving left or right as the volume of sound in that equaliser band varies.

Let's face it, the biggest flaw is the hardware design. It is simply the wrong chip and wrong technique.
There is no need to multiplex anything if you just use simple shift registers. Why multiplex when there is no need.

Mike, I don't think the OP is planning to multiplex the display. I think the design is to use 21 x 4017 (in 7 groups of 3 cascaded 4017), but so far the prototype only has 3 x 4017.

21 chips is certainly not the way I would have designed it either. I might have used multiplexing, or rather used chips to do the multiplexing like 4 x max7219 or 2 x ht16k33. Or maybe I would just have used ws2812 led strips.

Mike, I don't think the OP is planning to multiplex the display.

He hasn't any choice, using those chips only one LED for each chip is on at any one time.

Ahh, the penny has dropped Mike! You are assuming the OP is aiming for a moving bar style display. I have been assuming a moving dot style display. If you are right, I absolutely agree with you, a bad design.

If he wanted a moving dot then it is still a bad design. I would use an up / down counter going into a binary to decimal decoder driving the LEDs. Then you could clock the counter with a suitable delay so that it matched the scaled number from the MSGEQ7 chip.