(Solved) 7-Segment-Multiplexer Library not working

Hello this is my first post.
Lately I've been trying to multiplex some seven segments with a 74hc595 and I had success.

But when I try to convert the sketch into a lib it doesn't work.
It compiles fine, but when i upload it to the Arduino nothing happens.

Heres my code:

Sketch:

#include <SevenSegmentBitPatterns.h>
#include <SevenSegmentMuxer.h>

#define SER     8
#define SCK     10
#define EN      9
//#define SCLR    11

#define DISP1  2
#define DISP2  3
#define DISP3  4
#define DISP4  5
void signal();

int ar = 0;
uint8_t dig1,dig2 ,dig3,dig4;
SevenSegmentMuxer sm ;// sm(SER,SCK,EN,DISP1,DISP2,DISP3,DISP4);

void setup(){
  pinMode(13,1);
  //sm  = SevenSegmentMuxer(SER,SCK,EN,DISP1,DISP2,DISP3,DISP4);
  //  sm.setupPins();
  // sm.initTimer();
  signal();
  sm.begin(SER,SCK,EN,DISP1,DISP2,DISP3,DISP4);
  signal();
}

void loop(){
#define RANGE 10

  int16_t  ar = analogRead(5);
  if(ar > 1000);
  else{
    if (dig4 < RANGE) {
      dig4++;
    }
    if (dig4 == RANGE) {
      dig4 = 0;
      if (dig3 < RANGE) {
        dig3++;
      }
      if (dig3 == RANGE) {
        dig3 = 0;
        if (dig2 < RANGE) {
          dig2++;
        }
        if(dig2== RANGE){
          dig2 = 0;
          dig1++;
          if (dig1 == RANGE) {
            dig1 = 0;
            dig2 = 0;
            dig3 = 0;
            dig4 = 0;
          }
        }
      }
    }
    sm.setDigits(dig1,dig2,dig3,dig4);
    signal();
    delay((int)(ar));
  }


}

void signal(){
  digitalWrite(13,HIGH);
  digitalWrite(13,LOW); 
}//signal

Header:

/**
 *  This Program multiplexes up to four seven segmenst display via the shift register 74HC595.
 *  The multiplexing is entirely interrupt driven, so all you need is to set the digits of the 
 *  display(0-F) via SetDigits(). 
 *  It makes Timer0a throw an interrupt every 2 ms, withis this time frame the multiplexing is done.
 *  
 *  @author: micropro
 *  @date : 01.04.2010
 *
 *
 *
 **/
 #ifndef SSM_H
 #define SSM_H
 
 #include <WProgram.h>
 #include <SevenSegmentBitPatterns.h>
 #include <avr/io.h>
 #include <avr/interrupt.h>
 #include <inttypes.h>
 
#define PRESCL  256
#define MUXTIME  0.002 // 2 ms
#define TIM2A_COMP_VAL ((F_CPU/PRESCL)*MUXTIME) // calculate the compare value



#define NUMBER_OF_DISPLAYS 4 // needed for update()
//uint8_t ser,sck,en,disp1,disp2,disp3,disp4;
class SevenSegmentMuxer{



// Function Prototypes
public:
SevenSegmentMuxer();
void begin(uint8_t ser,uint8_t sck,uint8_t en,uint8_t disp1,uint8_t disp2,uint8_t disp3,uint8_t disp4);
void setDigits(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4);
void setDigitsBitwise(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4);
//void setupPins();
//void initTimer();
private:
//void switchDigit(uint8_t digit);
//void update();
//void displayDigit(uint8_t digit);

uint8_t numbers[] ;//= {ZERO,ONE,TWO,THREE,FOUR,FIVE,SIX,SEVEN,EIGHT,NINE,HEX_A,HEX_B,HEX_C,HEX_D,HEX_E,HEX_F};
uint8_t digit[4] ;//= {ZERO,ZERO,ZERO,ZERO};
//{HEX_F,HEX_A,DIG_I,DIG_L};

//volatile uint8_t updateCounter;
uint8_t _ser,_sck,_en,_disp1,_disp2,_disp3,_disp4;
//int dpon ;
};

#endif

Cpp-File:

 #include <SevenSegmentMuxer.h>
 

#define NUMBER_OF_DISPLAYS 4 // needed for update()
static void setupPins();
static void switchDigit(uint8_t digit);
static void displayDigit(uint8_t digit);
static void initTimer();
static void update();

static uint8_t numbers[] = {
  ZERO,ONE,TWO,THREE,FOUR,FIVE,SIX,SEVEN,EIGHT,NINE,HEX_A,HEX_B,HEX_C,HEX_D,HEX_E,HEX_F};
uint8_t digit[4]= {
  ZERO,ZERO,ZERO,ZERO};
//uint8_t ser,sck,en,disp1,disp2,disp3,disp4;
volatile uint8_t updateCounter = 0;
uint8_t _ser,_sck,_en,_disp1,_disp2,_disp3,_disp4;
//int dpon = 0x00;

void setupPins(){
  /*pinMode(this->ser,OUTPUT);
  pinMode(this->sck,OUTPUT);
  pinMode(this->en,OUTPUT); 
  pinMode(this->disp1,OUTPUT);
  pinMode(this->disp2,OUTPUT);
  pinMode(this->disp3,OUTPUT);
  pinMode(this->disp4,OUTPUT);
*/
  pinMode(_ser,OUTPUT);
  pinMode(_sck,OUTPUT);
  pinMode(_en,OUTPUT);
  pinMode(_disp1,OUTPUT);
  pinMode(_disp2,OUTPUT);
  pinMode(_disp3,OUTPUT);
  pinMode(_disp4,OUTPUT);
  
  }

  static void switchDigit(uint8_t dg){
  switch(dg){
  case 0:
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp1,HIGH); 
    break;
  case 1:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp2,HIGH); 
    break;
  case 2:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp3,HIGH); 
    break;
  case 3:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,HIGH); 
    break;
  default:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW);
    break;
  }
}//switchDigit

static void displayDigit(uint8_t dg){

  switchDigit(255); //disable all display 
  digitalWrite(_en,HIGH);
  //digitalWrite(SCLR,HIGH); 
  shiftOut(_ser,_sck,MSBFIRST,digit[dg]);
  /* extra clock pulse because SCK is connected to RCK (see 74HC595 datasheet)*/
  digitalWrite(_sck,HIGH);
  digitalWrite(_sck,LOW);

  digitalWrite(_en,LOW);

 // delayMicroseconds(200);
  switchDigit(dg);

} // displayDigit

void initTimer(){ 
  cli();

  /* Dont set TCCR2A ( and B) to zero straight, or else functions like delay() wont 
   work */
  // clear the currect timer config 
  TCCR2A &= ~((1<<WGM21) |(1<< COM2A0) | (1<< COM2A1));
  TCCR2B &= ~((1<<CS20) | (1<<CS21) | (1<< CS22));

  /* We want a prescaler of 256 and to enable CTC mode  */
  /* CTC means the counter will reset to zero when the comparematch interrupt occurs */
  TCCR2B |= (1<<CS22) | (1<<CS21); // prescaler = 256
  TCCR2A |= (1<<WGM21); // CTC-Mode
  OCR2A = (uint8_t) TIM2A_COMP_VAL; // fill compare register A
  TIMSK2 |= (1<<OCIE2A); // enable compareMatch Interrupt A

    sei();
}//initMuxer

static void update(){ // used once in the ISR, thus "inline", also we save 8 clockcycles

  if(updateCounter < (NUMBER_OF_DISPLAYS -1)){ 
    updateCounter++;
  }
  else {
    updateCounter = 0;
  }
 
  displayDigit(updateCounter);

}//update

ISR(TIMER2_COMPA_vect){
  update();
}//ISR

/*********************** End Static Functions  *************************/

SevenSegmentMuxer::SevenSegmentMuxer(){

}
/*
SevenSegmentMuxer::SevenSegmentMuxer(uint8_t serN,uint8_t sckN,uint8_t enN,uint8_t disp1N,uint8_t disp2N,uint8_t disp3N,uint8_t disp4N){

this->_ser = serN;
this->_sck = sckN;
this->_en  = enN;
this->_disp1 = disp1N;
this->_disp2 = disp2N;
this->_disp3 = disp3N;
this->_disp4 = disp4N;

/*
ser = serN;
sck = sckN;
en  = enN;
disp1 = disp1N;
disp2 = disp2N;
disp3 = disp3N;
disp4 = disp4N;

}
*/

void SevenSegmentMuxer::setDigits(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4){
  cli();
  this->digit[0] = this->numbers[dg1];
  this->digit[1] = this->numbers[dg2];
  this->digit[2] = this->numbers[dg3];
  this->digit[3] = this->numbers[dg4];
  sei();
}// setDigits

void SevenSegmentMuxer::setDigitsBitwise(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4){
  cli();
  this->digit[0] = dg1;
  this->digit[1] = dg2;
  this->digit[2] = dg3;
  this->digit[3] = dg4;
  sei();
}// setDigitsBitwise

 

 void SevenSegmentMuxer::begin(uint8_t serN,uint8_t sckN,uint8_t enN,uint8_t disp1N,uint8_t disp2N,uint8_t disp3N,uint8_t disp4N){
 this->_ser = serN;
this->_sck = sckN;
this->_en  = enN;
this->_disp1 = disp1N;
this->_disp2 = disp2N;
this->_disp3 = disp3N;
this->_disp4 = disp4N;
 setupPins();
 initTimer();
 }//begin

Bit-patterns:

// Bit configs
#ifndef BITPATTERN_H
#define BITPATTERN_H

#define DOT     0b10000000
#define ZERO    0b00111111
#define ONE     0b00000110
#define TWO     0b01011011
#define THREE   0b01001111
#define FOUR    0b01100110
#define FIVE    0b01101101
#define SIX     0b01111101
#define SEVEN   0b00000111
#define EIGHT   0b01111111
#define NINE    0b01101111

#define HEX_A   0b01110111
#define HEX_B   0b01111100
#define HEX_C   0b00111001
#define HEX_D   0b01011110
#define HEX_E   0b01111001
#define HEX_F   0b01110001

#define DIG_c   0b01011000
#define DIG_H   0b01110110
#define DIG_I   0b00110000
#define DIG_L   0b00111000
#define DIG_O   0b01011100
#define DIG_P   0b01110011
#define DIG_u   0b00011100
#define DIG_U   0b00111110 
#define DIG_R   0b01010000
#define DIG_S   FIVE
#define DIG_Y   0b01101110
#define DIG_MS  0b01000000

#define SEG_A   0b00000001
#define SEG_B   0b00000010
#define SEG_C   0b00000100
#define SEG_D   0b00001000
#define SEG_E   0b00010000
#define SEG_F   0b00100000
#define SEG_G   0b01000000
#define SEG_DP  0b10000000

#endif

I don't have any answers, but I do have some questions.

void signal(){
  digitalWrite(13,HIGH);
  digitalWrite(13,LOW);
}//signal

Do you think you'll be able to catch that blink? The LED is on for only a few clock cycles. At 16MHz, that's pretty short.

  int16_t  ar = analogRead(5);
  if(ar > 1000);
  else{
    if (dig4 < RANGE) {
      dig4++;
    }
    if (dig4 == RANGE) {

dig4 has some random value. So, it gets incremented. OK, I guess.

The rest of the manipulation of dig1, dig2, dig3, and dig4 (none of which have explicitly been set) occurs only if dig4 equals RANGE.

Finally:

when i upload it to the Arduino nothing happens.

Nothing? How do you know that?

You should start trying to debug this by calling sm.setDigits(), with specific values, in setup(). That way, you aren't trying to debug the sketch and the library at the same time.

Do you think you'll be able to catch that blink? The LED is on for only a few clock cycles. At 16MHz, that's pretty short.

You're right gonna change that.

dig4 has some random value. So, it gets incremented. OK, I guess.

The rest of the manipulation of dig1, dig2, dig3, and dig4 (none of which have explicitly been set) occurs only if dig4 equals RANGE.

They usually get initialized automatically with the value 0 afaik, so it works fine, but yes, it is a good idea to initialize it .

Nothing? How do you know that?

because the array "digit" gets initialized this way:
uint8_t digit[4]= { ZERO,ZERO,ZERO,ZERO};

So even if the main code fails, it should display four zeros.

EDIT: It seem s that the the different values (see loop) are being set , but not muxed by the interrupt.

because the array "digit" gets initialized this way:
uint8_t digit[4]= { ZERO,ZERO,ZERO,ZERO};

So even if the main code fails, it should display four zeros.

I don't see a relationship between the digit array and dig1, dig2, dig3, and dig4 that are passed to the sm.setDigits() function.

I still think that the way to test the library is to call the setDigits function in setup, with a known set of values:

sm.setDigits(5,2,8,1);

Then, either the digits light up, or they don't. You'd know whether the problem was with the sketch or the library.

EDIT: It seem s that the the different values (see loop) are being set , but not muxed by the interrupt.

What interrupt?

I solved it. the problem was that i declared the variables numbers[] digit[] and _ser, _sck (and so on) as a member of the class SevenSegmentMuxer, so that the functions called by the interrupt routine couldn't modify anything but pin 0 :facepalm: (because vars are initialized with zero). After I changed that it works perfectly fine now.

Now the lib looks like this:

/**
 *  This Program multiplexes up to four seven segmenst display via the shift register 74HC595.
 *  The multiplexing is entirely interrupt driven, so all you need is to set the digits of the 
 *  display(0-F) via SetDigits(). 
 *  It makes Timer0a throw an interrupt every 2 ms, withis this time frame the multiplexing is done.
 *  
 *  @author: micropro
 *  @date : 01.04.2010
 *
 *
 *
 **/
 
 #include <SevenSegmentMuxer.h>
 #include <avr/interrupt.h>

#define NUMBER_OF_DISPLAYS 4 // needed for update()
static void setupPins();
static void switchDigit(uint8_t digit);
static void displayDigit(uint8_t digit);
static void initTimer();
static void update();
static void signal();
// MUST BE STATIC, or else the interrupt functions are not able to modify it
static uint8_t numbers[] = {
  ZERO,ONE,TWO,THREE,FOUR,FIVE,SIX,SEVEN,EIGHT,NINE,HEX_A,HEX_B,HEX_C,HEX_D,HEX_E,HEX_F};
static uint8_t digit[4]= {
  ZERO,ZERO,ZERO,ZERO};

volatile uint8_t updateCounter = 0;
static uint8_t _ser,_sck,_en,_disp1,_disp2,_disp3,_disp4;
//int dpon = 0x00;

static void setupPins(){
  /*pinMode(this->ser,OUTPUT);
  pinMode(this->sck,OUTPUT);
  pinMode(this->en,OUTPUT); 
  pinMode(this->disp1,OUTPUT);
  pinMode(this->disp2,OUTPUT);
  pinMode(this->disp3,OUTPUT);
  pinMode(this->disp4,OUTPUT);
*/
  pinMode(_ser,OUTPUT);
  pinMode(_sck,OUTPUT);
  pinMode(_en,OUTPUT);
  pinMode(_disp1,OUTPUT);
  pinMode(_disp2,OUTPUT);
  pinMode(_disp3,OUTPUT);
  pinMode(_disp4,OUTPUT);
  
  }

  static void switchDigit(uint8_t dg){
   
  switch(dg){
  case 0:
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp1,HIGH);     
      
      break;
  case 1:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp2,HIGH); 
    
      break;
  case 2:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp4,LOW); 
    digitalWrite(_disp3,HIGH); 
    signal();
      break;
  case 3:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,HIGH); 
   
      break;
  default:
    digitalWrite(_disp1,LOW); 
    digitalWrite(_disp2,LOW); 
    digitalWrite(_disp3,LOW); 
    digitalWrite(_disp4,LOW);
    
      break;
  }
}//switchDigit

static void displayDigit(uint8_t dg){

  switchDigit(255); //disable all display 
  digitalWrite(_en,HIGH);
  //digitalWrite(SCLR,HIGH); 
  
  shiftOut(_ser,_sck,MSBFIRST,digit[dg]);
  /* extra clock pulse because SCK is connected to RCK (see 74HC595 datasheet)*/
  digitalWrite(_sck,HIGH);
  digitalWrite(_sck,LOW);

  digitalWrite(_en,LOW);

 // delayMicroseconds(200);
  switchDigit(dg);

} // displayDigit

static void initTimer(){ 
  cli();

  
  // clear the currect timer config 
  TCCR2A &= ~((1<<WGM21) |(1<< COM2A0) | (1<< COM2A1));
  TCCR2B &= ~((1<<CS20) | (1<<CS21) | (1<< CS22));

  /* We want a prescaler of 256 and to enable CTC mode  */
  /* CTC means the counter will reset to zero when the comparematch interrupt occurs */
  TCCR2B |= (1<<CS22) | (1<<CS21); // prescaler = 256
  TCCR2A |= (1<<WGM21); // CTC-Mode
  OCR2A = (uint8_t) TIM2A_COMP_VAL; // fill compare register A
  TIMSK2 |= (1<<OCIE2A); // enable compareMatch Interrupt A

    sei();
}//initMuxer

static void update(){ // used once in the ISR, thus "inline", also we save 8 clockcycles

  if(updateCounter < (NUMBER_OF_DISPLAYS -1)){ 
    updateCounter++;
  }
  else {
    updateCounter = 0;
  }
 
  displayDigit(updateCounter);

}//update
static void signal(){
digitalWrite(13,1);
//delay(1);
digitalWrite(13,0);
}

ISR(TIMER2_COMPA_vect){
 
  update();
}//ISR

/*********************** End Static Functions  *************************/

SevenSegmentMuxer::SevenSegmentMuxer(){

}
/*
SevenSegmentMuxer::SevenSegmentMuxer(uint8_t serN,uint8_t sckN,uint8_t enN,uint8_t disp1N,uint8_t disp2N,uint8_t disp3N,uint8_t disp4N){

this->_ser = serN;
this->_sck = sckN;
this->_en  = enN;
this->_disp1 = disp1N;
this->_disp2 = disp2N;
this->_disp3 = disp3N;
this->_disp4 = disp4N;

/*
ser = serN;
sck = sckN;
en  = enN;
disp1 = disp1N;
disp2 = disp2N;
disp3 = disp3N;
disp4 = disp4N;

}
*/

void SevenSegmentMuxer::setDigits(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4){
  cli();
  digit[0] = numbers[dg1];
  digit[1] = numbers[dg2];
  digit[2] = numbers[dg3];
  digit[3] = numbers[dg4];
  sei();
}// setDigits

void SevenSegmentMuxer::setDigitsBitwise(uint8_t dg1,uint8_t dg2,uint8_t dg3,uint8_t dg4){
  cli();
  digit[0] = dg1;
  digit[1] = dg2;
  digit[2] = dg3;
  digit[3] = dg4;
  sei();
}// setDigitsBitwise

 

 void SevenSegmentMuxer::begin(uint8_t serN,uint8_t sckN,uint8_t enN,uint8_t disp1N,uint8_t disp2N,uint8_t disp3N,uint8_t disp4N){
 /*this->_ser = serN;
this->_sck = sckN;
this->_en  = enN;
this->_disp1 = disp1N;
this->_disp2 = disp2N;
this->_disp3 = disp3N;
this->_disp4 = disp4N;*/
 _ser = serN;
_sck = sckN;
_en  = enN;
_disp1 = disp1N;
_disp2 = disp2N;
_disp3 = disp3N;
_disp4 = disp4N;
 
 setupPins();
 initTimer();
 }//begin

I solved it.

Excellent.

But, why would the interrupt function need to modify numbers[], digits[], or any other values?

But, why would the interrupt function need to modify numbers[], digits[], or any other values?

It's because i connected the displays as shown in this schematic:

So I need a routine which changes the outputs in a defined interval.
The Timer2 causes every two milliseconds an interrupt, which cycles through displays and feeds them with a bit-pattern. Actually just one display is enable at a time, but they switch fast enough for the human eye not noticing it ( maybe flies can :D) .

PS: the schematic above is lacking a bunch of resistors.