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 delay1. 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)2.

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

---

1 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.

2 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.

• Nick Gammon: However, if the check is false it imposes a 10ms delay. - it always imposes a 10 ms delay.
• Majenko: @NickGammon That's true. I misread "result = " as "return". Easily done.
• Nick Gammon: Maybe someone else can come up with a reason for having that delay in there because I sure as hell can't. - I suspect that they expect it to be called once (as per the usual examples of doing it after Serial.begin()) and they found by testing that if it only just became ready you might miss a character or two, so they threw in a delay as well. Or maybe they are trying to not lock up the CPU in a tight loop, in which case an else might have been handy. It's hard to know what they were thinking.
• Majenko: delay() is a tight loop, so that kind of negates your second point. As for your first I for one am always in the habit (when used in setup) of adding my own delay. Note that other systems (like chipKIT) don't impose that 10ms delay on you since it really is pointless.
• Majenko: @EdgarBonet I have submitted a pull request with a better system for monitoring the line state to the Arduino repository: github.com/arduino/Arduino/pull/3624

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

• aalku: 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.
• Nick Gammon: You might have mentioned that. Intentions aren't always obvious from code. See amended reply above.
• aalku: That helps on the delay but I think it does not work. Maybe Windows 10 reports the port to be open all the time or something. I'll have to put on a led or something to tell me when Serial reports true.
• Nick Gammon: Honestly, I think trying to see if you are connected is not a particularly good idea. If you want to debug some of the time, have a switch you can press if you want to turn it on.