Please need help from mentors.

Hello :slight_smile:

[EDIT]Sorry for the bad formatting. i use TextMate with tabs spacing=4 if you want to copy/paste the code for good visibility. Oh and for those interested, i have remodeled some makefiles and the TextMate integration is flawless on my setup now :slight_smile: PM me if you need it, or if you think i should consider put my work here too[/EDIT]

i received my Arduino a couple of days ago. i have started reading about electronics about a week ago. i understand everything, and i'm able to do powerful stuff already. So i was thinking about starting a small library for my Arduino, and i wanted to have your general advices. For instance... I started with a LED and a Debug class, here comes the dilemma.

Consider this class, located in "LED.h":

#ifndef PQR_LED_h
#define PQR_LED_h

#include "Debug"
#include <WProgram.h>

namespace PQR {

class LED {
public:
	// Constructors / Destructor.
	/**/					LED				(byte pinNumber, bool initialState=false)
	: _pin(pinNumber), _state(initialState) {
		_leds				= (LED**)(_leds ? realloc(_leds, sizeof(LED*)*_count) : malloc(sizeof(LED*)));
		_leds[_count]		= this;
		_count				++;
	}
	
	// Global setup method, needs to be called once at the very begining of the program.
	static void				setup			() {
		byte		pin;
		for (byte i=0; i<_count; i++) {
			pin				= _leds[i]->_pin;
			pinMode			(pin, OUTPUT);
			digitalWrite	(pin, _leds[i]->_state ? HIGH : LOW);
		}
	}
	
	// Accessors.
	inline bool				state			() const	{ return _state; }
	inline void				setState		(bool v)	{ if (_state==v) return; _state = v; digitalWrite(_pin, _state ? HIGH : LOW); }
	inline bool				invert			()			{ setState(!_state); }
	
	// Factory management.
	inline static byte		count			()			{ return _count; }
	inline static LED&		at				(byte i)	{ return *_leds[i]; }
	
	// Debug a LED to debugging.
	#ifdef DEBUG
	inline friend Debug&	operator<<		(Debug &d, LED const &l) {
		return				d << "L@" << (int)l._pin << "=" << l._state;
	}
	#endif

private:
	// Static members.
	static LED**			_leds;
	static byte				_count;
	// Members.
	byte					_pin;
	bool					_state;
};

LED**	LED::_leds		= 0;
byte	LED::_count		= 0;

}

#endif

It's very easy to use, an Arduino "sketch" could look like this:

#include <PQR/LED>
using namespace PQR;
// Declare as many LEDs as you want.
LED	l2	(12, HIGH);
LED	l1	(13, LOW);
void setup () {
	LED::setup();  // The LED static "setup" method takes care of configuring all declared LEDs ports as OUTPUTS.
}
void loop () {
	// It's very easy to iterate thru LEDs:
	for (byte i=0; i<LED::count(); ++i)
		LED::at(i).invert();
		// [edit] Was originally LED::at(i)->invert(); thanks Nick Gammon for pointing out the typo.[/edit]
}

So as you may notice, the invert and the setState methods of the LED class are calling the digitalWrite function, which i believe is not very nice for later use of the PIC. So my dilemma would be to make the call do absolutely nothing in the hardware, and instead do something like that:

void loop () {
	// It's very easy to iterate thru LEDs:
	for (byte i=0; i<LED::count(); ++i)
		LED::at(i)->invert(); // Which would not do anything on hardware output yet.
	LED::commit(); // Which would make a smart binary use of PORTA / PORTB / PORTC to make sure all I/O are simultaneous.
}

What do you think? Is there such a library already around? My goal is to have a very small memory footprint, as well as a very tiny compiled code. For instance, my debugging class looks like that (You may reuse it, but please give credits if you do):

#ifndef PQR_Debug_h
#define PQR_Debug_h

#ifdef DEBUG_FUNCTION
#	define Dbg PQR::Debug() << "Dbg (" << __PRETTY_FUNCTION__ << "): "
#	define Wrn PQR::Debug() << "Wrn (" << __PRETTY_FUNCTION__ << "): "
#	define Crt PQR::Debug() << "Crt (" << __PRETTY_FUNCTION__ << "): "
#else
#	define Dbg PQR::Debug() << "Dbg: "
#	define Wrn PQR::Debug() << "Wrn: "
#	define Crt PQR::Debug() << "Crt: "
#endif

namespace PQR {

class Debug {
public:
	/**/			Debug			() {}
	/**/			~Debug			() {
		#ifdef DEBUG
		Serial.println('.');
		#endif
	}
	// Global setup method, needs to be called once at the very begining of the program.
	static void		setup			(unsigned int rate=9600) {
		#ifdef DEBUG
		Serial.begin		(9600);
		#endif
	}
	
	// Main debugging method.
	#ifdef DEBUG
	template<class V>
	Debug&			operator<<		(V const &v) 	{ Serial.print(v); };
	Debug&			operator<<		(bool const b)	{ Serial.print(b ? 'Y' : 'N' ); };
	#else
	template<class V>
	Debug&			operator<<		(V const &v)	{ return *this; };
	#endif
};

}

#endif

Then in your sketch, debugging via serial basically becomes a breeze:

#define DEBUG
#define DEBUG_FUNCTION
#include <PQR/Debug>

void setup () {
	#ifdef DEBUG
	Debug::setup	();
	#endif
}

void loop () {
	Dbg				<< "This is informative: " << l1;
	Wrn				<< "This requires your attention: " << l1;
	Crt				<< "This is critical: " << l1;
	l1.invert		();
	delay			(500);
}

The output looks like:

Dbg (void loop()): This is informative: L@13=Y.
Wrn (void loop()): This requires your attention: L@13=Y.
Crt (void loop()): This is critical: L@13=Y.
Dbg (void loop()): This is informative: L@13=N.
Wrn (void loop()): This requires your attention: L@13=N.
Crt (void loop()): This is critical: L@13=N.

As you might have understood, you can chain whatever streams you want (E.g. Dbg << "Toto" << 3 << var << '@' << "Blah":wink: and this would work.
Also, if you disable the "#define DEBUG" line, debug lines don't get COMPILED (Really, that's my goal), so you instantly get your PIC memory when you need it.
The dilemma about it: Since i'm very new to Arduino and PICs in general, i have no idea what people usually use for serial debugging. Is it ALWAYS the Serial.print/ln stuff? Or is the Serial1 Serial2 Serial3 etc used too? And is there anything other than that around for debugging which i should consider to integrate into my debugging class for later use?

Any idea?

Thanks,
Pierre.

That's an interesting debug library you have there, and being able to utilize the streaming operators is nice, but I prefer a KIS version myself, that's always worked just fine for me:

#define _DEBUG
//#define _PID_DEBUG

#ifdef _DEBUG
#define DBG(m)  Serial.println(m)
#else
#define DBG(m)
#endif

As for writing IO manipulation code for both AVR and PIC micros, that'll be a bit more complicated. The underlying methods of manipulating the IO points aren't the only problem. The PIN mappings are going to be completely different as well. Arduino has a layer of abstraction itself to hide the different Port and pin mappings from the user as it is.

If you look at this Pinout for the Atmega168 DIP (which is pin for pin the same as the Atmega328), you'll see that things aren't quite so straightforward.

Dear Jraskell,

Thanks for answering my post! That's very interesting information about the IO pins which are layout differently on various ATMegas. So here comes two questions to my mind: Let's say the user is using a vanilla version of the Arduino IDE (That's kind of light for an IDE :smiley: But at least it's nice for beginners in electronics), in what prep roc vars would be the interesting information? i see in wiring.c a few hints: AVR_ATmega128, but are there the "standard" ones? Is there an official tech spec for the Arduino source code, which tells what's subject to change, what's deprecated, etc? If such document exists, that would be a matter of being smart enough while hooking to PORTB / PORTC etc registers, is that correct?

If you look at this Pinout for the Atmega168 DIP (which is pin for pin the same as the Atmega328), you'll see that things aren't quite so straightforward. http://arduino.cc/en/Hacking/PinMapping168"

i understand the wiring. That would make a whole bunch of checks when instantiating a LED to know on what PORT it is, and the risk is that a few things would not be doable at compile-time... Too bad, i'll have to keep using plain low-level PORTX binary manipulation then :smiley:

About debugging, i don't really know what a KIS version is, but if that's what you're showing me, well. i started using such thing around 10 years ago in my programs, and i really quickly got disgusted by all those consecutive lines of DBG("The variable is"); DBG(var); DBG("\r\n"); and i quickly realized that i needed something more effective. Hence my first project on the Arduino, which is a Debugging class :smiley: i plan to extend it a little bit, to allow any kind of Serial to be used but again, since that first attempt in micro-controlers is very a new hobby, i have really no idea about pitfalls yet... i guess i'll discover them soon.

Again, thank you very much for your help! BTW, how sunny is it now in New Hampshire?
Pierre.

There are 10 kinds of people, those who know binary, and those who don't.

Good one. As of now, exists only 1111 1111 000 0000 0000 0000 0000 0000 odd numbers... Well. My joke isn't as funny, you have to know the floating point mantissa to understand it :smiley:

KIS = Keep it simple

I see that you are doing, but in the LED class you have introduced both a malloc and then a realloc on assigning a new pin. I try to avoid complicated memory manipulation on these microcontrollers - not enough memory, too much possibility of fragmentation.
The LED class depends on the debug class, and as soon as debug is turned on, stores a bit more string in memory, its not much but it all adds up. Nothing stops (not even the comments) LEDs being created in other procedures or classes, which will work but has even more possibility of fragmenting memory. The LED class says destructor - there is not one.

Debug does a serial.begin. If you are already using the serial port for another purpose, debug will do a second serial.begin. Even worse, it will change the speed of the port to 9600. Suppose you are not using 9600?

I don't want to come over as picky, but you have to watch for side effects in any code, so I would try to keep debug code as simple as possible even it ends up as a string of print(i);

Hey ShellyCat and thanks for your time!

in the LED class you have introduced both a malloc and then a realloc on assigning a new pin. I try to avoid complicated memory manipulation on these microcontrollers - not enough memory, too much possibility of fragmentation.

Can you please develop on that? 1) i don't think realloc'ing a static array of void* whose maximum size would never exceed the output count is complicated, or maybe i'm not seeing right? About the fragmentation issue you're mentioning, i'll think about it.

The LED class depends on the debug class, and as soon as debug is turned on, stores a bit more string in memory, its not much but it all adds up.

Yes and no. The debugging class is really to be disabled when not required (Eg. Debug build / Release build). So yes, it adds a little overhead to the program, but isn't it really useful to have everything needed for properly debugging a program? Or do you guys use another kind of facility i should be aware of? Also, you may have not noticed that the operator<< for the LED class is forced inline, which means that if you don't use the method at least once, it's code doesn't get compiled at all (Try it by yourself, take my classes, compile a program with a loop streaming a led to the debug, and compile the same program without it. You'll understand what i mean).

Nothing stops (not even the comments) LEDs being created in other procedures or classes, which will work but has even more possibility of fragmenting memory.

Very interesting point. To avoid such thing, i first thought about making my LED::__ctor as protected and explicit, so if some evil programer doesn't read the doc, he would get thrown out by a compilation error. Instanciation would be done via a static factory method (Eg. LED::declare(13, LOW)) and would also be able to check if already instantiated on this pin to avoid re-instanciating an object pointing to the same hardware area. Right now i'm beginning to work directly with DDRx and PORTx / PINx, so there is no "LED" class anymore, and a much more generic IOPin class.

The LED class says destructor - there is not one.

Yes good point, i forgot to remove the comment :smiley: There initially was a destructor handling another realloc etc, but i thought that the LED class would be for beginners who would require any given LED object to live on the entire program's lifetime. i'll still thinking about the best (And most generic solution).

Debug does a serial.begin. If you are already using the serial port for another purpose, debug will do a second serial.begin.

Well not exactly. Debug::setup() which is static does a Serial.begin(), but as i explained, Debug::setup shall only be used once at the beginning of the program (Hence the name of the method). Also, it takes a parameter defaulted to 9600 but you can override this default.

Even worse, it will change the speed of the port to 9600. Suppose you are not using 9600?

That's exactly why i posted here. i really need to understand the typical uses of the serial port you guys do. In my case, i "start" a debugging session and it stays alive for the whole duration of my program. Now you've said that, i would really like you to develop on this point. For instance, are there people using the hardware serial with the Serial object and with another object at the same time?

Thanks a lot for all your comments, that's very interesting!

If you're doing a debug class, make sure you put the debug strings into progmem.

There are 10 kinds of people, those who know binary, and those who don't.

There are two kinds of people in the world. Those that divide people into two kinds, and those that don't.

// Declare as many LEDs as you want.

LED l2 (12, HIGH);
LED l1 (13, LOW);

Please, please don't do that. I stared at that for a while thinking "how did that compile?".

You have the first LED called l2 ( L-2 ) and assign it to LED 12 (twelve). That is just so confusing, to use a dataname that looks on a forum posting, and in the IDE, like a number. If your eyes easily spotted the difference between:

// Declare as many LEDs as you want.
LED	l2	(12, HIGH);

and:

// Declare as many LEDs as you want.
LED	12	(12, HIGH);

... then they are better than mine.


This doesn't compile:

void loop () {
	// It's very easy to iterate thru LEDs:
	for (byte i=0; i<LED::count(); ++i)
		LED::at(i)->invert();
}

I get:

sketch_oct20b.cpp: In function 'void loop()':
sketch_oct20b:11: error: base operand of '->' has non-pointer type 'PQR::LED'

Hey Nick Gammon, thank you for answering my post!

Please, please don't do that. I stared at that for a while thinking "how did that compile?".
You have the first LED called l2 ( L-2 ) and assign it to LED 12 (twelve). That is just so confusing, to use a dataname that looks on a forum posting, and in the IDE, like a number. If your eyes easily spotted the difference between:

Actually this is surprising myself that i typed that. i was just trying to make a very quick example of the use of the class directly in the message window if the forum. Hence the next error too. Of course, in real condition, the variable naming would look like something more self-explainatory such as "redLed", "blueLed", etc.

sketch_oct20b.cpp: In function 'void loop()':
sketch_oct20b:11: error: base operand of '->' has non-pointer type 'PQR::LED'

Silly me! Again, i didn't even try to compile the example i gave. It should read: LED::at(i).invert(); of course because LED::at() returns a ref to a LED and not a ptr. i will try to modify my initial post about that former matter.

Thanks for your time, and have a cool day :slight_smile:
Pierre.

Hello AWOL,

If you're doing a debug class, make sure you put the debug strings into progmem.

i'm looking at this page right now: http://www.arduino.cc/en/Reference/PROGMEM but i have to say it explains the "how" (Which i perfectly understand), but the "why" is still foggy in my mind. In my understanding, the only reason to do that would be to avoid loosing the precious SRAM, is that right? But when doing something like that:Dbg << "Toto";
Isn't the string "Toto" a const char[] not being put in SRAM due to it's const nature? How is the compiler using the explicit / implicit consents of data? What would be the solution to put it in progmem? i have done something which works, but would you mind telling me if it's ok? Lets say i want to make the LED's debugging operator<< PROGMEM friendly ([edit]Moved stuff to static in class LED, and surrounded it with DEBUG tests.[/edit]):

#ifndef PQR_LED_h
#define PQR_LED_h

#include "Debug"
#include <WProgram.h>
#include <avr/pgmspace.h>

namespace PQR {

class LED {
public:
	// Constructors.
	/**/					LED				(byte pinNumber, bool initialState=false)
	: _pin(pinNumber), _state(initialState) {
		_leds				= (LED**)(_leds ? realloc(_leds, sizeof(LED*)*_count) : malloc(sizeof(LED*)));
		_leds[_count]		= this;
		_count				++;
	}
	
	// Global setup method, needs to be called once at the very begining of the program.
	static void				setup			() {
		byte		pin;
		for (byte i=0; i<_count; i++) {
			pin				= _leds[i]->_pin;
			pinMode			(pin, OUTPUT);
			digitalWrite	(pin, _leds[i]->_state ? HIGH : LOW);
		}
	}
	
	// Accessors.
	inline bool				state			() const	{ return _state; }
	inline void				setState		(bool v)	{ if (_state==v) return; _state = v; digitalWrite(_pin, _state ? HIGH : LOW); }
	inline bool				invert			()			{ setState(!_state); }
	
	// Factory management.
	inline static byte		count			()			{ return _count; }
	inline static LED&		at				(byte i)	{ return *_leds[i]; }
	
	// Debug a LED to debugging.
	#ifdef DEBUG
	inline friend Debug&	operator<<		(Debug &d, LED const &l) {
		char			buf		[8];
		strcpy_P				(buf, (char*)&PQR::LED::dbgFmt);
		sprintf					(buf, buf, l._pin, l._state);
		return					d << buf;
	}
	#endif

private:
	// Debugging strings.
	#ifdef DEBUG
	static char const PROGMEM	dbgFmt[];
	#endif
	// Static members.
	static LED**				_leds;
	static byte					_count;
	// Members.
	byte						_pin;
	bool						_state;
};

#ifdef DEBUG
char const PROGMEM	LED::dbgFmt[]	= "L@%u=%u";
#endif
LED**				LED::_leds		= 0;
byte				LED::_count	= 0;

}

#endif

Then in my loop, this works:

#define DEBUG
#include <PQR/Debug>
#include <PQR/LED>

using namespace PQR;

LED		redLed		(13, LOW);

void setup () {
	#ifdef DEBUG
	Debug::setup	();
	#endif
	LED::setup		();
}

void loop () {
	redLed.invert			();
	Dbg						<< redLed;
	delay					(1500);
}

The output of this code is, as expected:

Dbg: L@13=1.
Dbg: L@13=0.
Dbg: L@13=1.
Dbg: L@13=0.
// And so on...

Is that what you suggested, put any static string in the PROGMEM? What exactly would that change over a const char[] being declared on the fly?

Thanks,
Pierre.

What exactly would that change over a const char[] being declared on the fly?

It saves a lot of RAM that you may wish to use for other things.
It doesn't matter if you qualify a string with const, the compiler will cause it to be copied to RAM.
The AVR is a Harvard architecture processor, with separate address spaces for RAM and program memory.

Hey AWOL, thanks!

Would you mind looking at the LED class and telling me if the PROGMEM stuff is being done correctly (i plan to get rid of the LED class, it's a playground for now).

It saves a lot of RAM that you may wish to use for other things.
It doesn't matter if you qualify a string with const, the compiler will cause it to be copied to RAM.
The AVR is a Harvard architecture processor, with separate address spaces for RAM and program memory.

That i don't understand. When using PROGMEM, the temporary buffer used for grabbing the PROGMEM content is put in RAM anyways, right? So the only difference is that it gets freed implicitly when going out of scope, ok. BUT: wouldn't it be the exact same pattern with a const char[] (Eg. allocated in scope, freed when going out of scope)?

[edit]is there a way to know what memory is used in various places of the hardware at runtime? Eg check SRAM, PROGMEM, etc? So i can experiment and see by myself what's going on under the hood? Also, what would be the starting address of a variable pointing to PROGMEM and another regular SRAM variable, in terms of offset? Would it be an constant offset based on the ATMega chip version, or would it change from one exec to another on the same chip?[/edit]

What buffer?
If all you're doing is printing (and that's about all the debug needs to do), when you copy from PROGMEM, you only need a single character buffer.

Think about the architecture.
The only permanent storage is the flash.
Before "main" runs, all strings are copied from flash to RAM, unless you've qualified them with PROGMEM.

hickscorp:
That i don't understand. When using PROGMEM, the temporary buffer used for grabbing the PROGMEM content is put in RAM anyways, right? So the only difference is that it gets freed implicitly when going out of scope, ok. BUT: wouldn't it be the exact same pattern with a const char[] (Eg. allocated in scope, freed when going out of scope)?

If you access stuff in PROGMEM, you can copy it out a byte at a time. So one byte of RAM is used. If you use a literal constant the compiler generates code to copy all of the literals into RAM at the start, and then they never go out of scope.

Nick and AWOL, thanks a lot, this is getting clear for me now. And it's very interesting too.

AWOL said:

What buffer? If all you're doing is printing (and that's about all the debug needs to do), when you copy from PROGMEM, you only need a single character buffer.

Look at my implementation, i need a buffer to retrieve the data from PROGMEM, so this buffer gets allocated. But Nick Gammon gives me a good solution which is to use a global buffer. [edit flag="before you scream at me"]i realize this answer would make you think i didn't understand the whole purpose of the thing. i did. At any time of the program exec, no SRAM strings would be allocated, only the current buffer being in use.[/edit]

Nick Gammon said:

If you access stuff in PROGMEM, you can copy it out a byte at a time. So one byte of RAM is used. If you use a literal constant the compiler generates code to copy all of the literals into RAM at the start, and then they never go out of scope.

Using a global buffer sounds good, however it pops another question in my mind. In the case of using my debug function in an ISR, let's say the ISR got called when my program was in the middle of a strcpy_P to that same buffer in another part of my program, then wouldn't it generate some random behavior? Would i need as many buffers as the total count of my ISRs?

[edit]Guys, how do you handle race condition in such micro controllers architecture, when using a global variable for read / write in multiple parts of a program[/edit]

Thanks again a lot, the whole thinking is new to me (Programming is not tho, so i guess i won't need too long before understanding everything i need to avoid common mistakes).
Best!
Pierre.

In the case of using my debug function in an ISR,

But you wouldn't ever be doing that, would you?

AWOL:

In the case of using my debug function in an ISR,

But you wouldn't ever be doing that, would you?

Wondering why you say that. The only thing that pops into my head would be that Serial.print() uses delay, itself unavailable in ISRs? Or am i missing something fundamental here?

Why would Serial.print use delay?
Think about it - an interrupt is meant to be FAST, but serial comms are SLOOOOOOOW

hickscorp:

AWOL:

In the case of using my debug function in an ISR,

But you wouldn't ever be doing that, would you?

Wondering why you say that. The only thing that pops into my head would be that Serial.print() uses delay, itself unavailable in ISRs? Or am i missing something fundamental here?

Using Serial to debug an ISR is like using a sledge hammer to adjust the idle on a finely tuned super car.