Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functional Interrupts #2745

Merged
merged 2 commits into from
Aug 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions cores/esp8266/FunctionalInterrupt.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include <FunctionalInterrupt.h>


// Duplicate typedefs from core_esp8266_wiring_digital_c
typedef void (*voidFuncPtr)(void);

// Helper functions for Functional interrupt routines
extern "C" void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, voidFuncPtr userFunc, void*fp , int mode);

// Structure for communication
struct ArgStructure {
std::function<void(void)> reqFunction;
};

void interruptFunctional(void* arg)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igrr it's a bit late, but shouldn't this have the ICACHE_RAM_ATTR tag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, under the current model it should have ICACHE_RAM_ATTR.

{
((ArgStructure*)arg)->reqFunction();
}

void attachInterrupt(uint8_t pin, std::function<void(void)> intRoutine, int mode)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igrr what about here? should this be std::function<void ICACHE_RAM_ATTR (void)> ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more tricky. You can't add attributes to function prototypes like that, and furthermore the invoke method of std::function (which gets called when you use operator() on the function object) is not placed into IRAM either (it's a part of libstdc++).

I think the only way to work around this issue is to implement interrupt masking for the duration of SPI flash operation (#3579), then IRAM_ATTRs will not be needed at all.

{
// use the local interrupt routine which takes the ArgStructure as argument
__attachInterruptArg (pin, (voidFuncPtr)interruptFunctional, new ArgStructure{intRoutine}, mode);
}
15 changes: 15 additions & 0 deletions cores/esp8266/FunctionalInterrupt.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef FUNCTIONALINTERRUPT_H
#define FUNCTIONALINTERRUPT_H

#include <stddef.h>
#include <stdint.h>
#include <functional>

extern "C" {
#include "c_types.h"
#include "ets_sys.h"
}

void attachInterrupt(uint8_t pin, std::function<void(void)> intRoutine, int mode);

#endif //INTERRUPTS_H
32 changes: 25 additions & 7 deletions cores/esp8266/core_esp8266_wiring_digital.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ extern int ICACHE_RAM_ATTR __digitalRead(uint8_t pin) {
*/

typedef void (*voidFuncPtr)(void);
typedef void (*voidFuncPtrArg)(void*);

typedef struct {
uint8_t mode;
void (*fn)(void);
void * arg;
} interrupt_handler_t;


static interrupt_handler_t interrupt_handlers[16];
static uint32_t interrupt_reg = 0;

Expand All @@ -127,31 +130,44 @@ void ICACHE_RAM_ATTR interrupt_handler(void *arg) {
while(!(changedbits & (1 << i))) i++;
changedbits &= ~(1 << i);
interrupt_handler_t *handler = &interrupt_handlers[i];
if (handler->fn &&
(handler->mode == CHANGE ||
if (handler->fn &&
(handler->mode == CHANGE ||
(handler->mode & 1) == !!(levels & (1 << i)))) {
// to make ISR compatible to Arduino AVR model where interrupts are disabled
// we disable them before we call the client ISR
uint32_t savedPS = xt_rsil(15); // stop other interrupts
handler->fn();
xt_wsr_ps(savedPS);
uint32_t savedPS = xt_rsil(15); // stop other interrupts
if (handler->arg)
{
((voidFuncPtrArg)handler->fn)(handler->arg);
}
else
{
handler->fn();
}
xt_wsr_ps(savedPS);
}
}
ETS_GPIO_INTR_ENABLE();
}

extern void ICACHE_RAM_ATTR __attachInterrupt(uint8_t pin, voidFuncPtr userFunc, int mode) {
extern void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, voidFuncPtr userFunc, void *arg, int mode) {
if(pin < 16) {
interrupt_handler_t *handler = &interrupt_handlers[pin];
handler->mode = mode;
handler->fn = userFunc;
handler->arg = arg;
interrupt_reg |= (1 << pin);
GPC(pin) &= ~(0xF << GPCI);//INT mode disabled
GPIEC = (1 << pin); //Clear Interrupt for this pin
GPC(pin) |= ((mode & 0xF) << GPCI);//INT mode "mode"
}
}

extern void ICACHE_RAM_ATTR __attachInterrupt(uint8_t pin, voidFuncPtr userFunc, int mode )
{
__attachInterruptArg (pin, userFunc, 0, mode);
}

extern void ICACHE_RAM_ATTR __detachInterrupt(uint8_t pin) {
if(pin < 16) {
GPC(pin) &= ~(0xF << GPCI);//INT mode disabled
Expand All @@ -160,6 +176,7 @@ extern void ICACHE_RAM_ATTR __detachInterrupt(uint8_t pin) {
interrupt_handler_t *handler = &interrupt_handlers[pin];
handler->mode = 0;
handler->fn = 0;
handler->arg = 0;
}
}

Expand All @@ -176,7 +193,7 @@ void initPins() {
for (int i = 12; i <= 16; ++i) {
pinMode(i, INPUT);
}

ETS_GPIO_INTR_ATTACH(interrupt_handler, &interrupt_reg);
ETS_GPIO_INTR_ENABLE();
}
Expand All @@ -186,3 +203,4 @@ extern void digitalWrite(uint8_t pin, uint8_t val) __attribute__ ((weak, alias("
extern int digitalRead(uint8_t pin) __attribute__ ((weak, alias("__digitalRead")));
extern void attachInterrupt(uint8_t pin, voidFuncPtr handler, int mode) __attribute__ ((weak, alias("__attachInterrupt")));
extern void detachInterrupt(uint8_t pin) __attribute__ ((weak, alias("__detachInterrupt")));