Attiny44 issue with muliplexing digits on a 7-segment display

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

Hi guys,

 

I'm having some strange behavior on my attiny44, that I can't find the root cause of. Hopefully you can help.

 

Description of the problem:
I am using the attiny44 to drive 2 seven-segment displays. So since the attiny44 doesn't have 14 I/O pins, I decided to multiplex the digits. I am using 7 pins for the 7 segments of the display, and the common cathode of each digit is connected to a MOSFET so that I can switch which digit is being displayed. So 7 pins for the 7-segment displays, 2 pins for the number of digits. In my code I multiplex the digits every 10 ms. So for 10 ms, the tens digit will be on, then for 10 ms, the ones digit will be on. I've got this working normally, except there is some sporadic buggy behavior. Every once in a while, the digits won't multiplex. Instead of the 2 mosfet pins switching every 10 ms, they stop switching for about 250 ms. This can be seen in the graph I've taken of the mosfet pins I took on an oscilloscope:

 

 

and you can see the exact behavior I'm talking about several times in this 20 second video:

https://youtu.be/NEx-5vWIai4

 

and here is the function that is called periodically (every 10 ms) to multiplex the digits:
 

void multiplexDigits( void )
{
    static digit_e whichDigit = ONES;

    if( whichDigit == ONES )
    {
        MOSFET_REG &= ~( 1 << TENS_PIN );   //clear tens
        MOSFET_REG |= ( 1 << ONES_PIN );    //set ones
        SetDisplay( Ones );
        whichDigit = TENS;
    }
    else if( whichDigit == TENS )
    {
        MOSFET_REG &= ~( 1 << ONES_PIN );   //clear ones
        MOSFET_REG |= ( 1 << TENS_PIN );    //set tens
        SetDisplay( Tens );
        whichDigit = ONES;
    }
}

 

The timing of these digits blanking was always the exact same from one test run to the next with the code the same. For example, I would notice that the ones digit would blank at the following times: 0, 9, 18, 29, 44, 46 etc (seconds from start), and a similar pattern for the tens digit. When I reset, the digits would blank at the exact same times I had previously recorded, no matter how many times I reset. So this leads me to believe that this is deterministic and not random as I had first thought. That doesn't really help me narrow down the root cause very much, but at least it's something.

 

This is what I've tried:
I've simplified my infinite loop in main to be calling only this function (multiplexDigits) every 10 ms, and none of my other functions. The problem still happens.

 

I've tried changing the clock speed to see if that's somehow causing the issue. The problem still happens.

 

I tried changing the optimization levels of my code. The problem still happens.

 

I changed the rate at which I'm multiplexing the digits (instead of every 10 ms, tried calling the function every 12, 15, 20 ms, etc.). The problem still happens.

 

For each of these changes I made, I noticed that it would change the timing of when the digits would go blank. So instead of my above example of the ONES digit blanking at the 0, 9, 18, 29, 44, 46 seconds mark, it would have a new pattern, which was consistent every time I reset the attiny.

 

If you've read this far, thank you! I hope I've been clear in describing the problem. If not, let me know what I can clarify. I really hope to get to the bottom of this and will greatly appreciate any help or even just advice on how to continue debugging this.

 

Thanks!

This topic has a solution.
Last Edited: Fri. Aug 9, 2019 - 01:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The problem is probably in some other code you

have not shown.  A guess: you are using compare

match of a timer and once in a while it gets missed

and instead of resetting the counter it has to go all

the way 'round to 0.

 

--Mike

 

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

You need to share more code. We can't see what interrupts might be running. We can't see how you generate the 10ms timing.

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

As a slight improvement, you may also want to:

 

turn off the fet (now both fets off)

set up the display segments

turn on the required fet

 

this will give the "crispest"  display

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

How you call this function to flip digit selection? Is that an interrupt? a time delay routine?  Please post more of your code, mostly this time delay and the flipping caller.

Every multiplex I wrote in my life, used a simple timer interrupt routine, doing the dirty job automatically.

Anyhow, multiplex of 100Hz is a little bit drastic, just wasting clock cycles.  You can go down to 40~50Hz and still not seeing any flickering for two digits.

 

Tip: If you don't need to use the decimal point in the digits, you will be using only 7 port pins to drive the segments, so you can use this unused eighth pin of the same 7 segments port, do drive two small transistors, the second inverting the first, each driving one 7Seg display.  So, you don't need to use two extra port pins for the flipping.  For example, suppose this unused pin is the bit 7, so, the Tens display would have ORED 0x80 to its value, the Units display would have ANDed 0x7F, it means, bit 7 would select one or another display.

Wagner Lipnharski
Orlando Florida USA

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

First, I want to apologize for not replying before now.  Soon after I posted in April, life got very busy (and I mean packing up my life, changing jobs, and moving over 1000 miles) and I haven’t gotten back to really even looking at this project until this week.  Not to excuse myself, I should have at least acknowledged and thanked you all for your insight so far.  Which brings me to my second point: Thank you all for your replies!  It is incredible that there are people all over the world that have the same passion I do and knowledge to help me get answers to a problem when I don’t know anyone in my personal life with the expertise to weigh in.  Thank you all for your time so far, and any more advice you’re kind enough to give in the future.

 

Now to hopefully give you all a better understanding of my problem, I’m posting all the code (at the bottom of this reply).  I have pared down my code to just have the functionality of multiplexing the 2 digits of the display, and it is all working as intended except for the bug where 1 digit at a time flickers off for 1/4 second every once in a while.

 

Mike,

Yes, I am using the 8-bit timer 0 in CTC mode.  I get what you’re saying about missing a compare match and then it having to go all the way around again.  Although I find it unlikely that that is the case since my compare match interrupt should happen every 1 ms and the digits stop multiplexing for about 250 ms.  Is it possible that it could be missing the compare 25 times in a row?

Here’s the code I have that sets up timer0:

void timer0_init( void )

{

    cli();

    TCCR0A |= ( 1 << WGM01 );     //sets mode to CTC

    TCCR0B |= ( 1 << CS01 );      //clock select is divided by 8.

    OCR0A = 124;                      //sets TOP to 124 so the timer will reset every 1 ms.

    TIMSK0 = ( 1 << OCIE0A );     //Enable output compare interruptS

    sei();

}

Earlier, I set up the clock prescaler to 8 so the whole system has a clock speed of 1 MHz, rather than 8 MHz:

 

CLKPR = (1<<CLKPCE); // enable prescaler change
CLKPR = 3;           //set prescaler

And here’s the ISR that runs when timer 0 value is compared to OCR0A:

ISR( TIM0_COMPA_vect )
{
    cli();
    ticks ++;
    sei();
}

ticks is then used in this timer function

 

Bool isTimerElapsed( Timer_ts *myTimer )
{
    Bool timerElapsed = FALSE;

    if( ticks >= myTimer->endTime )
    {
        timerElapsed = TRUE;

        //reset the timer:
        myTimer->endTime = ticks + myTimer->period;
    }

    return timerElapsed;
}

 

Which is then called in my super-loop to determine if 10 ms has passed and the two digits should be multiplexed again:

if( isTimerElapsed( &timer10ms_s ) )
{
    multiplexDigits();
}

 

balisong42,

You bet.  I should have shared more code in the first place.  Please see the code in my response to Mike above, as well as the full code below.

 

avrcandies,

Thanks! That is a more logical way of ordering it.  I’ve changed that in my code as you can see below.

 

wagnerlip,

The digits are flipped every 10 ms, based on a timer I made that elapses every 10 ms.  The timer gets its time in ms from a interrupt on a compare match on 8 bit timer0, set up to clear on a compare every 1 ms.  Please see the code I’ve included for that.
I think I will decrease the frequency of multiplexing the digits. You’re right, there’s no reason it needs to be that fast!  I originally had it being every 10ms because I was calling some other stuff every 10 ms and thought I’d just put that in there as well.

Good idea with the 1 pin controlling 2 transistors! I don’t know why I hadn’t thought of that.  That will definitely help as I was hoping to be able to use one more pin for a button input.  Thank you!

 

Here's my code:

(download here if you prefer to view it that way)

/*================================= main.c =================================*/

#include "Project.h"
#include "display.h"
#include "timer.h"
#include "hardwareInit.h"

int main()
{
	Timer_ts timer10ms_s;
	Timer_ts timer1s_s;
	int i = 0;

	initDisplay();
    SetClockPrescale(8);	//internal clock divided by 8 = 1 MHz, from hardwareInit
	timer0_init();			//set up timer0 for 1 ms overflow
	customTimer_init( &timer10ms_s, 10 );
	customTimer_init( &timer1s_s, 1000 );

	while(1)
	{

		if( isTimerElapsed( &timer10ms_s ) )
        {
			multiplexDigits();
        }
		if( isTimerElapsed( &timer1s_s ) )
		{
			UpdateDisplayValues( i );
			i++;
		}
	}

	return 0;
}

/*================================= timer.h =================================*/

#pragma once

typedef uint32 Ticks_t;

typedef struct {
    Ticks_t endTime;
    Ticks_t period;
} Timer_ts;

Ticks_t getTicks( void );

void customTimer_init( Timer_ts * myTimer, Ticks_t period );

Bool isTimerElapsed( Timer_ts * myTimer );

void timer0_init( void );

/*================================= timer.c =================================*/

#include "Project.h"

//own header
#include "timer.h"

#include <avr/interrupt.h>

static Ticks_t ticks = 0;

void customTimer_init( Timer_ts *myTimer, Ticks_t period )
{
    myTimer->period = period;
    myTimer->endTime = ticks + period;
}

Bool isTimerElapsed( Timer_ts *myTimer )
{
    Bool timerElapsed = FALSE;

    if( ticks >= myTimer->endTime )
    {
        timerElapsed = TRUE;

        //reset the timer:
        myTimer->endTime = ticks + myTimer->period;
    }

    return timerElapsed;
}

void timer0_init( void )
{
    cli();
    TCCR0A |= ( 1 << WGM01 );	//sets mode to CTC
    TCCR0B |= ( 1 << CS01 );	//clock select is divided by 8.
	OCR0A = 124;				//sets TOP to 124 so the timer will reset every 1 ms.
    TIMSK0 = ( 1 << OCIE0A );	//Enable output compare interruptS
	sei();
}

ISR( TIM0_COMPA_vect )
{
    cli();
	ticks ++;
    sei();
}

Ticks_t getTicks( void )
{
	return ticks;
}

/*================================= display.h =================================*/

#pragma once

void clearDisplay();

void initDisplay();

void SetDisplay( uint8 value);

void UpdateDisplayValues( uint8 value);

void multiplexDigits( void );

/*================================= display.c =================================*/
//functions for using a 2 digit 7-segment display where the 2 separate digits are multiplexed

#include "Project.h"

//own header
#include "display.h"

typedef enum
{
    ONES,
    TENS
} digit_e;

static uint8 Tens;
static uint8 Ones;

void initDisplay()
{
	//Set up 7 pins for the LED anodes as outputs
    DISPLAY_DDR |= ( 1 << DISP_DDPIN1 ) | ( 1 << DISP_DDPIN2 ) |
                   ( 1 << DISP_DDPIN3 ) | ( 1 << DISP_DDPIN4 ) |
                   ( 1 << DISP_DDPIN5 ) | ( 1 << DISP_DDPIN6 ) |
                   ( 1 << DISP_DDPIN7 );
	//Set up 2 pins to switch the MOSFETS.
    MOSFET_DDR |= (1<< ONES_DDPIN )|(1<< TENS_DDPIN );

}

void UpdateDisplayValues( uint8 value)
{
	Tens = value / 10;
	Ones = value % 10;
}

void SetDisplay( uint8 value)
{
  clearDisplay();

   switch(value){
     case 0:
         DISPLAY_REG |= (1<<top)|(1<<tl)|(1<<tr)|(1<<br)|(1<<bl)|(1<<bottom);
	   break;
     case 1:
         DISPLAY_REG |= (1<<tr)|(1<<br);
	   break;
     case 2:
         DISPLAY_REG |= (1<<top)|(1<<tr)|(1<<middle)|(1<<bl)|(1<<bottom);
	   break;
     case 3:
         DISPLAY_REG |= (1<<top)|(1<<tr)|(1<<middle)|(1<<br)|(1<<bottom);
	   break;
     case 4:
         DISPLAY_REG |= (1<<tl)|(1<<tr)|(1<<middle)|(1<<br);
	   break;
     case 5:
         DISPLAY_REG |= (1<<top)|(1<<tl)|(1<<middle)|(1<<br)|(1<<bottom);
	   break;
     case 6:
         DISPLAY_REG |= (1<<top)|(1<<tl)|(1<<middle)|(1<<br)|(1<<bl)|(1<<bottom);
	   break;
     case 7:
         DISPLAY_REG |= (1<<top)|(1<<tr)|(1<<br);
	   break;
     case 8:
         DISPLAY_REG |= (1<<top)|(1<<tl)|(1<<tr)|(1<<middle)|(1<<br)|(1<<bl)|(1<<bottom);
	   break;
     case 9:
         DISPLAY_REG |= (1<<top)|(1<<tl)|(1<<tr)|(1<<middle)|(1<<br)|(1<<bottom);
	   break;
     default:
         break;
   }
}

void multiplexDigits( void )
{
    static digit_e whichDigit = ONES;

    if( whichDigit == ONES )
    {
        MOSFET_REG &= ~( 1 << TENS_PIN );   //clear tens
        SetDisplay( Ones );
        MOSFET_REG |= ( 1 << ONES_PIN );    //set ones
        whichDigit = TENS;
    }
    else if( whichDigit == TENS )
    {
        MOSFET_REG &= ~( 1 << ONES_PIN );   //clear ones
        SetDisplay( Tens );
        MOSFET_REG |= ( 1 << TENS_PIN );    //set tens
        whichDigit = ONES;
    }
}

void clearDisplay()
{
    DISPLAY_REG &= ~( ( 1 << top ) | ( 1 << middle ) | ( 1 << bottom ) | ( 1 << tl ) | ( 1 << tr ) | ( 1 << bl ) | ( 1 << br ) );
}

/*================================= hardwareInit.h =================================*/

#pragma once

void SetClockPrescale( uint16 divisor );

/*================================= hardwareInit.c =================================*/
#include "Project.h"

//own header
#include "hardwareInit.h"

void SetClockPrescale( uint16 divisor )
{
	switch( divisor )
	{
		case 1:
			CLKPR = (1<<CLKPCE); 	// enable prescaler change
			CLKPR = 0; 				//set prescaler
			break;

		case 2:
			CLKPR = (1<<CLKPCE);
			CLKPR = 1;
			break;

		case 4:
			CLKPR = (1<<CLKPCE);
			CLKPR = 2;
			break;

		case 8:
			CLKPR = (1<<CLKPCE);
			CLKPR = 3;
			break;

		case 16:
			CLKPR = (1<<CLKPCE);
			CLKPR = 4;
			break;

		case 32:
			CLKPR = (1<<CLKPCE);
			CLKPR = 5;
			break;

		case 64:
			CLKPR = (1<<CLKPCE);
			CLKPR = 6;
			break;

		case 128:
			CLKPR = (1<<CLKPCE);
			CLKPR = 7;
			break;

		case 256:
			CLKPR = (1<<CLKPCE);
			CLKPR = 8;
			break;

		default:
			#warning Not a valid prescale divisor
			break;
	}
}

/*================================= Project.h =================================*/
//Project level #defines and #includes that the whole project should have

#pragma once

#include <avr/io.h>
#include "typedefs.h"
#include "speedometer_io_mapping.h"

#define F_CPU   1000000UL       //change this if you change the clock prescale divisor

/*================================= typedefs.h =================================*/

#pragma once

typedef unsigned char uint8;
typedef unsigned char Bool;
typedef unsigned int uint16;
typedef unsigned long uint32;
typedef float real32;

typedef enum
{
	NONE = 0,
	ONE = 1,
	TWO = 2,
	LONG = 3
} PressType_te;

#define FALSE 0
#define TRUE (!FALSE)

//==============================================================================
// Constants
//==============================================================================
#ifndef YES
#define YES  1
#endif 

#ifndef NO
#define NO  0
#endif 

#ifndef OFF
#define OFF  0
#endif 

#ifndef ON
#define ON  1
#endif 

#ifndef TOGGLE
#define TOGGLE  2
#endif 

#ifndef TRUE
#define TRUE  1
#endif

#ifndef FALSE
#define FALSE 0
#endif 

#ifndef HIGH
#define HIGH  1
#endif 

#ifndef LOW
#define LOW 0
#endif 

#ifndef ENABLE
#define ENABLE  1
#endif 

#ifndef DISABLE
#define DISABLE  0
#endif

#ifndef ENABLED
#define ENABLED  1
#endif 

#ifndef DISABLED
#define DISABLED 0
#endif

#ifndef SET
#define SET  1
#endif 

#ifndef CLEAR
#define CLEAR 0
#endif

#ifndef NULL
#define NULL  (0)
#endif  /* NULL */

/*================================= speedometer_io_mapping.h =================================*/
//define statements for different pins and registers used for this project

#pragma once

//used by display module (7 segment display)
#define DISPLAY_REG         PORTA
#define top                 PORTA2
#define bottom              PORTA3
#define middle              PORTA6
#define bl                  PORTA4
#define br                  PORTA0
#define tl                  PORTA5
#define tr                  PORTA1

#define MOSFET_REG          PORTB        //register that has the pins that control the MOSFETs
#define TENS_PIN            PORTB1		//pin that activates the tens place digit
#define ONES_PIN            PORTB0		//pin that activates the ones place
#define MOSFET_DDR          DDRB
#define ONES_DDPIN          DDB0
#define TENS_DDPIN          DDB1

#define DISPLAY_DDR         DDRA
#define DISP_DDPIN1         DDA0
#define DISP_DDPIN2         DDA1
#define DISP_DDPIN3         DDA2
#define DISP_DDPIN4         DDA3
#define DISP_DDPIN5         DDA4
#define DISP_DDPIN6         DDA5
#define DISP_DDPIN7         DDA6

 

 

 

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

The display updating needs to happen in the isr. Yes it can be done by polling time, but if you want to actually display anything useful that means your mcu is doing some work besides updating a display. If you don't use an isr for the update, then you will always be chasing 'glitches' in the display. Use an isr and you never have to worry about it- just feed it display data and it works. You can also easily add a brightness feature if using an isr.

 

A simple example for 2 digits-

https://godbolt.org/z/wXjhu_

 

Last Edited: Fri. Aug 9, 2019 - 02:52 AM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

If you are doing it with polling, since ticks is uint32_t, anytime you read ticks from non interrupt code you could be interrupted part way through reading the 32 bit value ie. it takes 4 reads to read the whole 32 bit value. So you need to block interrupts, read the 32 bit value into a local, then enable interrupts again.

In your ISR, why do you need the cli() and sei(), surely this is unnecessary?

You might also want ticks to be volatile (may not be necessary the way you are using it but generally doesn't hurt to make it volatile just in case).

 

Also, this doesn't handle the case where time has wrapped around from UINT32_MAX back through 0.

 

    if( ticks >= myTimer->endTime )

You want to test if ticks is 'equal to or ahead' of endTime, which assuming ticks and endTime are uint32_t, can be done with (it's early, hopefully I got this the right way round)

if (ticks - myTimer->endTime < 0x80000000)

or

if ((int32_t)(ticks - myTimer->endtime) >= 0)

 

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

Note that there are defined names for a 7 segment display - A is top,B next clockwise and so on. G is the middle segment.

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

curtvm wrote:

The display updating needs to happen in the isr. Yes it can be done by polling time, but if you want to actually display anything useful that means your mcu is doing some work besides updating a display. If you don't use an isr for the update, then you will always be chasing 'glitches' in the display. Use an isr and you never have to worry about it- just feed it display data and it works. You can also easily add a brightness feature if using an isr.

 

A simple example for 2 digits-

https://godbolt.org/z/YH9fmY

 

 

I am definitely chasing glitches!  But I want to avoid doing the multiplexing in the ISR itself.  There are several reasons for that:

1. I've always heard and thought that ISRs should be as short and uncomplicated as possible (and something about re-entrancy of operations in ISRs?)

2. I want to use the ISR to establish a millisecond time-base for my entire program.  In other words, there are other functions that are dependent on the time-keeping but they are going to be called at different frequencies (some every 10 ms, some every 100, etc).

 

Also, it seems like there should be an answer to this issue.  I need to know why it's happening, at the very least so I can understand an alternative to what I'm trying to do, if not to find a fix for it.

Thanks for your help and the example!  That example is definitely doing things different than me, so it's helping me learn.

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

MrKendo wrote:

If you are doing it with polling, since ticks is uint32_t, anytime you read ticks from non interrupt code you could be interrupted part way through reading the 32 bit value ie. it takes 4 reads to read the whole 32 bit value. So you need to block interrupts, read the 32 bit value into a local, then enable interrupts again.

 

 

YES!!  You genius!  I changed my code in timer.c to call my getTicks() function instead of just referencing ticks.  And I changed the getTicks() function to block interrupts like you said while it's reading value of ticks:

Ticks_t getTicks( void )
{
	Ticks_t retVal;
	cli();
	retVal = ticks;
	sei();
	return retVal;
}

And this completely fixed the problem!  The digits never flicker off anymore. Thank you!  This issue has been bothering me for so long, and you've solved it for me just like that. :)

 

Quote:

In your ISR, why do you need the cli() and sei(), surely this is unnecessary?

 

 

The reason for this is because in my full program I am going to have other interrupts, and I figured it would be bad to have interrupts interrupting other ISRs.  The tradeoff is I might miss some events that would normally cause an interrupt, but I think for my purposes that's ok.  If I'm way off base with this, let me know. 

 

Quote:
You might also want ticks to be volatile (may not be necessary the way you are using it but generally doesn't hurt to make it volatile just in case).

 

I have basic understanding of the volatile keyword.  As far as I know, it lets the compiler know that the variable might change unexpectedly, but I don't really understand when I should use it on one of my variables. Can you give some more insight on that?

 

Quote:

Also, this doesn't handle the case where time has wrapped around from UINT32_MAX back through 0.

 

 

I did think about that, but 4294967295 ms comes out to about 50 days, which is more than 2 orders of magnitude bigger than the amount of time I ever expect my device to be powered on.

 

Quote:

    if( ticks >= myTimer->endTime )

You want to test if ticks is 'equal to or ahead' of endTime, which assuming ticks and endTime are uint32_t, can be done with (it's early, hopefully I got this the right way round)

if (ticks - myTimer->endTime < 0x80000000)

or

if ((int32_t)(ticks - myTimer->endtime) >= 0)

 

 

I don't really understand the benefit of changing this around.  Can you elaborate?
Thank you again for all your help!

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

Kartman wrote:
Note that there are defined names for a 7 segment display - A is top,B next clockwise and so on. G is the middle segment.

 

Did not know that.  Thanks!

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

>1. I've always heard and thought that ISRs should be as short and uncomplicated as possible

 

So that you are not impeding on the things that do need to use the isr, and refreshing a multiplexed display is one that needs to be in the isr. The code to update the display is minimal, and is required to run no matter where its located. The refresh code probably takes about 50 cycles, so worst case scenario is you delayed another isr by 50 cycles (50us at 1MHz). If that is unacceptable, run your clock at 8MHz and the 50 clocks now takes only ~6us. 

 

>2. I want to use the ISR to establish a millisecond time-base for my entire program.

 

Change the ocra to get the ms you want, update a tick count each time, every 5 times through update the display., or use another timer if available- you don't need unused spare timers laying around doing nothing while another does the job of two.

 

 

 

 

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

carsondh wrote:
I have basic understanding of the volatile keyword.  As far as I know, it lets the compiler know that the variable might change unexpectedly, but I don't really understand when I should use it on one of my variables. Can you give some more insight on that?

That's about it, but bear in mind change unexpectedly is exactly what can happen when a variable is being updated in an ISR ie. it's unexpected from the point of view of any non ISR code using that variable.

Simple example, you set a flag in an ISR, and in your main loop you are waiting for that flag to be set

while (!flag) { /* empty */}

Normally the compiler would look at this and see that nothing is changing flag in the loop, so it doesn't need to keep reading flag, it only needs to read it once. It can then further optimise to say that since the body is empty, it doesn't need to do anything at all, and optimise the whole thing away. Making flag volatile prevents this optimisation, it forces the compiler to always perform a memory access, so it will now keep reading flag until it becomes true as intended.

Same kind of thing can happen when reading hardware registers, waiting for a specific bit to be set etc. This is one reason why all the hardware registers will have a volatile somewhere within how they are defined.

Whether you need to use volatile in any given situation will depend on the situation, but if you don't use it when it is needed your code doesn't work, if you do use it when it wasn't actually necessary, no problem, other than it might cost a few extra instructions. For this reason the rule of thumb is to make anything volatile if it is shared between ISR and non ISR.

In your case where you have a function to get_ticks, I don''t think it will make any difference. It awlays has to read ticks once in the function, volatile or not.

 

 

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

The reason for this is because in my full program I am going to have other interrupts, and I figured it would be bad to have interrupts interrupting other ISRs. 

I'm glad you have it working.

 

The classic Mega and Tiny micros will hold multiple pending interrupts, and will execute them one after another.

They finish the current interrupt, then execute one instruction from the main code, and then execute the next interrupt.

 

So if two interrupts fire while an interrupt is being processed, they will be executed, not missed, and they will all execute sequentially.

 

IF a specific interrupt fires twice, (or multiple times), before it has execute, then the additional firings will be missed.

There is only one bit to show that an interrupt needs to be executed, so if a particular interrupt keeps firing, it keeps setting its bit, but it is already set...

 

The Xmegas have a three level priority interrupt controller.  So a higher priority interrupt can interrupt a lower priority interrupt.

 

There are some newer Tinies, (essentially Xtinies, 0 and 1 series ???), that I've not worked with, (yet).

I'm not sure which style interrupt controller they have.

 

Good luck with the rest of your project.

 

JC

 

Edit:

Typos

 

Last Edited: Fri. Aug 9, 2019 - 03:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just to answer some of the other questions.

Your ISR is like this

carsondh wrote:

ISR( TIM0_COMPA_vect )
{
    cli();
	ticks ++;
    sei();
}

I haven't used ATTINY, but assuming it behaves like ATMEGA, the hardware disables interrupts globally (ie. clear I bit in SREG) when handling an interrupt, so you don't need the cli() at the start.

The hardware also enables interrupts globally when exiting the ISR (compiler will generate reti rather than ret instruction). So you don't need the sei() at the end.

 

About the timer wrapping. It sounds like you won't have it switched on long enough to wrap, so OK.

But to demonstrate the point, imagine it was just a uint8_t counter. So it counts up to 255 then wraps back to 0.

If you start out with time_now = 0, and set end_time to 10 ticks in the future, you have end_time = 10, and the test 'if (time_now >= end_time)' works as expected.

If time_now was, say, 250 however,  and you set end_time to 10 ticks in the future, you now have end_time = 4 (260 as a uint8_t is 4).

So now the test doesn't work as intended.

But the properties of using unsigned values in C means that (assuming these values are all uint8_t)  you have

250 + 10 = 4

4 - 250 = 10

240 - 250 = 246 (which is -10 as an int8_t assuming 2's complement)

So with a bit of thought, by doing t2 - t1 as unsigned values, you can handle the wrap and still work out if you are ahead or behind time_now.

 

 

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

You could, of course, include all the multiplex on your 1ms interrupt.

See, a simple variable counter 8 bits, could count 1ms interrups indefinetely, you could use the bit 3 of the counter as the flag for choose Tens or Units display.

This counter will count from zero to 255 and wrap around, bit 3 will be on half of the time, during 8ms.

Bit 3 could also be copied to the output pin that control one or another mosfet (via an inverter or the first mosfet).

This operation is so fast, very few instructions, it will not pose any overhead on the 1ms routine.

Also, the value of bit 3 will tell you which value you must port  out for the the 7 segments, tens or units, 3 or 4 instructions.

You don't even need to count the 8 x 1ms for then output the value, just output each 1ms and don't worry.

If you do it in assembly, it will occupy less than 10 instructions inside the 1ms interrupt, running at 1MHz, around 10us, 1/100 of the 1ms routine.

 

The bit that control the transistors could be the 8th bit on the same 7 bits that control the segments.

So, output 0b1xxx-xxxx send xxx to display 1, 0b0xxx-xxxx send xxx to display 0.

A simple copy of bit 3 of such counter to bit 7 of the output port, along with the 7segs from Tens or Units to bits 0~6 would do the trick.

 

It is wise to always keep an eye about how much time and instructions some routine could be made and where to install it.

Sometimes the monster is just created by ourselves, and in real the monster doesn't need to be there.

Wagner Lipnharski
Orlando Florida USA

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

MrKendo wrote:
So you don't need the sei() at the end.
It's worse than that - not just "don't need" but "must not have". You would probably get away with it in this case as I guess the timer period is set to be much longer than the time the ISR() takes to execute but the danger in doing sei() in an ISR is if the interrupting event might happen again while one was being handled. The first handling likely pushed registers to the stack on entry. If that gets interrupted then the second entry pushes some more and if that gets interrupted the thrird entry pushes more again. If that continues for a while the stack eats all available RAM.

 

It's odd because we never used to see that many posts with cli/sei in ISRs here but recently there seem to have been loads which suggests there may be a widely read, misguided internet article somewhere that is suggesting to people that this is "normal" practice.

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

A friend that is not very versed in AVR assembly made some mistakes with [sei][cli] in the code. I asked him why he thought it would be necessary. Of couse his answer was about to "avoid" or to "allow" interrupts to happen. Then I dedicated 10 minutes to explain to him how this works inside the chip, that an interrupt automatically disable [cli] interrupts when the chip jumps to the interrupt handler address, and the RETI instruction will reactivate the interrupt [sei] automatically.  Made some examples, considering interrupt handling routines that would be longer than another faster interrupt happens, and so on, and demonstrate how we need to pay special attention to avoid that, since it is OUR job to preview and make it right.  He understood, and his answer was "but I thought that [sei/cli] would take care of any interrupt trying to happen while serving a previous one".  Then I just said that a processor is just a processor, a piece of epoxy with some silicon inside, it does not make magic, yet.   

 

So, to Carsonhd, always put yourself in place of the chip, and try to make its job slowly, normally you will find a better way to do the same thing, without much mistakes.

 

This is why I love assembly and always use it, you learn so much about the chip, all its guts and behaviors. That piece of epoxy becomes your friend.

 

Wagner Lipnharski
Orlando Florida USA

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

@curtvm:

I'm still not really convinced I should be putting all the logic for multiplexing the display inside the ISR.  

Also, as far as using another timer as long as it's available, I don't like that idea either for the following reasons:

1. Extra code overhead (initializing multiple timers instead of 1, having multiple ISRs)

2. More peripherals being used means more power consumption.  I don't know how much of a difference it would make, but this is going to be a battery powered device so unless necessary, I'm going to avoid using a 2nd timer.

 

I think we just have different mindsets in the approach to this code.

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

@MrKendo:

 

Thanks!  That makes sense.  I think I have a better understanding of volatile now.  It really makes sense that the registers would be volatile since they can be updated without the program knowing it.

Good rule of thumb, and I've changed ticks to be volatile.

 

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

@DotJC:

 

Thanks! I did not know that interrupts would execute sequentially if one is triggered before another is finished.  That is good to know and definitely makes it easier to manage and think about.

 

Thanks for the good luck! I'm having fun with this project :)

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

@MrKendo:

 

Ah, I did not know that about the interrupts.  And you are correct, the attiny does do it the same was as ATMEGA.  From the attiny44a datasheet:

When an interrupt occurs, the Global Interrupt Enable I-bit is cleared and all interrupts are disabled. The user software can write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then interrupt the current interrupt routine. The I-bit is automatically set when a Return from Interrupt instruction – RETI – is executed.

 Good to know, thanks! I've gotten rid of the cli() and sei() calls in my ISR now that I know they are redundant.

 

 

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

@wagnerlip:

 

Interesting! I definitely see what you're saying now about using the unused 8th bit of the port for the 7 segments to drive the mosfet.  And using the 3rd bit of the timer/counter as the flag for which digit to display is a good idea too.  I don't think I will change my code to do that for this project, for the sake of readability and the fact that I'm not hurting for clock cycles as it is.  But I will definitely keep those ideas in mind for future projects!  And I am going to take your advice about using 1 pin to control the mosfets instead of 2.  I could definitely use an extra pin on this project!

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

clawson wrote:

It's worse than that - not just "don't need" but "must not have". You would probably get away with it in this case as I guess the timer period is set to be much longer than the time the ISR() takes to execute but the danger in doing sei() in an ISR is if the interrupting event might happen again while one was being handled. The first handling likely pushed registers to the stack on entry. If that gets interrupted then the second entry pushes some more and if that gets interrupted the thrird entry pushes more again. If that continues for a while the stack eats all available RAM.

 

 

It's odd because we never used to see that many posts with cli/sei in ISRs here but recently there seem to have been loads which suggests there may be a widely read, misguided internet article somewhere that is suggesting to people that this is "normal" practice.

 

I'm glad you said that.  I'm not totally sure where I got that idea from, but I think it was from one of my college classes.  I will be sure to keep this in mind in the future (and I've already changed it for this project).  Can you give me more detail on what you mean by "the first handling likely pushed registers to the stack on entry"?  Why would the value of the registers need to be moved to RAM?

Last Edited: Fri. Aug 16, 2019 - 01:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@wagnerlip:

 

I have had very little experience with assembly.  I understand the value in it, but C is just so much more human-readable!

Last Edited: Fri. Aug 16, 2019 - 02:07 AM