search

GreyGnome / EnableInterrupt

New Arduino interrupt library, designed for Arduino Uno/Mega 2560/Leonardo/Due
332 stars 73 forks source link

#define EI_NOTINT0 does not produce compilable code #38

Closed SteveBenz closed 7 years ago

SteveBenz commented 7 years ago

Please take this with a grain of salt, as I have like 12 total hours of Arduino time under my belt, but I believe that there are a number of spots (e.g. the first line of ei_external328.h) where you have:

if ! defined(EI_NOTINT0) && !defined (EI_NOTINT1)

Where what you meant is

if ! ( defined(EI_NOTINT0) && defined (EI_NOTINT1) )

(I'm working on something that uses the V-USB library with another library (the PS2 keyboard library) that also wants an interrupt. The comments in the v-usb library struck terror in my heart, so I was loathe to change that, and the PS2 keyboard library is in pretty rough shape, so I modified that to use your library, because I couldn't fathom how I could modify the system attachInterrupt, which is apparently incompatible with v-usb.)

michalelektryk commented 7 years ago

No the first one is the right one. Byuy it would be better to write !(a|b)

GreyGnome commented 7 years ago

I actually prefer the first form. To me, it reads, "If EI_NOTINT0 is not defined, and EI_NOTINT1 is not defined, perform the following..." The other way reads, "If EI_NOTINT0 or EI_NOTINT1 are defined, you do not want to perform the following..." I think it's a matter of taste more than anything. I find the logic of NOT(A or B) to be less readable in this case.

michalelektryk commented 7 years ago

Yes, after a moment of thinking I have to say that you are right

On 11 Sep 2017 2:58 pm, Mike Schwager notifications@github.com wrote:

I actually prefer the first form. To me, it reads, "If EI_NOTINT0 is not defined, and EI_NOTINT1 is not defined, perform the following..." The other way reads, "If EI_NOTINT0 or EI_NOTINT1 are defined, you do not want to perform the following..." I think it's a matter of taste more than anything. I find the logic of NOT(A or B) to be less readable in this case.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/GreyGnome/EnableInterrupt/issues/38#issuecomment-328521034, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AF_o8eOYR3FJP_RHrvs_GCjfzzJbIe_zks5shS5sgaJpZM4LYrVz.

GreyGnome commented 7 years ago

As we concur with one another, I will close this issue. https://youtu.be/i5j1wWY-qus?t=70