Discussion:
Operator precedence
(too old to reply)
Test
2017-08-03 09:21:21 UTC
Permalink
Raw Message
I am making code more portable across SBCs. One type is using "digitalRead()" to
read a gpio pin status (high or low) and other is using a couple of lines of code
to do it. I'd like to convert all use digitalRead()

...
/* below reads a digital input pin 5 and return 00010000 if it is "on"/"high" or
otherwise return 000000000 zero */
kk=GPIO_READ(5);
i4=kk>>g; // i4 will get integer 1 or zero

I am trying to make it more portable. The other SBC uses code like this:
i4=digitalRead(5); //much simpler

I am defining as follows:

#ifdef TYPEASBC
#define digitalRead(X) ((GPIO_READ(X))>>X)
#endif
// ...and then use below for all types
i4=digitalRead(5);

I am not sure if it's ok or do I run into some operator precence problem. I am
using gcc and the SBC's are Arduino, Raspberry and others.

---
This email has been checked for viruses by AVG.
http://www.avg.com
Ben Bacarisse
2017-08-03 10:36:21 UTC
Permalink
Raw Message
Post by Test
I am making code more portable across SBCs. One type is using "digitalRead()" to
read a gpio pin status (high or low) and other is using a couple of lines of code
to do it. I'd like to convert all use digitalRead()
...
/* below reads a digital input pin 5 and return 00010000 if it is "on"/"high" or
otherwise return 000000000 zero */
kk=GPIO_READ(5);
i4=kk>>g; // i4 will get integer 1 or zero
(g == 5, yes?)

If you know that the result is always something non-zero if pin 5 is on
and zero otherwise you can write this lots of ways:

i4 = GPIO_READ(5) != 0;
i4 = !!GPIO_READ(5) != 0;
i4 = (_Bool)GPIO_READ(5); // C99
i4 = (bool)GPIO_READ(5); // C99 with <stdbool.h>

You might also consider whether you need a 0/1 result at all since C
treats any non-zero value as true. It depends on how you use i4.
Post by Test
i4=digitalRead(5); //much simpler
#ifdef TYPEASBC
#define digitalRead(X) ((GPIO_READ(X))>>X)
#endif
// ...and then use below for all types
i4=digitalRead(5);
I am not sure if it's ok or do I run into some operator precence problem. I am
using gcc and the SBC's are Arduino, Raspberry and others.
You should always put ()s round macro names in the macro body:

#define digitalRead(X) (GPIO_READ((X)) >> (X))

Note the I did not bother with any round GPIO_READ because it should
itself expand into something with () round it just as you have correctly
done with digitalRead.

Not everyone bothers with the ()S round the first X but others would say
it's simpler just to have a rigid rule.

I'd probably write

#define digitalRead(X) (GPIO_READ(X) != 0)
--
Ben.
James R. Kuyper
2017-08-03 16:23:52 UTC
Permalink
Raw Message
...
Post by Ben Bacarisse
Post by Test
#ifdef TYPEASBC
#define digitalRead(X) ((GPIO_READ(X))>>X)
#endif
// ...and then use below for all types
i4=digitalRead(5);
I am not sure if it's ok or do I run into some operator precence problem. I am
using gcc and the SBC's are Arduino, Raspberry and others.
I think you mean "parameter names"?
Post by Ben Bacarisse
#define digitalRead(X) (GPIO_READ((X)) >> (X)
Ben Bacarisse
2017-08-03 17:28:30 UTC
Permalink
Raw Message
Post by James R. Kuyper
...
Post by Test
#ifdef TYPEASBC
#define digitalRead(X) ((GPIO_READ(X))>>X)
#endif
// ...and then use below for all types
i4=digitalRead(5);
I am not sure if it's ok or do I run into some operator precence problem. I am
using gcc and the SBC's are Arduino, Raspberry and others.
I think you mean "parameter names"?
Yes, thanks.
--
Ben.
David Brown
2017-08-03 10:37:19 UTC
Permalink
Raw Message
Post by Test
I am making code more portable across SBCs. One type is using "digitalRead()" to
read a gpio pin status (high or low) and other is using a couple of lines of code
to do it. I'd like to convert all use digitalRead()
...
/* below reads a digital input pin 5 and return 00010000 if it is "on"/"high" or
otherwise return 000000000 zero */
kk=GPIO_READ(5);
i4=kk>>g; // i4 will get integer 1 or zero
i4=digitalRead(5); //much simpler
#ifdef TYPEASBC
#define digitalRead(X) ((GPIO_READ(X))>>X)
#endif
// ...and then use below for all types
i4=digitalRead(5);
I am not sure if it's ok or do I run into some operator precence problem. I am
using gcc and the SBC's are Arduino, Raspberry and others.
Why not use:

bool i4 = digitalRead(X);

Then i4 is set to "false" if digitalRead(X) returns 0, or "true" otherwise.

If you don't want to use "bool", you can use !! to turn any non-zero
value into 1.
s***@casperkitty.com
2017-08-03 14:11:09 UTC
Permalink
Raw Message
Post by Test
/* below reads a digital input pin 5 and return 00010000 if it is "on"/"high" or
otherwise return 000000000 zero */
kk=GPIO_READ(5);
i4=kk>>g; // i4 will get integer 1 or zero
I would suggest using two "!" operators consecutively: kk=!!(GPIO_READ(5)),
or better yet, wrap that in a macro like

#define PUMP_READY() (!!(GPIO_READ(5))

If a compiler doesn't know that GPIO_READ(5) will always return 32 or 0,
then a statement like "if ((GPIO_READ(5)) >> 5) ..." would require the
compiler to perform a useless shift operation before branching. Most
compilers, however, should be able to recognize that a statement of the
form "if (!(condition)) X; else Y;" is equivalent to "if (condition) Y;
else X;", and thus "if (!!(condition)) X; else Y;" would be equivalent
to "if (condition) X; else Y;" without any extra steps. Since the most
common use of I/O reads would be as branch conditions, that's the most
important case to optimize.

With regard to operator precedence, either your form or mine should work
fine if the definition of GPIO_READ(x) is even remotely reasonable. Just
make sure that you have a set of parentheses directly around the invocation
of GPIO_READ and you should be set. Putting a set of parentheses around
the argument to GPIO_READ may or may not be a good idea. If your code
never passes anything to GPIO_READ other than macros that describe pins
and you never use those macros for any other purpose, then something like

#define PUMP_READY_PIN 23
#define GPIO_SET(x) (!!(GPIO_READ(x))
if (GPIO_READ(PUMP_READY_PIN)) ...

could be readily adapted to a GPIO-reading function that requires two
arguments, merely by changing the definition of PUMP_READY_PIN to e.g.

#define PUMP_READY_PIN PORTA,23

while everything else stayed the same. If GPIO_SET(x) had wrapped x in
parentheses, such a definition would fail if "x" could be a macro like
the second PUMP_READY_PIN definition.
Loading...