Ram Continuously Being Eaten

Ok. ill try to make this as short as possible.
The project is basically some form of a prototype for a polyphonic synthesiser.

there are 4 switches connected to an edge triggered mono pulse generator connected to an encoder whos outputs and 'valid' output is connected to the arduino duemilanove(Atmega 328) board.

4Switches -> pulse Generator -> encoder -> Arduino

The 'valid' output of encoder is connected to the Arduino pin 2 to act as an external interrupt.
when any key is pressed or released(edge triggered), the valid, will drop low for a few millis thus triggering the external interrupt on the arduino.
the arduino will then read the value of the encoder outputs and process it.

Simultaneously, there is a timer interrupt designed to output sound samples.

All variables shared between interrupts are DECLARED VOLATILE.
There are NO LOCAL VARIABLES.

The problem is, that each time i press or release the switch about 20 to 30 bytes of ram is eaten up. The following is the code:

#include <SD.h>
Sd2Card a;
uint8_t buff[512];

volatile double notefact[4]={0,0,0,0};
volatile int8_t noteID[4] = {-1,-1,-1,-1};
volatile int32_t noteTime[4] = {0,0,0,0};
volatile uint8_t temp_byte, temp2_byte, temp_bytefinal,finalbit;
volatile double temp_dbl;
uint8_t * heapptr, * stackptr;

volatile const int sampleRate = 25000;
#define SRRatio 267904.57886781300191011839336786/sampleRate

#define TWOPOWER1_12 1.0594630943592952645618252949463
#define TWOPOWER_1_12 0.94387431268169349664191315666753

volatile int8_t note = -1;


ISR(TIMER1_COMPA_vect)          // timer compare interrupt service routine
{
  temp2_byte = 0;
  for(int i=0; i<4;++i)
    if(noteID[i]+1)
    {
      temp2_byte += buff[((uint32_t)(notefact[i]*noteTime[i]+0.5)) & 0x000001FF];
      noteTime[i]++;
      
    }
  temp_byte = temp2_byte<<2;
  
  
  //OUTPUT TO BE CODED
}


inline uint8_t readNote()
{
  //Function to Read The value from the encoder  

  temp_byte = !digitalRead(A2);
  temp_byte <<= 1;
  temp_byte |= !digitalRead(A1);
  temp_byte <<= 1;
  temp_byte |= !digitalRead(A0);
  Serial.print("Note: "); Serial.println(temp_byte);
  delay(300);
  
  return temp_byte;
}

inline double noteFactor(uint8_t y)
{
  //some calculation pertaining to frequency of the note
  temp_dbl=1;
  
  while(y>0)
  {
    temp2_byte *= TWOPOWER1_12; 
  }
  return temp_dbl*SRRatio;
}

void alterNote()
{
  //The next three lines of code are for checking the ram, disabling timer compare interrupts
  //And enabling all other interrupts in order to allow for Serial transfer(i beleive this needs interrupts Enabled);

  TIMSK1 &= ~(1<<OCIE1A);
  check_mem();

  sei();		 //Enable all interrupts
  Serial.print("RA1: ");
  Serial.println((int)(stackptr - heapptr)); //Print amount of RAM
  
  note = readNote();
  if(note == 4) note = 0;
  else if (note == 5) note = 4;
  else if (note == 6) note = 7;
  else if(note == 7) note = 11;
  
  
  for(int i=0; i<4; ++i)
    if(noteID[i] == note)
    {
      notefact[i] = 0;
      noteID[i] = -1;
      noteTime[i] = 0;
      return;
    }
  for(int i=0; i<4; ++i)
    if(noteID[i] == -1)
    {
      notefact[i] = noteFactor(note);
      noteID[i] = note;
      noteTime[i] = 0;
      return;
    }
  TIMSK1 |= (1<<OCIE1A); //Enable Timer Compare Interrupts
}

void setup()
{
  Serial.begin(9600);

  //Initialise Serial connection and Memory Card
  if(a.init(SPI_HALF_SPEED,10))
    Serial.println("yay");
    
  else
    Serial.print("Oh dear!");
  delay(1000);
  
  noInterrupts();
  
  //Attach External Interrupt
  attachInterrupt(0,alterNote,FALLING);
  
  //Load The Sound Data  stored in the 512 Bytes of Sector 21 of the SD card
  a.readBlock(21,buff);

  //SETUP TIMER REGISTERS
  TCCR1A = 0;
  TCCR1B = 0;
  TCNT1  = 0;

  OCR1A = 1600;            // compare match register 16MHz/256/2Hz
  			   // This is a particularly High value.
			   // Intended value is 80
  
  TCCR1B |= (1 << WGM12);   // CTC mode
  TCCR1B |= (1 << CS11);    // 8 ptemp_bytecaler 
  
  check_mem();
  
  interrupts();
  delay(300);
  
  Serial.print("RAM: ");Serial.println((heapptr-stackptr));
  delay(300);
  
  TIMSK1 |= (1 << OCIE1A);  // enable timer compare interrupt*/
  
}

//Sets stackptr and heapptr in order to calculate memory
void check_mem() {
  stackptr = (uint8_t *)malloc(4);          // use stackptr temporarily
  heapptr = stackptr;                     // save value of heap pointer
  free(stackptr);      // free up the memory again (sets stackptr to 0)
  stackptr =  (uint8_t *)(SP);           // save value of stack pointer
}


void loop()
{
  
}

Moderator edit: Mind your language

Any Explanation would be helpful
Thanks in Advance

Allocating an object on the heap does not sound like a smart way to find the top of the heap, since the memory management library will (hopefully!) reuse memory that has been released previously. You may find it's just that your dynamic allocation is walking through the heap and not that you actually have a leak.

There is a standard snippet of code to measure the free memory without allocating memory on the heap, and I'm sure a search of the forum will find it.

I know but if that were the case, it would mean that the ram space shown is greater then the ram space present. however, I notice that with subsequent pressing of switches, the ram shown, continually decreases until it overflows and the program resets. BTW this code is easily testable. just need a switch on pin 2 as external interrupt. all other pins may return random data. that is not of much concern. so I would be very grateful for any effort put to debug this.

maharjun:
I know but if that were the case, it would mean that the ram space shown is greater then the ram space present. however, I notice that with subsequent pressing of switches, the ram shown, continually decreases until it overflows and the program resets. BTW this code is easily testable. just need a switch on pin 2 as external interrupt. all other pins may return random data. that is not of much concern. so I would be very grateful for any effort put to debug this.

I don't trust your code to measure free memory, and there is a perfectly good implementation which does the same thing which I do trust, so I suggest you use that. Among other things, does it not strike you as somewhat dangerous to do dynamic memory allocation and deallocation within an interrupt?

You've got several problems there.

  • You can't use Serial.println in an interrupt routine. Well, OK, you can. But don't expect it to work.
  • You shouldn't be using delay() in an interrupt routine. The whole point to an interrupt routine is to be as brief as possible.
  • It looks like your attempts to debug a possible stack problem are only making things worse.

Remove the messing around with the stack. Put the Serial.println code and delay() in loop().

Pete

There is no point in creating constants with so many decimals like:
#define TWOPOWER_1_12 0.94387431268169349664191315666753

That will be copied into a float and floats only have 7 digits of precision.

#define TWOPOWER1_12
#define TWOPOWER_1_12

Lovely choice of names too. No room for confusion there!

Pete

  while(y>0)
  {
    temp2_byte *= TWOPOWER1_12; 
  }

When does this loop exit?


There are NO LOCAL VARIABLES.

  for(int i=0; i<4; ++i)

Apart from i?

PeterH:
Allocating an object on the heap does not sound like a smart way to find the top of the heap, since the memory management library will (hopefully!) reuse memory that has been released previously.

Silly me, but what memory management library?

You know: malloc/free.

The thing being used here:

//Sets stackptr and heapptr in order to calculate memory
void check_mem() {
  stackptr = (uint8_t *)malloc(4);          // use stackptr temporarily
  heapptr = stackptr;                     // save value of heap pointer
  free(stackptr);      // free up the memory again (sets stackptr to 0)
  stackptr =  (uint8_t *)(SP);           // save value of stack pointer
}

Mind you, isn't there a bug in free? So maybe the real problem is that the diagnostics are wrong?

Judging by this thread:

http://arduino.cc/forum/index.php/topic,112432

The OP finds memory both slow, and being eaten. It's not a good day for him, regarding memory. Oh well, maybe it's being eaten slowly.

Oh great, good old automatic overhead on a processor with 2k RAM. And stdio uses it, but only for fdevopen() and even that can be avoided.

Running stdio without malloc()

By default, fdevopen() requires malloc(). As this is often not desired in the limited environment of a microcontroller, an alternative option is provided to run completely without malloc().

The macro fdev_setup_stream() is provided to prepare a user-supplied FILE buffer for operation with stdio.

More news to me. My thought is screw malloc() the same way I don't use C++ String objects.

IMO, it's just more of the bathtub on a bicycle approach to MCU's I like to think of as "let's throw a sink in too, and think about 'need' for a toilet.

Implementation details

Dynamic memory allocation requests will be returned with a two-byte header prepended that records the size of the allocation. This is later used by free(). The returned address points just beyond that header. Thus, if the application accidentally writes before the returned memory region, the internal consistency of the memory allocator is compromised.

The implementation maintains a simple freelist that accounts for memory blocks that have been returned in previous calls to free(). Note that all of this memory is considered to be successfully added to the heap already, so no further checks against stack-heap collisions are done when recycling memory from the freelist.

The freelist itself is not maintained as a separate data structure, but rather by modifying the contents of the freed memory to contain pointers chaining the pieces together. That way, no additional memory is reqired to maintain this list except for a variable that keeps track of the lowest memory segment available for reallocation. Since both, a chain pointer and the size of the chunk need to be recorded in each chunk, the minimum chunk size on the freelist is four bytes.

When allocating memory, first the freelist is walked to see if it could satisfy the request. If there's a chunk available on the freelist that will fit the request exactly, it will be taken, disconnected from the freelist, and returned to the caller. If no exact match could be found, the closest match that would just satisfy the request will be used. The chunk will normally be split up into one to be returned to the caller, and another (smaller) one that will remain on the freelist. In case this chunk was only up to two bytes larger than the request, the request will simply be altered internally to also account for these additional bytes since no separate freelist entry could be split off in that case.

If nothing could be found on the freelist, heap extension is attempted. This is where __malloc_margin will be considered if the heap is operating below the stack, or where __malloc_heap_end will be verified otherwise.

If the remaining memory is insufficient to satisfy the request, NULL will eventually be returned to the caller.

When calling free(), a new freelist entry will be prepared. An attempt is then made to aggregate the new entry with possible adjacent entries, yielding a single larger entry available for further allocations. That way, the potential for heap fragmentation is hopefully reduced. When deallocating the topmost chunk of memory, the size of the heap is reduced.

A call to realloc() first determines whether the operation is about to grow or shrink the current allocation. When shrinking, the case is easy: the existing chunk is split, and the tail of the region that is no longer to be used is passed to the standard free() function for insertion into the freelist. Checks are first made whether the tail chunk is large enough to hold a chunk of its own at all, otherwise realloc() will simply do nothing, and return the original region.

When growing the region, it is first checked whether the existing allocation can be extended in-place. If so, this is done, and the original pointer is returned without copying any data contents. As a side-effect, this check will also record the size of the largest chunk on the freelist.

If the region cannot be extended in-place, but the old chunk is at the top of heap, and the above freelist walk did not reveal a large enough chunk on the freelist to satisfy the new request, an attempt is made to quickly extend this topmost chunk (and thus the heap), so no need arises to copy over the existing data. If there's no more space available in the heap (same check is done as in malloc()), the entire request will fail.

Otherwise, malloc() will be called with the new request size, the existing data will be copied over, and free() will be called on the old region.

el_supremo:
You've got several problems there.

  • You can't use Serial.println in an interrupt routine. Well, OK, you can. But don't expect it to work.
  • You shouldn't be using delay() in an interrupt routine. The whole point to an interrupt routine is to be as brief as possible.
  • It looks like your attempts to debug a possible stack problem are only making things worse.

Remove the messing around with the stack. Put the Serial.println code and delay() in loop().

Pete

If you call delay() in the interrupt routine the interrupt routine will stay there, holding onto its stack frame, for ever I believe. Next interrupt comes in and consumes another stack frame, calls delay(), repeat until stack has consumed all memory - sounds like what is happening here, nothing to do with heap allocation.

In some circumstances you might have to call delayMicroseconds() with a small delay value, but that routine does not wait for a timer interrupt like delay() does, so is safe (although long waits are anti-social to the rest of the program and other interrupt handlers).

Interrupt routines should be coded to run as fast as possible, do the minimum necessary at that point in time to handle the interrupt condition (often this means inputting data to a buffer, or outputting from a buffer, and/or setting a flag variable or two). Interrupt routines generally are meant to keep the external hardware happy, not to do any actual processing.

While an interrupt routine is running its own interrupt condition and other 'lower-priority' interrupts are prevented by the hardware from having an effect until you return from the interrupt routine. Here 'lower priority' is a property of the processor design that you can't change. (Note the precise semantics of interrupt handling are different between different processors, you have to read the datasheet for the correct information).