tags:

views:

132

answers:

3

I am trying to write a function in C that will shift out the individual bits of a byte based on a clock signal. So far I have come up with this...

void ShiftOutByte (char Data)
{
    int Mask = 1;
    int Bit = 0;
    while(Bit < 8)
    {
        while(ClkPin == LOW);
        DataPin = Data && Mask;
        Mask = Mask * 2;
        Bit++;
    }
}

where DataPin represents the port pin that I want to shift data out on and ClkPin is the clock port pin.

I want the device to shift out 8 bits, starting on the LSB of the byte. For some reason my output pin stays high all the time. I am certain that the port pins are configured properly so it is purely a logical issue.

+5  A: 

You want to use &, not &&. && is the logical and, whereas & is the bitwise and.

DeadMG
+1  A: 

You have a typo, that has replaced one operator with another that has a similar function. That is, && is guaranteed to produce the answer 1 if both of its operands are logically true. Since you are almost certainly testing with Data not zero, then this is the case for all eight iterations of your loop.

This may be masking a more subtle issue. I don't know what your target architecture is, but if DataPin is effectively a one-bit wide register, then you probably need to be careful to not try to assign values other than 0 or 1 to it. One way to achieve that is to write DataPin = !!(Data & Mask);. Another is to shift the data the other direction and use a fixed mask of 1.

RBerteig
+3  A: 

Your solution is almost there, but there a few problems:

  • When clock goes high, you will pump out data in a burst before the clock has a chance to go low again, so you need to pause while clock is high before you check for it being low (unless the hardware pauses execution until clock goes low). This can be done either at the end of the loop, or at the beginning. I chose the beginning in the sample below, because it allows you to return from the function while clock is still high, and do some processing while it is still high.
  • You used the logical and (&&) instead of bitwise and (&) as others have pointed out
  • I am not familiar with your architecture, so I can't say if DataPin can only accept 0 or 1. But that is also another point where things may go wrong. Data & Mask will return a bit that is shifted left (1, 2, 4, 8, ...) depending on the bit position.

So, in summary, I would do something like:

void ShiftOutByte (unsigned char Data)
{
    int Bit;
    for(Bit=0; Bit < 8; ++Bit)
    {
        while(ClkPin == HIGH);
        while(ClkPin == LOW);
        DataPin = Data & 1;
        Data >>= 1;
    }
}

Note: I have used a for loop instead of the while ... Bit++ pattern for clarity of purpose.

Dysaster
All true. Thanks for the help.
Jordan S
You should use `unsigned char` here, to ensure that the eighth bit is shifted correctly.
caf
@caf You are right about using unsigned char - but not for correctness. You would get 8 correct bits pumped out with char, but you may end up with 0xFF instead of 0x00 after shifting a signed char, which is not relevant here. But it is good practice anyway. Editing answer to fix it.
Dysaster
@Dysaster: Unfortunately, the result of right shifting a negative value is simply "implementation-defined", with no further constraints.
caf
@caf I learned something new today. Thanks. I always thought a right shift on a signed value would be an arithmetic right shift, but a quick search revealed that you are right.
Dysaster