How to Convert my_str[4] ={1,1,1,1} to my_char =0b00001111?

Go To Last Post
22 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi to all!

Which is the best way to convert
my_str[4] ={0,0,0,0,0,1,1,1} to
my_char = 0b00000111 (--> 7 Decimal )

???

Thank you!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't quite see what type my_str is. It seems to have for elements, yet you assign 8 numbers to it. Is it a word? Anyway, if you just want to convert that into a character, you could use

char ch=0;
for(int i=0; i < 8; i++) {
 if(my_str[i]) ch |= (1 << (8-i));
}

or something like that.
Zoltan

Last Edited: Fri. Feb 1, 2008 - 04:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Assuming my_str is declared as a numeric type then you need to think about the binary representation like this:

mychar = my_str[0] * 2 ^0 +
         my_str[1] * 2^1 +
         my_str[2] * 2^2 +
         my_str[3] * 2^3

Try writing a loop to do the job where the index of my_str varies. Note how the index is the same as the power of 2 required in the sum. Think about how you would refactor the sum to eliminate the exp function and change the multipication of two as a left shift.

Edit: Zoltan beat me to it and gave you a first attempt at an answer. It is not quite correct and could be improved to remove the (8-i) calculation.

--Mike

Last Edited: Fri. Feb 1, 2008 - 04:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First, my_str[4] should be declared as my_str[8]. Second, the best way is to iterate 8 times, shift a temp one bit, read the value of my_str[i] and if is not-zero, then set the bit in temp depending upon the direction of your shift (0x80 for right shifting or 0x01 for left shifting).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

zoltanvoros wrote:
I don't quite see what type my_str is. It seems to have for elements, yet you assign 8 numbers to it. Anyway, if you just want to convert that into a character, you could use

char ch=0;
for(int i=0; i < 8; i++) {
 if(my_str[i]) ch |= (1 << i);
}

or something like that.
Zoltan

You need to shift ch whether not or not my_str[i] is non-zero.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
for (i = 0, my_char =0; i < sizeof(my_str); my_char <<= 1)
    my_char += my_str[i++];

Your teacher should tell you off for writing that.
Your compiler should tell you off for the wrong number of initialisers.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Your compiler should tell you off for the wrong number of initialisers.
I agree the code has some issues, but why would the compiler complain about multiple initializers?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kmr wrote:
You need to shift ch whether not or not my_str[i] is non-zero.

Kevin,

I think, I've fixed that. Correct me, if I am wrong.
Cheers,
Zoltan

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kmr wrote:
I agree the code has some issues, but why would the compiler complain about multiple initializers?

Because 8 were provided for something with a dimension of 4. At the very least you might expect "warning: too many initializers for my_str[]" or something?

Of course (though I'm not sure if folks would consider this bad practice?):

type?? my_str[] ={0,0,0,0,0,1,1,1};

would be fine -leaving the compiler to auto-dimension it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kevin,

He has declared an array of size 4 with 8 initialisers.

I also note that I should have read the array backwards, so that if it really was 4 wide the result would only shift into the bottom 4 bits.

David.

Last Edited: Fri. Feb 1, 2008 - 04:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

My philosophy is not to give the answer right away unless the OP has made some sincere attempts to find the answer first and is just stuck. Otherwise where is the learning if people spoon feed you the answer. And more to the point, how do you tell if the answer is even correct?

--Mike

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Mike,

I'm having trouble with your "more than half" - care to name and shame?

(in fact besides zoltranvoss' inclusion of the shift in the conditional I can't spot more?)

EDIT: Ah, it appears some of your post may have disapparated in a puff of its own logic while I wrote this!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

(in fact besides zoltanvoros' inclusion of the shift in the conditional I can't spot more?)

Now I am really curious to see where the problem is with that shift. I am not saying it's correct (I only believe it should be, and I don't see where it could be wrong), so if it's wrong, point it out :?
Zoltan

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WOW! thanks for the speed guys!

Well, my mistake, I meant unsigned char my_str[8] ={0,0,0,0,0,1,1,1} and don't my_str[4] ={0,0,0,0,0,1,1,1} ...sorry!

I didn't implemented it yet, so I did not see this error..

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Because 8 were provided for something with a dimension of 4.
Yes, I'm aware of that issue as mentioned in my post. The "too many initializers" post had only the for loop in it so I thought the poster was referring to the multiple initializers in the quoted loop, not the number of initializers in the array definition not was not quoted.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

zoltanvoros wrote:
I think, I've fixed that. Correct me, if I am wrong.
Yes, I see your fix. In general, since the AVR's can only shift one bit at a time, rather than your code of
char ch=0; 
for(int i=0; i < 8; i++) { 
 if(my_str) ch |= (1 << (8-i)); 
}

it is more efficient to shift one bit at a time

unsigned char val; // not initialized since all 8 bits will be filled
for (unsigned char i = 0; i < 8; i++) {
  val <<= 1;
  if (my_str[i]) val |= 0x01;
}

[i]Edit:
For comparison, here is the assembly output of AVR GCC with -Os for your code

  5a:	60 e0       	ldi	r22, 0x00	; 0
  5c:	70 e0       	ldi	r23, 0x00	; 0
  5e:	fb 01       	movw	r30, r22
  60:	e0 5a       	subi	r30, 0xA0	; 160
  62:	ff 4f       	sbci	r31, 0xFF	; 255
  64:	80 81       	ld	r24, Z
  66:	88 23       	and	r24, r24
  68:	81 f0       	breq	.+32     	; 0x8a 
  6a:	40 91 68 00 	lds	r20, 0x0068
  6e:	28 e0       	ldi	r18, 0x08	; 8
  70:	30 e0       	ldi	r19, 0x00	; 0
  72:	26 1b       	sub	r18, r22
  74:	37 0b       	sbc	r19, r23
  76:	81 e0       	ldi	r24, 0x01	; 1
  78:	90 e0       	ldi	r25, 0x00	; 0
  7a:	02 c0       	rjmp	.+4      	; 0x80 
  7c:	88 0f       	add	r24, r24
  7e:	99 1f       	adc	r25, r25
  80:	2a 95       	dec	r18
  82:	e2 f7       	brpl	.-8      	; 0x7c 
  84:	48 2b       	or	r20, r24
  86:	40 93 68 00 	sts	0x0068, r20
  8a:	6f 5f       	subi	r22, 0xFF	; 255
  8c:	7f 4f       	sbci	r23, 0xFF	; 255
  8e:	68 30       	cpi	r22, 0x08	; 8
  90:	71 05       	cpc	r23, r1
  92:	29 f7       	brne	.-54     	; 0x5e 
  94:	08 95       	ret

and mine

0000005a 
: 5a: e0 e6 ldi r30, 0x60 ; 96 5c: f0 e0 ldi r31, 0x00 ; 0 5e: 80 91 69 00 lds r24, 0x0069 62: 88 0f add r24, r24 64: 80 93 69 00 sts 0x0069, r24 68: 80 81 ld r24, Z 6a: 88 23 and r24, r24 6c: 29 f0 breq .+10 ; 0x78 6e: 80 91 69 00 lds r24, 0x0069 72: 81 60 ori r24, 0x01 ; 1 74: 80 93 69 00 sts 0x0069, r24 78: 31 96 adiw r30, 0x01 ; 1 7a: 80 e0 ldi r24, 0x00 ; 0 7c: e8 36 cpi r30, 0x68 ; 104 7e: f8 07 cpc r31, r24 80: 71 f7 brne .-36 ; 0x5e 82: 08 95 ret

Your assembly has a loop within a loop that performs a variable number of shifts depending upon the (8-i) value.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kmr wrote:
...it is more efficient to shift one bit at a time

unsigned char val; // not initialized since all 8 bits will be filled
for (unsigned char i = 0; i < 8; i++) {
  val <<= 1;
  if (my_str[i]) val |= 0x01;
}

OK, thanks for pointing that out. Also, could we improve on the speed by eliminating the "if" statement altogether. Something like this:

unsigned char val; // not initialized since all 8 bits will be filled
for (unsigned char i = 0; i < 8; i++) {
  val <<= 1;
  val |= (0x01 & my_str[i]);
}

I don't exactly know how much an "if" costs, but it's quite expensive, isn't it? I have never been so much pressed for speed that I had to care about it.
Zoltan

EDIT: I have found your comparison between the codes. It's cute. :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, the code is smaller/faster without the if statement. So, assuming my_str really only has 0 and 1 in it and not 0 and something-that-is-non-zero, not using the if is more efficient. As for the cost, one can always look at the assembly output.

;;  for (unsigned char i = 0; i < 8; i++) { 
;;    val <<= 1; 
;;    val |= (0x01 & my_str[i]); 
;;  } 

  5a:	e0 e6       	ldi	r30, 0x60	; 96
  5c:	f0 e0       	ldi	r31, 0x00	; 0
  5e:	80 91 69 00 	lds	r24, 0x0069
  62:	88 0f       	add	r24, r24
  64:	80 93 69 00 	sts	0x0069, r24
  68:	90 91 69 00 	lds	r25, 0x0069
  6c:	81 91       	ld	r24, Z+
  6e:	81 70       	andi	r24, 0x01	; 1
  70:	89 2b       	or	r24, r25
  72:	80 93 69 00 	sts	0x0069, r24
  76:	80 e0       	ldi	r24, 0x00	; 0
  78:	e8 36       	cpi	r30, 0x68	; 104
  7a:	f8 07       	cpc	r31, r24
  7c:	81 f7       	brne	.-32     	; 0x5e 
  7e:	08 95       	ret
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

zoltanvoros wrote:
I have found your comparison between the codes. It's cute. :)
I'm glad to have added to the cuteness of the thread, but that wasn't my goal. It was to show that in questions of efficiency of code to be compiled, one can simply examine to output of the compiler for a definitive answer (that, of course, can vary across compilers and optimization choices).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Note that 8-i is incorrect, it should be 7-i since the shift range (of an 8 bit variable) is 0-7, and i is in the range of 0-7. (this is a logical error, and has no effect on the efficiency of the code) Any shift by 8 would push the value beyond the limit of the 8bit char variable, and would be truncated off.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks a Lot all of you guys!

now my code changed FROM:

/*for(cINDEX = 10; cINDEX > 5; cINDEX--)
{
cAUX = CONVERT_STREAM_VALUE(cRECEIVED_STREAM[cINDEX]);

if (cAUX == 1)
{
cAUX <<= (10-cINDEX);
cCOMMAND |= cAUX;
}
else
{
cAUX = 1;
cAUX <<= (10-cINDEX);
cAUX = ~cAUX;
cCOMMAND &= cAUX;
}
}*/

TO:
cCOMMAND = 0;
for(cINDEX = 10; cINDEX > 5; cINDEX--)
{
cCOMMAND <<= 1;
if (cRECEIVED_STREAM[cINDEX]) cCOMMAND |= 0x01;
}

thanks again for the help!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Mike,

I'm having trouble with your "more than half" - care to name and shame?

(in fact besides zoltranvoss' inclusion of the shift in the conditional I can't spot more?)

EDIT: Ah, it appears some of your post may have disapparated in a puff of its own logic while I wrote this!

I went off to go verify my suspicions before posting again. I don't have an AVR to hand so I used MinGW. The solution by david.prentice results in an answer of 14 instead of 7 because there is an extra shift. Remember that the iterator gets executed before the test (K&R):
for  (exp1; exp2; exp3) statement

is the same as

exp1;
while (exp2) {
    statement
    exp3;
}

So I guess that is 2 out 3 are incorrect. Here is my version of the solution that is correct and readable:

   my_char = 0;
   for (i = 0; i < sizeof(my_str); i++) {
      my_char = (my_char * 2) + my_str[i];
   } 

BTW I created a new thread on the related subject of C style and efficiency.

--Mike