byte mirror code don't work

am trying to write a function to mirror a byte but it always yields 00000000 :
pls what’s the errorhere

byte mirror_byte (byte my_byte){
boolean bit_array[8];
byte mirror;

byte constant=1;
bit_array[0]= ( constant & my_byte > 0x00);
for(int i=1; i<=7;i++){
constant = constant * 2 ;
bit_array*= ( constant & my_byte > 0x00);*
}
constant=128;
mirror = 0x00;
if(bit_array[0]==1)
mirror = mirror | constant;
for(int i=1; i<=7;i++){
constant = constant /2 ;
if(bit_array*==1)
mirror = mirror | constant;
_
}_
return mirror;
_
}[/quote]*_

You need to use code tags (the # button), not quote tags for code…

byte mirror_byte (byte my_byte){
  boolean bit_array[8];
  byte mirror;

  byte constant=1;
  bit_array[0]= ( constant & my_byte  > 0x00);
  for(int i=1; i<=7;i++){
    constant = constant * 2 ;
    bit_array= ( constant & my_byte  > 0x00);  // cannot assign an int to an array variable
  }

  constant=128;
  mirror = 0x00;
  if(bit_array[0]==1)
    mirror = mirror | constant;
  for(int i=1; i<=7;i++){
    constant = constant /2 ;
    if(bit_array==1)
      mirror = mirror | constant;
  }
  return mirror;
}

Firstly that doesn’t compile (see the comment I added) presume you mean bit_array[i].

Secondly you are special casing the zero bit for no clear reason, making the code complicated.

Thirdly you are using a bit array as intermediate storage which isn’t needed, overall this
is a simple function if done by shifts in a single loop:

byte mirrow_byte (byte in)
{
  byte out = 0 ;
  for (byte i = 0 ; i < 8 ; i++)
  {
    out <<= 1 ;  // use shifts for bits, not *2 and /2, since shifting is the intention
    out |= in & 1 ;  // shint one bit at a time to/from the least significant end
    in >>= 1 ;
  }
  return out ;
}

(I find it easier to write a simple version than spot the bug(s) in the complex version!)

I took the time and trouble to render your illegibly quoted code and put it in code tags, and you took them off and returned them to quote tags. W..T..F ?

I had a go at my own and shaved a few bytes off the compiled size. Without a loop it may also be faster, gonna test soon.

EDIT: appears it is heavily inlined. Still not sure on the timing, but would be quite large in a function.

struct BitByte{
 int A : 1;
 int B : 1; 
 int C : 1; 
 int D : 1; 
 int E : 1; 
 int F : 1; 
 int T : 1; 
 int P : 1; 
};

const char MirrorByte( const char c ){

  union{
    char b;
    BitByte a; 
  } f = {c};
  
  union{
    BitByte g; 
    char h;
  } i = { f.a.P, f.a.T, f.a.F, f.a.E, f.a.D, f.a.C, f.a.B, f.a.A };  
  
  return i.h; 
}

void setup() {
  Serial.begin( 9600 );
  char c = 0xFE;
  Serial.println( ( unsigned char ) c, HEX );
  Serial.println( ( unsigned char ) MirrorByte(  c ), HEX );
}

void loop() {}

The original code treats bit 0 as a special case, which is just wrong. It also looks far too complicated.

If you want a simple solution, just use a for loop to process bits 0 .. 7, bitRead() to read the bit value from the input and bitWrite() to set the corresponding bit in the output.

If you want a fast solution, define a 256 byte lookup table in progmem.

Please USE CODE TAGS when posting code.

What about this:

void setup(){
  Serial.begin(115200);
  byte c = B11000101;
  Serial.println(c, 2);
  Serial.println(reverse(c), 2);
}//setup()

void loop(){}//()

byte reverse(byte x){
    x = (((x&0xaa)>>1)|((x&0x55)<<1));
    x = (((x&0xcc)>>2)|((x& 0x33)<<2));
    return((x>>4)|(x<<4));
}//reverse

How many minutes did you spend looking at that (or testing it) to confirm that it worked correctly for all possible input values?

Google "bit twiddling"

happy now?

char buffer[20];
boolean flag;


void setup(){
  Serial.begin(115200);
  for(byte i = 0; i < 255; i++)
    if(reverse(reverse(i)) != i){
      Serial.println(i);
      flag = true;
    }
  if(flag) Serial.println("failed");
  else Serial.println("success");
}//setup()

void loop(){}//()

byte reverse(byte x){
    x = (((x & 0xaa) >> 1) | ((x & 0x55) << 1));
    x = (((x & 0xcc) >> 2) | ((x & 0x33) << 2));
    return((x >> 4) | (x << 4));
}//reverse

See how simply robtillaart answered this question in a previous thread:

http://forum.arduino.cc/index.php?topic=54304.msg388532#msg388532

If it helps any, here is my solution.

byte Mirror(byte ary)
{
  byte h, l;
  byte table[16] = {
    0x0, 0x8, 0x4, 0xC,
    0x2, 0xA, 0x6, 0xE,
    0x1, 0x9, 0x5, 0xD,
    0x3, 0xB, 0x7, 0xF
  };
  h = table[ (ary & 0xf0) >> 4 ]; // Notice the closing bracket is different from the one below. This is because of the look up table is written as 0xN and not 0x0N 
  l = table[ ary & 0x0f ] << 4;
  //Serial.println( l | h ,HEX);
  return (l | h ); 
}

With a nibble swap instruction, that's a really good solution.

Thanks

AWOL:
Google “bit twiddling”

The trouble with all these clever solutions is that you have to either trust that they’re right, or work painfully through to prove to yourself that they are. It’s a triumph of ingenuity over clarity, and great to show what a smart coder you are (or what a great Googler) but unless you need to squeeze every instruction byte or CPU cycle out of the solution, this sort of obfuscation isn’t called for. The most straight forward solution is a for loop calling bitRead() and bitWrite().

The most straight forward solution is a for loop calling bitRead() and bitWrite().

I agree, but for some reason my IDE Version 1.0.5 does not know what the bitWrite() function is, even after I reinstalled it.

So I made that mirror function like it is, (I did not Google it) and it seems to work rather well.

Added: Strange thing, I just downloaded version 1.0.5 on my old Windows XP laptop, and the bitWrite function works, but the exact same sketch on my new windows 8 laptop, with the same IDE version does not compile. It says "bitWrite was not declared in this scope", So I copied the IDE from my XP laptop, to my Windows 8 laptop, and the bitWrite function now works.

I just reinstalled the IDE from the download page, and again it works. Maybe something just wasn't downloading correctly, the first 3 times. :( But never the less it works now.

HazardsMind: So I made that mirror function like it is, (I did not Google it) and it seems to work rather well.

Yes, same goes here. I tried to do it without a loop, which was easier than I thought.

Its good however that others have posted their own and excerpts from Sean Anderson's bit hacks. Out of curiosity, I'm going to profile them to see what each one can do. I want to make an assumption that the short and sweet 64-bit versions will not pull though in a timely fashion, i'm curious to see what the three non-looped versions can do compared to a loop.

The results are in, they were partly as I expected.

There are six algorithms used. I named each one under the person who posted it, not who wrote it.

Order of appearence in thread:

The method used is simple, each iteration consists of:

  • Loop through each value of a byte.
  • Run the mirror function twice then verify the result is the same as the original value.

Here are the results.
The fastest function is 28% faster than its closest alternative.
It is 98% faster than the slowest.

If you are tight on flash memory MarkT’s version is 68% smaller than the next version,
if you are after speed, my version takes the cake.

I have attached the sketch and output captures for Uno & Mega.

mirror.ino (5.07 KB)

UNO mirror test.log (4.47 KB)

MEGA mirror test.log (5.1 KB)

Nice work, thank you.


Strange thing, I just downloaded version 1.0.5 on my old Windows XP laptop, and the bitWrite function works, but the exact same sketch on my new windows 8 laptop, with the same IDE version does not compile.

Maybe windows 8 doesn’t work with bits any more? :wink:

pYro_65:
The results are in, they were partly as I expected.

An older test (2011) for the same problem - http://forum.arduino.cc/index.php?topic=54304.msg389923#msg389923 -

  1. global lookup table with 256 values - in flash - beats all in speed : < 1 uSec / flip
  2. global lookup table with 16 values - idem - is a good runner up: < 3 usec / flip

(2) is almost identical too HazardsMinds except for - global array vs local array.

For the footprint the array should be in PROGMEM… (as PeterH pointed out earlier in this thread!)

 void setup()
{
  Serial.begin(115200);
}

// big lookup table
prog_uchar flip[256] PROGMEM = { 0,128,64,192,32,160,96,224,16,144,80,208,48,176,112,240,8,136,72,200,40,168,104,232,24,152,88,216,56,184,120,248,4,132,68,196,36,164,100,228,20,148,84,212,52,180,116,244,12,140,76,204,44,172,108,236,28,156,92,220,60,188,124,252,2,130,66,194,34,162,98,226,18,146,82,210,50,178,114,242,10,138,74,202,42,170,106,234,26,154,90,218,58,186,122,250,6,134,70,198,38,166,102,230,22,150,86,214,54,182,118,246,14,142,78,206,46,174,110,238,30,158,94,222,62,190,126,254,1,129,65,193,33,161,97,225,17,145,81,209,49,177,113,241,9,137,73,201,41,169,105,233,25,153,89,217,57,185,121,249,5,133,69,197,37,165,101,229,21,149,85,213,53,181,117,245,13,141,77,205,45,173,109,237,29,157,93,221,61,189,125,253,3,131,67,195,35,163,99,227,19,147,83,211,51,179,115,243,11,139,75,203,43,171,107,235,27,155,91,219,59,187,123,251,7,135,71,199,39,167,103,231,23,151,87,215,55,183,119,247,15,143,79,207,47,175,111,239,31,159,95,223,63,191,127,255};

void loop()
{
  unsigned long start = micros();
  for (int i=0; i<256; i++)
  {
    volatile uint8_t b = i;
    b = pgm_read_byte_near(flip + b);
    b = pgm_read_byte_near(flip + b);
  }
  Serial.println(micros() - start);
  
  for (int i=0; i<256; i++)
  {
    volatile uint8_t b = i;
    b = pgm_read_byte_near(flip + b);
    b = pgm_read_byte_near(flip + b);
    Serial.print(b);
    Serial.print(',');
  }
  Serial.println();
  while (1);
}

468 usec (512 flips) => less than 1 usec / flip (including loop overhead)