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

ESP8266/ESP32: Use FunctionalInterrupt or attachInterruptArg by default #39

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mcspr
Copy link

@mcspr mcspr commented May 1, 2019

supercede #15
take 2 on #38

Smaller Encoder::attach_interrupt, encoder state arg is stored by the Core instead of us
Two possible implementations, as FunctionalInterrupt status in current Core is uncertain. attachInterruptArg directly exposes that functionality

Note, that only ESP8266 Core >= 2.5.1 places functional interrupt handler in IRAM

Some questions:
Should esp platform check go into a separate header? like interrupt_config.h
Is there a reason we must have .cpp file to build this code as stub library?

edit after latest push

@mcspr mcspr changed the title Functional interrupt ESP8266/ESP32: Use FunctionalInterrupt by default May 1, 2019
@mcspr
Copy link
Author

mcspr commented May 1, 2019

It is still not buildable though. I raised a question in espressif/arduino-esp32 gitter chat about this issue.
https://gitter.im/espressif/arduino-esp32?at=5cc95f25990feb451831fdb9

Linking .pioenvs/lolin32/firmware.elf
.pioenvs/lolin32/src/Basic.pde.cpp.o: In function `Encoder::update(Encoder_internal_state_t*)':
Basic.pde.cpp:(.iram1[Encoder::update(Encoder_internal_state_t*)]+0x3d): dangerous relocation: l32r: literal placed after use: .literal._ZN7Encoder6updateEP24Encoder_internal_state_t

Otherwise, idk of a fix other than moving Encoder::update to a global function.

@mcspr
Copy link
Author

mcspr commented May 4, 2019

Important thing about examples and ESP8266/ESP32 - we cannot use the pins 6-11 (inclusive). From wroom datasheet:

  • Pins SCK/CLK, SDO/SD0, SDI/SD1, SHD/SD2, SWP/SD3 and SCS/CMD, namely, GPIO6 to GPIO11 are connected
    to the integrated SPI flash integrated on the module and are not recommended for other uses.

Same applies for most ESP8266 boards. But, in some configurations it is possible to use pins 9&10.

@mcspr mcspr changed the title ESP8266/ESP32: Use FunctionalInterrupt by default ESP8266/ESP32: Use FunctionalInterrupt or attachInterruptArg by default May 4, 2019
@tablatronix
Copy link

Surely there is a better way to implement this PR.. still touches lot of code

@tablatronix
Copy link

Maybe this should be PR to esp8266 branch and merged so we could at least test it easier ?

@mcspr
Copy link
Author

mcspr commented Aug 16, 2019

How though? My main interest was to have compatible solution for both boards, but we are at a standstill with update() move for some reason (I have not changed it at all, just made global method instead of static class one)

  • we need IRAM_ATTR / ICACHE_RAM_ATTR somewhere. In hypothetical case, where Encoder class is declared and only then implemented, implementation can ignore the attr:
class Encoder {
...
    static void IRAM_ATTR update(...);
...
};

void Encoder::update(...) {
...
}

Sidenote there is also an arduino-devel mailing list discussion regarding attribute use:
https://groups.google.com/a/arduino.cc/forum/#!topic/developers/56VPC9t2MPc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants