I received the following following a build:
In teh FOR loop I have this:
And for 'adc_avg_array I have:
uint16_t adc_avg_array[16]; //16 element storage of ADC readings
I set up a 16 element array of unsigned integers as the ADC result is 12 bits.
As I try and figure out whats up, anyone have any ideas why I am getting this warning? I follow Johans belief that Warnings are potential errors so I really would like this to build clean of course.
JIm
Looks like you should have < rather than <=.
Gcc complains if you don’t have an integer index. Normally you’d use brackets with sizeof() eg sizeof(adc_avg_array)
Normally I’d use memset() to clear your array.
And rather than an average, maybe consider using a low pass filter. Requires less ram and is tunable for the response you want.
Looks like you should have < rather than <=.
I tried your suggestion and I get the same warning back.....
<headscratch>
Jim
you should have < rather than <=
Indeed.
The size (length) of the array is 16 - so the highest index is 15
note that you could just do
memset( adc_avg_array, 0, sizeof adc_avg_array );
And for 'adc_avg_array I have:
uint16_t adc_avg_array[16]; //16 element storage of ADC readingssizeof gives you the number of bytes occupied by the array - not the number of elements.
cpluscon wrote:you should have < rather than <=Indeed.
The size (length) of the array is 16 - so the highest index is 15
note that you could just do
memset( adc_avg_array, 0, sizeof adc_avg_array );
Gcc complains if you don’t have an integer index. Normally you’d use brackets with sizeof() eg sizeof(adc_avg_array)
Normally I’d use memset() to clear your array.
Ok, I can do that, but in teh other warning, in that spot I have:
/********add the array elements together and then divide by size of array********/
for(uint8_t i = 0; i < sizeof(adc_avg_array); i++)
{
adc_average_result = ((adc_average_result) + (adc_avg_array[i]));
}
adc_average_result = ((adc_average_result) / sizeof(adc_avg_array)); //calculate the averageYEah, the math is probably screwy but the warning points to:
adc_average_result = ((adc_average_result) + (adc_avg_array[i]));
which I declared 'adc_average_result' as:
uint16_t adc_average_result;
so I dont know why the compiler is irritated.....
<more head scratching>
Jim
I think #6 and #7 overlapped ?
sizeof gives you the number of bytes occupied by the array - not the number of elements.
Ahhhhh!
Ok, but if I declare the array as 16 integer elements, can I use Sizeof?
JIm
The usual idiom is:
sizeof(adc_avg_array) / sizeof(adc_avg_array[0])
But I would usually do
#define ADC_AVG_LEN 16 uint16_t adc_avg_array[ADC_AVG_LEN];
EDIT
But the memset version works as previously shown - because that one does want the number of bytes.
I think #6 and #7 overlapped ?
Yes, somewhat.
I guess then I need to create a constant that can be compared against instead of using sizeof()?
JIm
LOL - another overlap!
LOL - another overlap!
At least the thoughts are on teh same track!
JIm
The solution seems to be a mix of cpluscon and awneil.
cpluscon was correct in I needed to remove the '=' as it would have had the for loop increment one too many times.
Andys idea of defining a constant to compare against cleared up the type conflict.
Thanks gents. It builds....but does it work? 
JIm
The usual idiom is:sizeof(adc_avg_array) / sizeof(adc_avg_array[0])
So I guess you could do:
#define SIZEOF_ARRAY(X) sizeof(X) / sizeof(X[0])
if you do a lot of this stuff ...
awneil wrote:The usual idiom is:sizeof(adc_avg_array) / sizeof(adc_avg_array[0])So I guess you could do:
#define SIZEOF_ARRAY(X) sizeof(X) / sizeof(X[0])if you do a lot of this stuff ...
Oh boy, here we go again...
I simply did:
#define adc_avg_len 16 //size of adc elements
and then did this:
for(uint8_t i = 0; i < sizeof(adc_avg_len); i++ )
{
adc_avg_array[i] = 0; //write all 0's to each location in the array
}problem went away.
Once I get things going the way I want I can try everyone elses ideas...I need a stable foundation where I know the results before I start getting creative.
JIm
I simply did:
#define adc_avg_len 16 //size of adc elementsand then did this:
for(uint8_t i = 0; i < sizeof(adc_avg_len); i++ ) { adc_avg_array[i] = 0; //write all 0's to each location in the array }
That may build, but won't work.
sizeof(adc_avg_len) will likely be 2 - ie, the size of one single int.
What you need there is:
for(uint8_t i = 0; i < adc_avg_len; i++ )
{
adc_avg_array[i] = 0; //write all 0's to each location in the array
}ie, no sizeof.
I need a stable foundation where I know the results before I start getting creative.
+1
That may build, but won't work.
sizeof(adc_avg_len) will likely be 2 - ie, the size of one single int.
as I read this I realised my boo boo....live and learn.
JIm
so I dont know why the compiler is irritated.....
Another take :
#include <stdint.h>
#define lengthof(x) (sizeof(x)/sizeof(x[0]))
void out(uint16_t val);
int main() {
uint16_t adc_average_result = 0;
uint16_t adc_avg_array[16]; //16 element storage of ADC readings
/********add the array elements together and then divide by size of array********/
for(uint8_t i = 0; i < lengthof(adc_avg_array); i++)
{
adc_average_result = (uint16_t)(adc_average_result + adc_avg_array[i]);
}
adc_average_result = ((adc_average_result) / lengthof(adc_avg_array)); //calculate the average
out(adc_average_result);
}
macro via C Programming/Arrays and strings - Wikibooks, open books for an open world
for(uint8_t i = 0; i < 16; i++)
In reality you can do it this way. Some may object to the redundancy, and the risk of magic number discrepancy. But it is correct, and reads well.
Generally there are two approaches shown here for array count processing. IMO, the sizeof approach should be reserved for arrays that aren't explicitly sized, and may grow or shrink for whatever reason:
int syntaxarray[] = {
// values
};
int nsyntaxarray = sizeof(syntaxarray)/sizeof(int);
//..
for(i=0;i<nsyntaxarray;i++)
In your present case, this is likely the most sensible.
#define NSAMPLES 16 uint16_t array[NSAMPLES]; //... for(i=0;i<NSAMPLES;i++)
I find this monster lurking in the depths of my code:
// // Array Length Calculation // Stolen from https://stackoverflow.com/questions/4415524/common-array-length-macro-for-c // #define ARRAY_LENGTH(arr) ((sizeof(arr)/sizeof(0[arr])) / ((size_t)(!(sizeof(arr) % sizeof(0[arr])))))
TLDR; For those who don't wish to read the original stackoverflow question.
If you pass in a pointer to an array instead of the actual array variable (a common mistake apparently) you'll get a division-by-zero error from the compiler.
awneil wrote:
That may build, but won't work.
sizeof(adc_avg_len) will likely be 2 - ie, the size of one single int.
as I read this I realised my boo boo....live and learn.
JIm
Coding before the design was complete (i hope)!
The sizeof(anything) is of type size_t, which is a positive integer. If a library function can return a negative size_t, it's a ssize_t.
How many bits it is depends on the machine/compiler/etc. I'm currently setup here for a 32 bits so I can't check for AVR. Seems unlikely it's as small as 8 bits.
It may not make sense to compare this with a value of another integral type, e.g. a uint8_t, certainly without a cast.
awneil wrote:sizeof gives you the number of bytes occupied by the array - not the number of elements.
Ahhhhh!
Ok, but if I declare the array as 16 integer elements, can I use Sizeof?
JIm
Use (sizeof(array)/sizeof(the type of the array elements))
Remember that sizeof gives you the declared size, not the occupied size. For example if you declare a 10 byte array to hold a string and you put a 5 byte string plus a null terminator, you still get sizeof returning 10.
Jim
Not an issue for AVR but, in general, sizeof will also include any padding ...
My first thought on reading this was "why are you quoting "Error List"? it usually tells you nothing". If you want to study a build error look at the Output tab where the complete error (over several lines) is shown - not some one line summary that offers no context.
BTW, like others here, I habitually use:
#define NUM_ELEMENTS(x) ( sizeof(x) / sizeof(x[0]) )
then use as:
typedef struct {
int n;
char c;
long l;
} data_t;
data_t my_array[17];
...
for (int i = 0; i < NUM_ELEMENTS(my_array); i++) {
process( my_array[i] );
}of course the alternative is:
typedef struct {
int n;
char c;
long l;
} data_t;
#define ARRY_SIZE 17
data_t my_array[ARRAY_SIZE];
...
for (int i = 0; i < ARRAY_SIZE; i++) {
process( my_array[i] );
}
