Atmega bit shifting

[yalect]

Hello,
thank you for your reply
I made this modification for my program where it makes checking for first bit status if it's 1 PIND0 = 1 if it's 0 PIND1 = 1 but it doesn't work well where it works for first bit status of 0 or PIND1 = 1 only, maybe you have an Idea.
 void main() {
  int bits = 100;
  int compare = 0x00000001;
  int count, DATA,
  DDRC = 0xff;
  DDRD = (1<<PIND0)|(1<<PIND1);
  while(1)
  {
    for (count = 0; count < 8; count++)
    {
      DATA = (bits >> count);  // output = number shifted right * count
      PORTC = DATA;
      if( DATA & compare)
     {
     PORTD = (1<< PIND0);
     }
     else
    {
     PORTD = (1<< PIND1);
     }

     Delay_ms(1000);
    }
 }
}
thank you

[rstofer]

All of your variables are 8 bit quantities living in at least a 16 bit data type. 

From the declaration of 'compare', it seems like you are using a 32 bit CPU yet the ports are 8 bits.  I'm confused...  What processor are you using?

I would declare all the variables as 8 bit unsigned values.  Either as uint8_t or unsigned char.  I prefer uint8_t.

There is a missing ; after DATA in the declaration

Code: [Select]

int main(void)
{
  uint8_t bits = 0b01100100;          //use an unsigned 8 bit data type
  unsigned char compare = 0b00000001; // another style of unsigned 8 bit data type
  uint8_t count, data;

  DDRC = 0xff;
  DDRD = (1 << PIND0) | (1 << PIND1);  // see below re: PIND0 and PIND1

  while(1)
  {
    for (count = 0; count < 8; count++)
    {
      data = (bits >> count);
      PORTC = data;
      if (data & compare)
      {
        // I don't know how PIND0 and PIND1 are defined.
        // If they are defined as a mask instead of a count,
        // we should probably use something like:
        // PORTD = (1 << 0);
        // and
        // PORTD = (1 << 1);
        PORTD = (1 << PIND0);
      }
      else
      {
        PORTD = (1 << PIND1);
      }
      // Delay_ms(1000); // not declared in my Arduino setup
    }
  }
  return 0; // never returns but keep the compiler happy
}

I have no way to test the code, I don't know what processor you are using.  It does compile...

I am really unclear on how PIND0 and PIND1 are defined.  Your code doesn't mention a header file and I probably don't have it anyway.  If PIND1 is a mask like 0b00000010 instead of a count like 0b00000001 then the output will be in the wrong bit position.

You will need to remove the // in front of Delay_ms().

[yalect]

thank you
yes, I simulated that code with Proteus simulation software and it works normally as I said the problem it's not checking state of 1 for first bit or PIND0 not working.
thank you

[rstofer]

Written for the Arduino UNO, the code looks like this:

Code: [Select]

void setup() {
DDRC = 0xff;
DDRD = (1 << PIND0) | (1 << PIND1);
}

void loop() {
uint8_t bits = 0b01100100;          //use an unsigned 8 bit data type
unsigned char compare = 0b00000001; // another style of unsigned 8 bit data type
uint8_t count, data;

for (count = 0; count < 8; count++)
{
data = (bits >> count);
PORTC = data;
if (data & compare)
{
PORTD = (1 << PIND0);
}
else
{
PORTD = (1 << PIND1);
}
delay(1000);
}
}

I can watch PORTD and it does something reasonable but PORTC isn't complete and I didn't bother to try to watch it.  A project for the reader...

This was tested using the vMicro plug-in to Microsoft Visual Studio Community Edition.

The definitions for PIND0 and PIND1 are fine.

[rstofer]

thank you
yes, I simulated that code with Proteus simulation software and it works normally as I said the problem it's not checking state of 1 for first bit or PIND0 not working.
thank you

Simulation isn't hardware and, in my view, cheap hardware and an oscilloscope (or even LEDs) are better.  PORTC isn't completely available on the Arduino UNO but it is available on the Arduino Mega 2560.  On the UNO, only bits 0..5 are available.

Don't overlook the possibility that the simulator has more problems than the code.

[rstofer]

If you were expecting to see the very first bit in PIND0 or PIND1, you won't.  You have shifted bits before you got that far in your code.  You can do the output first and then the shift, something like this.

Code: [Select]

void setup() {
 DDRC = 0xff;
 DDRD = (1 << PIND0) | (1 << PIND1);
}

void loop() {
 uint8_t bits = 0b01100100;          //use an unsigned 8 bit data type
 unsigned char compare = 0b00000001; // another style of unsigned 8 bit data type
 uint8_t count, data;
 
 data = bits;   // set up the initial value in 'data'
 for (count = 1; count < 8; count++)  // we will already see the zeroth pattern
 {
  PORTC = data;  // send data to output
  if (data & compare)  // set up least significant bit
  {
   PORTD = (1 << PIND0);
  }
  else
  {
   PORTD = (1 << PIND1);
  }
  delay(1000);             // let the user see the pattern
  data = (bits >> count);  // finally, shift the pattern
 }
}

You can clever up the for statement to include initializing data:

Code: [Select]

for (data = bits, count = 1; count < 8; count++)


[Nusa]

Ok, I pulled out real hardware (Atmega328p uno board) and tried it. I'm using the Arduino IDE found at www.arduino.cc . I've never even looked at Protuse, so ask me no questions about that.

First off, the comma instead of a semi-colon comes out as a compile error for me, because it results in an attempt to redefine DDRC. Fix that. I don't know why your environment doesn't flag it.

Second, the compiler wasn't happy with void main(), so I changed it to int main() as the easiest solution.

Third, I discovered what is probably your core problem. When you use a main() function (instead of the setup/loop functions this IDE prefers), YOU become responsible for hardware initialization. It's not done for you! Calling init() at the top of main() should do it. init() sets up the system timers, which is important for correct delay times. Also, which is quite important in this case, it disconnects TX and RX from the UART used to program the board. Those happen to be the two pins on port D you're using for the LED outputs!

Last, my uploads failed at first. I'd only used 220 ohm resistors on the LED's, and that was enough of a load to disrupt the UART communications. I was too lazy to switch resistors, so I just unplugged the LED's during the upload, then plugged them back in.

After that, the code worked as written.

[yalect]

thank you
yes, if we try to realize that code with arduino we face that problems but my project not based on arduino but for using Atmega32 as processor with other base components so the problem it differs.

[Nusa]

Don't get hung up on the word Arduino. The hardware is not much more than a breakout board for the processor, regardless of which processor is actually implemented on a particular board.

Atmega32 is certainly part of the family, extremely similar to the Atmega328 in function. So much so that TX and RX are on the same pins of port D on both chips. They both have clocks that need initializing. If you have a UART on TX/RX, then that's the same too.

[rstofer]

The code I posted in Reply #28 should work for the ATmega32 - it doesn't presuppose the Arduino infrastructure.

No doubt there will need to be some #include statements to pull in header files but your code didn't show those.  The code in Reply #28 shifts before display so you might want to use the concepts in Reply #30 if you want to see the initial pattern before shifting.

[phil from seattle]

First off, the comma instead of a semi-colon comes out as a compile error for me, because it results in an attempt to redefine DDRC. Fix that. I don't know why your environment doesn't flag it.

I believe that is legal C but not C++.  It's a bad coding style in general as it is easy to overlook when reading a for loop. Best to keep only for-loop related stuff in the for(; statement.

[rstofer]

First off, the comma instead of a semi-colon comes out as a compile error for me, because it results in an attempt to redefine DDRC. Fix that. I don't know why your environment doesn't flag it.

I believe that is legal C but not C++.  It's a bad coding style in general as it is easy to overlook when reading a for loop. Best to keep only for-loop related stuff in the for(; statement.

The semi-colon referenced by Nusa above was the one missing after the declaration of 'data'.  That definitely caused a compiler error.

I'm not real fond of the comma separator either.  That is actually the first and only time I have ever written it that way.  I don't tend to use the '? :' conditional pair either.  Nevertheless, they are valid C and if you are to know the language it is worth knowing that they exist.

I could make the argument that the comma separator in the for ( ; ; ) is good practice because it clearly shows what is initialized at the beginning of the loop.  I'm not sure I'll try...

[phil from seattle]

Man, I hate those emojis that inadvertently warp a programming language construct.

[rstofer]

Man, I hate those emojis that inadvertently warp a programming language construct.

I didn't notice that earlier.  Kind of a PITA.  Anyway, I had too many semicolons..

[Bruce Abbott]

I made this modification for my program where it makes checking for first bit status if it's 1 PIND0 = 1 if it's 0 PIND1 = 1 but it doesn't work well where it works for first bit status of 0 or PIND1 = 1 only, maybe you have an Idea.

I changed the comma after DATA to a semicolon and compiled it with AVR GCC in Atmel Studio 4, then stepped through the code in the simulator.  As far as I can tell the code works perfectly (bit 0 is 0 so D1 goes high, bit 1 is 0 so D1 stays high, bit 2 is 1 so D1 goes low and D0 goes high etc.)

[yalect]

Hi,
thank you bruce
I want to ask that you just changed the comma after DATA to a semicolon from first program or you make other modifications and which simulator you are using?
thank you
 

[Bruce Abbott]

Hi,
thank you bruce
I want to ask that you just changed the comma after DATA to a semicolon from first program or you make other modifications and which simulator you are using?
thank you

No other changes apart from renaming Delay_ms to _delay_ms. Compiler was AVR GCC 4.3.3 and I used the AVR simulator that comes with Atmel Studio 4.

[paulca]

Slight off topic, but I had gave myself a challenge last night and almost got it working before I was too tired to solve the bug and had to go to bed.

Given an 8 bit byte print (serial) groups of bits whether 0 or 1.

Say for example the byte is an IC status register and you want to print something like:

Data Rate:  1001

When given the byte:

00010010

And the datasheet exert:

Bits   Purpose
===========
7-5: Status
4-1:  Data Rate
0:  Other Stuff

You want to just print:  1001

[rstofer]

mask = 0b00010000 // mask the high order bit first
for bit = 0 to 3
  if (value & mask)
    print 1
  else
    print 0
  mask >>= 1 // mask the next lower bit
end for

[paulca]

Mine was a little more generic and ambitious, but flawed.  This is actually version two.

Code: [Select]

void printBits(byte myByte, byte mask){
  for(int pos = 7; pos >= 0; pos--){
 
    byte maskBit = (mask << pos);
    if( maskBit & 0x80 ) {     
        byte shifted = (myByte << pos);
     
        if( shifted & 0x80 ) {
          Serial.print('1');
        } else {
          Serial.print('0');
        }
     }
  }
}

Bizarrely it prints all the right bits and the right values, but in the wrong order.

So:
printBits( b10000000, b11100000 )
Prints: 001

As an academic I'm curious to figure out how to get it working.  As a practicallist/professional I am going to something different:

void printBit( byte theByte, int theBit ) {
    theByte >>= theBit;
    if( theByte & 0x1 ) {
       Serial.print("1")
    } else {
       Serial.print("0")
    }
}

I'm sure there is probably an arduino function for it.  There is an instruction to test a bit in register in AVR ASM.

Then call it multiple times.

It's less elegant and less pedantic, less formal and less smarty pants, but it's 100 times faster to write and 100 time easier to understand.  Get the job done.

EDIT: What kicked this off was trying to print a debug function which human readibly formats the contents of an ADC config register.

Code: [Select]

Config register: 10000101 10000011 = 1000010110000011
Status: 1
Multiplex: 000
PGA: 010
Mode: 1
Data Rate: 001
Comparator Mode: 0
Comparator Polarity: 0
Comparator latch: 0
Comparator queue: 11

You can see the bug.

[rstofer]

Why wouldn't you use a set of bit fields in a struct?  You can then name the fields and deal with them as a normal entity.

https://docs.microsoft.com/en-us/cpp/c-language/c-bit-fields

https://www.geeksforgeeks.org/bit-fields-c/

That doesn't solve the printing of the bit fields in left to right order but my pseudo code above does that.  The only issue is that I assume a constant width of 4 bits and that isn't the right answer for variable width fields.

[rstofer]

Maybe add a parallel struct with the same field names (different variable name, of course) where each entry is an unsigned char and contains the width of the named field.

print_my_stuff(bitfield_struct.field, field_width_struct.field) to print the named bitfield a specific width.

in print_my_stuff, have a mask initialized to 0x01 and shift it left field_width - 1 places before you start the for loop.
Each time through the loop, shift the mask right 1 position.

[Bruce Abbott]

Why wouldn't you use a set of bit fields in a struct?  You can then name the fields and deal with them as a normal entity.

Two reasons:-

1. Bitfields are not portable. The bits can be stored in whatever order and alignment the compiler sees fit.

This probably isn't important when referencing hardware registers in an MCU, because the compiler should be designed to match the hardware and the code is not portable anyway. However it might be an issue if porting code from from one compiler/platform to another (I remember having this problem on the Amiga, where two different compilers allocated the bits in different directions).

2. It's not generic.

Structures are defined at compile time, so you would need a separate structure for each bitfield size and position you wanted to print. Again this probably wouldn't be a problem for hardware registers which are predefined.

I am currently porting code for a GAL programmer which stores up to 5892 bits in an array. The PC program simply used 1 Byte per bit, but the ATmega328 doesn't have enough RAM for that so the bits are packed into bytes. The bits need to be accessed individually, sometimes in natural order and sometimes reversed, and may also be bit-pair or byte-reversed. Each GAL type has a different bit map, so to use structures I would need several to suit each chip. Instead the program has arrays of bit numbers which are used to index the bits according to the patterns required, and functions that set and test each bit individually.

[paulca]

where two different compilers allocated the bits in different directions).

I did my entire GCSE Computer Science in reverse order to what we were taught.  I remembered when I got home and thought I'd failed.  Obviously the examiner understood endian-ness.  Luckily I had written my bit values in that order in my working out in the first question.  I passed.

On "Endian".  I absolutely hate that term.  It requires the concept of an "end".  Is that the left end or the right end?  Big left or MSB first.  Big right, MSB last would be better.

Anyway, my code ended up working.  I split it in two to make it easier to track what was wrong.  I can recombine it into one function later if I want to.  It certainly helped as I had quite a few issues getting the ADC set up.  I spend an hour wondering why the config register was not reading back what I set, until I went back to the data sheet and found the first bit "Status" going 0 means it's working LOL.  I had the logic analyser out and everything.  DOH!

[phil from seattle]

I dunno, why not use Serial.print((0x0F & (myByte>>1)), BIN);
Granted, it doesn't suppress the lead 4 bits. Personally, I'd live with that.