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

11 posts / 0 new
Author
Message

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)

*/
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

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.

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... ;-)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

"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

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

JohanEkdahl wrote:

At best.

At worst, they can be misleading.

Top Tips:

1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

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

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.

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.-

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