Is there any problem about calling if (Serial) many times?

Question

I am trying to debug a problem that I think may be related to the debug code itself and I tried to use theese macros:

#define DEBUG_PRINT(x) { if (Serial) { Serial.print(x);} }
#define DEBUG_PRINTLN(x) { if (Serial) { Serial.println(x);} }

That works a bit but everything starts running really slow and my sketch get to hung up quite fast. Maybe it is timing related but does not seems like that.

if I change that by this the problem is gone:

#define DEBUG_PRINT(x) { if (true) { Serial.print(x);} }
#define DEBUG_PRINTLN(x) { if (true) { Serial.println(x);} }

I'm using Arduino Pro Micro (Leonardo).

Is there any problem about calling if (Serial) so many times?

Full sketch here

Answer 1

There is no problem, no, but yes it will slow you down.

This is the source code to the boolean operator for the Serial constructor for CDC:

Serial_::operator bool() {
    bool result = false;
    if (_usbLineInfo.lineState > 0)
        result = true;
    delay(10);
    return result;
}

As you can see from that all it does is check the value of a variable. However it also imposes a 10ms delay¹. Personally I'd have written it as:

Serial_::operator bool() {
    return (_usbLineInfo.lineState > 0);
}

And on top of that I'd have made it inline.

You might want to experiment by editing CDC.cpp to replace the bool operator with that smaller, leaner, code and see if it improves things (or breaks them completely)².

Edit: I have submitted a pull request to the Arduino GIT repository with a better method of monitoring the line state and imposing the delay(10) when it is actually needed rather than at every call to the bool operator regardless of need.

• https://github.com/arduino/Arduino/pull/3624

---

¹ This is done to allow the port to complete opening at the PC end before data is allowed to be sent. A better system would only perform the delay() at the moment of port opening rather than every call to the operator.

² Note that this would remove the delay which may be a bad thing as you may lose the first few bytes of the first debug message. A better system is in the pull request mentioned in the edit to this answer.

Answer 2

The real flaw is testing if Serial is available all the time. For one thing, that wouldn't work on a Uno etc. How about something like this?

// make true to debug, false to not
#define DEBUG true

// conditional debugging
#if DEBUG 

  #define beginDebug()  do { Serial.begin (115200); while (!Serial) { } } while (0)
  #define Trace(x)      Serial.print   (x)
  #define Trace2(x,y)   Serial.print   (x,y)
  #define Traceln(x)    Serial.println (x)
  #define Traceln2(x,y) Serial.println (x,y)
  #define TraceFunc()   do { Serial.print (F("In function: ")); Serial.println (__PRETTY_FUNCTION__); } while (0)

#else
  #define beginDebug()  ((void) 0)
  #define Trace(x)      ((void) 0)
  #define Trace2(x,y)   ((void) 0)
  #define Traceln(x)    ((void) 0)
  #define Traceln2(x,y) ((void) 0)
  #define TraceFunc()   ((void) 0)
#endif // DEBUG


long counter;
unsigned long start;

void setup() {
  start = micros ();

  beginDebug ();
  Traceln (F("Commenced debugging!"));
  TraceFunc ();  // show current function name

}  // end of setup

void foo ()
  {
  TraceFunc (); // show current function name
  }

void loop() 
{

  counter++;
  if (counter == 100000)
  {
    Traceln (F("100000 reached."));
    Trace (F("took "));
    Traceln (micros () - start);
    counter = 0;
    foo ();
  }  // end of if

}  // end of loop

That only does the Serial.begin() once, and the test for waiting for Serial to become available once.

Then, depending on the DEBUG define, the debugging code is executed or not. The fancy ((void) 0) calls get optimized away completely by the compiler if DEBUG is false, thus making them have no effect if debugging is off (which wouldn't apply to your code).

---

(Edited to add)

As you can deduce from my macros I wanted to debug IF the serial port is connected. I don't want to wait for it to be connected.

Well it wasn't immediately obvious from your code. In that case I suggest you test in the main loop function every second or so, to see if Serial is true. If so, set a flag. Test that flag in your debug function. That way you aren't slowing down every single Serial.print by 10 ms.

Like this amended code:

// make true to debug, false to not
#define DEBUG true

// conditional debugging
#if DEBUG 

  #define beginDebug()  do { Serial.begin (115200); } while (0)
  #define Trace(x)      do { if (serialConnected) Serial.print   (x); }     while (0)
  #define Trace2(x,y)   do { if (serialConnected) Serial.print   (x, y); }  while (0)
  #define Traceln(x)    do { if (serialConnected) Serial.println (x); }     while (0)
  #define Traceln2(x,y) do { if (serialConnected) Serial.println (x, y); }  while (0)
  #define TraceFunc()   do { if (serialConnected) { Serial.print (F("In function: ")); Serial.println (__PRETTY_FUNCTION__);} } while (0)

#else
  #define beginDebug()  ((void) 0)
  #define Trace(x)      ((void) 0)
  #define Trace2(x,y)   ((void) 0)
  #define Traceln(x)    ((void) 0)
  #define Traceln2(x,y) ((void) 0)
  #define TraceFunc()   ((void) 0)
#endif // DEBUG

const byte LED = 13;

bool serialConnected;
unsigned long timeSerialLastTested;

long counter;
unsigned long start;

void setup() {
  start = micros ();

  beginDebug ();
  Traceln (F("Commenced debugging!"));
  TraceFunc ();  // show current function name

  pinMode (LED, OUTPUT);
}  // end of setup

void foo ()
  {
  TraceFunc (); // show current function name
  }

void loop() 
{

  // every second, see if the Serial port is available
  if (millis () - timeSerialLastTested >= 1000)
    {
    timeSerialLastTested = millis (); 
    serialConnected = Serial;  
    digitalWrite (LED, !digitalRead (LED));  // blink LED
    }

  counter++;
  if (counter == 100000)
  {
    Traceln (F("100000 reached."));
    Trace (F("took "));
    Traceln (micros () - start);
    counter = 0;
    foo ();
  }  // end of if

}  // end of loop