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

Log in or register to post comments
Go To Last Post
22 posts / 0 new
Author
Message
jaguar98
jaguar98's picture
Level: Wannabe
Joined: Fri. May 4, 2007
Posts: 66 View posts
#1
Posted by jaguar98: Fri. Feb 1, 2008 - 04:16 PM
  • 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!

Tags:

Tools, Compilers and General Programming
Top
zoltanvoros
zoltanvoros's picture
Level: Hangaround
Joined: Mon. Aug 6, 2007
Posts: 288 View posts
Posted by zoltanvoros: Fri. Feb 1, 2008 - 04:35 PM
  • 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
Top
mikeperks
mikeperks's picture
Level: Hangaround
Joined: Tue. Nov 6, 2007
Posts: 181 View posts
Location: Austin, TX
Posted by mikeperks: Fri. Feb 1, 2008 - 04:35 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
Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 04:36 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).

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 04:37 PM
  • 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.

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
Bronze Markdavid.prentice
david.prentice's picture
Level: 10k+ Postman
Joined: Sat. Feb 12, 2005
Posts: 30460 View posts
Location: Wormshill, England
Posted by david.prentice: Fri. Feb 1, 2008 - 04:37 PM
  • 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.

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 04:40 PM
  • 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?

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
zoltanvoros
zoltanvoros's picture
Level: Hangaround
Joined: Mon. Aug 6, 2007
Posts: 288 View posts
Posted by zoltanvoros: Fri. Feb 1, 2008 - 04:41 PM
  • 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

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101096 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Fri. Feb 1, 2008 - 04:43 PM
  • 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.

Top
Bronze Markdavid.prentice
david.prentice's picture
Level: 10k+ Postman
Joined: Sat. Feb 12, 2005
Posts: 30460 View posts
Location: Wormshill, England
Posted by david.prentice: Fri. Feb 1, 2008 - 04:45 PM
  • 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
Top
mikeperks
mikeperks's picture
Level: Hangaround
Joined: Tue. Nov 6, 2007
Posts: 181 View posts
Location: Austin, TX
Posted by mikeperks: Fri. Feb 1, 2008 - 04:47 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

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101096 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Fri. Feb 1, 2008 - 04:50 PM
  • 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!

Top
zoltanvoros
zoltanvoros's picture
Level: Hangaround
Joined: Mon. Aug 6, 2007
Posts: 288 View posts
Posted by zoltanvoros: Fri. Feb 1, 2008 - 05:04 PM
  • 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

Top
jaguar98
jaguar98's picture
Level: Wannabe
Joined: Fri. May 4, 2007
Posts: 66 View posts
Posted by jaguar98: Fri. Feb 1, 2008 - 05:07 PM
  • 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..

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 05:25 PM
  • 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.

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 05:31 PM
  • 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.

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
zoltanvoros
zoltanvoros's picture
Level: Hangaround
Joined: Mon. Aug 6, 2007
Posts: 288 View posts
Posted by zoltanvoros: Fri. Feb 1, 2008 - 05:55 PM
  • 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. :)

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 06:04 PM
  • 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

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
kmr
kmr's picture
Level: Raving Lunatic
Joined: Thu. Apr 1, 2004
Posts: 3720 View posts
Location: New Mexico
Posted by kmr: Fri. Feb 1, 2008 - 06:09 PM
  • 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).

Kevin Rosenberg
http://b9.com
http://kevin.hypershots.com

Top
glitch
glitch's picture
Level: Raving Lunatic
Joined: Sat. Jan 12, 2002
Posts: 8620 View posts
Location: Canada
Posted by glitch: Fri. Feb 1, 2008 - 06:19 PM
  • 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.

Top
jaguar98
jaguar98's picture
Level: Wannabe
Joined: Fri. May 4, 2007
Posts: 66 View posts
Posted by jaguar98: Fri. Feb 1, 2008 - 07:52 PM
  • 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!

Top
mikeperks
mikeperks's picture
Level: Hangaround
Joined: Tue. Nov 6, 2007
Posts: 181 View posts
Location: Austin, TX
Posted by mikeperks: Fri. Feb 1, 2008 - 07:52 PM
  • 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

Top