user14790: This is my first library so feel free to burn me and vote this down :D
This is a library that provides me with Watchdog deep sleep on my ATtiny85 projects.
In the test sketch I use the watchdog timer's interrupt vector but I want to place this within the library. How can I do this?
I have tried to add this to the init method but that did not work out;
powernap.h:26: error: expected unqualified-id before string constant
     ISR(WDT_vect) {
Also; feel free to shoot at the rest of the library, I am here to learn.
Test Sketch
#include "powernap.h"

#define LED 0

Napper napper = Napper();

volatile boolean f_wdt = 1;

ISR(WDT_vect) {
  f_wdt=1;
}

void setup() {
  delay(2000);
  napper.setup(8); // configure watchdog timer
}

void loop() { // blink
  pinMode(LED, OUTPUT);
  digitalWrite(LED, LOW);
  delay(200);
  digitalWrite(LED, HIGH);
  delay(200);
  digitalWrite(LED, LOW);
  delay(200);

  napper.sleep(); // deep sleep
}

powernap.cpp
#include "powernap.h"

 #ifndef cbi
   #define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
 #endif
 #ifndef sbi
   #define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
 #endif

Napper::Napper() {

}

void Napper::init(float initial) {
//  volatile boolean f_wdt = 1;

//  ISR(WDT_vect) {
//  f_wdt=1;
//  }
}

void Napper::setup(int cycle) {
    byte bb;
    int ww;
    if (cycle > 9 ) cycle=9;
    bb=cycle & 7;
    if (cycle > 7) bb|= (1<<5);
    bb|= (1<<WDCE);
    ww=bb;

    MCUSR &= ~(1<<WDRF);
    // start timed sequence
    WDTCR |= (1<<WDCE) | (1<<WDE);
    // set new watchdog timeout value
    WDTCR = bb;
    WDTCR |= _BV(WDIE);
}

void Napper::sleep() {
    cbi(ADCSRA,ADEN);                    // switch Analog to Digitalconverter OFF

    set_sleep_mode(SLEEP_MODE_PWR_DOWN); // sleep mode is set here
    sleep_enable();

    sleep_mode();                        // System sleeps here

    sleep_disable();                     // System continues execution here when watchdog timed out
    sbi(ADCSRA,ADEN);                    // switch Analog to Digitalconverter ON
}

powernap.h
#ifndef Fader_h
#define Fader_h

#if ARDUINO >= 100
  #include "Arduino.h"
#else
  #include "WProgram.h"
  #include "pins_arduino.h"
  #include "WConstants.h"
#endif

#include <avr/sleep.h>
#include <avr/wdt.h>
#include <util/delay.h>

class Napper {
    public:
        Napper();

        void init(float initial);
        void setup(int cycle);
        void sleep();

    private:
        int flag;

};

#endif


user30005: It is hard to help you improve your library or code if you don't tell others what your code pieces are trying to do.
In general, you will have a piece of code that sets up the whole thing, a piece of code that will be executed in the isr, and other helper code pieces, like getting status and clear status, as appropriate.
I often use a function pointer that the user can hook to the user handler so my isr portion of the code is fixed. Just hook to different functions in the user space and you are ready to go.
Obviously other ways exist, each with their pros and cons.

user7508: To answer your actual question, you just have to define ISR(WDT_vect)
outside any function.
Now, for the general critique, let me start by quoting Antoine de Saint
Exupéry: “It seems that perfection is attained not when there
is nothing more to add, but when there is nothing more to remove.”
There are quite a few things to remove from your code, like unused
variables, parameters or methods. I understand that you may be tempted
to leave useless stuff in there anticipating that you may need it in a
future, extended version. But please, don't do that. Try to keep clean
and lean any code that you post in public, and only add extra stuff when
you actually need it.
I would remove most of the #includes from powernap.h. If the compiler
needs one of these to understand your .h, then live it there. If it's
only needed to compile your library, then it belongs to your .cpp.
Also, there is no point in making your library dependent on the Arduino
core, as you are using no core functionality. By removing this
dependency, you can make your library more versatile, as it can be used
both for Arduino and non-Arduino projects.
The line #ifndef Fader_h looks weird. Standard practice is to use the
name of the .h file, capitalized. Don't depart from common practice
unless you have some reason to do so.
I would also add static to your methods, this way they can be used
without having to instantiate an object, like in
Napper::setup(WDTO_4S);.
You are using three different syntaxes for setting and clearing bits in
I/O registers. Your code will look more consistent if you choose one and
stick with it.
I almost forgot... you have to make sure the timed sequence is done
atomically, i.e. it's not interrupted.
Below is a version of the library that implements the suggestions above:
powernap.h
#ifndef POWERNAP_H
#define POWERNAP_H

#include <stdint.h>

class Napper {
    public:
        Napper() {}
        static void setup(uint8_t cycle);
        static void sleep();
};

#endif  // defined(POWERNAP_H)

powernap.cpp
#include <avr/interrupt.h>
#include <avr/sleep.h>
#include <util/atomic.h>
#include "powernap.h"

// This interrupt is used only as a wakeup source.
EMPTY_INTERRUPT(WDT_vect);

// Configure the watchdog as an interrupt source.
// `cycle' should be an integer from 0 to 9, or one of the constants
// WDTO_XXS from <avr/wdt.h>.
void Napper::setup(uint8_t cycle) {
    uint8_t prescaler;         // prescaler bits in WDTCR
    if (cycle > 9) cycle = 9;  // largest supported value
    prescaler = cycle & 7;     // bits WDP2:WDP0 are right aligned in WDTCR
    if (cycle > 7) prescaler |= _BV(WDP3);

    MCUSR &= ~_BV(WDRF);  // clear this bit: it overrides WDE
    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
        WDTCR |= _BV(WDCE)    // start timed sequence
              |  _BV(WDE);
        WDTCR =  prescaler    // set new watchdog timeout value
              |  _BV(WDIE);   // enable watchdog interrupt
    }
}

void Napper::sleep() {
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);
    sleep_mode();
}


14790: <p>This is my first library so feel free to burn me and vote this down :D
This is a library that provides me with Watchdog deep sleep on my ATtiny85 projects.</p>

<p>In the test sketch I use the <code>watchdog timer's interrupt vector</code> but I want to place this within the library. How can I do this?</p>

<p>I have tried to add this to the <code>init</code> method but that did not work out;<br>
<code>powernap.h:26: error: expected unqualified-id before string constant
     ISR(WDT_vect) {</code></p>

<p>Also; feel free to shoot at the rest of the library, I am here to learn.</p>

<p><strong>Test Sketch</strong></p>

<pre><code>#include "powernap.h"

#define LED 0

Napper napper = Napper();

volatile boolean f_wdt = 1;

ISR(WDT_vect) {
  f_wdt=1;
}

void setup() {
  delay(2000);
  napper.setup(8); // configure watchdog timer
}

void loop() { // blink
  pinMode(LED, OUTPUT);
  digitalWrite(LED, LOW);
  delay(200);
  digitalWrite(LED, HIGH);
  delay(200);
  digitalWrite(LED, LOW);
  delay(200);

  napper.sleep(); // deep sleep
}
</code></pre>

<p><strong>powernap.cpp</strong></p>

<pre><code>#include "powernap.h"

 #ifndef cbi
   #define cbi(sfr, bit) (_SFR_BYTE(sfr) &amp;= ~_BV(bit))
 #endif
 #ifndef sbi
   #define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
 #endif

Napper::Napper() {

}

void Napper::init(float initial) {
//  volatile boolean f_wdt = 1;

//  ISR(WDT_vect) {
//  f_wdt=1;
//  }
}

void Napper::setup(int cycle) {
    byte bb;
    int ww;
    if (cycle &gt; 9 ) cycle=9;
    bb=cycle &amp; 7;
    if (cycle &gt; 7) bb|= (1&lt;&lt;5);
    bb|= (1&lt;&lt;WDCE);
    ww=bb;

    MCUSR &amp;= ~(1&lt;&lt;WDRF);
    // start timed sequence
    WDTCR |= (1&lt;&lt;WDCE) | (1&lt;&lt;WDE);
    // set new watchdog timeout value
    WDTCR = bb;
    WDTCR |= _BV(WDIE);
}

void Napper::sleep() {
    cbi(ADCSRA,ADEN);                    // switch Analog to Digitalconverter OFF

    set_sleep_mode(SLEEP_MODE_PWR_DOWN); // sleep mode is set here
    sleep_enable();

    sleep_mode();                        // System sleeps here

    sleep_disable();                     // System continues execution here when watchdog timed out
    sbi(ADCSRA,ADEN);                    // switch Analog to Digitalconverter ON
}
</code></pre>

<p><strong>powernap.h</strong></p>

<pre><code>#ifndef Fader_h
#define Fader_h

#if ARDUINO &gt;= 100
  #include "Arduino.h"
#else
  #include "WProgram.h"
  #include "pins_arduino.h"
  #include "WConstants.h"
#endif

#include &lt;avr/sleep.h&gt;
#include &lt;avr/wdt.h&gt;
#include &lt;util/delay.h&gt;

class Napper {
    public:
        Napper();

        void init(float initial);
        void setup(int cycle);
        void sleep();

    private:
        int flag;

};

#endif
</code></pre>
 30005: <p>It is hard to help you improve your library or code if you don't tell others what your code pieces are trying to do.</p>

<p>In general, you will have a piece of code that sets up the whole thing, a piece of code that will be executed in the isr, and other helper code pieces, like getting status and clear status, as appropriate.</p>

<p>I often use a function pointer that the user can hook to the user handler so my isr portion of the code is fixed. Just hook to different functions in the user space and you are ready to go.</p>

<p>Obviously other ways exist, each with their pros and cons.</p>
 7508: <p>To answer your actual question, you just have to define <code>ISR(WDT_vect)</code>
outside any function.</p>

<p>Now, for the general critique, let me start by quoting <a href="https://en.wikipedia.org/wiki/Antoine_de_Saint-Exup%C3%A9ry" rel="nofollow noreferrer">Antoine de Saint
Exupéry</a>: “It seems that perfection is attained not when there
is nothing more to add, but when there is nothing more to remove.”
There are quite a few things to remove from your code, like unused
variables, parameters or methods. I understand that you may be tempted
to leave useless stuff in there anticipating that you may need it in a
future, extended version. But please, don't do that. Try to keep clean
and lean any code that you post in public, and only add extra stuff when
you actually need it.</p>

<p>I would remove most of the <code>#include</code>s from powernap.h. If the compiler
needs one of these to understand your .h, then live it there. If it's
only needed to compile your library, then it belongs to your .cpp.</p>

<p>Also, there is no point in making your library dependent on the Arduino
core, as you are using no core functionality. By removing this
dependency, you can make your library more versatile, as it can be used
both for Arduino and non-Arduino projects.</p>

<p>The line <code>#ifndef Fader_h</code> looks weird. Standard practice is to use the
name of the .h file, capitalized. Don't depart from common practice
unless you have some reason to do so.</p>

<p>I would also add <code>static</code> to your methods, this way they can be used
without having to instantiate an object, like in
<code>Napper::setup(WDTO_4S);</code>.</p>

<p>You are using three different syntaxes for setting and clearing bits in
I/O registers. Your code will look more consistent if you choose one and
stick with it.</p>

<p>I almost forgot... you have to make sure the timed sequence is done
atomically, i.e. it's not interrupted.</p>

<p>Below is a version of the library that implements the suggestions above:</p>

<p><strong>powernap.h</strong></p>

<pre><code>#ifndef POWERNAP_H
#define POWERNAP_H

#include &lt;stdint.h&gt;

class Napper {
    public:
        Napper() {}
        static void setup(uint8_t cycle);
        static void sleep();
};

#endif  // defined(POWERNAP_H)
</code></pre>

<p><strong>powernap.cpp</strong></p>

<pre><code>#include &lt;avr/interrupt.h&gt;
#include &lt;avr/sleep.h&gt;
#include &lt;util/atomic.h&gt;
#include "powernap.h"

// This interrupt is used only as a wakeup source.
EMPTY_INTERRUPT(WDT_vect);

// Configure the watchdog as an interrupt source.
// `cycle' should be an integer from 0 to 9, or one of the constants
// WDTO_XXS from &lt;avr/wdt.h&gt;.
void Napper::setup(uint8_t cycle) {
    uint8_t prescaler;         // prescaler bits in WDTCR
    if (cycle &gt; 9) cycle = 9;  // largest supported value
    prescaler = cycle &amp; 7;     // bits WDP2:WDP0 are right aligned in WDTCR
    if (cycle &gt; 7) prescaler |= _BV(WDP3);

    MCUSR &amp;= ~_BV(WDRF);  // clear this bit: it overrides WDE
    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
        WDTCR |= _BV(WDCE)    // start timed sequence
              |  _BV(WDE);
        WDTCR =  prescaler    // set new watchdog timeout value
              |  _BV(WDIE);   // enable watchdog interrupt
    }
}

void Napper::sleep() {
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);
    sleep_mode();
}
</code></pre>
