2

I'm using an AVR-atmega processor which controls a RTC. In the atmega, the time from RTC is saved as a struct:

typedef struct {
uint8_t    year;
uint8_t    month;
uint8_t    hours;
uint8_t    minutes;
"and so on..."
} time_t;

I have a global variable "time" (volatile time_t *time;) in the atmega.

I have written a C program to send an array with the current time via USB to the atmega in order to set the correct time on the RTC.

in the USB function on the atmega the code is as follows:

time = (void *) data;

where data is the array sent from my C program with the current time. is this the right way to do it? my problem is now that when I try to read the time from the atmega, i.e. the atmega returns my struct, I get random values now and then but most of the time I get time a actually set.

Hope I explained it somewhat understandable..

Thanks

// Noxet

Noxet
  • 236
  • 4
  • 16
  • What's `data`? Should you be copying it out instead of just assigning a pointer? – Carl Norum May 23 '13 at 22:09
  • You can try to do a function for it, as you can see in http://stackoverflow.com/questions/10468128/how-do-you-make-an-array-of-structs-in-c. – Mihai8 May 23 '13 at 22:11
  • data is the parameter from the USB function, declared as "uchar *data" – Noxet May 24 '13 at 12:06

4 Answers4

2

You may be seeing issues due to alignment of the the time struct. The compiler is free to pad your struct if it wants align it on short or word boundaries. You could try instructing the compiler not to do any padding by use of __packed or a #pragam pack. E.g

typedef __packed struct {
uint8_t    length;
uint16_t   weight;
} Fish;

Because this struct is 3 bytes wide some compilers will pad it (either at the beginning or end) to word align it (make it 4 bytes wide) in memory - to improve access speed etc. If its packed it will be strictly 3 bytes wide.

Edit: syntax might be as below for AVR

typedef __attribute__((packed)) struct { ...

I think the point Eric is trying to make (which IS correct) is that you also need to becareful about the scope of your data pointer. If that data is limited in scope i.e will get freed during the lifetime of your use of time then you need to copy the bytes from data to a time_t struct rather than just pointing to memory that may be freeed (as Eric suggests using a memcpy).

Ricibob
  • 7,505
  • 5
  • 46
  • 65
  • What happens if the compiler pad my struct and how does it affect the data stored in it? Thanks for your answer. – Noxet May 24 '13 at 11:55
  • If your time_t struct is say 7 bytes wide the compiler may add in a byte of padding (any where in the struct depending on the field widths, ie at the front, on the end, or in the middle) to word allign the struct in memory. It doesnt effect normal use of the struct - but if you want to interpret (cast) a block of memory as (to) the struct then that byte padding will cause an offset. As a rule if you are going to interpret raw memory as a struct that struct must be packed (no compiler padding). – Ricibob May 24 '13 at 12:28
  • Aha, okej. I'll definitley try this. You might have an answer to my comment above? how my code differs from the code in v-usb. Thanks! – Noxet May 25 '13 at 11:54
  • Casting the data function arg to a struct is fine if your only using the struct inside the function. Its not OK if the struct has a scope/lifetime outside the function call (ie is global) and might see the data its pointing to be cleaned up underneith it - in this case you need to copy. If its a padding/alignment or endiness issue the 2 methods (both pointer assignent and copy) will produce the same (wrong) result. – Ricibob May 27 '13 at 12:18
2

well we have 3 issues at play here that I will address one by one:

1 - alignement in a struct an architecture's ABI can specify that all values be aligned on some arbitrary word boundary, so a struct like:

struct TheStruct{
    char a;
    int  man;
}

may really be stored in memory as (assuming 32 bit int and word):

char [0] 
pad  [1]
pad  [2]
pad  [3] 
int  [4]
int  [5]
int  [6]
int  [7]

This can be different in AVR and intel...

2 - endianness

I think both AVR and Intel will be little endian... so this likely wont be an issue here, but:

A computer storing the 32 bit int 0x01234567 could store this in memory as:
[0x01] [0x23] [0x45] [0x67] big endian or
[0x67] [0x45] [0x23] [0x01] little endian

3 - network vs host byte order
I really am not sure how this gets sorted out throught the USB drivers, but it is possible that going the the uart will switch its endianness again... this is a subset of endianness see: wikipedia

If I were you I would write code to deal with the buffers in the bytes precisely:

int value1 = 0x1234567

char * buffer =  calloc(1,bufferSize);
buffer[0] = value1 & 0xff000000 > 6;
buffer[1] = value1 & 0x00ff0000 > 4;
buffer[2] = value1 & 0x0000ff00 > 2;
buffer[3] = value1 & 0x000000ff;
...

it is a little tiresome, and might be easier to do in assembly really, but I think that is the best way to get a solid data interchange.. be sure to document it.

Grady Player
  • 14,399
  • 2
  • 48
  • 76
  • Thanks for your answer, i will look these things up. I just did it this way because in the usbFunctionSetup used by v-usb in the AVR I have "uchar data[8]" as parameter and in the documentation they set their struct variable "usbRequest_t *rq" equal to data i.e. usbRequest_t *rq = (void *) data; and there it works fine but when i try to do it the way i did, it just goes bananas.. I don't see the difference between my code and their. – Noxet May 25 '13 at 14:03
0

So it's almost 10 years ago, but I believe the basic issue is that you did not allocate memory for the time_t structure. You allocated a pointer to the structure, and then used that pointer to copy the structure contents, but still don't have memory allocated for the structure data. So, sometimes it works, and other times it gets overwritten with other data after you wrote to some random data area. You can allocate data, then have a variable that is a pointer to the struct:

typedef struct {
uint8_t    year;
uint8_t    month;
uint8_t    hours;
uint8_t    minutes;
"and so on..."
} time_t;

time_t st_time;  // allocate memory for structure time (time_t is a type)
time_t *ptr_time = &st_time;  // for example usage below

void set_time_first_way( time_t *pt ) {  // correct way
  st_time = *pt;  // copy by dereferencing pointer
}

void set_time_second_way( time_t *pt ) {  // bad way
  ptr_time = pt;  // copy address by setting pointers
}

But in this second case you just saved the calling address to ptr_time (overwriting the address to the actual st_time structure). That's probably not what you want.

dminear
  • 111
  • 6
  • Who doesn't love a good necro post xD But no, actually that was not the issue at all. The data from the RTC was stored in a buffer (allocated). The main issue was due to alignment. I naively assumed that all variables in a struct lie next to eachother without padding, which is of course not always the case. – Noxet May 12 '23 at 21:13
-2

Try something like

char data[10];
struct time_t myTime;
myTime.year = 2013; 
// ... snip
memcpy(data, myTime, sizeof(myTime)); // copy myTime to the array

To receive it, it is the same operation with the parameters reversed

memcpy(myTime, data, sizeof(myTime)); // copy data into myTime

Note that as soon as you have pointers in your struct, this trick will no longer work.

Eric
  • 19,525
  • 19
  • 84
  • 147
  • Ok, but I can still declare my variable as "voltile time_t *time" ? I don't have pointers inside my struct so i guess it should be okey. Would there be a difference if I declared my variable as "volatile struct time_t time"? Thanks // Nox – Noxet May 24 '13 at 11:51
  • 1
    structs are NOT necessaraly "contigous in memory" - compilers can and do pad structs to do short or word alignment. – Ricibob May 24 '13 at 12:33
  • Your answer may in fact be correct - in that it might just be a relative lifetime/scope issue on 'data' vs 'time' because he is not copying the data - so that is fine. Its just contigous statements thats illegit. – Ricibob May 24 '13 at 12:54
  • But even with padding it should work both ways. The padding will be copied into the array back and forth – Eric May 24 '13 at 14:15
  • @Noxet : volatile doesn't make a difference. volatile is only to prevent the compiler from removing important variables during the optimization process – Eric May 24 '13 at 18:56
  • Thank you for your answers. The reason I set the struct equal to the variable data is that it's done this way in the USB function "usbFunctionSetup" where the incoming data is "char data[8]" and you set the struct "usbRequest_t" equal to the data i.e. usbRequest_t = (void *) data; so I thought I would just do the same thing. but since my code does not work, what is the difference? – Noxet May 25 '13 at 11:53