digitalWriteFast() lesson

Projects Programming
1

This simple function using the digitalWriteFast library would give me an 'else' without a previous 'if' error. I was confused, because that should compile with a normal function.

void setDirection(int8_t d)
{
  if (d == CW)
    digitalWriteFast(MOTOR_DIRECTION_PIN, 1);
  else if (d == CCW)
    digitalWriteFast(MOTOR_DIRECTION_PIN, 0);
 else
    ; // fail
}

It turns out that digitalWriteFast() is a #define, with its own nested if/else structure. When it gets substituted, the result fails. Adding braces fixes the problem.

void setDirection(int8_t d)
{
  if (d == CW)
  {
    digitalWriteFast(MOTOR_DIRECTION_PIN, 1);
  }
  else if (d == CCW)
  {
    digitalWriteFast(MOTOR_DIRECTION_PIN, 0);
  }
  else
    ; // fail
}

This is likely the case with other libraries too.

2 Likes
2

Generally, it is a good idea always use a braces in "if-else" statements,

1 Like
3

WHICH digitalWriteFast() are you talking about?

It's possible to set up macro-based, multi-line "functions" to always parse as single "statements", so that they won't cause errors like this. Apparently the one you are using doesn't do that. :frowning: (I'd consider this a bug worth reporting...)

1 Like
4

if it has a if / else structure then you should be fine. if it's just an if then indeed it might become an issue as your else would not be attached to the right if.

which library are you using and on which platform?

5

I suggest always use braces. If you ever need to add another line you can forget to add a brace

More importantly, how is this compiled? Is the else linked to one or two?

If(one)
  If(two)
    DoTwo();
  else
    DoOne();

If(one)
  If(two)
    DoTwo();
else
  DoOne();
6

Agree. This should not happen.

The library is digitalWriteFast by Watterott. Available in Arduino's library manager.

This is a code snippet for digitalWriteFast

#define digitalWriteFast(P, V) \
if (__builtin_constant_p(P)) { \
  BIT_WRITE(*__digitalPinToPortReg(P), __digitalPinToBit(P), (V)); \
} else { \
  digitalWrite((P), (V)); \
}
7

Thanks, braces around multiple lines -- sure.

I never put braces around single lines, and most folks do not either. Many examples in Arduino's own libraries.

8

If you auto indent the code it will be revealed :slight_smile:

Or if you study carefully how selection and compound statements work then it’s obvious too.

You should not think in terms of lines but really in terms of identifying statements.

9

I've submitted Fast functions not single-statement safe... · Issue #7 · ArminJo/digitalWriteFast · GitHub

If you auto indent the code it will be revealed

Not when some of the offending code is hidden by a macro.
It's not THAT obvious. My first impression was that the if statement that the fast macro expands to IS a single statement, and it ought to work.

But you get:

 if (x)  //user code
   if (__builtin_constant_p(P)) {
      BIT_WRITE(*__digitalPinToPortReg(P), __digitalPinToBit(P), (V));
   } else {
      digitalWrite((P), (V));
   }
   ;  //user code
else ...   //user code

and that extra semicolon breaks things.

10

Of course - I meant for the code he posted

11

It's not the extra semicolon that breaks things. It breaks even without the semicolon:

 if (x)  //user code
   if (__builtin_constant_p(P)) {
      BIT_WRITE(*__digitalPinToPortReg(P), __digitalPinToBit(P), (V));
   } else {
      digitalWrite((P), (V));
   }
else ...   //user code

That's not valid code as you can't have if (...) { ... } else { ... } else { ... }.

12

That's a shame. The correct way to write this structure in C++ is

#define digitalWriteFast(P, V) \
  do { \
    if (__builtin_constant_p(P)) { \
      BIT_WRITE(*__digitalPinToPortReg(P), __digitalPinToBit(P), (V)); \
   } else { \
      digitalWrite((P), (V)); \
   } \
} while (0)

This way it behaves like an ordinary C++ statement and works fine whether or not there is a brace around an else block.

I would make a pull request but the last edit to the repo was 8 years ago and I guess there would be all kinds of latent defects uncovered if you changed it now. For example, you can currently write digitalWriteFast(...) // Note omitted semicolon and that will work. If you fixed the code, that would cease to compile.

FWIW, all of this is possibly irrelevant if the core you're using defines all these functions as static inline in the relevant headers. If that is the case, the compiler will often inline and elide any constants. Unfortunately, the core I use, the esp32 core, does not. But fortunately it has other ways to program digital outputs to switch at higher frequencies that don't involve using the CPU.

13

It would be valid - you forgot an if

Perfectly right on the do while(0) that’s it’s often done

14

Oh, I think you're right. The second else would attach to the first if in that case. The semicolon just separates it from the first if, causing an "else without if" error.

(Of course, I'm not suggesting the solution is to omit the semicolons. The solution is to update the library code as has been done already.)

15

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.