Go Down

Topic: Is this AVR sleep code correct? (Read 2919 times) previous topic - next topic

Marciokoko

#75
Sep 14, 2018, 12:50 am Last Edit: Sep 14, 2018, 03:27 am by Marciokoko
OK I came back today at 3pm and it was off.  One battery measures 1.2v and the other 2.3v so I'm recharging.  So that's not very good at all.  It ran from about 753pm to 753am (12 hours) plus ill say half way between 753am and 3pm, so like 10am, (3 hours), thats about 15 hours.  If 1 battery has 3200 mah, since they were in series, thats about 3200mah/15 hours ~ 213mamps.  Maybe, I mean if the nano draws about 30mA just sitting there and the vibrating motor would draw about 300mA but it only ran for about 2 seconds each time, thats 300ma * (2/60) = 10mA each run, for 15 runs = 150mA.  So 150 + 30 ~ 180mA.  And maybe the l298 loses some as heat as well so would that seem normal?

DKWatson, you said you ran an UNO for 40 hours on 8800mah pack, which is a bit more than 2x mine, actually 8800/3200 = 2.75 and I got 15 hours x 2.75 = 41 hours, if the UNO consumed as much as a nano which it doesnt, but mine had a dc motor on it so I guess it sounds ok.

So back to the design board, Ill try one more run with the same battery setup with no sleep just to confirm.  Then Ill try the same setup with the sleep code since I have a hunch that even though the dc motor pulls more current, since it only operates for 2 seconds each run, the nano will obviously represent a larger bulk of the power consumption and thus should represent some nice power savings.

Marciokoko

OK I finished the second test with the brand new 18650 pair in series.  The results were:

Started:
8.42V @ 739pm mon sep 17

Checked again:
@ 739pm tue Sep 18 still going strong

Checked again:
@ 500am Wed Sep 19 dying (only nano led on)

So it drained somewhere between 739p and 500a next day.  So again taking the simple average that puts it at 12am which would be 4 hours past the full day of 24, so 28 hours.  The batteries may have not been fully charged the first test that lasted 15 hours.  So I went from 15 to 28 hours.  Average might be 20 hours?  And if these are supposed to have around 3000 mah that means it drew around 150 on average.

Now I'll test with sleep code.


DKWatson

Don't forget that when mfg quotes mAh it usually means fully charged to fully discharged. At some point, a cut-off voltage, everything will stop working and you may still have 30-40% capacity left.
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

Marciokoko

Ok Im about to run the sleep code in order to compare with the no sleep code duration.  Just making sure this is correct; if I want a run every 60 minutes, I need 60 minutes * 60 secs = 3600 secs/8secs per sleep = 450 cycles, so the code would be:

Code: [Select]

#include <avr/wdt.h>            // library for default watchdog functions
#include <avr/interrupt.h>      // library for interrupts handling
#include <avr/sleep.h>          // library for sleep
#include <avr/power.h>          // library for power control
#include <util/delay.h>


#if (defined (__AVR_ATmega328P__) || defined (__AVR_ATmega328__))
    #define UNO
    #define LED_PORT        PORTB
    #define LED_PIN         PB5
    char BOARD[]      = {"m328p"};
#endif

int nbr_sleeps=0;

#define led 13
#define motorPin 10

inline void     flash_led()     {LED_PORT |= (1 << LED_PIN); _delay_ms(30);
                                    LED_PORT &= ~(1 << LED_PIN);}
                                   
ISR(WDT_vect){
        // not hanging, just waiting
        // reset the watchdog
        wdt_reset();
}

void configure_wdt(void){
  cli();                           // disable interrupts for changing the registers
  MCUSR = 0;                       // reset status register flags
                                   // Put timer in interrupt-only mode:                                       
  WDTCSR |= 0b00011000;            // Set WDCE (5th from left) and WDE (4th from left) to enter config mode,
                                   // using bitwise OR assignment (leaves other bits unchanged).
  WDTCSR =  0b01000000 | 0b100001; // set WDIE: interrupt enabled
                                   // clr WDE: reset disabled
                                   // and set delay interval (right side of bar) to 8 seconds
  sei();                           // re-enable interrupts
 
}

void sleep(int ncycles){
    nbr_sleeps ++;
    power_adc_disable();
    sleep_mode();
    flash_led();
    sleep_disable();
    power_all_enable();
}
void setup(){
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);
    digitalWrite(pulsePin, HIGH);
   
    //set_sleep_mode(SLEEP_MODE_IDLE);
    pinMode(led, OUTPUT);
    pinMode(pulsePin, OUTPUT);
    configure_wdt();
}

void loop(){
    sleep(1);
    if (nbr_sleeps==450) runMotor();
}


void runMotor() {
  //Turn motor on
 digitalWrite(motorPin, HIGH);
 delay(20000);
 digitalWrite(motorPin, LOW);
     nbr_sleeps=0;

}

DKWatson

Where is your wdt_reset() function?
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

DKWatson

Regardless, I use a simple iasm statement,

#define wdr()       __asm__ __volatile__ ("wdr \n")

And I believe you must execute a reset immediately before setting WDTCSR. Part of the 4 clk security fail-safe.

Why are you passing an unused parameter to sleep()? You should discard the sleep_disable() command and disable ADC  in setup if you're never going to use it. Disabling ADC should be a fundamental action in every sketch unless you specifically require it. Arduino turns everything on assuming the user wants everything. ADC is a big power drain.
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

Marciokoko

#81
Sep 20, 2018, 10:59 pm Last Edit: Sep 21, 2018, 05:22 am by Marciokoko
Here is my code, somewhere along the way it got all screwed up:

Code: [Select]

#include <avr/wdt.h>            // library for default watchdog functions
#include <avr/interrupt.h>      // library for interrupts handling
#include <avr/sleep.h>          // library for sleep
#include <avr/power.h>          // library for power control
#include <util/delay.h>

//1. NOT SURE WHAT THIS ALL DOES
#if (defined (__AVR_ATmega328P__) || defined (__AVR_ATmega328__))
    #define UNO
    #define LED_PORT        PORTB
    #define LED_PIN         PB5
    char BOARD[]      = {"m328p"};
#endif

//2. Defines variables
#define LED 13
#define motorPin 10
int nbr_sleeps=0;
//2.1 WHAT IS THIS FOR?
volatile int f_wdt=1;


//3. Flashes LED function - Sorry, i prefer this format for now
void flash_led(){
  pinMode (LED, OUTPUT);
  for (byte i = 0; i < 10; i++){
    digitalWrite (LED, HIGH);
    delay (50);
    digitalWrite (LED, LOW);
    delay (50);
    }
  pinMode (LED, INPUT);
  }
 
//inline void     flash_led()     {LED_PORT |= (1 << LED_PIN); _delay_ms(30);
//                                    LED_PORT &= ~(1 << LED_PIN);}


//4. Interrupt Service Routine - not sure what it does - gets called when wd times out after 8 secs
//Some examples say wdt_reset, others wdt_disable and others have if 0 then 1 else wdt overrun...
//I think if you disable it at first in ISR, you call wdt_reset later in code before sleeping and
//that enables it again?
//Otherwise you check for it being 0 and set it to 1 and later in loop if 1, flash led, =0 and sleep
ISR(WDT_vect){
        // reset the watchdog or pat the dog such that the arduino wont reboot, but why?
        wdt_disable();
        //nick gammon uses wdt_disable in Sketch H
}

//4.1 This is the other option, check if f_wdt==0 which means disabled?  If so, reset it like wdt_reset?
//Else if f_wdt is not 0 disabled and ISR has been called then the timer has been overrun?
//ISR(WDT_vect){
//  if(f_wdt == 0)
//  {
//    f_wdt=1;
//  }
//  else
//  {
//    Serial.println("WDT Overrun!!!");
//  }
//}

//5. Set sleep mode and configure_wdt
void setup(){
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);   
    //set_sleep_mode(SLEEP_MODE_IDLE);
    pinMode(LED, OUTPUT);
    configure_wdt();
}

//6. Configure varios wdt parameters like ADC off, cli/sei, flags etc...stuff i dont understand anyway
//7. Greatest confusion.  This order is:  setup, config wdt, loop, flash, sleep, runmotor@450
//In Sketch H the order is loop, flash, config wdt, sleep...
//Main difference is config wdt in Sketch H happens in every loop, only happens 1x on mine.
void configure_wdt(void){

  //Does this go here?
  ADCSRA = 0;

  cli();                           // disable interrupts for changing the registers
  MCUSR = 0;                       // reset status register flags
                                   // Put timer in interrupt-only mode:                                       
  WDTCSR |= 0b00011000;            // Set WDCE (5th from left) and WDE (4th from left) to enter config mode,
                                   // using bitwise OR assignment (leaves other bits unchanged).
  WDTCSR =  0b01000000 | 0b100001; // set WDIE: interrupt enabled
                                   // clr WDE: reset disabled
                                   // and set delay interval (right side of bar) to 8 seconds
  sei();                           // re-enable interrupts
 
}

void sleep(){
    nbr_sleeps ++;
    power_adc_disable(); // doesnt ADCSRA=0 take care of that?
    //where is power_all_disable?  Is it just an option and we chose just the adc?
    //wdt_reset(); should go here right, since I disabled it in ISR?
    //where is the sleep_enable();  is it in power_all_enable();?
    sleep_mode();
    // took out the flash_led() and moved it to loop because its where i understand it better...
    sleep_disable();
    power_all_enable();
}

void loop(){
  flash_led();
  sleep();
  if (nbr_sleeps==450) runMotor();
}


void runMotor() {
  //Turn motor on
  digitalWrite(motorPin, HIGH);
  delay(20000);
  digitalWrite(motorPin, LOW);
  nbr_sleeps=0;
}


I have a few questions in there that Ill resume here:

1. Not sure what the if/endif test does.  Im guess it distinguishes between a 328 and a 328P for where the LED is connected to?  Which I guess only makes sense when using your suggested inline function but for my clarity's sake, I replaced it with a normal flash led function as you'll see.

2. Here are the variables, not sure what the f_wdt=1 is for.  This may be a remnant from another tutorial?

3. This is the new led flashing function I replaced yours with.  Sorry

4. The infamous ISR.  I guess this is gets called after? the watchdog times out and reboots the mcu.  That is the reason why we need to pat the dog, right?  However we disable it here which is when the watchdog has already rebooted the mcu and so we disable it so the mcu wont get rebooted by accident.  Later on we wdt_reset(); in code such that the watchdog is available to us once again?

5. After every reboot/boot:
a. setup()
a. 1 set sleep mode
b. configure_wdt()
b.1 turn off adc
b.2 whatever cli does (oh, locks down registers for writing to them?)
b.3 mcusr
b.4 8 second register thing
b.5 whatever sei does (unlocks registers again?)
c. loop()
c.1 flash_led()
d. sleep()
d.1 turn off adc again, this might be my mistake again, should leave just one, the ADCSRA one i think.
d.2 shouldnt i have power_all_disable here?
d.3 shouldnt I wdt_reset() somewhere in here?
d.4 shouldnt I have sleep_enable() somewhere in here?
d.5 sleep_mode()
d.6 sleep_disable()
d.7 power_all_enable()

However in Sketch H he does things differently:
a. setup()
b. loop()
b.1 flashed_led()
b.2 turn off adc
b.3 no cli, why?
b.4 mcusr
b.5 8 second register
b.6 no sei because cli wasnt called i guess
b.7 wdt_reset()
b.8 sleep_mode
b.9 noInterrupts
b.10sleep_enable
b.11brown out stuff
b.12Interrupts
b.13sleep_cpu (sleep_mode)
b.14sleep_disable()
b.15no power_all_enable

Main difference, configures wdt only at setup() vs in every loop().  Why?

Marciokoko

#82
Sep 23, 2018, 04:00 am Last Edit: Sep 24, 2018, 04:38 am by Marciokoko
I'm running this code but the problem is that the l298n doesn't sleep with the mcu.  So it's draining power constantly from it. 

Code: [Select]
#include <avr/interrupt.h>      // library for interrupts handling
#include <avr/sleep.h>          // library for sleep
#include <avr/power.h>          // library for power control
#include <util/delay.h>


//2. Defines variables
#define LED 13
//#define motorPin 10
int nbr_sleeps=0;
//2.1 WHAT IS THIS FOR?
volatile int f_wdt=1;
//2.2 L298N VARIABLES
int enA = 10;
int in1 = 7;
int in2 = 6;


ISR(WDT_vect){
        // reset the watchdog or pat the dog such that the arduino wont reboot, but why?
        wdt_disable();
        //nick gammon uses wdt_disable in Sketch H
}


//5. Set sleep mode and configure_wdt
void setup(){
  //Serial.begin(9600);
  //Serial.println("setting up");
  //5.1 L298N setup
  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);   
  pinMode(LED, OUTPUT);
}


void sleep(){
  nbr_sleeps ++;
  ADCSRA = 0;
  MCUSR = 0;                       // reset status register flags
  WDTCSR |= 0b00011000;            // Set WDCE (5th from left) and WDE (4th from left) to enter config mode,
  WDTCSR =  0b01000000 | 0b100001; // set WDIE: interrupt enabled
  wdt_reset();
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);   
  noInterrupts ();           // timed sequence follows
  sleep_enable();
  // turn off brown-out enable in software
  MCUCR = bit (BODS) | bit (BODSE);
  MCUCR = bit (BODS);
  interrupts ();             // guarantees next instruction executed
  sleep_cpu (); 
  // cancel sleep as a precaution
  sleep_disable();
}

void loop(){
  sleep(); //sleep suff
  //flash_led();

  if (nbr_sleeps==450) runMotor();
}
//8 sec/cycle * 450 cycles = 3600 seconds = 1hr


void runMotor() {
  //Turn motor on
  //flash_led();
  digitalWrite(in1, HIGH);
  digitalWrite(in2, LOW);
  analogWrite(enA, 200);
  delay(2000);
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  nbr_sleeps=0;
}

DKWatson

Re: #81
1. The conditional compilation was based on the added portion (that isn't there) that set PORT and PIN for the on-board LED based on the processor. It also defines a name that can be tested elsewhere for additional conditional compilation. In this case however, it is meaningless and can simply be replaced by a reference to pin 13.

2. f_wdt appears to be a flag. When the watchdog timer runs out it will cause a system reset. Don't forget that WDT really has nothing to do with sleep, we simply use that interrupt to wakeup. After waking up, you have the amount of time left on WDT to reset it or the system resets. What will often be done is that any of a variety of tasks during the wakeup will set the flag. Before going back to sleep, the flag is tested and if its okay WDT is reset and everything ticks along. The logic is that if something happens, and infinite loop or waiting for an input that never arrives, the flag will not get set, WDT will not be reset and the system will reboot.

3. I'm not really that attached to it.

4. I suppose. I'm not accustomed to using Arduino functions so I can only infer what wdt_disable() does by it's name. I don't see any wdt_enable() if that's the correct function call, so I don't see where the WDT is re-enabled. power_all_enable() does not do it.

5. WDT only needs to be configured once. ADC only needs to be disabled/shut_down only once unless power_all_enable() is invoked which will power up the ADC. cli() and sei() disable/enable all interrupts. sleep does not need to be enable/disabled. The processor will not go to sleep unless you tell it to, which is what the sleep_mode() call does.

W.R.T. #82
If you want to put the L298n to sleep as well, high-side load a 2N2222 transistor with power to the servo and connect the base to an unused pin (through about a 1K resistor, but do the math on that first, I don't know anything about your motors).
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

Marciokoko

Ok thanks, Im working on cleaning up the code but I need to understand it thoroughly.  I want to make sure, only up to here, if Ive correctly separated the code that, since its only run once, can go in setup vs the code that must be run before/after each sleep so its in the sleep() function:

A- Ill keep your define and inline void for flash led, for neatness.
B- Ill throw up sleep_enable() and sleep_disable() as I understand that they are unnecessary and maybe just old references
C- Ill keep the ISR with wdt_disable() since I understand that the f_wdt flag is more for making sure the loop doesnt hang at some point and I dont need that, for now.
D- Since WDT only has to be configured once, im trying to separate the config-ONCE code from the sleep-REPETITIVE code like so:

Code: [Select]

void configure_wdt(){
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);   
    MCUSR &= ~(1<<WDRF); //or MCUSR = 0;
    ADCSRA = 0; //turns off ADC
    power_adc_disable() //PRR extras
    SPCR = 0; //turns off spi
    power_spi_disable();// turns off clock to spi PRR extras
    power_all_disable(); // PRR extras
    WDTCSR |= (1<<WDCE) | (1<<WDE); //or WDTCSR = bit (WDCE) | bit (WDE);
    WDTCSR = 1<<WDP0 | 1<<WDP3;// or WDTCSR = bit (WDIE) | bit (WDP3) | bit (WDP0);
    WDTCSR |= _BV(WDIE);
}


and

Code: [Select]

void sleep(){
    nbr_sleeps ++;
    wdt_reset();
//  sleep_enable();
    sleep_mode();
//EXECUTION AFTER ISR RESUMES HERE...
//  sleep_disable();
//  power_all_enable(); //so this powers up the ADC again, so I commented it out

}

DKWatson

Looks OK as far as snippets go. Presumably there will be some code later in place of //EXECUTION AFTER ISR RESUMES HERE...
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

Marciokoko

Ok!

The reason I ask is that in Nick Gammon's Sketch H: https://www.gammon.com.au/forum/?id=11497

He has the MCUSR=0, ADCSRA=0, WDTCSR bits, wdt_reset and set_sleep_mode and sleep_cpu() calls in the loop instead of a one time run setup. 

I only have the wdt_reset() and sleep_mode() {or sleep_cpu()} calls in the loop, which I guess are the ones that really need to be in the loop because I have to wdt_reset() every time as well as....well, sleep_mode() :-)

Btw, if I call wdt_disable() in the ISR, how does it get "re-enabled"?

So my final code would be:

Code: [Select]

//NEED POWER SAVINGS SPECIALLY WITH L298N
//V OF 2X18650 FULLY CHARGED DROPPED FROM 8.44-7.98 IN
//FROM 1142A TO 715P ~ 7.5HRS
//
#include <avr/wdt.h>            // library for default watchdog functions
#include <avr/interrupt.h>      // library for interrupts handling
#include <avr/sleep.h>          // library for sleep
#include <avr/power.h>          // library for power control
#include <util/delay.h>


//1. DEFINE LED PIN FOR COND COMP
#if (defined (__AVR_ATmega328P__) || defined (__AVR_ATmega328__))
    #define UNO
    #define LED_PORT        PORTB
    #define LED_PIN         PB5
    char BOARD[]      = {"m328p"};
#endif

//2. Defines variables
#define LED 13
#define fet 5
//#define motorPin 10
int nbr_sleeps=0;
//2.1 WHAT IS THIS FOR?
volatile int f_wdt=1;
//2.2 L298N VARIABLES
int enA = 10;
int in1 = 7;
int in2 = 6;
 
inline void     flash_led()     {LED_PORT |= (1 << LED_PIN); _delay_ms(30);
                                    LED_PORT &= ~(1 << LED_PIN);}


//3. Interrupt Service Routine - disable wdt as soon as comeback
//Some examples say wdt_reset, others wdt_disable and others have if 0 then 1 else wdt overrun...
//I think if you disable it at first in ISR, you call wdt_reset later in code before sleeping and
//that enables it again?
//Otherwise you check for it being 0 and set it to 1 and later in loop if 1, flash led, =0 and sleep
ISR(WDT_vect){
        // disables wdt upon wake up so it wont fire accidentally while we do stuff before sleep again
        wdt_disable();
        //does it get re-enabled by sleep_mode()?  wdt_reset() might be a better idea.
}
//4. Set WDT config parameters - ONCE
void configure_wdt(){
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);   
    MCUSR &= ~(1<<WDRF); //or MCUSR = 0;
    ADCSRA = 0; //turns off ADC
    power_adc_disable() //PRR extras
    SPCR = 0; //turns off spi
    power_spi_disable();// turns off clock to spi PRR extras
    power_all_disable(); // PRR extras
    WDTCSR |= (1<<WDCE) | (1<<WDE); //or WDTCSR = bit (WDCE) | bit (WDE);
    WDTCSR = 1<<WDP0 | 1<<WDP3;// or WDTCSR = bit (WDIE) | bit (WDP3) | bit (WDP0);
    WDTCSR |= _BV(WDIE);
}

//5. Set some other stuff up in setup
void setup(){
  //Serial.begin(9600);
  //Serial.println("setting up");
  //5.1 L298N setup
  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);   
  pinMode(fet, OUTPUT);
  digitalWrite(fet, LOW);
  pinMode(LED, OUTPUT);
}

//6. Could maybe move some parameters, like ADC off back into the sleep()-loop()
void sleep(){
   nbr_sleeps ++;
   wdt_reset();
   sleep_mode();
  //EXECUTION AFTER ISR RESUMES HERE...
  //power_all_enable(); //so this powers up the ADC again, so I commented it out

}
void loop(){
  flash_led(); //signalling about to sleep
  sleep(); //sleep suff
  if (nbr_sleeps==4) runMotor();
}
//8 sec/cycle * 450 cycles = 3600 seconds = 1hr


void runMotor() {
  //Turn motor on
  flash_led(); //signal about to run motor
  //turn on mosfet
  digitalWrite(fet, HIGH);
  delay(500);
  //Now run motor
  digitalWrite(in1, HIGH);
  digitalWrite(in2, LOW);
  analogWrite(enA, 200);
  delay(2000);
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  digitalWrite(fet, LOW);
  nbr_sleeps=0;
}


So it starts up in setup(), then goes to loop() where it flash_led(), sleep, check if nbr_sleeps=4 and if so, run the motor by setting D5 to High.

DKWatson

That whole sequence of steps that initialize WDT must be performed to enable/re-enable which is why in Nick's example it's in the loop. If you don't disable, you don't have to re-enable, just make certain you have wdr() or wdt_reset or whatever you use for reset, strategically place so that a reset occurs before the timer runs out. Remember that for safety/security reasons enabling WDT is a timed sequence of instructions so as to avoid accidental invocation. That is why there is no such thing as wdt_enable() unless you write your own macro/function.
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

Marciokoko

OK so after which point does wdt_reset() become necessary to prevent reboot?  After setting the bits or after calling sleep_mode()?

Go Up