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

digitalPinToInterrupt #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gagarcr
Copy link
Contributor

@gagarcr gagarcr commented Mar 11, 2021

According to:
https://www.arduino.cc/reference/en/language/functions/external-interrupts/attachinterrupt/

"attachInterrupt(pin, ISR, mode) (Not recommended. Additionally, this syntax only works on Arduino SAMD Boards, Uno WiFi Rev2, Due, and 101.)"

Instead, it proposes to use:
"attachInterrupt(digitalPinToInterrupt(pin), ISR, mode) (recommended)"

According to:
https://www.arduino.cc/reference/en/language/functions/external-interrupts/attachinterrupt/

"attachInterrupt(pin, ISR, mode) (Not recommended. Additionally, this syntax only works on Arduino SAMD Boards, Uno WiFi Rev2, Due, and 101.)"

Instead, it proposes to use:
"attachInterrupt(digitalPinToInterrupt(pin), ISR, mode) (recommended)"
@PaulStoffregen
Copy link
Owner

PaulStoffregen commented Mar 11, 2021

Did you test this on any boards? If so, which ones? And which pins did you use during those tests?

@gagarcr
Copy link
Contributor Author

gagarcr commented Mar 11, 2021

I have only tested it in the Arduino Nano Every

@PaulStoffregen
Copy link
Owner

PaulStoffregen commented Mar 11, 2021

Does that board define CORE_INT0_PIN, CORE_INT1_PIN, CORE_INT2_PIN, etc? My understanding is Arduino doesn't define this abstraction for any of their boards.

If you only test this change on boards which don't define those symbols, well, that means your change is essentially untested. It must be actually tested on boards which define those names.

@kescholm
Copy link

kescholm commented Oct 6, 2021

It might be helpful to know the pin can be checked to ensure it can be used as an interrupt:

    uint8_t pin_interrupt = digitalPinToInterrupt(PIN);
    if (pin_interrupt == NOT_AN_INTERRUPT) { /* fail here */  }

However, utility/interrupt_pins.h pin interrupt values are already hard-coded:

// Arduino Uno, Duemilanove, Diecimila, LilyPad, Mini, Fio, etc...
#elif defined(__AVR_ATmega328P__) || defined(__AVR_ATmega328PB__) ||defined(__AVR_ATmega168__) || defined(__AVR_ATmega8__)
  #define CORE_NUM_INTERRUPT	2
  #define CORE_INT0_PIN		2
  #define CORE_INT1_PIN		3

So the switch statement already identifies the proper pin value:

case CORE_INT0_PIN:
    interruptArgs[0] = state;
    attachInterrupt(0, isr0, CHANGE);
    break;

It is equivalent to

    attachInterrupt(digitalPinToInterrupt(CORE_INT0_PIN), isr0, CHANGE);

or

    attachInterrupt(digitalPinToInterrupt(pin), isr0, CHANGE);

So I'd either: 1) leave it 2) modify interrupt_pins.h if it misses some cases or boards
Also perhaps a fallback can be done with digitalPinToInterrupt instead of preprocessor errors:

#if !defined(CORE_NUM_INTERRUPT)
#error "Interrupts are unknown for this board, please add to this code"
#endif
#if CORE_NUM_INTERRUPT <= 0
#error "Encoder requires interrupt pins, but this board does not have any :("
#error "You could try defining ENCODER_DO_NOT_USE_INTERRUPTS as a kludge."
#endif

@ezhik1
Copy link

ezhik1 commented Feb 25, 2022

any progress on this issue? I've actually stumbled on this PR after I made local changes to attachInterrupt( digitalPinToInterrupt( but still no success on a Seeeduino Xiao. created an issue #77 for that.

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.

4 participants