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

3
  • What's data? Should you be copying it out instead of just assigning a pointer? Commented May 23, 2013 at 22:09
  • You can try to do a function for it, as you can see in stackoverflow.com/questions/10468128/…. Commented May 23, 2013 at 22:11
  • data is the parameter from the USB function, declared as "uchar *data" Commented May 24, 2013 at 12:06

4 Answers 4

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).

Sign up to request clarification or add additional context in comments.

4 Comments

What happens if the compiler pad my struct and how does it affect the data stored in it? Thanks for your answer.
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).
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!
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.
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.

1 Comment

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.
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.

1 Comment

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.
-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.

6 Comments

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
structs are NOT necessaraly "contigous in memory" - compilers can and do pad structs to do short or word alignment.
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.
But even with padding it should work both ways. The padding will be copied into the array back and forth
@Noxet : volatile doesn't make a difference. volatile is only to prevent the compiler from removing important variables during the optimization process
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.