[SOLVED] possible memory leak between setup() and loop() int array values

Hello. Unfortunately this has now cost me the project but for my sanity I would like to know the issue. I have some experience with C++ but mainly C# so basic errors, or lack of understanding of the Arduino are likely.

We were using an Uno R3, multiplexing (CD74HC4067) 14 ultrasonic range finders (Halija Seafront 5V). Sensors were positioned around a rectangular base to give ~360 coverage for people approaching to control a strip of LEDs - for art..

Calibrating a baseline value from either setup() or loop() (multiple approaches were attempted), and storing these values to be compared with real time data in loop() resulted in (apparent) memory leaks in Serial.

The programme compiles to 24% memory and 41% RAM to sample, compare, calculate the LED response and turn on the lights, then repeat. The output and light functions are removed and commented (//):

//Main.ino
/*
 * IF PERSON COMES WITHIN THE GL_interactionRange FADE LIGHTS TO INTERACTION COLOUR
 * CALCULATE DISTANCE FOR ALL SENSORS WITHIN INIT CYCLE - SET TO GL_DEFAULTRANGE[]
 * ONLY ACCEPT VALUES BELOW THIS DEPTH
 */

#include <FastLED.h>

#define GL_numOfSensors 14                     
#define GL_lightDataPin 12                         

#define GL_sensorInitCycles 20                  //(20) No. of cycles to initialise the sensors
#define GL_maxDetectionRange 800           
#define GL_interactionRange 70.0              

byte GL_pingPin = 9;                                
byte S0 = 8;                                            // MULTIPLEXER PINS
byte S1 = 7;
byte S2 = 6;
byte S3 = 5;
byte GL_echoPin = 13;                             

byte GL_pinsForInput[1] = { GL_echoPin };                          
byte GL_pinsForOutput[5] = { S0, S1, S2, S3, GL_pingPin };         

int GL_sensorDefaultRange[GL_numOfSensors] = {};               // Static objects data []
int GL_sensorData[GL_numOfSensors] = {};                           // Global SensorData []

/*
 *
 *PROBLEM STARTS WITH GL_DefaultRange[] allocation and use throughout the programme
 *
 */

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

  initSensors();          //This sets DefaultRange[] before real time data
  //initOutputs();

  Serial.println( "STARTING" );
}

byte _idCounter = 5;

void loop() {

  /*
   * DEBUG - write all sensor numbers each 5 lines for ease of reading data
   */
  
  if( _idCounter == 5 )
  {
    _idCounter = 0;
    Serial.println();
    for( byte id = 0; id < GL_numOfSensors; id ++ )
    {
      Serial.print(id);
      Serial.print("  ");
    }
    
    Serial.println();
    for( byte id = 0; id < GL_numOfSensors; id ++ )
    {
      Serial.print( GL_sensorDefaultRange[id]);             // Sanity check the default ranges
      Serial.print("  ");
    }
    Serial.println(); 
  }
  _idCounter++;

  // SensorManager - Calculate all depths and set - GL_sensorData[GL_numOfSensors]
  getDepthFromAllSensors();
  

  // DetectionManager - Find the nearest sensor value within interaction and default ranges
  // Use this value to set the fade of the LEDs in the LightManager - for(ALL LEDS).setHue( %total fade )
  identifyPeople();

  // OutputManager - Select outputs based proximity - Light / Sound
  // Currently only turns on lights - FastLED.Show();
  //LightAndSound();
  
  // Wait for system to stabilize - multiple delays in sensor code
  delay(20);

initSensors() is handled by SensorManager, below:

//SensorManager.ino

/*
 * SET ALL PINS FOR MULTIPLEXER - CYCLE THROUGH ALL SENSORS FOR GL_sensorInitCycles
 * SET THESE - GL_DefaultRange[] = GL_SensorData[] (stable from init cycles)
 * THIS ASSIGNMENT APPEARS TO CAUSE THE ISSUE WHEN THE GL_DefaultRange[] IS QUERIED
 * WHEN COMMENTED OUT THERE DOES NOT APPEAR TO BE AN ISSUE
 */

volatile byte initCounter = 4;
volatile bool sensorsInit = true;
int _inDepth = 0;

void initSensors()
{
  for( int initPin = 0; initPin < 5; initPin++ )
  {
    pinMode( GL_pinsForOutput[initPin], OUTPUT );
    digitalWrite( GL_pinsForOutput[initPin], LOW );
  }
  
  pinMode( GL_echoPin, INPUT );

  for( byte calib = 0; calib < GL_sensorInitCycles; calib++ )
  {
    getDepthFromAllSensors();
    delay(20);
    Serial.print(calib); 
    Serial.print(" ");
  }

  Serial.println();
  Serial.println("Calbiration values");

  for( byte cal_ID = 0; cal_ID < GL_numOfSensors; cal_ID++ )
  {
    volatile int val = GL_sensorData[cal_ID];        // Assigning a new local variable to separate the 
                                                                       // memory locations - this was a guess instead of 
                                                                       // direct assignment between arrays


    GL_sensorDefaultRange[cal_ID] = ( val - 5 );  //(-5) Allow for small amount of noise after stabilised
    Serial.print(GL_sensorDefaultRange[cal_ID]);  // Sanity check
    Serial.print(" ");
  }  

  Serial.println();
}

int _depth = 0;
      
void getDepthFromAllSensors()
{
  for( byte sensorID = 0; sensorID < GL_numOfSensors; sensorID++ )
  {
    selectMuxPin(sensorID);

    _depth = ( getDuration() * 0.034 / 2 );

    if ( _depth != 0 )
    {
      GL_sensorData[sensorID] = (( 2 ) * GL_sensorData[sensorID] + _depth ) / 3;
    }
    
/*
    Serial.print("id ");
    Serial.print( i );
    Serial.print(" data ");
    Serial.print( _depth );
    Serial.print(" ");
    Serial.print(GL_sensorData[i]); 
    Serial.println();
*/          
    delay(20);    
  }
  //Serial.println();
  
  for( byte outSensorID = 0; outSensorID < 14; outSensorID++ )
  {
    Serial.print(GL_sensorData[outSensorID]); 
    Serial.print(" ");
  }
  Serial.println();
  
}

long getDuration()
{
  long _duration = 0;
  
  // Set the sonar to zero and wait to stabilize      
  digitalWrite( GL_pingPin, LOW );
  delayMicroseconds(200);

  // Set the sonar to high - wait for 8 pulses - set to zero
  digitalWrite( GL_pingPin, HIGH );
  delayMicroseconds(30);
  digitalWrite( GL_pingPin, LOW );
  
  // Read the time of flight - calculate the distance    
  _duration = pulseIn( GL_echoPin, HIGH, 20000 );

  return _duration;
}

void selectMuxPin( int pin )
{  
  for( byte index = 0; index < 16; index++ )
  {   
    if( pin & (1<<index))
    { digitalWrite( GL_pinsForOutput[index], HIGH ); }
    else
    { digitalWrite( GL_pinsForOutput[index], LOW ); }
  }
}

Sanity checks in Serial show the GL_DefaultRange[] to be populated, however, when querying this array against the GL_sensorData[] there is a persistent error in the Serial output. See top DEBUG (counter==5) in main loop(). The "id" values shown here give an inconsistent "?" character - suggesting a memory leak. Comparison is made here DetectionManager - identifyPeople():

int closest =  GL_maxDetectionRange;     // Outside of sensor range
volatile int sensorData = 0;

void identifyPeople()
{
  // Reset tests for nearest person and lightDepthData
  closest = GL_maxDetectionRange;
  
  for( byte _sensorID = 0; _sensorID < GL_numOfSensors; _sensorID++ )
  {
    sensorData = GL_sensorData[_sensorID];

    Serial.print( _sensorID );
    Serial.print( " sml " );
    Serial.print( sensorData );
    Serial.print( " " );
    Serial.print( GL_sensorDefaultRange[_sensorID] );
    Serial.print( " " );

    // If data is less than the default range and closest
    // Possible braces on each && clause may help
    if( sensorData < GL_sensorDefaultRange[_sensorID] && sensorData < closest )
    {
      closest = sensorData;
    }

    Serial.println( closest );    
  }

  Serial.print( "closest " );
  Serial.print( closest );
  Serial.println();
  
  fadeAll( closest );        // Calculate %fade required (background to full interaction) For all LEDs
                                   // This all works fine

Whenever these two sets of arrays are compared there is a Serial issue. The "volatile int val" in initSensors() is used to attempt to separate the memory locations in assignment, although this was a best guess at a memory allocation issue.

Other than this I cannot think of why the values held in GL_DefaultRange[] cannot be compared with GL_sensorData[]. For the sake of this issue the system appears stable and relatively responsive given the number of delays and smoothing in sensor data, without moving over to the NewPing library, and would have resulted in a completion of the project.

Any and all help would be greatly appreciated as I'd love to know why I couldn't get over this final hurdle in delivering the project. Sorry for any issues with the post. All the best.

What is this doing?

void selectMuxPin( int pin )
{  
  for( byte index = 0; index < 16; index++ )
  {   
    if( pin & (1<<index))
    { digitalWrite( GL_pinsForOutput[index], HIGH ); }
    else
    { digitalWrite( GL_pinsForOutput[index], LOW ); }
  }
}

GL_pinsForOutput has only five elements, but you're using sixteen.

wildbill:
What is this doing?

It's a bit shifting operation to compare the integer value of "pin" in. Where bits align or not it sets the output to 1 or 0 in the pinsForOutput {S0, S1, S2, S3} to represent the correct configuration in the multiplexer. It's basically an auto-configuration for the multiplexer.

It can handle up to 16 channels, hence the limit on the loop to prevent overflowing into the pinsForOutput{} configuration. Hope this helps.

eayajb:
Hope this helps.

It never helps to access beyond the bounds of an array, which that code does unconditionally.

What limit?

If that code overruns the array and happens to do a digitalWrite to the serial output pin you will get garbage in the serial monitor.

gfvalvo:
It never helps to access beyond the bounds of an array, which that code does unconditionally.

Thanks, this could be the issue. I can test a more direct approach.

Output of this loop is four bool operations (representing up to 15 == 1111) on the multiplexer pins. The "limit" is held in the comparison ( pin & (1<<index) ), where the output will be 1 or 0 depending on the bit representation of index < 16 i.e. up to 15. This compares the representation of each in selecting true or false to set the pins. The operation does not compare past four bits, and so only accesses the first four articles of the array GL_pinsForOutput[].

Output for this function where pin = 1-17 would read: 0000, 0001, 0010, ... ,1111 and terminate at 15 given the above loop.

Or so I believe. I have not tested this with 17, but Serial.print 1 or 0 for each case in the main getDepthFromAllSensors() for(numOfSensors) SelectMUX(), reads as above.

Your understanding of what that loop does is flawed. It does sixteen digitalWrites. The only thing your bit twiddling does is to determine whether to write high or low. You have an overflow.

wildbill:
Your understanding of what that loop does is flawed. It does sixteen digitalWrites. The only thing your bit twiddling does is to determine whether to write high or low. You have an overflow.

Balls.. I mean thanks, but balls.

Looking at the output of the loop with Serial in place... :

void selectMuxPin( int pin )
{  
  for( byte index = 0; index < 16; index++ )
  {   
    if( pin & (1<<index))
    { digitalWrite( GL_pinsForOutput[index], HIGH ); Serial.print("1"); }
    else
    { digitalWrite( GL_pinsForOutput[index], LOW ); Serial.print("0"); }
    Serial.print("_");
  }
  Serial.println();
}

... only shows four writes. Although this seems to be the prevailing answer and is now starting to make sense.
Unfortunately I don't have an Arduino anymore to test, but thank you.

eayajb:
... only shows four writes. Although this seems to be the prevailing answer and is now starting to make sense.
Unfortunately I don't have an Arduino anymore to test, but thank you.

The expression (1<<index) will yield the following values as the loop increments:

0x0001
0x0002
0x0004
0x0008
0x0010
0x0020
0x0040
0x0080
0x0100
0x0200
0x0400
0x0800
0x1000
0x2000
0x4000
0x8000

The expression pin & (1<<index) will return 0 or not zero. If it returns 0 then a LOW will be written to GL_pinsForOutput[index] which may be up to 16 times or index values 0 thru 15.

I ran this:

byte GL_pingPin = 9;                                
byte S0 = 8;                                            // MULTIPLEXER PINS
byte S1 = 7;
byte S2 = 6;
byte S3 = 5;
byte GL_echoPin = 13;                             

byte GL_pinsForOutput[5] = { S0, S1, S2, S3, GL_pingPin };    
void setup()
{
  Serial.begin(115200);
  selectMuxPin(3);
}

void selectMuxPin( int pin )
{
  for ( byte index = 0; index < 16; index++ )
  {
  Serial.print("Index: ");
  Serial.println(index);  
    if ( pin & (1 << index))
    {
      //digitalWrite( GL_pinsForOutput[index], HIGH );
      Serial.print("1");
    }
    else
    {
      //digitalWrite( GL_pinsForOutput[index], LOW );
      Serial.print("0");
    }
    Serial.print("_");
  }
  Serial.println();
}

void loop()
{
}

I commented out the digitalWrites to prevent any untoward effects

Which gives me this:

Index: 0
1_Index: 1
1_Index: 2
0_Index: 3
0_Index: 4
0_Index: 5
0_Index: 6
0_Index: 7
0_Index: 8
0_Index: 9
0_Index: 10
0_Index: 11
0_Index: 12
0_Index: 13
0_Index: 14
0_Index: 15
0_

Since GL_numOfSensors is 14 then then there would at most be 3 writes of a HIGH to the output pins and those would only occur when the index is in the range 0 to 3. It is the LOW writes that exceed the array bounds as wildbill demonstrated.

Thanks @ ToddL1962 and wildbill, that really helps to see it laid out. There's some truth to this method somewhere, but back to the drawing board. There are plenty of less "elegant" - working solutions out there.

It's a strange one that the programme never showed any signs of instability or overflow until I introduced the calibration routine. False flag operations..

Thanks everyone. Back to school for me.

eayajb:
Thanks @ ToddL1962 and wildbill, that really helps to see it laid out. There's some truth to this method somewhere, but back to the drawing board. There are plenty of less "elegant" - working solutions out there.

It's a strange one that the programme never showed any signs of instability or overflow until I introduced the calibration routine. False flag operations..

Thanks everyone. Back to school for me.

The simple fix would be to change

  for ( byte index = 0; index < 16; index++ )

to

  for ( byte index = 0; index < 4; index++ )

Since you know GL_numOfSensors is at most 4 bits.