Go Down

Topic: [Solved] Array of structs or pointer indexing ... Options / Best practice? (Read 9943 times) previous topic - next topic

Dyslexicbloke

starforgelabs, points well understood and accepted in the spirit that they are offered

I have absolutely no issue with being told when I am wrong, even less being told I'v been lucky.
What is a little hard to take is unqualified comments ... You cant do that / You don't understand ...

Don't get me wrong I do not think for a second that I am entitled to or 'deserve' answers, but it gets a little frustrating when some one takes the time to tell me how badly wrong I am without telling me why.

I will go back to my original concept which was an array of types, one member of which is a pointer to a variable that I want refer to by name at runtime.

Can you help me understand this pointer issue a bit further ...
You seem to be saying that I should never increment/decrement pointers because I cant know what is where and cant therefore rely on the information I will get back.
This is because unless the variable is part of an array its location will chance even if its instance is never destroyed once its been created.

Is that correct, It seems counter intuitive ... Am I missing something?
Also why memory allocated to a global variable change in the first place?

Nick,
I would very much welcome you going on about It .. Providing you were telling me how to do things as opposed to how not to.
I have had some advice already, some from you too I think ... There has been so much.
This page http://www.cplusplus.com/reference/clibrary/cstring/ was suggested and I think it will be what I need for now.
If I have any issues I will post specific requests for help.
That said If there is anything I need to know in order to use the functions on that Page please feel free to put me right

If I knew where the box was I would probably still want to think outside it!

Feel free to be blunt ... Its how I learn.

nickgammon

Sadly, memory allocation has bugs at present. Boilerplate:

Please note that, at present, the String library has bugs as discussed here and here.

In particular, the dynamic memory allocation used by the String class may fail and cause random crashes.

I recommend reworking your code to manage without String. Use C-style strings instead (strcpy, strcat, strcmp, etc.).


Even if you code perfectly, the memory bugs will bite you.

I would be happy to make constructive suggestions if I had a better idea of what you are trying to do with your code.
Please post technical questions on the forum, not by personal message. Thanks!

More info: http://www.gammon.com.au/electronics

PeterH


Can you help me understand this pointer issue a bit further ...
You seem to be saying that I should never increment/decrement pointers because I cant know what is where and cant therefore rely on the information I will get back.
This is because unless the variable is part of an array its location will chance even if its instance is never destroyed once its been created.

Is that correct, It seems counter intuitive ... Am I missing something?
Also why memory allocated to a global variable change in the first place?


Pointers hold the memory location of a piece of data. If you have a pointer to a char, then it holds the address of the memory location where a char is stored. You can alter the value of the char and that is fine - the storage location does not change, the pointer does not change.

When you increment a decrement a pointer, you are changing it to refer to a memory location which is adjacent to the original location. The compiler promises to keep elements of an array next to each other, so it is legitimate to use pointer arithmetic to refer to adjacent elements of the array - as long as you avoid going past the end of the array. But if the thing pointed to is not within an array, you the programmer have no way of knowing what is stored next to the original storage location. In that case, incrementing/decrementing the pointer is wrong - you would be changing it to point to an arbitrary piece of memory that you have no reason to assume holds anything in particular.

Having said all that, you do not need to use pointer arithmetic at all to do what you're doing. You will have an array of structs, and each struct will contain a name and a pointer to a variable. You can access each element of the array using foo [ i ] notation so there is no need for pointer arithmetic, and when you come to dereference the pointer you will just use the '*' operator.

Will all your variables be of the same type? If not, your structure ought to hold multiple pointers (one for each type you need to support) of which only one would be non-null for a given item.

PaulS

Quote
and each struct will contain a name and a pointer to a variable.

Even simpler, of course, is for each instance of the struct to contain a name and a value, rather than a pointer to the value.
The art of getting good answers lies in asking good questions.

wildbill

How your example code with the incremented pointer ever worked was bothering me on the drive home. It did, I now suspect, because your junk variables are never used for anything and the compiler was able to optimize them away. Use them for something in the code somewhere and I imagine your example will stop working.

PeterH


Even simpler, of course, is for each instance of the struct to contain a name and a value, rather than a pointer to the value.


I haven't followed the whole discussion, but I understood that the goal was to include dynamic values in text messages. In that case being able to include the current value of a variable (without imposing any complex mechanism to deal with updates to that variable) would best be done by storing a pointer to the variable rather than the value.

Dyslexicbloke

wildbill,
I have tested that scenario and you are correct, using the vars breaks the pointer indexing as you predicted.
Actually that is good to know, because I understand it now.

So I am using arrays like I started with .... Only better now I have some idea what I am doing ...

This is working, thanks everyone.
Code: [Select]
// Test data

float BatVolts = 13.5;
float ACVolts = 236;
float WindSpeed = 3;
float PVCurrent = 20;

typedef struct{                                            // new type (xVar - Exposed variables)
  char Name[10];
  float * Ptr;
}xVar;
float * ValPtr;

xVar ExposedVars[20];                                      // make an array of xVars

void MapFloatName(float * NewVarPtr, char NewName[]){      // function to build array data, fetching the pointer in the process
  static int VarCount = 0;
  ExposedVars[VarCount].Ptr = NewVarPtr;
  strcpy (ExposedVars[VarCount].Name,NewName);
  VarCount++;
}


float * GetVarRef(char VarName[]){                         // function to lookup names and return a pointer to the apropriate variable
  int iLoopCount = 0;
  while(strlen(ExposedVars[iLoopCount].Name) > 0){
   Serial.print(VarName);
   Serial.print("  <->  ");
   Serial.println(ExposedVars[iLoopCount].Name);
   if(strcmp(ExposedVars[iLoopCount].Name,VarName)==0)
   {
     Serial.print(VarName);
     Serial.print(" found @ ");
     Serial.print(iLoopCount);
     return ExposedVars[iLoopCount].Ptr;
   }
    iLoopCount++;   
  }
  Serial.print(VarName);
  Serial.println(" not found");
  return NULL;
}

int freeRam () {
  extern int __heap_start, *__brkval;
  int v;
  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval);
}

void setup()
{
  pinMode(13,OUTPUT);
  digitalWrite(13,HIGH);
  Serial.begin(9600);
  Serial.println(freeRam ());
  MapFloatName(& BatVolts,"BatVolts");
  MapFloatName(& ACVolts,"ACVolts");
  MapFloatName(& BatVolts,"WindSpeed");
  MapFloatName(& ACVolts,"PVCurrent");
  Serial.println(freeRam ());
  Serial.println("");

  ValPtr = GetVarRef("BatVolts");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
 
  ValPtr = GetVarRef("BadName1");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
 
  ValPtr = GetVarRef("ACVolts");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
 
  ValPtr = GetVarRef("PVCurrent");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
 
  ValPtr = GetVarRef("WindSpeed");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
 
  ValPtr = GetVarRef("BadName2");
  if(ValPtr){
  Serial.print("  Value is ");
  Serial.println(* ValPtr);
  }
  Serial.println("");
}

void loop()

digitalWrite(13,!digitalRead(13));
delay(250);
}




If I knew where the box was I would probably still want to think outside it!

Feel free to be blunt ... Its how I learn.

nickgammon

Yes, but it is an incredibly convoluted way of doing it. This does the same thing (maps names to values) and doesn't use pointers (apart from the pointer to the constant string, which is the name of the item). No copying, no fiddling with taking addresses, etc.

Code: [Select]

float BatVolts = 13.5;
float ACVolts = 236;
float WindSpeed = 3;
float PVCurrent = 20;

// number of items in an array
#define NUMITEMS(arg) ((unsigned int) (sizeof (arg) / sizeof (arg [0])))

// structure for name/value mapping
typedef struct
  {                                     
  const char * name;
  float & val;
  } xVar;
 
xVar vals [] = {
  { "BatVolts",  BatVolts },
  { "ACVolts",   ACVolts },
  { "WindSpeed", WindSpeed },
  { "PVCurrent", PVCurrent },
};

void showValues ()
  {
  for (int i = 0; i < NUMITEMS (vals); i++)
    {
    Serial.print (vals [i].name);
    Serial.print (" = ");
    Serial.println (vals [i].val); 
    }
  }  // end of showValues
 
void setup ()
{
  Serial.begin (115200);
  showValues ();
  Serial.println ();
 
  BatVolts = 12;
  ACVolts = 240;
  WindSpeed = 4;
  PVCurrent = 18;
 
  showValues ();
 
} // end of setup

void loop () {}



Output:

Code: [Select]

BatVolts = 13.50
ACVolts = 236.00
WindSpeed = 3.00
PVCurrent = 20.00

BatVolts = 12.00
ACVolts = 240.00
WindSpeed = 4.00
PVCurrent = 18.00



I haven't done looking up names, but if I can print them, I can look one up and return it. I'll leave that as an exercise. ;)
Please post technical questions on the forum, not by personal message. Thanks!

More info: http://www.gammon.com.au/electronics

PaulS

Code: [Select]
void MapFloatName(float * NewVarPtr, char NewName[]){      // function to build array data, fetching the pointer in the process

This function expects a pointer to a float as the first argument.

Code: [Select]
  MapFloatName(& BatVolts,"BatVolts");
BatVolts IS NOT a float. &BatVolts is not a pointer to a float.

You MUST pass the function the correct kind of pointer.

Code: [Select]
  ExposedVars[VarCount].Ptr = NewVarPtr;
Storing a pointer to itself in the struct is silly. If you have the struct instance, because it is the nth element in the array, then you don't need a pointer to the instance. In the GetVarRef() method, you could simply return the index to the instance in the array.

Speaking of GetVarRef(),
Code: [Select]
float * GetVarRef(char VarName[]){                         // function to lookup names and return a pointer to the apropriate variable

     return ExposedVars[iLoopCount].Ptr;

The thing that you made Ptr point to is NOT a float, so Ptr is not a pointer to float, so this function is returning the wrong kind of thing, performing a cast that may not always do what you expect.

As Nick points out, nothing in what you are doing requires pointers at all. The array that you have is all that you need. Return the index into the array where that is appropriate. Access the data in the array instances by index.
The art of getting good answers lies in asking good questions.

Dyslexicbloke

PaulS
I may well be wrong, given my current applicable skills, or lack thereof, the possibility is high.
However I fail to see how your comments are helpful, even if they are accurate which I may not be the case for all of them.
Do you take pleasure from making people, or perhaps just me, look silly?
I have said, and will say again .... I know that I know very little, I welcome criticism and expect to be proven wrong regularly but I would also expect that I would get corrected as opposed to simply being berated.
If you are going to point out an error could you perhaps give some indication as to what the solution might be?
Frankly if you cant be bothered to do that I would prefer that you didn't bother at all.

Quote

Code: [Select]
void MapFloatName(float * NewVarPtr, char NewName[]){      // function to build array data, fetching the pointer in the process
This function expects a pointer to a float as the first argument.

Code: [Select]
MapFloatName(& BatVolts,"BatVolts");
BatVolts IS NOT a float. &BatVolts is not a pointer to a float.
[/quote]
Line 1 of the program ...
Code: [Select]
float BatVolts = 13.5;
How BatVolts not a float?
So how is &BatVolts not a pointer to a float?

SERIOUSLY This is a question not a challenge to your premise.
If I am doing something fundamentally silly, please correct me.

Quote
Code: [Select]
ExposedVars[VarCount].Ptr = NewVarPtr;
Storing a pointer to itself in the struct is silly. If you have the struct instance, because it is the nth element in the array, then you don't need a pointer to the instance. In the GetVarRef() method, you could simply return the index to the instance in the array.


ExposedVars is an array of xVars, xVars being a type structure containing a single char[10], Name, and a float pointer, Ptr.
NewVarPtr is a float pointer, the first argument in MapFloatName(....
The pointer being passed in are:-
Code: [Select]
 
  MapFloatName(& BatVolts,"BatVolts");                              // A pointer to the float BatVolts
  MapFloatName(& ACVolts,"ACVolts");                                // A pointer to the float ACVolts
  MapFloatName(& BatVolts,"WindSpeed");                           // These lines is wrong when I copped and pasted I didn't edit properly.
  MapFloatName(& ACVolts,"PVCurrent");                             // &WindSpeed &PVCurrent is what should have been there but them that is self explanatory really


In all cases, even the erroneous ones, the pointer assigned to NewVarPtr is a global float.
How is that in any way ... Storing a pointer to itself in the struct.

Again please educate me if I am wrong.

Quote
The thing that you made Ptr point to is NOT a float

Well what is it then? ...
If it isn't a pointer then is isn't because of the previous error which you have already pointed out. Twice ... really?
Or perhaps I am being disingenuous and you have spotted another fundamental error, which you haven't told me how to fix!

Quote
As Nick points out, nothing in what you are doing requires pointers at all.

Yes he does ..
He then go's on to offer some really useful looking code examples, although he doesn't comment them so I know what thy do or how/why they work.
Clearly he is showing me a different, probably better, way to define an array. But then if I knew it all meant I would already be using it.

After that his code is puts 'name / value' pares into an array, again using code that isn't explained and clearly beyond my current understanding.
He then points out that because I have name value pares in an array I don't need a pointer.

Well ... I don't understand the construct that is used to define xVar[]
Since Nick is saying that I don't need pointers then I assume that the ' * ' and ' & ' are doing something else.
But that aside I assume my values are now in an array of xVars called Vals so to refer to them I need:-

Code: [Select]
vals [i].val = analogRead(analogPin)   // Or something simmiler

Not very readable is it, for a new guy I mean or perhaps someone coming back to code months after writing it.
O, and my original readable floats ... BatVolts and ACVolts, the ones that I had originally planned to use:-

Code: [Select]
BatVolts = analogRead(analogPin)     // Or something simmiler

When unless I run a loop to copy their values to the corresponding array values, which seems ludicrous from both a time and space perspective, I would have to ditch them.

The point of this was to make both the main code and my display definition files readable and descriptive.
I don't see how Nicks code achieves that whilst the pointers do.

Now Nick may well have come into this conversation late and made some assumptions based on skim reading the thread, we have all don it.
You on the other hand hand know exactly what I am trying to do and why.

It is perhaps not unreasonable to concede that you clever folk may not need descriptive readable code to prevent you from getting lost.
I however, do need it.
I will also freely admit that there may be a better way to do this but to date it hasn't been brought to my attention and I have been discussing elements of these topics for some time now.

Lastly If all the criticism you level at this code is correct then it wouldn't work right ...
Perhaps I haven't tested some scenario that will break it, I'v been there before and been educated.

Again please tell me why its wrong and at the very least where to look for a solution if you cant give me one for some reason.

More than a little frustrated
Al




If I knew where the box was I would probably still want to think outside it!

Feel free to be blunt ... Its how I learn.

PeterH


This does the same thing (maps names to values) and doesn't use pointers (apart from the pointer to the constant string, which is the name of the item). No copying, no fiddling with taking addresses, etc.


It'd never occurred to me to use references other than for parameter passing, but now you've pointed it out that does seem pretty obvious. It's a neat approach, but using pointers instead of references would produce virtually identical code.

PaulS

Quote
Do you take pleasure from making people, or perhaps just me, look silly?

No. I was apparently mixing code from your latest post with some earlier stuff. I see that BatVolts is now a float, so I was wrong.
The art of getting good answers lies in asking good questions.

nickgammon


I don't see how Nicks code achieves that whilst the pointers do.


Yes, I skimmed. Sometimes a rewrite is better. I rewrite my own stuff when I get bogged down. I didn't comment everything because I find comments like this annoying:

Code: [Select]

  Serial.begin (115200);   // start serial comms at 115200 baud


Those comments don't add anything to the code IMHO.




The essence of my suggestion was in this array:

Code: [Select]

xVar vals [] = {
  { "BatVolts",  BatVolts },
  { "ACVolts",   ACVolts },
  { "WindSpeed", WindSpeed },
  { "PVCurrent", PVCurrent },
};


That ties each name (eg. "BatVolts") to a reference to the variable. The variable is not in the array, a reference is. A reference is a fancy C++ way of doing pointers (in effect) without actually have to use the * and & operators, so it is easy to use.

This lets you (as I demonstrated in the code) change the original values, eg.

Code: [Select]

BatVolts = 22;


The reference in the array now "points to" (can I say "refers to"?) the same original variable.

So if someone using the serial port wants to know the "BatVolts" value (by name) you simply scan the array with a simple "for" loop, looking for a string match, and then reply with the variable which is referenced (as I demonstrated with the printing).
Please post technical questions on the forum, not by personal message. Thanks!

More info: http://www.gammon.com.au/electronics

AWOL

#28
Oct 25, 2012, 10:10 pm Last Edit: Oct 25, 2012, 10:26 pm by Nick Gammon Reason: 1
Code: [Select]
{ "PVCurrent", PVCurrent },
A preprocessor stringify operator in a macro could help here.
"Pete, it's a fool (who) looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.
I speak for myself, not Arduino.

nickgammon

Here is my simplified version, using pointers instead of references. The code is almost identical. And it is still much easier to follow:

Code: [Select]

float BatVolts = 13.5;
float ACVolts = 236;
float WindSpeed = 3;
float PVCurrent = 20;

// number of items in an array
#define NUMITEMS(arg) ((unsigned int) (sizeof (arg) / sizeof (arg [0])))

// structure for name/value mapping
typedef struct
  {                                     
  const char * name;
  float * val;
  } xVar;
 
xVar vals [] = {
  { "BatVolts",  &BatVolts },
  { "ACVolts",   &ACVolts },
  { "WindSpeed", &WindSpeed },
  { "PVCurrent", &PVCurrent },
};

void showValues ()
  {
  for (int i = 0; i < NUMITEMS (vals); i++)
    {
    Serial.print (vals [i].name);
    Serial.print (" = ");
    Serial.println (*vals [i].val); 
    }
  }  // end of showValues
 
void setup ()
{
  Serial.begin (115200);
  showValues ();
  Serial.println ();
 
  BatVolts = 12;
  ACVolts = 240;
  WindSpeed = 4;
  PVCurrent = 18;
 
  showValues ();
 
} // end of setup

void loop () {}
Please post technical questions on the forum, not by personal message. Thanks!

More info: http://www.gammon.com.au/electronics

Go Up