Advanced programmers advice needed.

@Holmes4
Thanks for your reply.
I have been suspecting the heap and the stack but I can't get my head around how that would be possible.
I don't use new,alloc,malloc,String so basically that would mean I'm not using the heap.

As to the stack.
Seen the fact I don't reach setup that would mean that the constructors +main() +setup() blow the heap/stack
In my code (that is all the code in the project except for the arduino core code) there is not a single constructor that calls a method (that is apart from a base constructor if it needs a parameter)
Apart from: I'm overlooking a case: How can a stack/heap blow happen under these circumstances before setup()?

As a side note to show I'm aware of the stack usage:
As strings take so much room I also declared a global variable to do temporary string methods.

char commonlyUsedBuffer[commonlyUsedBuffersize];

An then in the code

m_LastMessageTimeStamp.ToString(commonlyUsedBuffer,commonlyUsedBuffersize);

This should reduce stack usage.

Now I think about this. In the old days I could provide a stack and heap size. How does this happen on the avr?

Best regards
Jantje

I'd look at the headers for the text orientated libraries and see what sort of size buffers they reserve.

I still can't see any code!

Mark

@AWOL
I verified the text oriented libraries. As I use 3 serial ports there are quite a lot of buffers. I even found an error saving 12 bytes :smiley:
But all these buffers are instantiated as global variables directly or indirectly. So they are counted for in the size statistics.
IMHO The remaining memory is really free for the stack and (not used) heap.

@holmes4
Some of the libraries are available in github. I'll update them and add the others but it seems I need to do a system upgrade first.

Due to debugging there is quite some trash in there but here is the main sketch

#include "ChargerBrains.h"
#include "SerialBridgeCommunicator.h"
//#include "FakeComunicator.h"
#include "LemCurrentSensor.h"
#include "JanServo.h"
#include "I2CLiquidCrystal.h"
#ifdef I_USE_MINI_BRIDGE
#include "MiniBridge.h"
#include "MiniProcess.h"
HardwareSerial &BridgeSerial = Serial1;
#else
#include <Bridge.h>
#include <HttpClient.h>
#endif
#include "gps_Library.h"

#define GPSTX_PIN 13
#define GPSRX_PIN 5
VoltMeter myChargerVoltmeter(A4);
LemCurrentSensor myCurrentSensor1(A0, A1);
LemCurrentSensor myCurrentSensor2(A2, A3);
SerialBridgeCommunicator myCommunicator;
//SerialCommunicator myCommunicator;
//FakeComunicator myCommunicator;
ChargerBrains myBrains;
SERIALTYPE myGPSserial(GPSTX_PIN, GPSRX_PIN);
GPSModule myGPS(9600UL, &myGPSserial);

JanServo myVoltageServo(4);
JanServo myPowerServo(6);

LiquidCrystal myLCD(0x38);
Stream *SerialInput = &Serial;
Stream *SerialOutput = &Serial;

const char mySketchName[] PROGMEM = "BatterijLader";
#define MAXFIELDS 65
FieldData AllFields[MAXFIELDS];

void setup()
{
	boolean triedToStartListener = false;
	int listenerStartWaitTime = 10000;
	delay(4000); //give the LCD slave some time

	myLCD.begin();
	myLCD.print("batterijlader");
	myLCD.setBackground(0, 0, 255);
	myLCD.setCursor(0, 1);
	myLCD.print(millis());

	Serial.begin(115200);
	//wait until serial is opened. If it is not opened it means I'm not debugging
	//so I can start the serial listener to feed the web site and log reader
	while (!Serial)
	{

		if (millis() > 120000UL) //40 seconds after startup start the serial listener
		{
			myLCD.setCursor(0, 0);
			if (triedToStartListener)
			{
				myLCD.print(F("listener started 2 times")); //no need to log this to serial as it is not running
				//Increase the time to wait for the script to start
				listenerStartWaitTime += listenerStartWaitTime;
			}
			myLCD.print(F("start bridge") );
			Bridge.begin();
			myLCD.clear();
			myLCD.print(F("bridge started"));
			triedToStartListener = true;

			//start the script on linux that will process the serial commands
			runShellCommand(F("/www/cgi-bin/jantje/startReadingArduino.sh"), commonlyUsedBuffer, commonlyUsedBuffersize);
			myLCD.clear();
			myLCD.print(F("reader started"));
			delay(listenerStartWaitTime); //make sure the started program is up and running
			//this should also enable serial so a break is not needed
		}
		delay(100);
		myLCD.setCursor(0, 1);
		myLCD.print(millis());

	}
	Serial.println((__FlashStringHelper *) mySketchName);
	Bridge.begin();

	Serial.println(F("bridge started"));
//	Serial.println(F("Test 2222222222222222222222222222222222222222222222222222222222222"));
//	//Serial.println(F("Test 2222222222222222222222222222222222222222222222222222222222222"));
//	//Serial.println(F("Test 2222222222222222222222222222222222222222222222222222222222222"));
//	//Serial.println(F("Test 2222222222222222222222222222222222222222222222222222222222222"));
//	//Serial.println(F("Test 2222222222222222222222222222222222222222222222222222222222222"));
//	Serial.println(F("Test 22222222222222222"));

	myCommunicator.serialRegister(F("Admin"));
	myBrains.serialRegister(F("brains"));
	myChargerVoltmeter.serialRegister(F("volt.charger"));
	myCurrentSensor1.serialRegister(F("current.bat1"));
	myCurrentSensor2.serialRegister(F("current.bat2"));
	myVoltageServo.serialRegister(F("Servo.volt"));
	myPowerServo.serialRegister(F("Servo.Power"));
	myGPS.serialRegister(F("GPS"));
	Serial.print(F("Current Datas "));
	Serial.println(curFieldData);
	Serial.println(F(" from "));
	Serial.println(MAXFIELDS);
	myCommunicator.setup();
	myBrains.setup();
	myChargerVoltmeter.setup();
	myCurrentSensor1.setup();
	myCurrentSensor2.setup();
	myVoltageServo.setup();
	myPowerServo.setup();
	myGPS.setup();
	Serial.println(F("setup done"));

}

void loop()
{
	static uint32_t lastlcdprint = 0;
//	myBattery1Voltmeter.loop();
//	myBattery2Voltmeter.loop();
	myChargerVoltmeter.loop();
	myCurrentSensor1.loop();
	myCurrentSensor2.loop();
	myVoltageServo.loop();
	myPowerServo.loop();
	myGPS.loop();

	myCommunicator.loop();
	myBrains.loop();
	if (millis() - lastlcdprint >= 2000)
	{
		snprintf(commonlyUsedBuffer, commonlyUsedBuffersize, "dV: %3i %3i %3i", myChargerVoltmeter.getVoltage() / 100, 0, 0);			//myBattery1Voltmeter.getVoltage() / 100, myBattery2Voltmeter.getVoltage() / 100);
		myLCD.clear();
		myLCD.print(commonlyUsedBuffer);
		//Serial.println(lcdBuffer);
		snprintf(commonlyUsedBuffer, commonlyUsedBuffersize, "dA:     %3i %3i", myCurrentSensor1.getCurrent(), myCurrentSensor2.getCurrent());
		//Serial.println(lcdBuffer);
		myLCD.setCursor(0, 1);
		myLCD.print(commonlyUsedBuffer);
		lastlcdprint = millis();
	}

}

As for my question on stack and heap. There is no need to specify sizes as explained here :
http://www.nongnu.org/avr-libc/user-manual/malloc.html

A small update.
I use a GPS module connected to pin 13 and 5. I don't think it is related but it looks as if it causes incomming data on Serial1.
I mean: The code works better without the gps connected (even though it worked 2 days ago :grin: )
But this addapted loop from bridge never finishes when the gps is connected.

  do {
  	Serial.println();
  	Serial.print("stream.available()=");
  	Serial.println(stream.available());
    dropAll();
    delay(1000);
  } while (stream.available() > 0);

Best regards
Jantje

As I use 3 serial ports

And GPS and Wire...

1 serial port is for GPS 1 for bridge and 1 for serial
And I use wire to communicate with the LCD
There is a reason why I stopped using the bootloader ]:smiley:
yes

Before setup() is called the main() function calls init() main() or init() create the instances of hardwareSerial for Serial, Serial1 etc. Each instance of hardwareSerial has 2x64 byte buffers. In addition to that the heap must be created and initialized. My theory is that doing all this is putting you over the 2.5k of sram!.

The compiler can only report static data space used. not anything allocated during startup.

There is a way (playground) to get the amount of free ram left at run time and it would be very easy to write a stub program to check how well the reported ram usage matches the actual at the beginning of setup().

Mark

@holmes4

The ( Hardware ) serial data is allocated statically:

void setup() {
  Serial.begin( 9600 );
  Serial1.begin( 9600 );
  Serial2.begin( 9600 );
}

void loop() { }

[quote author=Static SRAM]00800200 D __data_start
00800210 B __bss_start
00800210 D __data_end
00800210 D _edata
00800210 00000044 B rx_buffer
00800254 00000044 B tx_buffer
00800298 00000044 B rx_buffer1
008002dc 00000044 B tx_buffer1
00800320 00000044 B rx_buffer2
00800364 00000044 B tx_buffer2
008003a8 00000044 B rx_buffer3
008003ec 00000044 B tx_buffer3
00800430 00000022 B Serial
00800452 00000022 B Serial1
00800474 00000022 B Serial2
00800496 00000022 B Serial3
008004b8 00000004 B timer0_overflow_count
008004bc 00000004 B timer0_millis
008004c0 00000001 b timer0_fract
008004c1 B __bss_end[/quote]

Whats worse, is it appears the data for each Serial port is allocated regardless of what is used ( Serial3 is in SRAM, even though I only used 0,1 & 2 ).

This probably doesn't apply as the Leonardo only supports one serial line and serial over CDC ( statically allocated ). SoftwareSerial or similar is probably being used. Even then those libraries probably have statically allocated buffers too.

@Jantje, you said:

To be able to do so I overload the virtual SerialCommunicator::setReceivedMessage but this stopped working.

Where did you overload it, in SerialCommunicator or in SerialBridgeCommunicator

Well was worth a look.

Mark

pYro_65:
@holmes4

The ( Hardware ) serial data is allocated statically:

And so is software serial and altsoftware serial.

The reason is

class HardwareSerial : public Stream
{
...
    unsigned char _rx_buffer[SERIAL_BUFFER_SIZE];
    unsigned char _tx_buffer[SERIAL_BUFFER_SIZE];
  ...
};

Combined with

  extern HardwareSerial Serial1;

in hardwareSerial.h and because of that the needed

HardwareSerial Serial1(&UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1);

In other words the class gets instantiated as a global variable in the arduino core and as such the buffer gets added to the data section.
Remove the last 2 definitions and you save some space but lose the serial functionality.
That won't really help me as I need them.

pYro_65:
@Jantje, you said:

To be able to do so I overload the virtual SerialCommunicator::setReceivedMessage but this stopped working.

Where did you overload it, in SerialCommunicator or in SerialBridgeCommunicator

[EDIT: changed the code after the remark of Pyro]

Simplified the code looked like

class SerialCommunicator
{
protected:
		virtual void setReceivedMessage(const char* newMessage);
public:
		void loop();

};

class SerialBridgeCommunicator: public SerialCommunicator
{
	protected:
		virtual void setReceivedMessage(const char* newMessage)
                {
                  .......
                  SerialCommunicator::setReceivedMessage(newMessage);
                }
};

I changed it to

class SerialCommunicator
{
protected:
		void setReceivedMessage(const char* newMessage);
public:
		void loop();
};

class SerialBridgeCommunicator: public SerialCommunicator
{
	protected:
		void setReceivedMessage(const char* newMessage)
                {
                 the same code goes here
                }
	public:
		void loop()
                {
                 code copied from SerialCommunicator.loop
                }

};

By copying the code I changed the namespace and as such the correct setReceivedMessage is called. This was easy as setReceivedMessage is only called once.

In the mean time I have been able to do more code schrinking (I'll be an expert some day XD ) and I'm now at

Program:   29368 bytes (89.6% Full)
(.text + .data + .bootloader)

Data:       1958 bytes (76.5% Full)
(.data + .bss + .noinit)

I'm still experiencing problems. But with these figures (and my coding rules as mentioned above) I think we can rule out stack and heap problems in the pre setup code.

holmes4:
There is a way (playground) to get the amount of free ram left at run time and it would be very easy to write a stub program to check how well the reported ram usage matches the actual at the beginning of setup().

I see 2 problems here

  1. The report in setup does not take into account the top heap/stack usage before the processor came to setup
  2. When the problems happen the processor doesn't get to setup.

Best regards
Jantje

Is the 'serialRegister' function supposed to be named setReceivedMessage. I ask due to the derived class calling 'SerialCommunicator::setReceivedMessage(newMessage);' which is not there, and also means nothing is being overloaded and serialRegister is never defined.

The types of the functions don't match either, you'd need at least a c-style conversion for newMessage.

If the base class virtual functions require a derived implementation, make them pure-virtual ( you do not need the unused code then ).

@pYro_65
My bad I mixed up serialRegister and setReceivedMessage
serialRegister has nothing to do with the overload.
Best regards
Jantje

Quick and worth a go try reducing the size of the buffers in serial etc, if the program then gets to setup() then you know that you have a stack/heap usage problem during the program start up code. If it still fails to get in to setup() then odds on some kind of coding error in the constructors.

Mark

maybe I have not been all to clear about the nature of the problem.
Sometimes with the same sketch (no new upload) it works and then it stops working.
When it stops working it is after a cold start. When I simply reset the leonardo the behaviour seems to be is consistent. When I cold start it is not.
Which is pretty annoying as I go to sleep happy that it works and when I start the next day .......

This behaviour makes me think more in the area of uninitialize variables. However with -wall these are marked and -even though they do cost program memory- I have initialized all variables in my code.
Removing the invalid PROGMEM warnings below all the warnings that remain in my sketch when compiled with -wall

Description	Resource	Path	Location	Type
"USB_MANUFACTURER" redefined	USBCore.cpp	/batterijlader/arduino/core	line 59	C/C++ Problem
unused variable ‘current_config’	HardwareSerial.cpp	/batterijlader/arduino/core	line 329	C/C++ Problem
unused variable ‘c’	HardwareSerial.cpp	/batterijlader/arduino/core	line 115	C/C++ Problem
#warning this may not be correct	Tone.cpp	/batterijlader/arduino/core	line 210	C/C++ Problem
unused variable ‘r’	HID.cpp	/batterijlader/arduino/core	line 514	C/C++ Problem
#warning Timer 2 not finished (may not be present on this CPU)	wiring.c	/batterijlader/arduino/core	line 274	C/C++ Problem
unused variable ‘zero’	USBCore.cpp	/batterijlader/arduino/core	line 270	C/C++ Problem
#warning Timer 2 not finished (may not be present on this CPU)	wiring.c	/batterijlader/arduino/core	line 265	C/C++ Problem

Now I'm looking at this list I see this warning "#warning Timer 2 not finished (may not be present on this CPU)"
This is the code it points to

	// set timer 2 prescale factor to 64
#if defined(TCCR2) && defined(CS22)
	sbi(TCCR2, CS22);
#elif defined(TCCR2B) && defined(CS22)
	sbi(TCCR2B, CS22);
#else
	#warning Timer 2 not finished (may not be present on this CPU)
#endif

	// configure timer 2 for phase correct pwm (8-bit)
#if defined(TCCR2) && defined(WGM20)
	sbi(TCCR2, WGM20);
#elif defined(TCCR2A) && defined(WGM20)
	sbi(TCCR2A, WGM20);
#else
	#warning Timer 2 not finished (may not be present on this CPU)
#endif

Which is part of init() which is being called before setup. This feels like a lead.
I guess I best open an issue/thread on this one in the yun specific locations and try to remove the wire usage and see what that gives.

An update on the code on github.
Github decide to update their windows client to use .net4. Moreover they decided to update my installation which runs on XP without my permission. After upgrading my xp to the very latest version (I had turned off auto update for very good reasons) I found out .net4 is not supported on xp.

Best regards
Jantje

When it stops working it is after a cold start. When I simply reset the leonardo the behaviour seems to be is consistent. When I cold start it is not.

Could you explain this, by cold start do you mean upload? or restarting both processors on the yun, What do you mean by "consistent".

Mark

cold start= take power of system (that is from linino and leonardo) let it cool down (at least 10 seconds but prefably far more) and then repower.
consistent= 2 examples

Using SerialCommunicator instead of SerialBridgeCommunicator works
removing the overloaded setReceivedMessage and it works. Putting the overloaded setReceivedMessage back in and it failed.

Now setup is called but fails very early in the call Bridge.begin(); (this is my mini version of Bridge not the official bridge)
So I add some Serial.print statements to Bridge.begin() to find where it fails.
result setup is no longer called.
I removed the Serial.print statements and the setup works again and Bridge.begin() does not return smiley-roll

on the code.
Seems it is still supported on xp professional but doesn't get installed via update center.
So I could upload now.

Best regards
Jantje

Update
As I have been reducing the size i can now switch between altsoftserial and softserial.
It looked as if things were stable with softserial. But today I started the yun again and didn't want to move (with softserial)
serveral resets no ttyACM0 lcd gets no info ... (looks like dead)
Recompiled with altsoftware serial and uploaded and bam it works :fearful:

I also ran the available memory in the beginning of setup (when it is working) and I had 600+ bytes free. I calculated that around 20 bytes were used. Seemed okish to me since we are in main setup so some stack is being used I see 5 bytes in setup +2 times 2 for the function pointers.

I removed the memfree function changed some Serial.println to Serial.print and i don't get into the setup again.

In my experience this behaviour is cause by a memory overwrite. Something is overwriting and it depends on the compiler what is overwritten.

Any help is welcomed.
Best regards
Jantje

Sounds like incorrect use of a pointer some where.

Mark