Is my method of passing a struct as both value and reference correct?

Question

I have looked at the examples of passing a struct by both value and reference. My code compiles but is not working as it should. I am using C to program a micro-controller so it is hard to check if it is working properly, but I am not getting the desired output.

So, as per instructions, I first define my structure:

 struct package //define a structure type called package.
{
    unsigned char 
            wavType,startFreq1,startFreq2,startFreq3,startFreq4,
            stopFreq1,stopFreq2,
            stopFreq3,stopFreq4,step,dura,amp,sett; //define bytes to use 
};

In the main method I create an instance of it:

    struct package p; //create a new instance of Package

Now I pass it by reference (pointer - because I'm using C) to a function:

   getPackage(&p);

Within the function getpackage() I update the values of the respective elements of p:

 getPackage(struct package *p) //Get data package
{
p->wavType = receive();
p->startFreq1 = receive();
p->startFreq2 = receive();
p->startFreq3 = receive();
p->startFreq4 = receive();
p->stopFreq1 = receive();
p->stopFreq2 = receive();
p->stopFreq3 = receive();
p->stopFreq4 = receive();
p->step = receive();
p->dura = receive();
p->amp = receive();
p->sett = receive();

}

This is the receive function:

unsigned char receive(void)
{
 unsigned char dataR = 0x00;

for(signed char i = 0; i <=7 ;i++)
{

    dataR |= PORTBbits.RB1 << i; //move the value on the data pin to a bit in dataR

}

return dataR;
}

QUESTION: Will this correctly update the bytes in the package p? Also, does package p need to be returned if I want to use it elsewhere? I ask this because....

I now pass the package p, by value, into another function using:

sendSine(p);

This function makes use of the value of the bytes in the package p:

void sendSine(struct package p)
{

dataL = p.startFreq1; 
dataH = p.startFreq2; 
send(dataL,dataH);

dataL = p.startFreq3; 
dataH = p.startFreq4;
send(dataL,dataH);

}

I know the function send(dataL,dataH) is working because I have tested it by setting dataL and dataH by hand and I get the required result, so there must be an error along the way with the struct - I just cant figure out where... Can anyone help me with were it might be?

`dataR |= PORTBbits.RB1 << i;` - I don't know what you are trying to achieve using this, but it seems wrong. — , Jul 31 '13 at 19:55
As @H2CO3 suggests, make sure your `receive()` function is returning the data you are expecting to get. — jxh, Jul 31 '13 at 20:00
You mentioned "not working as it should". What errant activity have you noticed? — chux - Reinstate Monica, Jul 31 '13 at 20:00
Consider 'unsigned char x = PORTBbits.RB1; dataR |= x << i;`. You may have trouble shifting a 1 bit field. — chux - Reinstate Monica, Jul 31 '13 at 20:02
@H2CO3 By using: dataR |= PORTBbits.RB1 << i; am trying to move the value (be it a 1 or 0) in to dataR at the place specified by i. When I send I use: PORTAbits.RA5 = (dataH >> i) & 1 to move the bit in position i of dataH on to a port. I had thought (due to research) that this was the correct method of writing to a bit in a byte, if this is incorrect can you tell me the correct method please? In short, I am trying to write to a bit in a byte, at the position "i" — Tim M, Jul 31 '13 at 20:05
@TimMottram This implies that the status of RB1 changes to the next bit (assuming serial communication) after you read it. Is that the case? — , Jul 31 '13 at 20:08
The behaviour I am experiencing is that, after sending the data, the chip I am sending it to does not behave as expected is the data I had sent was correct. If you want more info let me know, but its all electronics.. — Tim M, Jul 31 '13 at 20:09
I'd recommend you to define your `signed char i` outside the for loop, most compilers would at least throw a warning depending on how it's being compiled. Now, you cannot pass a struct by value, it won't know what to do because there's no predefined function to copy your struct by value. — user2553780, Jul 31 '13 at 19:55
"I'd recommend you to define your signed char i outside the for loop" - why? - "Now, you cannot pass a struct by value" - what if I told you that you can? — , Jul 31 '13 at 19:56
@H2CO3 As for your first question, it's answered right there. As for the second question, I made a mistake but I haven't seen anywhere passing structs by value. I assume it's not good practice and probably would introduce awful performance specially if he's copiling for a microcontroller. — user2553780, Jul 31 '13 at 20:00
@H2CO3 yes this correct, I left out the clock and fsync parts as they were not part of the problem. Each time i pass through the loop the data is changing, also, I have checked that the data is correct (by watching the data line and the clock line). I wait for the clock to drop, then move the data bit into the i'th position of dataR. But if, as you suggest, my method of writing to a bit in a byte is incorrect then please show me the correct method :) — Tim M, Jul 31 '13 at 20:11
Hmm, do you think then that it is a problem with the assignment: I.E p->wavType = receive(); Will this (assuming that dataR populated correctly) set the value of wavType to the whatever was in dataR? — Tim M, Jul 31 '13 at 20:16
Again, avoid passing structs by value. See http://stackoverflow.com/questions/161788/are-there-any-downsides-to-passing-structs-by-value-in-c-rather-than-passing-a. People who have never built applications on embedded systems may say all sort of things, but on embedded systems is important to pay attention to good programming practices. — user2553780, Jul 31 '13 at 20:27
@user2553780 Ok, I'll look into that. However, when I had passed the struct as a pointer, when I tried to do, for example, dataH = p.wavType; I got an assignment error to do with it being a pointer..? — Tim M, Jul 31 '13 at 21:01
@Tim, if you have a pointer-to-struct, you access the content of the struct using the `->` operator: `p->wavType`. The dot operator is for direct structs, not pointers to. — This isn't my real name, Jul 31 '13 at 21:03
@user2553780 regardless of how unwise it might be to do so in a microcontroller context, the language does permit passing structs by value, so saying "you can't do it" is factually incorrect. (It's perfectly alright to say "it's a bad idea to do it", of course.) — This isn't my real name, Jul 31 '13 at 21:05
@ElchononEdelson I know, I was going to correct it but my post was moved as a comment and I couldn't edit it anymore. — user2553780, Jul 31 '13 at 21:11

score 0 · Accepted Answer · answered Aug 01 '13 at 05:38

0

The receive function seems to be the procedure in question. Try writing a stub-replacement for receive, such as:

    unsigned char receive(void)
    {
       unsigned char X = 'a';  
       // or whatever value you want to simulate as being received

       return X;
    }

and then try running your complete application, tf it works then go back and re-think your original receive per some of the comments that have already been made.

answered Aug 01 '13 at 05:38

JackCColeman

3,777
1
15
21

Thanks, you were correct, the receive function was where the error was occurring. First, I can't use a signed char to shift with, it has to be an int. Secondly (and fairly embarrassingly) I wasn't clearing dataR before I was getting new data. So the old values were persistent when the data received was a 0. Thanks for all your help. – Tim M Aug 05 '13 at 12:54

Is my method of passing a struct as both value and reference correct?

1 Answers1