Class Libraries and Strings, how do I make them play nice?

I am trying to learn class libraries with a fairly simple class and function to concatenate serial data and report the compiled parsed data through another function. My problem is that I can't seem to get it to compile without errors. The errors range from missing types to missing functions. They all seem to be related to the Strings deceleration in the class for the return result. This program is using multiple ultrasonic range finders and streams the data to a host. It is programed to have a control command set to manipulate different settings within the routines. The issues I am having are related to the parsing of the input commands.

Could you please give me some pointers as to what my misunderstanding is.

Thank you

Using the Ardunio 1.5.1.r2 IDE

Error report

In file included from SysComs.cpp:18:
SysComs.h:8: error: 'String' does not name a type
SysComs.h:13: error: 'String' does not name a type
SysComs.cpp: In constructor 'SysComs::SysComs()':
SysComs.cpp:26: error: '_result' was not declared in this scope
SysComs.cpp: In member function 'bool SysComs::encode(char)':
SysComs.cpp:35: error: '_result' was not declared in this scope
SysComs.cpp:36: error: 'millis' was not declared in this scope
SysComs.cpp:48: error: '_result' was not declared in this scope
SysComs.cpp:53: error: '_result' was not declared in this scope
SysComs.cpp: At global scope:
SysComs.cpp:60: error: 'String' does not name a type
SysComs.cpp: In member function 'unsigned int SysComs::length()':
SysComs.cpp:65: error: '_result' was not declared in this scope
SysComs.cpp: In member function 'long unsigned int SysComs::last_time()':
SysComs.cpp:69: error: 'millis' was not declared in this scope

SysComs.h

#ifndef SysComs_h
#define SysComs_h
class SysComs {
	public:
		SysComs();
		bool encode(char c); 
		String result() ;
		unsigned int length() ;
		unsigned long last_time() ;
	private:
		String _result;
		unsigned int _count;
		bool _valid;
		bool _end;
		unsigned long _last_time;
		unsigned long _in_time;
};
#endif

SysComs.cpp

/*
SysComs - Library for parsing serial input byte by byte without significantly delaying the main program
		Data structure command must be preceded by " $ " Results will strip leading $
			bool	<->char	SysComs.encode(char)	returns true at end of command <-> parse data input 
			string	<- 		SysComs.result()		returns string result value
			int	<-		SysComs.length()		returns length of string result
			float	<-		SysComs.last_time()	returns time last command received
	ie: 
			if (SysComs.encode(Serial.read())) {				// True if a complete command data structure found
				String CommandString = SysCom.result();			// Parse data to systems command routine
				int Command_Length = SysCom.length();			// Length of command ( without $ )
			} else {									// not a valid command fulfilled
				float Last_Command = SysComs.last_time();		// Time lapsed since last command received in mS
			}
*/
#include "SysComs.h"

SysComs::SysComs() {
	_count = 0;
	_valid = false;
	_last_time = 0;
	_in_time = 0;
	_end = false;
	_result = '\0';
}

bool SysComs::encode(char c) {
	_end = false;
	switch(c) {
		case '

main.h

#ifndef main_h
#define main_h

#if defined(ARDUINO) && ARDUINO >= 100
	#include <Arduino.h>
#else
	#include <WProgram.h>
	#include <pins_arduino.h>
#endif

#define POWER_RELAY 12
#define ESTOP	 0	
#define POWER_READ 1	

#endif

main.cpp

#include <String.h>
#include <Arduino.h>
#include "main.h"

#include "NewPing.cpp"
#include "SysComs.cpp"

SysComs SC;

void SetPower(boolean _state);
void SetSleep(boolean _state);
void ResetTimers() ;
void SonarScan();
void echoCheck() ;
bool ComCheck() ;
void Command(String d);
void StreamData() ;
void StreamStatus();

void setup() {
}

void loop() {
	ComCheck();					// Check for commands
}

bool ComCheck() {
	unsigned long td = millis();			// Store time for error correction
	while(Serial.available()) {			// While serial buffer full
		if (SC.encode(Serial.read())) {	// True if a complete command data structure found
			Command(SC.result());		// Parse data to systems command routien
			ResetTimers();			// Reset interval timers
			return true;			// Com check reported new data which delayed normal time cycles
		}
	}
	pto += millis() - td;				// Add read delay to time off set for echo accuracy
	return false;					// No new commands received
}

void Command(String d) {
	d.toUpperCase();					// Convert data to upper case
	if (d == "SLEEP") {
		SetSleep(true);
	}
	if (d == "WAKE") {
		if (PWR) {
			SetSleep(false);
		} else {
			Serial.println("$SRERR,*7");
		}
	}
	if (d == "POWERDOWN") {
		SetPower(false);
	}
	if (d == "POWERUP") {
		SetPower(true);
		delay (100);
	}
	if (d == "STATUS") {
		StreamStatus();
	}
	StreamStatus();
}

: // Command begin
_count = 0;
_valid = true;
_result = '\0';
_in_time = millis();
return _end;
case '\r': // Command end 
_end = true;
_valid = false;
_last_time = millis();
case '\n': // Command end 
_end = true;
_valid = false;
_last_time = millis();
}
if (_valid) {
_result += c; // Append data to result
_count++;; // increment length
}
if (_count > 25) { // Data too long
_count = 0; // Clear results
_result = '\0';
_end = false;
_valid = false;
}
return _end;
}

String SysComs::result() {
if (_end) return _result;
}

unsigned int SysComs::length() {
return _result.length();
}

unsigned long SysComs::last_time() {
return (millis() - _last_time);
}


main.h

§DISCOURSE_HOISTED_CODE_3§


main.cpp

§DISCOURSE_HOISTED_CODE_4§

Idle question (that I know the answer to). How is the SysComs class supposed to know what a String is?

What declare another include for <String.h> in SysCom.cpp?
I already have it declared in the main.cpp

I have tried adding <String.h> to SysCom.cpp & SysCom.h and I start getting duplicate declarations in the error reports.

Include tree:
main.cpp
Arduino.h
String.h
main.h
Newping.cpp
Newping.h
SysCom.cpp
SysCom.h

Delirious:
I have tried adding <String.h> to SysCom.cpp & SysCom.h and I start getting duplicate declarations in the error reports.

Each .cpp file is compiled separately. If the code in that file requires external declarations then you should ensure that the .cpp includes the relevant header file (directly or indirectly) so that it has them. You want to ensure that each external declaration is only made once. In simple cases that just means you only include each header file once. In more complex cases that isn't easy to achieve, so there is a convention that header files should use a conditional directive to ensure that the content is only included once. If you're getting duplicate declarations when you add an include file, it suggests that the file providing the declarations doesn't follow that convention.

I see in the 'include tree' list that you show two .cpp files under the main.cpp file. That implies that you have #included one .cpp file within another. I hope that's just a typo, but if not then that's wrong and should be removed. Since the .cpp files will also be compiled and linked into the final executable, all the global symbols in those included .cpp files would then be duplicated.

Each .cpp file is compiled separately. If the code in that file requires external declarations then you should ensure that the .cpp includes the relevant header file (directly or indirectly) so that it has them.

While not inaccurate, this is a little misleading. If a header file references a non-standard type, such as String, the include file for that type needs to be included in the header file, not the source file.

Sorry, but I am not very fluent in the terminology so bare with me, please. I have been coding for various languages on my own with no proper education in programing for years, and now I am looking to expand to more complex structures and routines. So if I am mistaken in my structure I would appreciate some detailed help as to what I am doing wrong. I believe I am on the right track since I managed to create a class function on my own that compiles as a valid structure, for the most part, until I started adding bi-directional parsing and included strings.

I understand about duplicate globals with regards to linking the cpp files in this manner. I made sure there are no identical variables that are not defined as strict to the specific routine.
As for the complex cases I presume you are referring to the "#ifndef/#endif" decelerations.

Then can I presume it would be safe to add this as a header to each _h library?

#ifndef String_h
#include <String.h>
#endif

#if defined(ARDUINO) && ARDUINO >= 100
	#include <Arduino.h>
#else
	#include <WProgram.h>
	#include <pins_arduino.h>
#endif

Then create a linker to all the files with:

Sonar.pde

#ifndef Sonar_h
#define Sonar_h

#ifndef String_h
#include <String.h>
#endif

#if defined(ARDUINO) && ARDUINO >= 100
	#include <Arduino.h>
#else
	#include <WProgram.h>
	#include <pins_arduino.h>
#endif

#include "main.h"
#include "NewPing.h"
#include "Syscoms.h"

#endif

But after doing this I get several pages of this for each function in every library:

C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/SysComs.cpp:60: multiple definition of `SysComs::result()'
main.cpp.o:/SysComs.cpp:60: first defined here
SysComs.cpp.o: In function `SysComs::encode(char)':
C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/SysComs.cpp:29: multiple definition of `SysComs::encode(char)'
main.cpp.o:/SysComs.cpp:29: first defined here
SysComs.cpp.o: In function `SysComs':
C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/SysComs.cpp:20: multiple definition of `SysComs::SysComs()'
main.cpp.o:/SysComs.cpp:20: first defined here
SysComs.cpp.o: In function `SysComs':
C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/SysComs.cpp:20: multiple definition of `SysComs::SysComs()'
main.cpp.o:/SysComs.cpp:20: first defined here
NewPing\NewPing.cpp.o: In function `NewPing':
E:\Prog\Arduino\arduino-1.5.1r2\libraries\NewPing/NewPing.cpp:15: multiple definition of `NewPing::NewPing(unsigned char, unsigned char, int)'
main.cpp.o:/NewPing.cpp:15: first defined here
NewPing\NewPing.cpp.o: In function `NewPing':
E:\Prog\Arduino\arduino-1.5.1r2\libraries\NewPing/NewPing.cpp:15: multiple definition of `NewPing::NewPing(unsigned char, unsigned char, int)'
main.cpp.o:/NewPing.cpp:15: first defined here
NewPing\NewPing.cpp.o: In function `NewPing::timer_stop()':

By the way I have removed all other include declerations except for the required _h for each individual _cpp ie: SysComs.cpp only includes SysComs.h

I know I could consolidate this command routine into a single application, but I need to understand how the class function works. This particular device will be attached to a host micro controller with 3 other identical sensor controllers. I will be using this class function to create the separate serial links and collect the data to be accessed by a PC. This requires the host controller to have at least 5 instances of this class function on 5 different serial connections.

ie:
SysComs Sen[Sensor_ID];
SysComs PC_Host;

If you include Arduino.h in the header file, which includes String.h, there is no reason to include String.h, too.

If you include Arduino.h in the header file, there is no reason to include it in the source file, too.

The String class is (incorrectly) defined in WString.h, and the include guard is String_class_h (also non-conventional).

So I could just include " #include <Arduino.h> " in each header? I am using IDE 1.5.1r2 so I don't need the backwards compatibility for the definitions.

So I could just include " #include <Arduino.h> " in each header?

Yes. Of course, the real answer comes when you fire up the text editor and make the change.

That obviously fixes many of the issues but there is still another problem with I had before and still can not make out.
Now all the classes seem to be broken. They are not being declared by their type correctly.

main.cpp:16: error: 'NewPing' does not name a type
main.cpp:23: error: 'SysComs' does not name a type
main.cpp: In function 'void ResetTimers()':
main.cpp:114: error: 'sonar' was not declared in this scope
main.cpp: In function 'void SonarScan()':
main.cpp:127: error: 'sonar' was not declared in this scope
main.cpp: In function 'void echoCheck()':
main.cpp:137: error: 'sonar' was not declared in this scope
main.cpp:138: error: 'US_ROUNDTRIP_CM' was not declared in this scope
main.cpp: In function 'bool ComCheck()':
main.cpp:144: error: 'SC' was not declared in this scope

Here is the call in main.cpp

NewPing sonar[SONAR_NUM] = {     		// Sensor object array.
  NewPing(2,3, MAX_DISTANCE), 			// Each sensor's trigger pin, echo pin, and max distance to ping.
  NewPing(4,5, MAX_DISTANCE),
  NewPing(6,7, MAX_DISTANCE),
  NewPing(8,9, MAX_DISTANCE),
  NewPing(10,11, MAX_DISTANCE),
};
SysComs SC;

NewPing worked prior to installing SysComs

I am calling the source within the IDE with

Sonar.pde

#ifndef Sonar_h
#define Sonar_h
#include <Arduino.h>
#include "main.h"
#include "NewPing.h"
#include "Syscoms.h"
#endif

Am I linking them incorrectly or some thing?

Ok i see what I did wrong, and starting to understand the compiling proccess a little more...
I needed the following header to be in main.cpp only

#include <Arduino.h>
#include "main.h"
#include "NewPing.h"
#include "Syscoms.h"

I now get a single error of:

e:/prog/arduino/arduino-1.5.1r2/hardware/tools/avr/bin/../lib/gcc/avr/4.3.2/../../../../avr/lib/avr5/crtm328p.o:(.init9+0x0): undefined reference to `main'

Would this be because I have the main loop routine being called from an include branched off the Sonar.pde?

if I rename main[.cpp & .h] to Sonar[.pde & .h] and fix .pde to include Sonar.h I get the duplicate definitions again but they only appear to be related to "NewPing.cpp

NewPing\NewPing.cpp.o: In function `NewPing':
E:\Prog\Arduino\arduino-1.5.1r2\libraries\NewPing/NewPing.cpp:15: multiple definition of `NewPing::NewPing(unsigned char, unsigned char, int)'
NewPing.cpp.o:C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/NewPing.cpp:15: first defined here
NewPing\NewPing.cpp.o: In function `NewPing':
E:\Prog\Arduino\arduino-1.5.1r2\libraries\NewPing/NewPing.cpp:15: multiple definition of `NewPing::NewPing(unsigned char, unsigned char, int)'
NewPing.cpp.o:C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/NewPing.cpp:15: first defined here
NewPing\NewPing.cpp.o: In function `NewPing::timer_stop()':
E:\Prog\Arduino\arduino-1.5.1r2\libraries\NewPing/NewPing.cpp:177: multiple definition of `NewPing::timer_us(unsigned int, void (*)())'
NewPing.cpp.o:C:\Users\DELIRI~1\AppData\Local\Temp\build966233769977545045.tmp/NewPing.cpp:177: first defined here
NewPing\NewPing.cpp.o: In function `NewPing':

Many thanks PeterH and PaulS. The problem has been resolved.

Obviously it was my mistake of the method for compiling the include libraries on top of the IDE duplicating the local library with the IDE repository.
The class function appears to be working correctly.

Once again thank you for the insight to guide me along the correct track.

Please feel free to close this thread.