Atmega bit shifting

[yalect]

Hello,
I would like to ask you that I want make program shift bits to the right or the left I found one but my query is to shift bits 8 time the program shift bits one time I don't know why this is my example :
void main() {
int bits = 100, count, DATA;
    DDRC |= 0xff;

  while(1)
 {

  for (count = 0; count < 8; count++);
 
  {
  DATA |=(bits >> count);/* shift right, then mask bottom bit*/

      PORTC |= DATA;
      Delay_ms(3000);

       }
      }
      }

[sleemanj]

Your english is confusing as to exactly what you are trying to achieve, but I think your problem is ` >> count ` should be ` >> 1 `

[yalect]

Hello,
sorry, really that was an error I modified the program I want to shift bits 8 times not one time as it does
void main() {
int bits = 100, count, DATA;
    DDRC |= 0xff;

  while(1)
 {

  for (count = 0; count < 8; count++);

  {
  DATA |=(bits >> 1);/*shift right, then mask bottom bit*/

      PORTC |= DATA;
      Delay_ms(3000);

       }
      }
      }

[Bruce Abbott]

There are several mistakes in your code:-

Code: [Select]

for (count = 0; count < 8; count++);
In C a semicolon terminates a statement, so this line does nothing except count from 0 to 7.

Code: [Select]

int bits = 100 ... // binary 0011 0000 0011 1001
Binary 0011000000111001 = decimal 1234, not 100. If a comment doesn't match the code it's worse than useless!

Code: [Select]

    DDRC |= 0xff;
    ...
    DATA |=(bits >> count);// shift right, then mask bottom bit
    ...
    PORTC |= DATA;
The '|=' operator means '(bitwise)OR equals', not 'equals'. 

And again the comment doesn't match the code. Nothing is 'masking the bottom bit'.

Here is what I think you actually wanted:-

Code: [Select]

void main()
{
  int bits = 100; // = binary number 00000000 01100100
  int count, DATA;
  DDRC = 0xff;
  while(1)
  {
    for (count = 0; count < 8; count++)
    {
      DATA = (bits >> count);  // output = number shifted right * count
      PORTC = DATA;
      Delay_ms(3000);
    }
  }
}


[yalect]

Thank you Bruce
you mean that program will shift the bits to the right 8 times and make the bits or the byte on the PORTC each time (because my Porgram I Post, it shift the bits one time and make them on PORTC), what if we replace count with 1, and write then DATA =(bits >> 1), it will work?
Thank you

[Nusa]

I recognize those now-disconnected comments from a code sample I gave you in another of your threads. https://www.eevblog.com/forum/microcontrollers/pin-to-send-data/msg1383938/#msg1383938

Your problems with this seem to be at a very basic level. Try reading and understanding this: https://en.wikipedia.org/wiki/Bitwise_operations_in_C. There are many tutorials out there if that's not enough.

[Bruce Abbott]

what if we replace count with 1, and write then DATA =(bits >> 1), it will work?

No. If you do this then then DATA will always be 'bits' shifted right by 1, and the output will be a stationary bit pattern.

Assuming you want the bits to 'scroll' across the output pins, there are two ways to do it:-

1. Always calculate DATA from the original number, with 'count' being the amount of shift required. The first time through the loop 'count' = 0 so there is no shift, second time shifted right 1 place, 3rd time 2 places etc. The value of DATA is regenerated each time.  This is how my code works.

2. Load the number into DATA, then shift it right 1 place each time through the loop (after outputting it). So DATA holds the next value to be output, and gets progressively shifted right. Do do this you need to make the following changes:-

Code: [Select]

    DATA = bits;    // load number into DATA _before_ entering the loop
    for (count = 0; count < 8; count++)
    {
      PORTC = DATA;        // output current contents of DATA
      DATA = (DATA >> 1);  // shift DATA right * 1 (will be output next time through the loop)
      Delay_ms(3000);
    }
  }
Method 2 is slightly more efficient on the ATmega because it doesn't have a barrel shifter, so shifting by more than 1 place has to be implemented as multiple 1 bit shifts.

 

[rstofer]

Or, you can use the C switch statement inside the for () and not even mess with shifting.

Code: [Select]

for (i = 0; i < 8; i++) {
     switch(i) {
        case 0 : PORTC = 0x01;
                     break;
        case 1 : PORTC = 0x02;
                     break;
        ...
        case 7 : PORTC = 0x80;
                     break;
        default : break;
      }
      delay(3000);
}

Clean and straightforward.


[phil from seattle]

For a lot less code (at least, C code).

Code: [Select]

uint8_t bitmap[8] = { 0x1,0x2,0x4,0x8,0x10,0x20,0x40,0x80};

for(i=0;i<8;i++){
    PORTC = bitmap[i];
//  PORTC = 1<<i;  // as an alternative, which ever gens the least code.
}
not sure the point of that sequence, though.

[Bruce Abbott]

Total code size with the algorithms presented so far (GCC 4.3.2 optimization level 0s):-

My algo #1,

Code: [Select]

    for (count = 0; count < 8; count++)
    {
      DATA = (bits >> count);// shift right * count
      PORTC = DATA;
    } 104 bytes

My algo #2,

Code: [Select]

for (count = 0; count < 8; count++)
    {
      PORTC = DATA;        // output current contents of DATA
      DATA = (DATA >> 1);  // shift DATA right * 1 (will be output next time through the loop)
    }94 bytes

rstofer,

Code: [Select]

for (count = 0; count < 8; count++) {
     switch(count) {
        case 0 : PORTC = 0x01;
                     break;
        case 2 : PORTC = 0x02;
                     break;
        case 3 : PORTC = 0x04;
                     break;
        case 4 : PORTC = 0x08;
                     break;
        case 5 : PORTC = 0x10;
                     break;
        case 6 : PORTC = 0x40;
                     break;
        case 7 : PORTC = 0x80;
                     break;
        default : break;
      }
}
174 bytes

phil from seattle #1,

Code: [Select]

uint8_t bitmap[8] = { 0x1,0x2,0x4,0x8,0x10,0x20,0x40,0x80};

for(count=0;count<8;count++){
    PORTC = bitmap[count];
}
156 bytes

phil from seattle #2,

Code: [Select]

for(count=0;count<8;count++){
    PORTC = 1<<count; 
}104 bytes

However, neither rstofer's nor phil from seattle's code does what is required, which is shifting an arbitrary bit pattern (eg. 1100100 or 11000000111001) progressively right. The case statements or array contents could be adjusted to suit a particular bit pattern, but modifying all those 'magic numbers' would be time-consuming and easy to mess up.

Burying a shift operation in case statements or array elements are examples of 'tricky' coding that should be avoided.  Unless you desperately need the fastest possible execution and/or smallest footprint, programs should be written for readability and understanding first, then optimized only if necessary. If shifting right is what you want to do, use the >> operator. Don't be afraid to use intermediate variables, spread statements over several lines, or break complex code up into simpler functions which make your intention clearer and the code easier to expand or modify. Avoid 'magic' numbers. Let the compiler take care of optimizing your code.
   
 

[Bruce Abbott]

The case statements or array contents could be adjusted to suit a particular bit pattern, but modifying all those 'magic numbers' would be time-consuming and easy to mess up.

And as an example of 'messing up', in copy/pasting I missed a case statement!

The correct rstofer code is:-

Code: [Select]

for (count = 0; count < 8; count++) {
     switch(count) {
        case 0 : PORTC = 0x01;
                     break;
        case 1 : PORTC = 0x02;
                     break;
        case 2 : PORTC = 0x04;
                     break;
        case 3 : PORTC = 0x08;
                     break;
        case 4 : PORTC = 0x10;
                     break;
        case 5 : PORTC = 0x20;
                     break;
        case 6 : PORTC = 0x40;
                     break;
        case 7 : PORTC = 0x80;
                     break;
        default : break;
      }
  }
which increased total code size to 182 bytes.

[phil from seattle]

...
Burying a shift operation in case statements or array elements are examples of 'tricky' coding that should be avoided.  Unless you desperately need the fastest possible execution and/or smallest footprint, programs should be written for readability and understanding first, then optimized only if necessary. If shifting right is what you want to do, use the >> operator. Don't be afraid to use intermediate variables, spread statements over several lines, or break complex code up into simpler functions which make your intention clearer and the code easier to expand or modify. Avoid 'magic' numbers. Let the compiler take care of optimizing your code.
   

I was simply responding to the prior posting with case statements. The code is nonsense because it just pumps changing bits out a data port with no explicit timing.

I disagree with your assessment of the coding style. Data driven schemes are perfectly fine and quite understandable to someone moderately skilled in programming and able to hold a couple of simultaneous thoughts in their head. It's a staple of FSMs. One could argue that bit shifting is equally confusing. I suppose you are against the use of pointers, subclassing, virtual functions and so on. In addition, comments should be placed to make it clear what's going on.

And, I don't disagree with "let the compiler optimize" sentiment but too many hack programmers don't understand that they need to get the algorithm right first.  Let the optimizer take care of allocating registers and such.

[Bruce Abbott]

I was simply responding to the prior posting with case statements

Fair enough.

One could argue that bit shifting is equally confusing.

When the intention is to shift bits? I think not.

Where bit shifting would be confusing is when you really wanted to (for example) divide a number by 2. Perhaps in that case a bit shift operation would be more efficient, but you should write the code you want and let the compiler decide how to implement it

I suppose you are against the use of pointers,

Pointers are an abomination!

[NorthGuy]

The correct rstofer code is:-

Code: [Select]

for (count = 0; count < 8; count++) {
     switch(count) {
        case 0 : PORTC = 0x01;
                     break;
        case 1 : PORTC = 0x02;
                     break;
        case 2 : PORTC = 0x04;
                     break;
        case 3 : PORTC = 0x08;
                     break;
        case 4 : PORTC = 0x10;
                     break;
        case 5 : PORTC = 0x20;
                     break;
        case 6 : PORTC = 0x40;
                     break;
        case 7 : PORTC = 0x80;
                     break;
        default : break;
      }
  }


Even The Queen couldn't make the code more complicated. This is equivalent to:

Code: [Select]

PORTC = 0x01;
PORTC = 0x02;
PORTC = 0x04;
PORTC = 0x08;
PORTC = 0x10;
PORTC = 0x20;
PORTC = 0x40;
PORTC = 0x80;

[yalect]

Thank you Bruce
talking about shifting to the right if we take state of bit for Var within the program let's say bits if we want to read it we will make & operation or mask it so I wrote this program to detect state of that bit 1 or 0 with PIND0 and 1 of PORTD but the program didn't work well as I expected might I don't understand how it works if we  shift right number 100 my result was (for detecting the first bit of bits) :
**************************************************
Number shifted  *  1st bit  *   detected bit
***************************************
11100100      *          0       *       1
00110010      *          0       *       0
00011001      *          1       *       0
00001100      *          0       *       1
00000110      *          0       *       0
00000011      *          1       *       0
00000001      *          1       *       1
********************************************
it doesn't make sense?!

void main() {
  int bits = 100;
  int compare = 0x0000001;
  int count, DATA;
  DDRC = 0xff;
  DDRD = (1<< PIND0);
  DDRD = (1<< PIND1);
  while(1)
  {
    for (count = 0; count < 8; count++)
    {
      DATA = (bits >> count);  // output = number shifted right * count
      PORTC = DATA;
      Delay_ms(1000);
      if ((bits >> count) & compare)
     {
     PORTD = (1<< PIND0);
     }
     else
    {
     PORTD = (1<< PIND1);
     }
    }

  }
}
maybe you have an Idea?
thank you

[Nusa]

Off by one. Try moving your delay to the bottom of the for loop so the port C and D results happen at the same time.

[rstofer]

The case statements or array contents could be adjusted to suit a particular bit pattern, but modifying all those 'magic numbers' would be time-consuming and easy to mess up.

And as an example of 'messing up', in copy/pasting I missed a case statement!

The correct rstofer code is:-

Code: [Select]

for (count = 0; count < 8; count++) {
     switch(count) {
        case 0 : PORTC = 0x01;
                     break;
        case 1 : PORTC = 0x02;
                     break;
        case 2 : PORTC = 0x04;
                     break;
        case 3 : PORTC = 0x08;
                     break;
        case 4 : PORTC = 0x10;
                     break;
        case 5 : PORTC = 0x20;
                     break;
        case 6 : PORTC = 0x40;
                     break;
        case 7 : PORTC = 0x80;
                     break;
        default : break;
      }
  }
which increased total code size to 182 bytes.

No it isn't and this leads to another claimed error as the code not having a delay.  My original code had a delay at the bottom of the 'for' loop.

It's one thing to count emitted code, it's another to count cycles actually taken.  Both the table lookup and the case statement take consistent time to execute (and the table is better,  by far!) while m >> n takes n shifts because the Atmega doesn't have a barrel shifter and both m and n are arbitrary (presumably).  All the compiler can do is emit looping code over a single shift.  That may be just the right solution!  Or not...

I thought the original problem was to display some kind of pattern on the output.  If that's not true then the case statement might be a poor choice.  OTOH, if the output is more or less random depending on the count (and it's hard to tell from the original code) then a case statement is ideal.  There need be no relationship between outputs of successive cases.  The case statement also allows for feature creep.  When count = 3, for example. Maybe when that pattern comes up I need to launch an internet connection.  Who knows?

I CHOSE a right-to-left pattern.  It could have been left-to-right or it could have been any other pattern.  Maybe I want 2,3,5,7,11,13,17,19 - the first 8 prime numbers.  It's a little awkward to do with >>.

The case statement doesn't have to be inside a for loop.  It could be some kind of state machine with very little effort.  A super loop and a state variable is all it takes.

But if the goal is actually m >> n, just say so.  The compiler knows full well how to implement this.

[NivagSwerdna]

but my query is to shift bits 8 time the program shift bits one time

I'm a bit late to the party but the OP's question was how to shift 8 bits at a time.

unsigned long bigNumber = 0xAABBCCDD;
 
  byte lsb;
 
  lsb = bigNumber & 0xff;
  Serial.println(lsb,HEX);
 
  bigNumber >>= 8;
  lsb = bigNumber & 0xff;
  Serial.println(lsb,HEX);

  bigNumber >>= 8;
  lsb = bigNumber & 0xff;
  Serial.println(lsb,HEX);

  bigNumber >>= 8;
  lsb = bigNumber & 0xff;
  Serial.println(lsb,HEX);

DD
CC
BB
AA

[rstofer]

but my query is to shift bits 8 time the program shift bits one time

I'm a bit late to the party but the OP's question was how to shift 8 bits at a time.

If this is as simple as breaking up a 32 bit value into 8 bit bytes, there is no need to shift.  You can 'union' the 32 bit value with an array of 4 each 8-bit values and be done with it.

Code: [Select]

#include <stdio.h>
#include <stdint.h>

int main(void)
{
union {
uint32_t word;
uint8_t  bytes[4];
} u;

u.word = 0x11223344;

for (int i = 3; i >= 0; i--) {
printf("%4x\n",u.bytes[i]);
}
}

This prints

Code: [Select]

  11
  22
  33
  44

ETA:  To print the bytes LSB first, change the for loop:

Code: [Select]

for (int i = 0; i < 4; i++)

The printf() part stays the same.

[Bruce Abbott]

I wrote this program to detect state of that bit 1 or 0 with PIND0 and 1 of PORTD but the program didn't work well as I expected might I don't understand how it works if we  shift right number 100 my result was (for detecting the first bit of bits) :
**************************************************
Number shifted  *  1st bit  *   detected bit
***************************************
11100100      *          0       *       1
00110010      *          0       *       0
00011001      *          1       *       0
00001100      *          0       *       1
00000110      *          0       *       0
00000011      *          1       *       0
00000001      *          1       *       1
********************************************
it doesn't make sense?!

I think your problem is here,

Code: [Select]

  DDRD = (1<< PIND0);
  DDRD = (1<< PIND1);The first line makes only D0 an output and all other pins inputs. The second line makes only D1 an output and all others (including D0) inputs. The result is only D1 is an output.

To make both D0 and D1 outputs, do this:-

Code: [Select]

DDRD = (1<<PIND0)|(1<<PIND1); // D0 and D1 are outputs, all others inputsor this:-

Code: [Select]

DDRD = (1<<PIND0); // D0 is output, all others input
DDRD |=(1<<PIND1); // D1 is output, all others unchanged

[hamster_nz]

Although not of any importance in this case, one thing to be careful of when using I/O ports is that reading the value of a port does not always return the last value written to the Port's output registers, it reads the state of the port's input buffer.

This is especially tricky when masking off bits... "PORTD |= 1;" might end up clearing other bits in the output register as an unintended side effect.

All sorts of ugly problems lurk here - especially when you need to update ports during an interrupt, giving rise to all sorts of hard to debug race conditions.

[rstofer]

Although not of any importance in this case, one thing to be careful of when using I/O ports is that reading the value of a port does not always return the last value written to the Port's output registers, it reads the state of the port's input buffer.

This is especially tricky when masking off bits... "PORTD |= 1;" might end up clearing other bits in the output register as an unintended side effect.

All sorts of ugly problems lurk here - especially when you need to update ports during an interrupt, giving rise to all sorts of hard to debug race conditions.

For some CPUs, it is better to keep a shadow copy of what has been sent to the register in a variable.  Manipulate the variable as desired and then send it to the port.  Never read-modify-write if you can avoid it.  Work from the shadow copy.

[NorthGuy]

For some CPUs, it is better to keep a shadow copy of what has been sent to the register in a variable.  Manipulate the variable as desired and then send it to the port.  Never read-modify-write if you can avoid it.  Work from the shadow copy.

PICs have built-in shadow variables called LAT.

[rstofer]

For some CPUs, it is better to keep a shadow copy of what has been sent to the register in a variable.  Manipulate the variable as desired and then send it to the port.  Never read-modify-write if you can avoid it.  Work from the shadow copy.

PICs have built-in shadow variables called LAT.

Some do, the PIC 16F family does not.

Section 9.2 for example

http://ww1.microchip.com/downloads/en/devicedoc/33023a.pdf

The PIC 18 series does have the LAT registers.

Page 3-18 for example

http://ww1.microchip.com/downloads/en/DeviceDoc/39500a.pdf

This whole read-modify-write issue really started with the PIC 16 series (AFAIK).  It was corrected in the PIC 18 series.  It is important to read the datasheet to find out what side effects a read-modify-write will cause.  It is not wise to just assume that you are reading the latch when, in fact, you are reading the pin.

[NorthGuy]

Some do, the PIC 16F family does not.

Most of PIC16Fs do. The ones without LATs are old, obsolete, overpriced and, in addition to missing LATs, have lots of other problems. These are better forgotten. Newer PIC16s (the part numbers starting from PIC16F1) not only have LAT but they have lots of serious improvements. Not to mention they're much cheaper.