What is going wrong in this code to display a four digits number on 7 segments displays?

Go To Last Post
11 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi, I want to display the number stored in the register OCR1A, which is a number ranging from 800 to 2200. This a 50Hz PWM signal using Timer1.  I am really starting to learn AVR so I'm no expert on this. I'm reading different sources and references about programming AVR so I can achieve what I am looking for. Below I post a code which I intended to display the value in OCR1. The four 7 segment displays are multiplexed in PORTC. This did not work. Can anyone explain me what is going wrong? I need to be able to show a value on the display so after that I want to add other features to my program, for example, incrementing/decrementing the value of OCR1A using push buttons with software deboucing. 

 

This is the code: (By the way, I am programming an ATMEGA16)

 

#define F_CPU 8000000     // 8MHZ crystal
#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

unsigned short digits[3];
unsigned short digit;

void shownumber(unsigned char n)
{
/*This function writes a digits given by n to the display
Note: n must be less than 9 */

if(n<10)
{
switch (n)
{
case 0:
PORTC=0x3F;
break;

case 1:
PORTC=0x06;
break;

case 2:
PORTC=0x5B;
break;

case 3:
PORTC=0x4F;
break;

case 4:
PORTC=0x066;
break;

case 5:
PORTC=0x6D;
break;

case 6:
PORTC=0x7D;
break;

case 7:
PORTC=0x07;
break;

case 8:
PORTC=0x7F;
break;

case 9:
PORTC=0x6F;
break;
}
}
}

//-------------- Returns mask for common cathode 7-seg. display
unsigned short mask(unsigned short num) {
switch (num) {
case 0 : return 0x3F;
case 1 : return 0x06;
case 2 : return 0x5B;
case 3 : return 0x4F;
case 4 : return 0x66;
case 5 : return 0x6D;
case 6 : return 0x7D;
case 7 : return 0x07;
case 8 : return 0x7F;
case 9 : return 0x6F;
} //case end
}

void splitn(unsigned short int number) {
/* This function breaks apart a given integer into separate digits
   and writes them to the display array i.e. digits[]
   
   */
if (number>=1000) {
digit = number / 1000;            // extract thousands digit
digits[3] = mask(digit);      // and store it to port_array
digit = (number / 100) % 10;      // extract hundreds digit
digits[2] = mask(digit);      // and store it to port_array
digit = (number / 10) % 10;       // extract tens digit
digits[1] = mask(digit);      // and store it to port_array
digit = number % 10;              // extract ones digit
digits[0] = mask(digit);      // and store it to port_array
}
else {
digit = (number / 100) % 10;      // extract hundreds digit
digits[2] = mask(digit);      // and store it to port_array
digit = (number / 10) % 10;       // extract tens digit
digits[1] = mask(digit);      // and store it to port_array
digit = number % 10;              // extract ones digit
digits[0] = mask(digit);      // and store it to port_array
}
}
int main(void){
DDRC = 0xFF;
DDRB |= 0x0F;
DDRD = 0xFF;
digit      = 0; // Initial
PORTB      = 0; //
PORTC      = 0; //
TCCR1A |= 1<<WGM11 | 1<<COM1A1;
TCCR1B |= 1<<WGM12 | 1<<WGM13 | 1<<CS11;
ICR1=19999; //set ICR1 to produce 50Hz frequency
cli();
TIMSK|=(1<<TOIE0);
TCCR0|=(1<<CS00)|(1<<CS01);
sei();
while (1){
OCR1A=1000;
splitn(OCR1A);
}
}
   
 ISR(TIMER0_OVF_vect)

{
   /*

   This interrupt service routine (ISR)
   Updates the displays

   */
   static unsigned char i=0;

   if(i==3)
   {
      //If on last display then come
      //back to first.
      i=0;
   }
   else
   {
      //Goto Next display
      i++;
   }

   //Activate a display according to i
   PORTB&=(0b11110000);
   PORTB|=(1<<i);

   //Write the digit[i] in the ith display.
   shownumber(digits[i]);
}

 

-.Electronics is Science.-

Last Edited: Sat. Oct 21, 2017 - 01:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

You've not told us what result you got. Do you get anything on the display? What have you done to narrow down the problem?
My first guess is you need to declare the digits array volatile.
Eg:
volatile uint8_t digits[3];

Don't expect to write a reasonably complex piece of code and have it work first time. There are a number of blocks that all have to work in order to get the result you want. You need to prove each of these.when you do this, you narrow down the problem. One small problem is much easier to solve than one big, complex problem.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

XavierPacheco wrote:
This did not work.

What did happen?

 


 

ATmega16 has JTAG on port C pins 2..5. It is enabled by default, overriding "normal" digital I/O functionality of those pins. Have you un-programmed the JTD fuse? 

 

If "no", then don't start with trying to un-program it - fiddling with fuses is a risky thing unless you know exactly what you're doing. Instead start with moving the whole handling of the display to another port, and see if it works better. If it does, then read up carefully on fuses and how to program them. Ask here if in any doubt, before attempting to change fuse values!

 


 

Be meticulous about structuring your code textually. No-one cares to even try to read flush-left code, like in your program above. Structuring your coded is the first favor you can do to yourself


Minor remark:

/*This function writes a digits given by n to the display
Note: n must be less than 9 */

Bad comment. It should read "less than 10" or "less than or equal to 9". Good comments are good. Wrong comments are confusing, eventually to yourself in a few months time... ;-)

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Sat. Oct 21, 2017 - 01:36 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

XavierPacheco wrote:
Can anyone explain me what is going wrong?
You are doing the conversion of the decimal digit into the LED bit pattern once too often.

shownumber() expects a decimal digit and outputs the respective bit pattern. But splitn() has already stored the bit pattern in digits[], not the decimal digits.

 

And another major problem of your code:

unsigned short digits[3];
...
digits[3] = mask(digit);      // and store it to port_array

 

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
Wrong comments are confusing

At best.

 

At worst, they can be misleading.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all.

 

Thanks for your comments. My code is working now. I can effectively show the value of OCR1A on the displays. The only thing missing now is to add push buttons to change the value of OCR1A. But for this I need deboucing and it is complicating things a little bit. I will read references to see what I can do.  Another question: As I already used interrupts to updates displays, do I need other interrupt for the deboucing or just a simple delay would work? if so, I need to set timer2 of ATMEGA16 because I have used Timer0 and Timer1, to interrupt (update displays) and to output  a PWM signal respectively. 

Is a state machine convenient in this case?

-.Electronics is Science.-

Last Edited: Sat. Oct 21, 2017 - 11:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

My suggestion would be to use your timer0 tick interrupt to debounce the pushbuttons. Rather than use the overflow, use the CTC mode and select a convenient tick time, like 5ms.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But I used Timer0 to update de displays. Can I use it for two purposes? The only timer I haven't used is Timer2. Isn't it good for debounce?

-.Electronics is Science.-

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

the display update doesn't take much time, so you can also do debounce in that isr.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you. Do you know any reference for me to look into how to include a debounce software in my ISR? 

-.Electronics is Science.-

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Debounce is very simple - basically you keep a count of the number of times the input is on or off. For example - if your timer tick is 5ms, each timer tick, sample the switch input and increment a counter if it is pressed else reset the counter. If the counter hets to, say, 10 (10 x 5ms = 50ms) then you can report the switch as pressed.

Debounce has been discussed here extensively - google avr debounce.