0

I am making a circuit, that will pull data out of the 4011 shift register. So far it works fine, but as my number rolls over some value, strange things start to happen.

This code will cycle all of my 4011's.

  typedef unsigned long u_long;

  for (u_long i = 0; i < 32; i++)
  {
    digitalWrite(CLOCK_in,0);
    delayMicroseconds(0.2);

    bool bit = digitalRead(DATA_in);

    Serial.print(bit,BIN);

    if (bit) out |= (1 << i); 

    digitalWrite(CLOCK_in,1);
  }
  Serial.println((u_long)out);
  Serial.println((u_long)out,BIN);

From the first "print()" I get:

00000000000000010000000000000000

Which is what I am expecting as this is my input (And my goal is to convert it to unsigned long decimal - highest number is on 32bits). From the next print(), howeer, I get 4294934528. And I think that is not correct. From the last print() function, where naive me expected the same answer as the first one, I get 11111111111111111000000000000000

Where is the point that I am missing? Is there some problem with the bitshift part?

phuclv
  • 37,963
  • 15
  • 156
  • 475
Kralik_011
  • 393
  • 1
  • 3
  • 14
  • `the first "print()"` `the next "print()"` `the last "print()"` - I see only one `print` inside `Serial.print`. Please be more specific. Where is the "next" print? Where is the "last" print? – KamilCuk Dec 16 '19 at 23:39
  • Please, post all the relevant code. If the error is related to the way variable `out` is printed, how can we help you if you don't even show how that variable is declared and initialized? All its lifecycle would be really welcome. – Roberto Caboni Dec 16 '19 at 23:49
  • 3
    One bug that probably has nothing to do with the problem you're seeing: delayMicroseconds() takes an unsigned int. In the code above, the 0.2 is a floating-point number, which the compiler automatically converts to an unsigned int, resulting in 0. So the code above has the same effect as delayMicroseconds(0). If you need a delay under 1 microsecond, you can probably get away with no delayMicroseconds() call. – Bradford Needham Dec 17 '19 at 01:01
  • 1
    instead of typdef'ing your own type, just use the standard `uint32_t` – phuclv Dec 17 '19 at 01:24

1 Answers1

1

There's one issue that I spotted right away without an MCVE

for (u_long i = 0; i < 32; i++)
{
    ...
    if (bit) out |= (1 << i);
    ...
}

In Arduino int is a 16-bit type, and integer literals are of int type by default. Shifting by more than the bit width invokes undefined behavior, therefore your code has UB when i > 15

To fix that use the L suffix to make it a long literal

if (bit) out |= (1L << i);

But there's no reason to loop with a slow 32-bit variable in Arduino. Just use int, or even better uint8_t. Also use uint32_t with the standard UINT32_C macro for the output type so that you don't need to figure out the correct suffix

uint32_t out;
for (int i = 0; i < 32; i++)
{
    ...
    if (bit) out |= UINT32_C(1) << i;
    ...
}

That said, you still need to provide an MCVE

phuclv
  • 37,963
  • 15
  • 156
  • 475