Array keeps returning only the last element. [C/Arduino]

Question

i've a problem with an array (called "Inputs" of type "GeneralInput") on Arduino,basically,no matter which element i try to have access to,the code always returns me the last element of that array. Here's part of the code:

//...include statements
//other initializations
GeneralInput *Inputs[19];

void setup() 
{
    //...
    //...
    InitializeInputs();
}

void InitializeInputs()
{
    //type 0 = pedal switch;  1 = volume pedal
    //type 2 = potentiometer;   3= switch;
    //pedal switches
    Inputs[0] = &GeneralInput(0,0,true,false,NULL,10);
    Inputs[1] = &GeneralInput(1,0,true,false,NULL,9);
    Inputs[2] = &GeneralInput(2,0,true,false,NULL,6);
    Inputs[3] = &GeneralInput(3,0,true,false,NULL,5);
    //volume pedal
    Inputs[4] = &GeneralInput(4,1,false,false,NULL,A2);
    //potentiometer
    Inputs[5] = &GeneralInput(5,2,false,true,mux2,5);
    Inputs[6] = &GeneralInput(6,2,false,true,mux2,6);
    Inputs[7] = &GeneralInput(7,2,false,true,mux2,7);
    Inputs[8] = &GeneralInput(8,2,false,true,mux2,8);
    Inputs[9] = &GeneralInput(9,2,false,true,mux2,9);
    Inputs[10] = &GeneralInput(10,2,false,true,mux2,10);
    Inputs[11] = &GeneralInput(11,2,false,true,mux2,11);
    //switch
    Inputs[12] = &GeneralInput(12,3,true,true,mux2,15);
    Inputs[13] = &GeneralInput(13,3,true,true,mux2,14);
    Inputs[14] = &GeneralInput(14,3,true,true,mux2,13);
    Inputs[15] = &GeneralInput(15,3,true,true,mux2,12);
    //joystick   
    Inputs[16] = &GeneralInput(16,3,true,true,mux1,2);  //switch
    Inputs[17] = &GeneralInput(17,2,false,true,mux1,1); //x axis
    Inputs[18] = &GeneralInput(18,2,false,true,mux1,3); //y axis
}

void loop() 
{  
    int length=0;
    //cycle through different inputs
    int startIndex=0,endIndex=0;
    //temp arrays
    byte toSendTmp[30]; 
    for(int i=0;i<30;i++)
      toSendTmp[i]=0;
    //...
    //..
    int packetIndex=0;
    for(int i=startIndex;i<endIndex;i++)
    {
         //if the input is updated,fill the array with the new data
         /*
          * When i try to have access to the i-element i always get
          * the last one instead.
          */
         if(Inputs[i]->Update())
         {
            toSendTmp[(packetIndex*3)] = Inputs[i]->GetID(); 
            toSendTmp[(packetIndex*3)+1] = Inputs[i]->GetType(); 
            toSendTmp[(packetIndex*3)+2] = Inputs[i]->GetValue();
            packetIndex++;
         }         
    }
    //....
    //...
}

And if needed here's the GeneralInput.h and GeneralInput.cpp code. NOTE: I can't tell if the array is always returning the last item or if every slot of the array is filled with a pointer to the same object (the last created).

Any idea on what i'm doing wrong?

Thanks in advance.

I'm surprised this is compiling. You can't take the address of a temporary object. — Joseph Mansfield
– Joseph Mansfield, Commented Jul 9, 2014 at 13:00
Thanks,you're right,i'll look into the issue since i can't use 'new' on Arduino. — MTKJacob
– MTKJacob, Commented Jul 9, 2014 at 13:41
@JosephMansfield You "can" take the address of just about anything in C (for example char** foo = &"bar"; is valid). However, there are only a limited number of situations where taking the address of something not typically addressed (like a local variable) is useful. — event44
– event44, Commented Jul 9, 2014 at 14:25

Holt · Accepted Answer · 2014-07-09 13:02:36Z

1

Your &GeneralInput are incorrect, in fact you create temporary objects and store their adresses in an array, but as soon as your GeneralInput object get destroy (same line as creation), a new object takes place at the same address:

// Create GeneralInput at address @
Inputs[0] = &GeneralInput(0,0,true,false,NULL,10);
// End of your temporary object, the `GeneralInput` object is destroyed but you still
// points to its address...
/* etc. */

You're getting the last value because the compiler always create the GeneralInput at the same address, so all Inputs[] point to the same address.

You need to dynamically create your GeneralInput:

Inputs[0] = new GeneralInput(0,0,true,false,NULL,10);

answered Jul 9, 2014 at 13:02

Holt

38.1k7 gold badges100 silver badges147 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

MTKJacob Over a year ago

Thanks,you're right,the problem is that on Arduino the "new" keyboard is not available so i'll need to found a workaround for that.Thanks again.

event44 · Accepted Answer · 2014-07-09 14:11:19Z

0

Every slot in the array has a pointer to the same memory location which is the occupied by the last element you create. By doing &GeneralInput(...) you are creating a GeneralInput object on the stack and retrieving its stack address. But since the GeneralInput object itself is never assigned to a variable, the memory it occupies is immediately available for reuse. This means that every GeneralInput object is created at the same address on the stack. The solution, however, isn't to change your code to something like

GeneralInput genInput = GeneralInput(...);
Inputs[...] = &genInput;

Code like that will still be filling your array with pointers to stack addresses. Those pointers will immediately become invalid when the function returns. You should be filling your array with something like

Inputs[...]  = (GeneralInput*)malloc(sizeof(GeneralInput));
*Inputs[...] = GeneralInput(...);

Using this method make sure that if your Inputs array ever reaches a point where you don't use it anymore loop over it freeing every element.

Edit: Arduino using C, so doesn't have new. Use malloc and free instead.

edited Jul 9, 2014 at 14:11

answered Jul 9, 2014 at 13:04

event44

6188 silver badges18 bronze badges

3 Comments

MTKJacob Over a year ago

Thanks for the answer,i appreciate the details,unfortunately i can't use "new" on arduino,i'll se to find a workaround for that,thanks again.

event44 Over a year ago

Oh that's right. I'll edit my answer in case you still want to use an array of pointers.

MTKJacob Over a year ago

Thanks,i still have the ""same"",but i think that now it's for another reason. Probably the data in the GeneralInput class keeps being overwritten as a static member instead of being related to his own object.

cup · Accepted Answer · 2014-07-09 20:59:19Z

As others have said, the problem is with the address of the temporary variables. You can get around the "new" problem by having default parameters.

class GeneralInput
{
public:
    GeneralInput(int a = 0, int b = 0, bool c = true, bool d = true, int* e = NULL, int f = 0);
    ...
};

Then declare your array - this takes GeneralInput with the default parameters

GeneralInput inputs[20];

Then in Initialize - then you won't have the new problem or the problem of temporaries disappearing at the end of the routine.

void InitializeInputs()
{
    inputs[0] = GeneralInput(0,0,true,false,NULL,10);
    ...
}

I don't know what the NULL points to but you might want to put in a copy operator for this if it is anything else other than copying the value. Not very efficient because it calls the constructor twice but that only happens at initialization.

Collectives™ on Stack Overflow

Array keeps returning only the last element. [C/Arduino]

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related