atmega328p watchdog confusion

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

Hi all

 

I was designing a circuit and thought to implement watchdog system reset(not interrupt driven)in case of any accidental freezing of the MCU(The MCU will work in a very harsh environment).

Please take a look at the code below.

#include <avr/io.h>
#include <avr/wdt.h>

int main(void)
{
    wdt_disable();
    /*
    some other setup
    */
    wdt_enable(WDTO_8S);
    while(1)
    {
        wdt_reset();
        /*
        some other task
        */
    }
}

Should this code work or should I turn on WDTON fuse?There are example on the web but they are not very clear and max example uses interrupt driven method.But my main target is only to reset the system in case of any freezing.

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

What is going to cause your processor to freeze?

"This forum helps those that help themselves."

"How have you proved that your chip is running at xxMHz?" - Me

"If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

SHARANYADAS wrote:
max example uses interrupt driven method

???  What do you mean by "interrupt driven method"?  What do you want it to do for you?

 

SHARANYADAS wrote:
Should this code work

Again, what do you expect it to do?  Will your posted fragment "work"?  It looks like it wil indeed enable the WDT.  There are some comments about your posted code, though.  In a "real" full application, doing a WDR blindly every main loop only tells you that the main loop is being run once in a while.  It doesn't help if e.g. your interrupts aren't firing so you get no timer ticks or sensor readings or host communications.  Do the reset with an "AND" of all important conditions.

 

Do a Google search for "ganssle watchdog".

 

I've said that before, and also about the AND in prior discussions.  Have you looked for those?

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/forum/w... [code looks identical to yours]

...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

The CPU will work in a very high frequency RF environment.

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

SHARANYADAS wrote:
The CPU will work in a very high frequency RF environment.

So, what does that directly have to do with watchdog operation?

 

Is that environment creating noise, such that Absolute Maximum Ratings of the AVR are violated?  If so, anything can happen including chip damage.  When operationd >>within<< limits, then tell what you are trying to protect against.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

From datasheet:

 

WDTON Fuse Unprogrammed:
In this mode, the Watchdog Timer is initially disabled, but can be enabled by writing the WDE bit to 1 without any restriction.
A timed sequence is needed when changing the Watchdog Time-out period or disabling an enabled Watchdog Timer.

 

WDTON Fuse Programmed:
In this mode, the Watchdog Timer is always enabled, and the WDE bit will always read as one. A timed sequence is needed when changing the Watchdog Time-out period.

 

Therefore, disabling the Watchdog Timer (to save power for example, mainly in sleep modes) is possible if the fuse is unprogrammed only.

 

Where to insert the instructions 'WDR' [equivalent to wdt_reset(), I guess], in a code, needs to be considered carefully.

And what to do after an MCU reset by the Watchdog also depends on the application.

 

Last Edited: Sun. Feb 11, 2018 - 10:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
        wdt_reset();
        /*
        some other task
        */

Impossible to say whether that will work without knowing what "some other task" is and how long it may be away doing it.

 

While you can opt for a single wdt_reset() in the core loop in main() that then relies on everything that is invoked in main() getting back within the WDT period. If things you call to are themselves "blocking" then you may want to wdt_reset()s in the loops where they block too.

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

I have wdt_reset() many times within the while loop.There are so many modules in my program.I just wanted to know how to initiate it.That's why i posted this skeleton code.

I was having trouble with the wdt_disable().With wdt disable,the cpu was stuck in a never ending reset.So i omitted it and now the reset is working as expected.I tested with a delay_ms more than wdt delay and also implemented it in my final coding...

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

Did you read the user manual?:

 

http://www.nongnu.org/avr-libc/u...

 

Note in particular the paragraph that starts "Note that for newer devices...". That applies to a 328P

 

(You put the wdt_disable() in .init3 and not at the start of main() so it is on the "other side" of the _do_copy_data() and _do_clear_bss() loops)

Last Edited: Tue. Feb 13, 2018 - 09:15 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

According to AVR132,there is no need to disable the wdt if i only want to reset the system.That's why I omitted the wdt_disable().Is my understanding correct?

 

 

 

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

The paragraph you highlighted is the exact reason the manual says:

Note that for newer devices (ATmega88 and newer, effectively any AVR that has the option to also generate interrupts), the watchdog timer remains active even after a system reset (except a power-on condition), using the fastest prescaler value (approximately 15 ms). It is therefore required to turn off the watchdog early during program startup, the datasheet recommends a sequence like the following:
 

#include <stdint.h>

#include <avr/wdt.h>

uint8_t mcusr_mirror __attribute__ ((section (".noinit")));

void get_mcusr(void) \

__attribute__((naked)) \

__attribute__((section(".init3")));

void get_mcusr(void)

{

mcusr_mirror = MCUSR;

MCUSR = 0;

wdt_disable();

}

Saving the value of MCUSR in mcusr_mirror is only needed if the application later wants to examine the reset source, but in particular, clearing the watchdog reset flag before disabling the watchdog is required, according to the datasheet.

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

Make a note that when the WD resets the MPU, it also "resets" its own timeout value to the smallest timeout, but remains enabled, so you need to quickly disable it or reset its timeout value upon reset.

At least it does for the 328/168/88/48 family.....

 

Jim

 

Mission: Improving the readiness of hams world wide : flinthillsradioinc.com

Interests: Ham Radio, Solar power, futures & currency trading - whats yours?

 

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

Ok...as my understanding goes,this is as follows(Please check the comments).Please correct me If i am wrong.

 

#include <stdint.h>

#include <avr/wdt.h>

uint8_t mcusr_mirror __attribute__ ((section (".noinit")));

void get_mcusr(void) \
__attribute__((naked)) \
__attribute__((section(".init3")));//This is the function definition

void get_mcusr(void)//This is the declaration

{
mcusr_mirror = MCUSR;
MCUSR = 0;
wdt_disable();
}

Now,where should I call the function get_mcuscr()?Should I call it at the first line of main?

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

SHARANYADAS wrote:
Now,where should I call the function get_mcuscr()?Should I call it at the first line of main?
You don't "call" it at all. You already said:

void get_mcusr(void) \
__attribute__((naked)) \
__attribute__((section(".init3")));//This is the function definition

that drops the code into .init3 as described in:

 

http://www.nongnu.org/avr-libc/u...

 

A simple example shows how this works. If I write a virtually "empty" C program first:

C:\SysGCC\avr\bin>type avr.c
#include <avr/io.h>

char text[] = "hello";
uint8_t buffer[10];

int main(void) {
        while(1);
}

C:\SysGCC\avr\bin>avr-gcc -mmcu=attiny13 -Os avr.c -o avr.elf

C:\SysGCC\avr\bin>avr-objdump -d avr.elf

avr.elf:     file format elf32-avr


Disassembly of section .text:

00000000 <__vectors>:
   0:   09 c0           rjmp    .+18            ; 0x14 <__ctors_end>
   2:   21 c0           rjmp    .+66            ; 0x46 <__bad_interrupt>
   4:   20 c0           rjmp    .+64            ; 0x46 <__bad_interrupt>
   6:   1f c0           rjmp    .+62            ; 0x46 <__bad_interrupt>
   8:   1e c0           rjmp    .+60            ; 0x46 <__bad_interrupt>
   a:   1d c0           rjmp    .+58            ; 0x46 <__bad_interrupt>
   c:   1c c0           rjmp    .+56            ; 0x46 <__bad_interrupt>
   e:   1b c0           rjmp    .+54            ; 0x46 <__bad_interrupt>
  10:   1a c0           rjmp    .+52            ; 0x46 <__bad_interrupt>
  12:   19 c0           rjmp    .+50            ; 0x46 <__bad_interrupt>

00000014 <__ctors_end>:
  14:   11 24           eor     r1, r1
  16:   1f be           out     0x3f, r1        ; 63
  18:   cf e9           ldi     r28, 0x9F       ; 159
  1a:   cd bf           out     0x3d, r28       ; 61

0000001c <__do_copy_data>:
  1c:   10 e0           ldi     r17, 0x00       ; 0
  1e:   a0 e6           ldi     r26, 0x60       ; 96
  20:   b0 e0           ldi     r27, 0x00       ; 0
  22:   ee e4           ldi     r30, 0x4E       ; 78
  24:   f0 e0           ldi     r31, 0x00       ; 0
  26:   02 c0           rjmp    .+4             ; 0x2c <__do_copy_data+0x10>
  28:   05 90           lpm     r0, Z+
  2a:   0d 92           st      X+, r0
  2c:   a6 36           cpi     r26, 0x66       ; 102
  2e:   b1 07           cpc     r27, r17
  30:   d9 f7           brne    .-10            ; 0x28 <__do_copy_data+0xc>

00000032 <__do_clear_bss>:
  32:   20 e0           ldi     r18, 0x00       ; 0
  34:   a6 e6           ldi     r26, 0x66       ; 102
  36:   b0 e0           ldi     r27, 0x00       ; 0
  38:   01 c0           rjmp    .+2             ; 0x3c <.do_clear_bss_start>

0000003a <.do_clear_bss_loop>:
  3a:   1d 92           st      X+, r1

0000003c <.do_clear_bss_start>:
  3c:   a0 37           cpi     r26, 0x70       ; 112
  3e:   b2 07           cpc     r27, r18
  40:   e1 f7           brne    .-8             ; 0x3a <.do_clear_bss_loop>
  42:   02 d0           rcall   .+4             ; 0x48 <main>
  44:   02 c0           rjmp    .+4             ; 0x4a <_exit>

00000046 <__bad_interrupt>:
  46:   dc cf           rjmp    .-72            ; 0x0 <__vectors>

00000048 <main>:
  48:   ff cf           rjmp    .-2             ; 0x48 <main>

0000004a <_exit>:
  4a:   f8 94           cli

0000004c <__stop_program>:
  4c:   ff cf           rjmp    .-2             ; 0x4c <__stop_program>

Notice that because I included the variables "text[]" and "buffer[]" that this caused the compiler to build _do_copy_data() (to copy "hello" from flash to RAM) and _do_clear_bss() (to set all of buffer to 0x00) into the image. They are routines in ".init4" so they come after the initial setup code in .init2:

00000014 <__ctors_end>:
  14:   11 24           eor     r1, r1
  16:   1f be           out     0x3f, r1        ; 63
  18:   cf e9           ldi     r28, 0x9F       ; 159
  1a:   cd bf           out     0x3d, r28       ; 61

but before the code that actually calls main():

  42:   02 d0           rcall   .+4             ; 0x48 <main>
  44:   02 c0           rjmp    .+4             ; 0x4a <_exit>

which is in .init9. So now when you add:

char text[] = "hello";
uint8_t buffer[10];

__attribute__((section(".init3"), naked)) void some_name_or_other(void) {
	DDRB = 0xAA;
}

int main(void) {
	while(1);
}

what happens is this:

  [SNIP]
00000014 <__ctors_end>:
  14:   11 24           eor     r1, r1
  16:   1f be           out     0x3f, r1        ; 63
  18:   cf e9           ldi     r28, 0x9F       ; 159
  1a:   cd bf           out     0x3d, r28       ; 61

0000001c <some_name_or_other>:
  1c:   8a ea           ldi     r24, 0xAA       ; 170
  1e:   87 bb           out     0x17, r24       ; 23

00000020 <__do_copy_data>:
  20:   10 e0           ldi     r17, 0x00       ; 0
  22:   a0 e6           ldi     r26, 0x60       ; 96
  24:   b0 e0           ldi     r27, 0x00       ; 0
  26:   e2 e5           ldi     r30, 0x52       ; 82
  28:   f0 e0           ldi     r31, 0x00       ; 0
  [SNIP]

The new code has been squeezed in between .init2 and .init4 so it is after the initial setup but before the _do_stuff() loops. It is not called and it's function name does not matter as it just appears "inline" between those other two .init sections. The use of "naked" in the __attribute__ was to say don't put usual function prologue and epilogue on this code. In particular this means it does not have a "RET" on the end as normal function code would (because at that point in .init3 nothing has CALLd so a RET would be disastrous).

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

Thanks a lot for the in depth explanation.I don't want the MCUCR backup.So,now i will write the code like this below.

 

#include <avr/io.h>
#include <avr/wdt.h>

void wdt_init(void) __attribute__((naked)) __attribute__((section(".init3")));

void wdt_init(void)
{
	MCUSR = 0;
	wdt_disable();

}

int main(void)
{
wdt_enable(WDTO_8S);
//other init
while(1)
{
//tasks
wdt_reset();
//some other tasks
}
}

Is it ok now?

Last Edited: Tue. Feb 13, 2018 - 07:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I implemented wdt_disable() as I mentioned in the previous post!Now I can see some random resets in the circuit.When I didn't implemented the wdt_disable(),the circuit was running properly.Why this is happening?Now my main.c code is as follows...

 

/*
 * sigalert_v1.0.c
 *
 * Created: 12-12-2017 13:09:07
 * Author : SHARANYA
 */
#include <avr/io.h>
#include <util/delay.h>
#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <avr/interrupt.h>
#include <avr/wdt.h>

#include "uart.h"
#include "twi.h"
#include "mcp3424.h"
#include "simcom.h"
#include "ds18b20.h"

#define sensor_pin PINB
#define sensor_ddr DDRB
#define sensor_port PORTB

#define sensor_1 1
#define sensor_2 2
#define sensor_3 3
#define sensor_4 4
#define sensor_5 5

void wdt_init(void) __attribute__((naked)) __attribute__((section(".init3")));

void wdt_init(void)
{
	MCUSR = 0;
	wdt_disable();

}

volatile uint8_t sensor_change = 0;

void enbl_pcintr0(void)
{
	PCICR |= (1<<PCIE0);
	PCMSK0 |= (1<<PCINT1);
	sei();
}

void disbl_pcintr0(void)
{
	PCICR = 0X00;
	PCMSK0 = 0X00;
	cli();
}

ISR (PCINT0_vect)
{
	sensor_change = 1;
}

uint8_t check_sensor(uint8_t sensor_no)
{
	uint8_t state;
	if(bit_is_set(sensor_pin,sensor_no))
	{
	_delay_ms(20);
	if(bit_is_set(sensor_pin,sensor_no))
	state = 1;
	}
	else
	{
	state = 0;
	}

	return state;
}

void check_state(void)
{
	wdt_reset();
	double temp = 0;

	if(check_sensor(sensor_1))
	{
		sim900_sendsms(8,0,0,"Alert,door open!!");
		sensor_change = 0;
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();
	}

	else if(sensor_change)
	{
		sim900_sendsms(8,0,0,"Alert,door movement detected!!");
		sensor_change = 0;
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();
	}

	temp = read_temp();

	if(temp == 0)
	{
		sim900_sendsms(8,0,0,"Error!!check temp sensor & connection");
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();
	}

	else if(temp>hrt)
	{
		sim900_sendsms(8,0,0,"Alert!!High temp detected...");
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();

	}

	if(read_volt()<=low_dc)
	{
		sim900_sendsms(8,0,0,"Alert!!Low DC detected...");
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();
	}
}

int main(void)
{
    wdt_enable(WDTO_8S);
	wdt_reset();
	sensor_ddr &= ~((1<<1)|(1<<2)|(1<<3)|(1<<4));//input for door sensors
	sensor_port |= (1<<1)|(1<<2)|(1<<3)|(1<<4);//pullup enable

	DDRB |= (1<<5);//Output for MOSFET
	PORTB &= ~(1<<5);//Turn the MOSFET ON to power up the modem	

	uint8_t alert_state = 1;//by default,enable alert
	uint32_t timer_count = 0;//Init a timer counter.Send sms approx every 1hr later if alert found disable

	uart_init();
	sim900_init();	

    while (1)
    {

	wdt_reset();

	if(alert_state)
	{
		check_state();
	}

	sim900_chksms();

	if(err_res)
	{
		sim900_init();
	}

	switch (usrcmd_no)
	{
	case 1:
		sim900_sendsms(1,0,read_temp(),0);
		usrcmd_no = 0;
		break;

	case 2:

		sim900_sendsms(2,0,read_amp(),0);
		usrcmd_no = 0;
		break;

	case 3:
		sim900_sendsms(3,0,read_volt(),0);
		usrcmd_no = 0;
		break;

	case 4:
		sim900_sendsms(4,0,low_dc,0);
		usrcmd_no = 0;
		break;

	case 5:
		sim900_sendsms(5,0,hrt,0);
		usrcmd_no = 0;
		break;

	case 6:
		alert_state = 1;
		timer_count = 0;
		sim900_sendsms(6,0,0,0);
		usrcmd_no = 0;
		break;

	case 7:
		alert_state = 0;
		timer_count = 0;
		sim900_sendsms(7,0,0,0);
		usrcmd_no = 0;
		break;

	case 8:
		sim900_sendsms(8,0,0,"Mobile no updated\r\n!!");
		usrcmd_no = 0;
		break;			

	}

	enbl_pcintr0();//Enable interrupt to detect door open/close during delay
	delay_s(30);//wait for 30s before next check
	disbl_pcintr0();//disable interrupt

	if(!(alert_state))
	{
		timer_count++;
	}

	if(!(alert_state) && (timer_count>=120))
	{
		sim900_sendsms(7,0,0,0);
		timer_count = 0;//reset timer count to send sms after 1hr
	}

	}
}

Is there something to do with fuse bits?Please help....

Last Edited: Wed. Feb 14, 2018 - 07:44 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
		wdt_reset();
		_delay_ms(5000);
		wdt_reset();

How long is your watchdog period?!?!? It surely cannot be more than 5 seconds. So this code is bound to cause the AVR to reset.

 

No sensible micro program is ever going to be using _delay_ms() calls anyway but if you do use it for 5 seconds you really need to do something like:

for (int i = 0; i < 50; i++) {
    _delay_ms(100);
    wdt_reset();
}

This way the WDR is done every 100ms. As long as that is well within the chosen WDT period it should be getting reset often enough.

 

But the fact is that a micro program that is going to use a watchdog should have that requirement designed in from the start. The watchdog is not something (like documentation!) that you can leave to the very last minute then pepper a few WDRs across the already written code. If it was designed in from the start then for every action of the code (where it waits for things like UDRE or RXC or whatever for example) you would be considering whether WDRs are needed. And you would never have chosen to use _delay_ms(5000)!

 

Did you somehow think that _delay_ms() contained WDRs perhaps? It doesn't. (apart from anything else they would skew the timing or at least have to be accounted for).

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

Ah, just scanned the code some more and spotted:

    wdt_enable(WDTO_8S);

so if it is 8 seconds then you might get away with those 5 second delays. But I'd still break the _delay_ms() down with WDRs anyway

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

I have already done a wdt_reset after _delay_ms within a for loop at another header file which is used for larger delays(30s).So what is your suggestion to avoid this

clawson wrote:
No sensible micro program is ever going to be using _delay_ms() calls

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

Professional micro projects almost always run a timer (maybe more than one though a large number of activities can be "ganged" off just a single tick). Typically such time ISR run at something like 10ms ticks. If you want something to delay you just wait until some tick count has counted to 500 lots of 10ms or whatever.