Beware: out of memory on promini

[Hamburger]

Hi,

just to inform interested parties.
While hunting down what seemed to be sensor problems it has turned into what seems like a shortage of SRAM.
The code/memory footprint has grown (too) large to run on promini or nano (2K sram each)!
Thanks to the ongoing efforts of Mr RcCam and Ziss we found that depending on particular configuration

Data: 1992 bytes
(.data + .bss + .noinit)

is being taken at compile time. This seems to be too huge to run safely with 2K sram.
If interested, read here http://www.multiwii.com/forum/viewtopic.php?f=8&t=941&start=40

Even if it turned out this was not the source of my sensor problems it has to be taken into account for future development and error hunting.

[Hamburger]

ok, I took Alex' latest source (compatible with arduino IDE 1.00 and Serial cleanup) from google rep, rev 411, and reduced sram usage by tweaking power meter and LCD strings by 128+44 bytes.
With my setup I end up at a comfortable (?)

Program: 20818 bytes
(.text + .data + .bootloader)

Data: 1673 bytes
(.data + .bss + .noinit)

Now this does run again on Arduino Nano for me, yeah!
If interested in the code, it is here: http://code.google.com/p/multiwii/sourc ... n412&r=412
(at the moment I am too lazy to remove my TRI60.h and the include, but no big deal for you)

[mr.rc-cam]

My advice to the MWC contributors is to stop any feature additions until the ram usage issue is resolved. And each new release needs a rom/ram size check (using worse case scenarios) to ensure run time reliability. Perhaps the avr-size report's results need to be included in a release's readme file for all to see. A config.h should be devised that provide the worst case setup for use with the mem size check.

BTW, I'm not worried about the ram shortage, since there are opportunities in the code to reclaim some of the ram bloat. My bigger fear is that now some MWC users will needlessly think their performance problems are somehow related to a "ram shortage" rather than from the endless other (and more likely) reasons their model does not fly to their satisfaction.

- Thomas

[SoundMTB]

Hi
I don't agree do the idear with Sram size.
I've pro mini 328 only with WMP+BM020 and my SRAM shows (tested with AVR-SIZE) 771 Bytes.
This is much less than 2K but I have I2C errors and Problems with the Bus.
I'm 99,9% sure there is no Hardware failure in my electronic it has to be an Software fould i think

[mr.rc-cam]

This is much less than 2K but I have I2C errors and Problems with the Bus.
I'm 99,9% sure there is no Hardware failure in my electronic it has to be an Software fould i think

If you have I2C errors then I am 99.99% sure you have hardware problems, not software issues. The I2C problems need to be discussed in the other I2C threads rather than here.

BTW, since the release of V1.9 and its ability to report I2C errors, there is belief that V1.9 has caused the I2C errors. But the I2C hardware issues have silently existed for a long time and it is responsible for reduced MWC performance in some models. To help relieve some of the V1.9 finger pointing I will post later today (in a new thread) that summarizes the I2C issue and offers some general advice on solving I2C hardware problems. Frankly, it's time we improved the hardware so that Alex and the MWC contributors can develop the best performing software.

- Thomas

[kataventos]

If you have I2C errors then I am 99.99% sure you have hardware problems, not software issues. The I2C problems need to be discussed in the other I2C threads rather than here. Frankly, it's time we improved the hardware so that Alex and the MWC contributors can develop the best performing software.

- Thomas

I to agree, all multiwii user´s have to triple check every single connection/solder, sensors, calibration routine (motors/proppelers) etc, before pointing code issues, this way the evolution is faster. It is true that in an open source project like this one, coders try to please all the users that use the most diferent and popular sensors, this is almost like microsoft x mac we want to use all sensors and shields on the market (microsoft) , ok with that, most of them are sponsers! but at least be sure that they are not faulty and if they are well mounted before pointing the code.

Sorry about this post here!

Cheers,
Henrique

[Danal]

One possible RAM reduction could come from moving all the LCD strings into PROGMEM.

Is this worthwhile? I can work on it, if desired.

[Hamburger]

One possible RAM reduction could come from moving all the LCD strings into PROGMEM.

Is this worthwhile? I can work on it, if desired.

yes, yes.

And by thetime that is not enough anymore (because flash is getting closer to limit as well) we could move the strings into eeprom with another sketch that needs to be run once to upload the strings. From then on the actual mwii sketch would simply use references into eeprom and not use neither flash nor sram.

But that is fantasy. Using PROGMEM is the way to go, imho. I say, do it.

[Danal]

The code in 'shared' at R423 or above has most strings moved to PROGMEM. It has been bench tested to ensure LCD function, but not yet flown.

Size Before, the R420 level of shared compiled for ProMini, with most options on:
Program 17292, SRAM 1717+Stack. This leaves about 331 bytes for stack on a ProMini.

After:
Program 17366, SRAM 1019+Stack. This leaves about 1029 bytes for stack on a ProMini.

SRAM reduction of 698 bytes. This should fix any ProMini bugs from lack of SRAM.


The changes are EXTENSIVE. About 12 to 14 hours total work (and I'm pretty fast). I would appreciate the comments at the beginning of LCD and in Output remaining intact, as I do this only for my ego.

[kataventos]

The changes are EXTENSIVE. About 12 to 14 hours total work (and I'm pretty fast). I would appreciate the comments at the beginning of LCD and in Output remaining intact, as I do this only for my ego.

Many thanks! I´m sure that all promini users will be very gratful.

Cheers,
Henrique

[ziss_dm]

Hi Danal,

I would not say I really like it. ;( You did big job separating strings/rearranging structs/etc.. But was it really necessary?

1) For inline strings there are really convenient macro PSTR(...)
2) It does not work for struct initializers, and there are 2 ways to overcome it:
- declare string separately(as you did)
- move strings inside struct as fixed-size array

The variable length strings packed better in the flash, but you need additional code to handle them.
The fixed size strings, easier to handle but have flash overhead. But, I think, more importantly it is much easier to read and manage:

Code: Select all

static lcd_param_t lcd_param[] PROGMEM = {
  {("PITCH&ROLL P"),    &P8[ROLL],      &__P}
, {("ROLL   P"),        &P8[ROLL],      &__P}, {("ROLL   I"), &I8[ROLL],   &__I}, {("ROLL   D"), &D8[ROLL],   &__D}
, {("PITCH  P"),        &P8[PITCH],     &__P}, {("PITCH  I"), &I8[PITCH],  &__I}, {("PITCH  D"), &D8[PITCH],  &__D}

So I did the test with 1.8. This is comparison of the r423 and attached LCD.pde

Code: Select all

Simple Config (gyro+acc):
           Var Strings                          Fixed Strings
Program:   14780 bytes                 Program:   14664 bytes
Data:        888 bytes                 Data:        870 bytes


Big Config (gyro+acc+baro+power meter):
           Var Strings                          Fixed Strings
Program:   19664 bytes                 Program:   19818 bytes
Data:       1078 bytes                 Data:       1054 bytes

As you can see it uses less SRAM and even less Flash for small number of lcd params.


So, the question is: do we really need to economize this flash?

regards,
ziss_dm

[Danal]

PSTR - thanks! It is easier and more readable.

Strings in the struct - The Arduino reference explicitly prohibits using any datatype not defined in pgmspace.h, such adcustom structs, with PROGMEM. This is repeated twice, with the second repeat noting that cryptic run time bugs can occur.

[ziss_dm]

Hi,

1) http://www.avrfreaks.net/index.php?name ... torder=asc
2) All prog_ types is just typedefs: http://www.nongnu.org/avr-libc/user-man ... space.html
3) You already violating "Arduino rule", declaring this:

Code: Select all

PROGMEM const void *lcd_param_ptr_table [] = {

4) If you want example how to put strunct in progmem, take a look in SoftwareSerial.cpp (officialy supplied with Arduino):

Code: Select all

typedef struct _DELAY_TABLE
{
  long baud;
  unsigned short rx_delay_centering;
  unsigned short rx_delay_intrabit;
  unsigned short rx_delay_stopbit;
  unsigned short tx_delay;
} DELAY_TABLE;

static const DELAY_TABLE PROGMEM table[] = 
{
  //  baud    rxcenter   rxintra    rxstop    tx
  { 115200,   1,         17,        17,       12,    },
....

regards,
ziss_dm

[Danal]

1) Read that already. And a lot more. Thanks anyway.
2) Yes, I realize that they are defs in the pgmspace.h library. Read that too.
3) Good catch. It should be prog_void.