Passing A Pointer - Have I improved things or made it worse?

Thread Starter

TechWise

Joined Aug 24, 2018
124
As I alluded to in a previous thread, I am trying to up my C skill level and write more efficient and robust code. I have run into another question. Let's say I am using my ADCs to measure three quantities. I've defined a struct to neatly wrap them up:
Code:
typedef struct measurements
{
    float i_a;
    float i_b;
    float i_c;
} measurements;
In main(), I create two instances of this struct: one to contain the raw results from the ADC, the other to contain the calibrated measurements which have been shifted and scaled:
Code:
measurements raw_measurements;  // Contains the raw ADC conversion results
measurements latest_measurements; // Contains the shifted and scaled measurements
I want to write a function to take the raw measurements and do the shifting and scaling, by which point "latest_measurements" should contain the correctly calibrated values. The most obvious solution seemed to be to pass "raw_measurements" into a function, create a temporary variable to hold the results then return it at the end:
Code:
measurements calibrate_measurements(measurements raw)
{
    measurements temp;
    temp.i_a = (raw.i_a - CURRENT_A_OFFSET) * CURRENT_A_GAIN;
    temp.i_b = (raw.i_b - CURRENT_B_OFFSET) * CURRENT_B_GAIN;
    temp.i_c = (raw.i_c - CURRENT_C_OFFSET) * CURRENT_C_GAIN;

    return temp;
}
Then in main(), I assign this to the "latest_measurements" variable:
Code:
latest_measurements = calibrate_measurements(raw_measurements);
Question: Is this a good and/or standard approach? I am thinking now that I could avoid the overhead of creating this temporary variable by passing two pointers as arguments: a pointer to the "raw_measurements" and a pointer to the "latest_measurements". What would be the pros and cons of this? I know that @WBahn mentioned in a previous thread that there would be "some access overhead and that might be too much" so perhaps could elaborate on this?
 

MrChips

Joined Oct 2, 2009
21,820
My practice would be to pass both arguments in the function call.

calibrate_measurements(raw_measurements, latest_measurements);
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
My practice would be to pass both arguments in the function call.

calibrate_measurements(raw_measurements, latest_measurements);
My understanding of this is that it would be "passing by value", during which the contents of "raw_measurements" and "latest_measurements" would be copied into the function and the original would be left alone. Then, after the function exits, the variables inside it would be destroyed and the original "raw_measurements" and "latest_measurements" would retain their original values outside the function.

Do you mean I should pass by reference so that the function declaration becomes
Code:
void calibrate_measurements(measurements *latest, measurements *raw)
and then call the function with:
Code:
calibrate_measurements(&latest_measurements, &raw_measurements,);
so that the actual contents of "latest_measurements" are altered rather than just copying them into the function and destroying them when it exits?
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
The most efficient way to do it is to pass both as pointers.

Bob
You mean as I've done in post 3 or by actually declaring the two structs as pointers, passing them as pointers and then dereferencing inside the function?

Like this:
Code:
measurements * raw_measurements;  // Contains the raw ADC conversion results
measurements * latest_measurements; // Contains the shifted and scaled measurements
Then the function would be:
Code:
void calibrate_measurements(measurements * raw, measurements * latest)
{
    latest->i_a = (raw->i_a - CURRENT_A_OFFSET) * CURRENT_A_GAIN;
    latest->i_b = (raw->i_b - CURRENT_B_OFFSET) * CURRENT_C_GAIN;
    latest->i_c = (raw->i_c - CURRENT_C_OFFSET) * CURRENT_C_GAIN;

    return;
}
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
Assuming that once converted to usable floats you don't need the raw A/D value again you could consider defining your measurements variables as unions of type int and float and let the compiler manage the conversion that way you lose the redundant data and your not held to a pointer data type.

https://www.tutorialspoint.com/cprogramming/c_unions.htm
This is an interesting possibility that I hadn't really thought of. You're right that once I cast the raw result to a float, I don't need the raw result anymore. So in a case like this, the compiler would assign enough space for a float which would initially only be partly filled by a 16-bit integer then be overwritten by a float.

The only problem I can see is that the integer is needed to calculate the float, so I would need to assign it to a temporary variable while the calculations were done then overwrite it with the float which seems to defeat the purpose, unless I'm not understanding correctly.
 

Marc Sugrue

Joined Jan 19, 2018
149
This is an interesting possibility that I hadn't really thought of. You're right that once I cast the raw result to a float, I don't need the raw result anymore. So in a case like this, the compiler would assign enough space for a float which would initially only be partly filled by a 16-bit integer then be overwritten by a float.

The only problem I can see is that the integer is needed to calculate the float, so I would need to assign it to a temporary variable while the calculations were done then overwrite it with the float which seems to defeat the purpose, unless I'm not understanding correctly.
I don't think you would have a problem, i recall doing something similar on an embedded system without an issue, the compiler will load the data into registers before writing it back. I just checked i did something like this:

union AtoDDATA {
unsigned int Value_uint;
float Value_f;
};

typedef struct Measurements{
union AtoDDATA a; // Now float or uint
union AtoDDATA b; // Now float or uint
union AtoDDATA c; // Now float or uint
} measurements;

//Foat is between 0 & 1

measurements->a.Value_uint = (unsigned int) (measurements->a.Value_f * 65535.0);
 
Last edited:

WBahn

Joined Mar 31, 2012
26,141
As I alluded to in a previous thread, I am trying to up my C skill level and write more efficient and robust code. I have run into another question. Let's say I am using my ADCs to measure three quantities. I've defined a struct to neatly wrap them up:
Code:
typedef struct measurements
{
    float i_a;
    float i_b;
    float i_c;
} measurements;
In main(), I create two instances of this struct: one to contain the raw results from the ADC, the other to contain the calibrated measurements which have been shifted and scaled:
Code:
measurements raw_measurements;  // Contains the raw ADC conversion results
measurements latest_measurements; // Contains the shifted and scaled measurements
I want to write a function to take the raw measurements and do the shifting and scaling, by which point "latest_measurements" should contain the correctly calibrated values. The most obvious solution seemed to be to pass "raw_measurements" into a function, create a temporary variable to hold the results then return it at the end:
Code:
measurements calibrate_measurements(measurements raw)
{
    measurements temp;
    temp.i_a = (raw.i_a - CURRENT_A_OFFSET) * CURRENT_A_GAIN;
    temp.i_b = (raw.i_b - CURRENT_B_OFFSET) * CURRENT_B_GAIN;
    temp.i_c = (raw.i_c - CURRENT_C_OFFSET) * CURRENT_C_GAIN;

    return temp;
}
Then in main(), I assign this to the "latest_measurements" variable:
Code:
latest_measurements = calibrate_measurements(raw_measurements);
Question: Is this a good and/or standard approach? I am thinking now that I could avoid the overhead of creating this temporary variable by passing two pointers as arguments: a pointer to the "raw_measurements" and a pointer to the "latest_measurements". What would be the pros and cons of this? I know that @WBahn mentioned in a previous thread that there would be "some access overhead and that might be too much" so perhaps could elaborate on this?
In general you want to avoid returning structures for the same reason that you want to avoid passing them by value -- they will chew up your stack and it takes time to push and pop them from the stack.

Instead, pass two pointers to your function, one for the new and one for the old.

Any time that you are using the dot derefence operator (i.e., temp.i_a), you are probably not doing it the way you should (there are always exceptions).

Instead, work with pointers to the structures.

Code:
void calibrate_measurements(measurements *cal, measurements *raw)
{
    cal->i_a = (raw->i_a - CURRENT_A_OFFSET) * CURRENT_A_GAIN;
    cal->i_b = (raw->i_b - CURRENT_B_OFFSET) * CURRENT_B_GAIN;
    cal->i_c = (raw->i_c- CURRENT_C_OFFSET) * CURRENT_C_GAIN;
}
The key to remember is that if you only work with pointers to structures, you need to dynamically allocate those structures before you use them the first time.
 

MrChips

Joined Oct 2, 2009
21,820
Do you mean I should pass by reference so that the function declaration becomes
Code:
void calibrate_measurements(measurements * latest, measurements *raw)
and then call the function with:
Code:
calibrate_measurements(&latest_measurements, &raw_measurements,);
so that the actual contents of "latest_measurements" are altered rather than just copying them into the function and destroying them when it exits?
You are correct. My mistake.
 

Analog Ground

Joined Apr 24, 2019
416
Perhaps another option is to put the raw and latest values in the same structure or make a structure of structures containing two of the measurement structures. Then, only one pointer is passed to the function. This approach may be attractive if the raw and latest values are tightly associated and retained together. It also may produce more efficient code since all values are (most likely) contiguous in memory.
 

BobTPH

Joined Jun 5, 2013
2,534
Unless you are taking 3 measurements so rapidly that you cannot afford the conversion between them, why not simply convert each value as it is read, never storing the raw count?

Bob
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
Unless you are taking 3 measurements so rapidly that you cannot afford the conversion between them, why not simply convert each value as it is read, never storing the raw count?

Bob
I'm actually taking 7 measurements simultaneously and I'm trying to keep the ISR as short as possible. I will probably do a very crude overcurrent protection check on the raw value so by the time I've done those comparisons that will add more computation to the ISR. The advice from TI was to keep the ISR as short as possible as it's not so easy to nest interrupts on this device.
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
In general you want to avoid returning structures for the same reason that you want to avoid passing them by value -- they will chew up your stack and it takes time to push and pop them from the stack.

Instead, pass two pointers to your function, one for the new and one for the old.

Any time that you are using the dot derefence operator (i.e., temp.i_a), you are probably not doing it the way you should (there are always exceptions).

Instead, work with pointers to the structures.

Code:
void calibrate_measurements(measurements *cal, measurements *raw)
{
    cal->i_a = (raw->i_a - CURRENT_A_OFFSET) * CURRENT_A_GAIN;
    cal->i_b = (raw->i_b - CURRENT_B_OFFSET) * CURRENT_B_GAIN;
    cal->i_c = (raw->i_c- CURRENT_C_OFFSET) * CURRENT_C_GAIN;
}
The key to remember is that if you only work with pointers to structures, you need to dynamically allocate those structures before you use them the first time.
Thanks for this. The code you've suggested above is my preferred suggestion for now as it's similar to what I have and easy for me to understand.
Could you clarify your final point for me? If the structure only contains a fixed number of fixed sized floats, and I create an instance of the structure before referring to it, why would it need to be dynamically allocated?
 

Analog Ground

Joined Apr 24, 2019
416
I'm actually taking 7 measurements simultaneously and I'm trying to keep the ISR as short as possible. I will probably do a very crude overcurrent protection check on the raw value so by the time I've done those comparisons that will add more computation to the ISR. The advice from TI was to keep the ISR as short as possible as it's not so easy to nest interrupts on this device.
I would keep the raw values, at least for awhile, for test and debug.
 

Thread Starter

TechWise

Joined Aug 24, 2018
124
Perhaps another option is to put the raw and latest values in the same structure or make a structure of structures containing two of the measurement structures. Then, only one pointer is passed to the function. This approach may be attractive if the raw and latest values are tightly associated and retained together. It also may produce more efficient code since all values are (most likely) contiguous in memory.
I had considered this and I had also considered having a struct for voltage measurements, one for current measurements and one for auxilliary inputs and then placing those inside a larger struct which would contain all the measurements. I decided against it at the finish up as there seemed to be no reason to further classify the measurements as my controller makes use of all of them at once anyway.
 

Analog Ground

Joined Apr 24, 2019
416
I had considered this and I had also considered having a struct for voltage measurements, one for current measurements and one for auxilliary inputs and then placing those inside a larger struct which would contain all the measurements. I decided against it at the finish up as there seemed to be no reason to further classify the measurements as my controller makes use of all of them at once anyway.
I might combine all the measurements into one structure, raw and calibrated, if for no other reason than it makes it very easy to see all the values at once with a source level debugger. Especially if all the data is obtained in a single input scan. Also, I can easily control alignment of the data in memory. Passing around one pointer might prove convenient. Just a matter of taste and style.
 

WBahn

Joined Mar 31, 2012
26,141
Thanks for this. The code you've suggested above is my preferred suggestion for now as it's similar to what I have and easy for me to understand.
Could you clarify your final point for me? If the structure only contains a fixed number of fixed sized floats, and I create an instance of the structure before referring to it, why would it need to be dynamically allocated?
When you declare your structure and your typedef, you are only telling the compiler what one of these things will look like, but not actually allocating any memory for it.
 

xox

Joined Sep 8, 2017
495
When you declare your structure and your typedef, you are only telling the compiler what one of these things will look like, but not actually allocating any memory for it.
Why do you think the dynamic allocation would be necessary though? The variables "raw_measurements" and "latest_measurements" would be allocated on the stack, so that should be sufficient. He could then pass them to the function as described in post #3.
 

WBahn

Joined Mar 31, 2012
26,141
Why do you think the dynamic allocation would be necessary though? The variables "raw_measurements" and "latest_measurements" would be allocated on the stack, so that should be sufficient. He could then pass them to the function as described in post #3.
True. Do note that the context of saying that they would be need to be dynamically allocated was that they are never dereferenced using the dot operator, only the arrow. Now, if you allocate them in main() -- or made them global -- and never dereferenced them in main, you could get away with that. There's a lot to be said for keeping the main() as lean as possible, though that can go against the notion of max performance in an embedded application.
 
Top