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

pin macro digitalPinHasPWM returns invalid results #1831

Closed
jnsbyr opened this issue Mar 29, 2016 · 3 comments · Fixed by #1833
Closed

pin macro digitalPinHasPWM returns invalid results #1831

jnsbyr opened this issue Mar 29, 2016 · 3 comments · Fixed by #1833

Comments

@jnsbyr
Copy link

jnsbyr commented Mar 29, 2016

Basic Infos

Hardware

Hardware: ESP8266 - all boards
Core Version: 2.1.0

Description

The macro digitalPinHasPWM(p) from variants/.../pins_arduino.h has a similar problem as A0 from issue #1766.

digitalPinHasPWM returns true for pins 1 to 17 with an Adafruit Huzzah board. But it should return true for 0 to 16 (or even better 0 to 5 and 12 to 16, because you should not do PWM on your flash IOs). The reason is that the macro is intended to return a boolean (0/1) result but current macro returns NOT_A_PIN for pin 17 but that resolves to -1 and that is true but should be false and it otherwise returns the pin number and for pin 0 that is false but should be true.

With the macro digitalPinToInterrupt the case is a little bit different. It should return the number of the external interrupt for a specific pin to be used with attachInterrupt(). With AVR architecture this results in 0 for pin 2 and 1 for pin 3 and otherwise NOT_AN_INTERRUPT (-1) for all other pins being an offset for the interrupt vector address of INT0. For the ESP8266 the current solution seems to return the right values but the macro should be defined using the constant NOT_AN_INTERRUPT instead of NOT_A_PIN.

Possible Solution

  • minimum change
#define digitalPinToInterrupt(p)    (((p) < EXTERNAL_NUM_INTERRUPTS)? (p) : NOT_AN_INTERRUPT)
#define digitalPinHasPWM(p)         (((p) < NUM_DIGITAL_PINS)? 1 : 0)
  • saveguard approach
#define digitalPinHasPWM(p)         (((p) < NUM_DIGITAL_PINS && ((p) < 6 || (p) > 11))? 1 : 0)

in all variants of pins_arduino.h

This issue blocks firmata/arduino#278.

@igrr
Copy link
Member

igrr commented Mar 29, 2016

@jnsbyr
I'm sorry, i haven't noticed your comment on the old issue (#1766).
Please let me know if #1833 looks good to you.

@jnsbyr
Copy link
Author

jnsbyr commented Mar 29, 2016

@igrr
No problem - it was my fault to add to an old issue. The diffs look good. Will test it.

@jnsbyr
Copy link
Author

jnsbyr commented Apr 1, 2016

@igrr
Tested by replacing the variants folder of the 2.1.0 release with the version of commit 0f719e8 using an Adafruit Huzzah board. The result for digitalPinHasPWM is now as expected:
pins 0,1,2,3,4,5,12,13,14,15,16

Thank you very much!

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

Successfully merging a pull request may close this issue.

2 participants