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

22 posts / 0 new
Author
Message

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!

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

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

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).

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.

```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.

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?

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

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.

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

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

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!

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

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..

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.

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.

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. :)

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
```

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).

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.

Thanks a Lot all of you guys!

now my code changed FROM:

/*for(cINDEX = 10; cINDEX > 5; 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;
}

thanks again for the help!

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