I can't figure this out

Post here about your Arduino projects, get help - for Adafruit customers!

Moderators: adafruit_support_bill, adafruit

Please be positive and constructive with your questions and comments.
Locked
User avatar
sting
 
Posts: 173
Joined: Sun Jan 17, 2010 7:44 pm

I can't figure this out

Post by sting »

I have several projects that use the same set of code and I am attempting to build a library so I don't have to duplicate the code that is the same. My problem is, in the code that I have that works, it makes assumptions about the size of arrays that is not the same in all of the applications.

This is what works

Code: Select all

#define deviceCnt 4
#define thermoSize 8

byte thermo[deviceCnt][thermoSize];

void lookUpThermo(){
  //Serial.println("--Thermo Search started--");
  int thermoIndex = 0;
  while (ow.search(thermo[thermoIndex])){
    ...
  }
}
I am attempting to do something like, but It seems that I don't understand memory allocation enough

Code: Select all

#define deviceCnt 4
#define thermoSize 8

byte *thermo;

void setup() {
  thermo = (byte*) malloc(deviceCnt * thermoSize * sizeof(byte));
  ...
}

void lookUpThermo(){
  //Serial.println("--Thermo Search started--");
  thermoIndex = 0;
  //tell search to go to the appropriate location in array
  while (ow.search((byte*)thermo[thermoIndex * thermoSize])){
    ...
  }
Any suggestions would be appreciated
Thanks in advance

User avatar
danmalec
 
Posts: 13
Joined: Fri Nov 18, 2011 3:53 pm

Re: I can't figure this out

Post by danmalec »

One approach would be to keep thermo as a 2D array thusly:

Code: Select all

byte **thermo;

void setup() {
  thermo = (byte **)malloc(deviceCnt * sizeof(byte *));

  int row;
  for (row=0; row<deviceCnt; row++) {
    thermo[row] = (byte *)malloc(thermoSize * sizeof(byte));
  }
  ...
}
I've had success using this myself in toy projects; but, I haven't run any sort of longevity tests on it, so I can't speak to how nicely the dynamically allocated memory works over time (this approach won't necessarily give one contiguous block of memory the way your approach would).

User avatar
adafruit_support_bill
 
Posts: 88086
Joined: Sat Feb 07, 2009 10:11 am

Re: I can't figure this out

Post by adafruit_support_bill »

I can't speak to how nicely the dynamically allocated memory works over time
Shouldn't be an issue if it is only done in setup(). It is best to avoid it in the loop() though. With limited memory, the heap can easily get fragmented beyond usability.

User avatar
westfw
 
Posts: 2008
Joined: Fri Apr 27, 2007 1:01 pm

Re: I can't figure this out

Post by westfw »

You're probably running into a disagreement with the compiler on whether the array is organized by columns or by rows... Since you'd rather have a two-dimensional array, I'd suggest dispensing with the idea of doing the indexing yourself, and have the compiler do all the work. For instance (this is a unix program; I compiled it and it seems to do the right thing):

Code: Select all

#include <stdio.h>
#include <stdlib.h>

#define deviceCnt 4
#define thermoSize 8

typedef struct thermo {int a[deviceCnt][thermoSize] ;} thermo_array_t;

thermo_array_t *thermo;

void setup() {
    int i,j;
    thermo = (thermo_array_t *) malloc(sizeof(*thermo));
    for (i=0; i < deviceCnt; i++) {
        for (j=0; j<thermoSize; j++) {
            thermo->a[i][j] = 100*i + j;
        }
    }
}

void printThermo(int a[thermoSize])
{
    int i;
    for (i=0; i<thermoSize; i++) {
        printf(" %d", a[i]);
    }
}

int main() {    
    int device;

    setup();
    for (device=0; device<deviceCnt; device++) {
        printf("Device data for #%d:", device);
        printThermo(thermo->a[device]);
        printf("\n");
    }
}

Code: Select all

 ./a.out
Device data for #0: 0 1 2 3 4 5 6 7
Device data for #1: 100 101 102 103 104 105 106 107
Device data for #2: 200 201 202 203 204 205 206 207
Device data for #3: 300 301 302 303 304 305 306 307
I don't think there is any overhead associated with wrapping a structure around the array, and it make things a lot clearer (at least to me.)

User avatar
philba
 
Posts: 387
Joined: Mon Dec 19, 2011 6:59 pm

Re: I can't figure this out

Post by philba »

Which dimension in the array changes? If it's both, there is no way to make one piece of code work properly for just a 2 dim array since at least one dimension size must be known at compile time. I would do something similar to what wes suggests but create a structure per device that holds a one dimensional array of device samples. Then create an array of structure pointers that point to the individual malloc'd device structures. This way you can have differing sample set sizes, even in the same program. You could even put the sample size in the structure to do your own bounds checking. That way, it's unambiguous (to a human and the compiler) what's going on. I have to admit - >30 years of c/c++ and I still can't remember if C is row or column major.

Code: Select all

struct t_thermoDevice { 
    int sampleCount;
    int samples[1];
};

t_thermoDevice *tdevices[NDEVICES];

void setup(){
    for(int i=0;i<NDEVICES;i++) {
        tdevices[i] = (t_thermoDevice *) malloc( sizeof(t_thermoDevice) + sizeof(int)*(NSAMPLES-1));
        tdevices[i]->sampleCount = NSAMPLES;
    }
}
The samples array size of 1 is a fiction to allow the compiler to know that it's a linear array. No bounds checking is done. so, tdevices->samples[20] would work just fine assuming there is data there.

User avatar
danmalec
 
Posts: 13
Joined: Fri Nov 18, 2011 3:53 pm

Re: I can't figure this out

Post by danmalec »

@adafruit_support - gotcha, I'm still adapting to the limited memory constraint. Thanks for confirming it should be OK in a setup method.

@philba
there is no way to make one piece of code work properly for just a 2 dim array since at least one dimension size must be known at compile time.
I follow you on needing more data than just the array for the code to work properly, but I'm missing the point you're making about needing to know at least one dimension at compile time. As long as the run time code keeps and respects the lengths (in a struct or otherwise), I'm not seeing where having both dimensions dynamic introduces issues in and of itself (design-wise, I like your approach of bundling the lengths within the struct and allowing flexibility in sample set sizes). Randomly sizing a char array:

Code: Select all

char **thermo;
int deviceCnt;
int thermoSize;

void setup() {
  int deviceIdx, thermoIdx;
  char data;
  
  // Determine array size at runtime
  randomSeed(analogRead(0));  
  deviceCnt = 1 + random(4);
  thermoSize = 1 + random(4); 

  // Allocate an array of pointers
  thermo = (char **)malloc(deviceCnt * sizeof(char *));
  
  // Allocate an array of data in each array entry
  for (deviceIdx=0; deviceIdx<deviceCnt; deviceIdx++) {
    thermo[deviceIdx] = (char *)malloc(thermoSize * sizeof(char));
  }
  
  // Populate resulting array
  data = 'A';
  for (deviceIdx=0; deviceIdx<deviceCnt; deviceIdx++) {
    for (thermoIdx=0; thermoIdx<thermoSize; thermoIdx++) {
      thermo[deviceIdx][thermoIdx] = data++;
    }
  }
  
  Serial.begin(9600);
}

void print_row(char array[], int len) {
  int idx;
  
  Serial.print("ROW_FUNC: ");
  for (idx=0; idx<len; idx++) {
    Serial.print(array[idx]);
  }
  Serial.println();
}

void loop() {
  Serial.print("deviceCnt: ");
  Serial.println(deviceCnt);
  Serial.print("thermoSize: ");
  Serial.println(thermoSize);
  
  int deviceIdx, thermoIdx;

  // Print out array directly
  for (deviceIdx=0; deviceIdx<deviceCnt; deviceIdx++) {
    Serial.print("ROW: ");
    for (thermoIdx=0; thermoIdx<thermoSize; thermoIdx++) {
      Serial.print(thermo[deviceIdx][thermoIdx]);
    }
    Serial.println();
  }

  // Pass array to function to print
  for (deviceIdx=0; deviceIdx<deviceCnt; deviceIdx++) {
    print_row(thermo[deviceIdx], thermoSize);
  }
  
  delay(2000);
}
Yields the following in the serial monitor on a sample run:

Code: Select all

deviceCnt: 3
thermoSize: 2
ROW: AB
ROW: CD
ROW: EF
ROW_FUNC: AB
ROW_FUNC: CD
ROW_FUNC: EF

User avatar
philba
 
Posts: 387
Joined: Mon Dec 19, 2011 6:59 pm

Re: I can't figure this out

Post by philba »

Say you declare char a[3][4]; That makes for 3 rows of 4 columns (or is it 4 and 3? doesn't matter for this discussion) and allocates a 12 byte block of memory, So, to index a[j], you need to know at least the number of columns to compute 4*i+j. If the compiler doesn't know the number of columns, it can't figure it out. That value must be know at compile time. There's no way to tell compiled C code the column size. You could overlay the array with a linear array and do the math yourself but that's kind of clumsy

Now, since Arduino libraries get recompiled along with the code that uses them, it is possible to arrange to have a define constant for your array sizes but that feels clumsy, too.

What I like about the solution I outlined is that it's only a short step from that to an object oriented solution where you create a class object "Thermo" and attach a set of methods to it. So you can have things like thermo1.scaleFarenheit(), thermo1.filter(), thermo1.addSample(x) or some such. what ever you want to do. Could be some nicely elegant code and makes a library a lot more user friendly.

User avatar
danmalec
 
Posts: 13
Joined: Fri Nov 18, 2011 3:53 pm

Re: I can't figure this out

Post by danmalec »

I'm afraid we may be talking past each other a bit... I agree with you that the solution you outlined is closer to an object oriented approach and is headed in a cleaner direction than the 2D array (especially when talking about building a library). Also, I'm in agreement with you on the compiler needing the column count for a statically allocated array.

Ignoring larger design issues for a minute, my claim is that we can get something that behaves like a dynamically sized 2D array in code (tho in fact it is an array of references to arrays) and which the compiler will handle correctly. From my code sample above:
  • define char **a
    assign a malloc'ed array of char *'s of length m into a
    then assign a malloc'ed array of char of length n into each entry in a.
Later on, the compiler will see a[j] as - take the pointer value in a, add i (pointer math, so really i * sizeof(char *)), dereference the value, add j (pointer math, really j * sizeof(char)), and dereference the value. The compiler won't need to know the value of n since the type of a won't be an array of arrays of size n.

User avatar
philba
 
Posts: 387
Joined: Mon Dec 19, 2011 6:59 pm

Re: I can't figure this out

Post by philba »

Yes, I see what you are doing. And, it appears to work correctly in GCC. I went looking at the specifications (GCC, c99 and the 2008 ISO draft) and couldn't find it explicitly called out. It seems that it's well embedded in the informal definition of C all the way back to the K&R compiler in the early 70s. main(argv, argc), with argv declared as char **, for example. So, it's probably not going to break anytime soon. Personally, I'd opt for a design that is simpler to understand, especially if newbies will be seeing it.

Locked
Please be positive and constructive with your questions and comments.

Return to “Arduino”