Problem creating library

Hello guys,
I've created myself a little custom library to enable some debugging within the Arduino IDE. (It is quite similar to the public domain library under SmartDebug/smartdebug.h at master · hobbyelektroniker/SmartDebug · GitHub) if you want to check it out. So here is the problem: When I put all the code in a smartdebug.h file and include that in my main, everything works fine.

smartdebug.h :

#ifndef SMARTDEBUG_INCLUDED_
#define SMARTDEBUG_INCLUDED_
	#ifdef DEBUGMODE //define DEBUGMODE in main to enable debugging commands

		void DebugWait (String txt) {
		// clear buffer
		delay(100);
		char ch;
		while (Serial.available()) ch = Serial.read();
		ch = 0;

		Serial.println(txt);
		Serial.println(">press 'c' to continue");

		// wait for user to press 'c'
		do {
		if (Serial.available() > 0) ch = Serial.read();
		} while (ch != 'c');
		// clear buffer
		while (Serial.available()) ch = Serial.read();  
		}

		#define DEBUG_PRINTLN(txt) Serial.println(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val) Serial.print(txt); Serial.print(": "); Serial.println(val)
		#define DEBUG_WAIT(txt, condition) if (condition) DebugWait(txt)
	#else
		#define DEBUG_INIT(speed)
		#define DEBUG_PRINTLN(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val)
		#define DEBUG_WAIT(txt, condition)
	#endif
#endif

AFAIK, it is best practice to put function definitions etc. into a separate .cpp file. Consequently I split the abovementioned smartdebug.h in two:

smartdebug.h :

#ifndef SMARTDEBUG_INCLUDED_
#define SMARTDEBUG_INCLUDED_
	#ifdef DEBUGMODE //define DEBUGMODE in main to activate debugging commands

		void DebugWait (String txt);

		#define DEBUG_PRINTLN(txt) Serial.println(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val) Serial.print(txt); Serial.print(": "); Serial.println(val)
		#define DEBUG_WAIT(txt, condition) if (condition) DebugWait(txt)
	#else
		#define DEBUG_INIT(speed)
		#define DEBUG_PRINTLN(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val)
		#define DEBUG_WAIT(txt, condition)
	#endif
#endif

and a smartdebug.cpp file :

#include "smartdebug.h"

void DebugWait (String txt) {
// clear buffer
delay(100);
char ch;
while (Serial.available()) ch = Serial.read();
ch = 0;

Serial.println(txt);
Serial.println(">press 'c' to continue");

// wait for user to press 'c'
do {
if (Serial.available() > 0) ch = Serial.read();
} while (ch != 'c');
// clear buffer
while (Serial.available()) ch = Serial.read();  
}

But now I get two error messages when compiling (my board is a Teensy 3.2 if that matters):

C:...\Arduino\libraries\SmartDebug\smartdebug.cpp:3:17: error: variable or field 'DebugWait' declared void

void DebugWait (String txt) {

C:...\Arduino\libraries\SmartDebug\smartdebug.cpp:3:17: error: 'String' was not declared in this scope

Being no C++ expert, I have no idea what the hell is wrong here. I split the file up just like they do it in every C++ tutorial you can find out there. The only difference is that my function accepts a string whereas the examples I found have numeric or no inputs. Is the String input part of the problem?

Greets and thanks

Ouch, debugging by using Strings (and delay)? Are you trying to introduce bugs or remove them... ::slight_smile:

Thing to note when using separate .cpp en .h files is that each .cpp is compiled separate, without ANY notice of the other files. And an #include is just like copy-pasting the content in that location. So when the .h is included in your .cpp 'DEBUGMODE' is NEVER defined aka the prototype is not made.

Solution, just trust the linker. Just always define the function and create the prototype. If you don't use it in your code, the linker will not include it in the binary :wink: But I would suggest to move away from Strings ::slight_smile:

Well I figured that creating some custom commands for the Arduino IDE was the fastest and easiest way to do some rudimentary debugging (i. e. stopping the code at certain points and checking variables on the serial monitor). I know there are debugging tools out there, but AFAIK they all require additional hardware (and the knowledge to set it up) and are rather expensive. Why do you think I could create bugs by using strings/delays? The delay is to ensure that print outputs are really displayed and the strings are to explain what's going on so I think I do need them (Please don't flame me for not being a pro :cry: )

Now that you say it, I understand the problem with the function. Makes perfect sense of course. So I'll put my function definition back into the header. Just out of interest: Does the compiler compile files in a certain sequence (like main first or sth.) or is it random? And could one define variables that are shared between different cpp files?

Greets and thanks

able_archer_83:
Well I figured that creating some custom commands for the Arduino IDE was the fastest and easiest way to do some rudimentary debugging (i. e. stopping the code at certain points and checking variables on the serial monitor). I know there are debugging tools out there, but AFAIK they all require additional hardware (and the knowledge to set it up) and are rather expensive.

Not agains serial printing :wink:

able_archer_83:
Why do you think I could create bugs by using strings/delays?

If you would use strings that would also be fine. But you are NOT. You are using Strings (with a capital) which is NOT the same as strings :wink: See this topic a few down.

able_archer_83:
The delay is to ensure that print outputs are really displayed and the strings are to explain what's going on so I think I do need them (Please don't flame me for not being a pro :cry: )

Every print is displayed, also without delay. Okay, waiting a bit can be beneficial to read the monitor but note that they might mess with the normal program flow. So might introduce problems and or mask erros.

able_archer_83:
So I'll put my function definition back into the header.

NO! Just don't place the function prototype in the "#ifdef DEBUGMODE"! :wink:

able_archer_83:
Just out of interest: Does the compiler compile files in a certain sequence (like main first or sth.) or is it random?

IDE probably uses some sort of order but it does not matter.

able_archer_83:
And could one define variables that are shared between different cpp files?

Yes and no. For now, go with no and just pass everything to your library with functions. That's for 99% the neatest solution.

Add “#include <Arduino.h>” (or “#include <WString.h>”) if you want to use the ‘String’ type.

Add “#include <Arduino.h>” if you want to use the ‘Serial’ object.

Arduino.h gets included in your .ino file automatically, which is why it’s only a problem when your code is in a separate .cpp file.

Thank you guys.

Considering the String issue, I'm a bit confused now. I modified my code according to septillion's advice, removing the "String", so now I have (all in one file for simplicity):

#ifndef SMARTDEBUG_INCLUDED_
#define SMARTDEBUG_INCLUDED_
		
	#ifdef DEBUGMODE //define DEBUGMODE in main to activate debugging commands
	
		void DebugWait (char txt[]) {
		// clear buffer
		delay(100);
		char ch;
		while (Serial.available()) ch = Serial.read();
		ch = 0;

		Serial.println(txt);
		Serial.println(">press 'c' to continue");

		// wait for user to press 'c'
		do {
		if (Serial.available() > 0) ch = Serial.read();
		} while (ch != 'c');
		// clear buffer
		while (Serial.available()) ch = Serial.read();  
		}

		#define DEBUG_PRINTLN(txt) Serial.println(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val) Serial.print(txt); Serial.print(": "); Serial.println(val)
		#define DEBUG_WAIT(txt, condition) if (condition) DebugWait(txt)
	#else
		#define DEBUG_INIT(speed)
		#define DEBUG_PRINTLN(txt)
		#define DEBUG_PRINTLN_VALUE(txt, val)
		#define DEBUG_WAIT(txt, condition)
	#endif
#endif

That compiles, but I get two warnings:

In file included from Y:...\Teensy_Master\Teensy_Master.ino:3:0:

Teensy_Master: In function 'void setup()':
C:...\Documents\Arduino\libraries\SmartDebug/smartdebug.h:26:66: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]

#define DEBUG_WAIT(txt, condition) if (condition) DebugWait(txt)

^

Y:...\Teensy_Master\Teensy_Master.ino:99:3: note: in expansion of macro 'DEBUG_WAIT'

DEBUG_WAIT("Serial interface initialized successfully",true); //DEBUGCODE

Putting the function in a separate file threw an error at first, but it compiles (with the same warnings) if I include Arduino.h

This compilation works regardless of whether the "void DebugWait (char txt);" in the header is written above or beneath the #ifdef DEBUGMODE (which actually makes sense since the code doesn't call debugWait if DEBUGMODE is not defined).
Another question out of interest: How does the C++ compiler handle global variables? (AFAIK, it does not "jump". So say it encounters a variable that is supposed to be global but is defined in another file that has not been compiled yet. What would the compiler do in this case?

After all, can I use the "String" with confidence (considering it compiles without warnings compared to the version with char) or not?

When it comes to the delay issue, I think I don't need it for reading as I won't be reading in more than one character at a time (which should work without a delay, right?). I inserted it here because my Debug.Wait function is called immediatley after Serial.begin and without the delay it just wouldn't print the "press c to continue" statement (function execution was correct though as the program did react to pressing c). Maybe the print failure was due to the fact that the Serial.begin command was not finished yet. So you say I don't need delays for printing in general?

Greets

Since DebugWait() is not going to modify txt, you should make it const:

void DebugWait (const char txt[]) {

In addition to fixing the warning, this will also make your code more foolproof. If you have a variable that you never intend to modify and you make it const, then if you have a bug in your code that accidentally attempts to modify the variable (for example, the classic using = instead of == in a comparison) then you get a nice compiler error that points you right to the bug. If you didn't make it const, you could spend hours trying to track down that bug. So best practices for programming is to make every variable that will not be modified const.

able_archer_83:
So say it encounters a variable that is supposed to be global but is defined in another file that has not been compiled yet. What would the compiler do in this case?

As long as there has been a declaration of the variable, it will carry on. If there has not been a declaration then it will be an error.

able_archer_83:
After all, can I use the "String" with confidence (considering it compiles without warnings compared to the version with char) or not?

No. Just because some code doesn't compile without warnings doesn't mean that it will work reliably. You need to have a very good understanding of how String works to use it safely. If you don't understand it, then there is a good chance your use of it will cause memory fragmentation that will cause your program to stop working after running fine for some amount of time; a very difficult thing to troubleshoot. String also uses a lot more memory than the equivalent code using strings (null terminated char arrays). So even if you do know how to use it safely, you still shouldn't use it.

able_archer_83:
I inserted it here because my Debug.Wait function is called immediatley after Serial.begin and without the delay it just wouldn't print the "press c to continue" statement

Some specific Arduino boards do have this issue of serial output not working correctly immediately after calling Serial.begin(). I've noticed that on the ESP8266 and the Uno WiFi Rev2 (though the latter has since been fixed). On most Arduino boards, no delay is needed. I don't really understand the purpose of DebugWait(). Are you sure the user will always call it immediately after Serial.begin()? If not, you are probably better off leaving it to them to add a delay after Serial.begin() in their own sketch if necessary

able_archer_83:
So you say I don't need delays for printing in general?

Correct. You only need it when using specific Arduino boards immediately after calling Serial.begin(). Any other time, no delay is needed to print.

pert:
Since DebugWait() is not going to modify txt, you should make it const:

Fixed it, thank you.

pert:
Some specific Arduino boards do have this issue of serial output not working correctly immediately after calling Serial.begin(). I've noticed that on the ESP8266 and the Uno WiFi Rev2 (though the latter has since been fixed). On most Arduino boards, no delay is needed. I don't really understand the purpose of DebugWait(). Are you sure the user will always call it immediately after Serial.begin()? If not, you are probably better off leaving it to them to add a delay after Serial.begin() in their own sketch if necessary
Correct. You only need it when using specific Arduino boards immediately after calling Serial.begin(). Any other time, no delay is needed to print.

Yeah my controller (Teensy 3.2) indeed seems to need the delay as well. The purpose of DebugWait is to provide an option for the user to halt the program at certain points. I put in the delay since I thought the print problem maybe was a general issue, but now that I have confirmation it is related to the Serial.begin I don't need it anymore.

Well, greetings and thanks to all of you again :slight_smile:

Oh, and if you don't mind: What did these warnings mean that i fixed by making char constant?

able_archer_83:
Oh, and if you don't mind: What did these warnings mean that i fixed by making char constant?

"warning: ISO C++ forbids converting a string constant to 'char*'"

A string literal/constant (characters in double-quotes) is a pointer to characters that should never be changed. A "const char *" type. If you pass one to a function that has an argument like "char *arg" or "char arg" then inside the function you could have:

arg[3] = 'Z';

and not get any compiler errors or warnings. This is clearly not a good thing. Functions to which you are passing a string literal (or other "const char *") are supposed to have their argument declared as 'const' so that the statement

arg[3] = 'Z';

will cause a compiler error. That requirement is stated in the ISO standard but not enforced in the compiler used by Arduino.