General Coding question- Setting int values

Log in or register to post comments
Go To Last Post
12 posts / 0 new
Author
Message
phippstech
phippstech's picture
Level: Rookie
Joined: Wed. Nov 29, 2017
Posts: 36 View posts
Location: Austin
#1
Posted by phippstech: Mon. Oct 5, 2020 - 10:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So I am having a little problem with a interrupt based timer I am trying to implement. My interrupt vector is:

ISR(TIMER0_COMPA_vect)
{
	milsec++;
	if(milsec == 1000)
	{
		buzz_stim++;
		milsec = 0;
	}
}

with the code block where this is used:

void buzzer_short()
{ 
        millis_timer(1);
	buzz_interval = buzz_stim;
        sbi(PORTB,7);
	interval_diff = buzz_interval - buzz_stim;
	if (interval_diff > 2)
	{
		cbi(PORTB,7);
	}
}

buzzer_short() is called out through out my program and I am trying to used it for a piezo buzzer, so when it's time for it to be called the buzzer sounds for a second or two and the turns off. I know the issue with my code is that buzz_interval is basically always equal to buzz_stim so the interval_diff is always zero. Being that I am still honing my coding skills, I'm just not sure how to write where every time this block is called, buzz_interval resets and doesn't reset until it's called next. Thanks!

 

P.S. This is based off of a millisecond timer 

void millis_timer(uint8_t millis)
{
    TCCR0A |=  (1<<WGM01);
    TCCR0B |= (1<<CS02)|(1<<CS00);
    OCR0A = millis*7.815 - 1;
    TIMSK0 |= (1<<OCIE0A);
    sei();
}

 

This topic has a solution.

Cody W Phipps

Tags:

Tools, Compilers and General Programming, solved
Last Edited: Sun. Oct 11, 2020 - 01:48 AM
Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101132 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. Oct 5, 2020 - 11:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

VOLATILE

Top
phippstech
phippstech's picture
Level: Rookie
Joined: Wed. Nov 29, 2017
Posts: 36 View posts
Location: Austin
Posted by phippstech: Mon. Oct 5, 2020 - 11:27 PM (Reply to #2)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So instead of:

int buzz_interval;

It should be:

volatile int buzz_interval;

in my global variables?

Cody W Phipps

Top
awneil
awneil's picture
Level: 10k+ Postman
Joined: Fri. Jul 1, 2005
Posts: 21413 View posts
Location: Basingstoke, Hampshire, UK
Posted by awneil: Mon. Oct 5, 2020 - 11:28 PM (Reply to #2)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
VOLATILE

and he wrote a whole Tutorial about it: https://www.avrfreaks.net/forum/tutcoptimization-and-importance-volatile-gcc

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 oscillator: https://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...
Top
ka7ehk
ka7ehk's picture
Level: 10k+ Postman
Joined: Fri. Nov 22, 2002
Posts: 21821 View posts
Location: Tangent, OR, USA
Posted by ka7ehk: Mon. Oct 5, 2020 - 11:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

At the very least

 

volatile uint16_t   milsec;
volatile uint16_t   buzz_stim;

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

Top
Kartman
Kartman's picture
Level: 10k+ Postman
Joined: Thu. Dec 30, 2004
Posts: 24779 View posts
Location: Melbourne,Australia
Posted by Kartman: Tue. Oct 6, 2020 - 01:41 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

rather than re-invent the wheel, here's how Arduino does it:

 

https://github.com/arduino/Ardui...

 

Top
kk6gm
kk6gm's picture
Level: Raving Lunatic
Joined: Fri. Sep 11, 2009
Posts: 3878 View posts
Location: Sacramento, CA
Posted by kk6gm: Tue. Oct 6, 2020 - 05:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OUCH!  Don't use floating point here! (*7.815).  Small/medium micros do much better with integers.

 

The 'volatile' issue has been mentioned.  Any variable shared between ISR and main code must be 'volatile'.

 

But I think this is what you are asking about.  You keep reloading buzz_interval with buzz_stim, then checking if they are different.  They will not be (at most, an interrupt may come along and increment buzz_stim afterwards, giving a difference of 1, but never >2).

 

You need something like this:

 

void buzzer_short()
{ 
    millis_timer(1);
    buzz_interval = buzz_stim;
    sbi(PORTB,7);
    do
    {
        interval_diff = buzz_stim - buzz_interval;
    }
    while (interval_diff <= 2);
    cbi(PORTB,7);
}

Also, if buzz_stim is more than 8 bits (on an 8-bit machine), you have to make all main code accesses to buzz_stim atomic.

Top
N.Winterbottom
N.Winterbottom's picture
Level: Posting Freak
Joined: Tue. Oct 1, 2002
Posts: 1863 View posts
Location: Redbourn, Herts, UK
This reply has been marked as the solution. 
Posted by N.Winterbottom: Tue. Oct 6, 2020 - 07:59 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The OP code won't suffer from lack of volatile modifier because it contains a bug:

 

volatile uint8_t buzz_stim; // DON'T FORGET THIS

void buzzer_short()
{
    millis_timer(1);
    buzz_interval = buzz_stim;
    sbi(PORTB, 7);

    /* There should be a loop here to continually check interval_diff */
    interval_diff = buzz_interval - buzz_stim;
    if (interval_diff > 2)
    {
        cbi(PORTB, 7);
    }

    /* This whole code is far from optimal but this addition will suffice */
    do  {
        /* I reversed the subtraction here - you had another bug */
        interval_diff = buzz_stim - buzz_interval;
    } while (interval_diff < 2);

    cbi(PORTB, 7);
}

 

Last Edited: Tue. Oct 6, 2020 - 08:00 AM
Top
phippstech
phippstech's picture
Level: Rookie
Joined: Wed. Nov 29, 2017
Posts: 36 View posts
Location: Austin
Posted by phippstech: Tue. Oct 6, 2020 - 08:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks N.Winterbottom and kk6gm for the do while suggestion and the rest for the volatile tip. One more thing to add to thing for it to work for me, the sbi(PORTB,7) needs to be pulled into the do loop:

 

void buzzer_short()
{ 
	millis_timer(1);
	buzz_interval = buzz_stim;
	do 
	{
		sbi(PORTB,7);
		interval_diff = buzz_stim - buzz_interval;
	} 
	while (interval_diff < 1);
	cbi(PORTB,7);
}

 

 

Kartman wrote:

rather than re-invent the wheel, here's how Arduino does it:

 

https://github.com/arduino/Ardui...

 

This looks like something you would put into Arduino sketch. 

Cody W Phipps

Top
kk6gm
kk6gm's picture
Level: Raving Lunatic
Joined: Fri. Sep 11, 2009
Posts: 3878 View posts
Location: Sacramento, CA
Posted by kk6gm: Tue. Oct 6, 2020 - 09:06 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

phippstech wrote:
One more thing to add to thing for it to work for me, the sbi(PORTB,7) needs to be pulled into the do loop

That doesn't make sense, and suggests that something else is not right.

 

Also, I see that you are now testing for < 1 (so == 0).  That test can appear to fail because buzz_stim could be incremented in the ISR right after it is loaded into buzz_interval.  This is true no matter what interval_diff you are looking for (looking for an interval of X will always result in an interval between X-1+tiny_delta and X), but the "wrong answer" will be most obvious when comparing to 0.

Top
phippstech
phippstech's picture
Level: Rookie
Joined: Wed. Nov 29, 2017
Posts: 36 View posts
Location: Austin
Posted by phippstech: Sun. Oct 11, 2020 - 01:48 AM (Reply to #10)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for that. I marked the correct solution 

Cody W Phipps

Top
Kartman
Kartman's picture
Level: 10k+ Postman
Joined: Thu. Dec 30, 2004
Posts: 24779 View posts
Location: Melbourne,Australia
Posted by Kartman: Sun. Oct 11, 2020 - 02:34 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

phippstech wrote:
This looks like something you would put into Arduino sketch. 

 

Err no. It what implements the Arduino millis() function - which is what you seem to be doing. Note the atomic access of the tick counter. If your tick counter is anything larger than a uint8_t, then you need to ensure this.

Top