A bitwise operation that im confused with

Question

Hi im working on stm32f4. I need to read 8 pin and sum the values after convert like binary to decimal. For example if the values of 8 bits are 000000110, my CardID must be 6. i used this, but there can be better way:

CardID = (GPIOD->IDR & GPIO_Pin_8) + 
         (GPIOD->IDR & GPIO_Pin_9)*2 + 
         (GPIOD->IDR & GPIO_Pin_10)*4 + 
         (GPIOD->IDR & GPIO_Pin_11)*8 + 
         (GPIOD->IDR & GPIO_Pin_12)*16 + 
         (GPIOD->IDR & GPIO_Pin_13)*32 + 
         (GPIOD->IDR & GPIO_Pin_14)*64 + 
         (GPIOD->IDR & GPIO_Pin_15)*128;

Hint: i know the value of GPIO_Pin_8,GPIO_Pin_9....GPIO_Pin_15.

Can we do the AND operation with a number of combinations?

#define GPIO_Pin_8                 ((uint16_t)0x0100)  /* Pin 8 selected */
#define GPIO_Pin_9                 ((uint16_t)0x0200)  /* Pin 9 selected */
#define GPIO_Pin_10                ((uint16_t)0x0400)  /* Pin 10 selected */
#define GPIO_Pin_11                ((uint16_t)0x0800)  /* Pin 11 selected */
#define GPIO_Pin_12                ((uint16_t)0x1000)  /* Pin 12 selected */
#define GPIO_Pin_13                ((uint16_t)0x2000)  /* Pin 13 selected */
#define GPIO_Pin_14                ((uint16_t)0x4000)  /* Pin 14 selected */
#define GPIO_Pin_15                ((uint16_t)0x8000)  /* Pin 15 selected */

@effenok: Why is that relevant? Anyway: OP did mention the MCU family he uses; this implies its meaning. — too honest for this site, Aug 02 '15 at 14:04
I got it, i was thinking wrong and its working now thanks all of you :) — abdullah cinar, Aug 02 '15 at 14:23
If one answer helped, you should think of accepting it. That is the way to say "thank you" on SO. — too honest for this site, Aug 02 '15 at 14:51
No the wrong part was mine not yours. I meant i was wrong and you helped me to make it correct. I am new at this site Sorry if i failed with english and thanks again :) — abdullah cinar, Aug 02 '15 at 15:01
[How and why to accept an answer](http://meta.stackexchange.com/a/5235/237804) (not sure it was clear) — anatolyg, Aug 02 '15 at 15:09
Lol i didnt think accepting an answer is a button, sorry im new here :D — abdullah cinar, Aug 02 '15 at 15:24

too honest for this site · Accepted Answer · 2015-08-02T17:53:17.353

3

This is likely wrong:

You should not read the input data register multiple times to get corresponding values. The register is defined volatile, so the compiler can not optimize away accesses.
masking with & does not return 0 or 1 (boolean result), but leaves the bit in the same position it has. The multiplications will move them to positions 15..22, while presumably they should be moved to bits 0..7.

As these are consecutive bits, the correct way would be:

// must be consecutive bits:
#define GPIO_CARD_ID_MASK  0xFF00U
#define GPIO_CARD_ID_SHIFT 8

CardID = (GPIOD->IDR & GPIO_CARD_ID_MASK) >> GPIO_CARD_ID_SHIFT;

This only reads the register once, thus avoiding race-conditions and - possibly more important here - saves many clock cycles. It also is (almost) self-explaining by using properly named macros, thus better maintainable.

Note: For the given architecture/MCU, the mask is actually not necessary (these are the upper 8 bits of an uint16_t), but good style. It documents which bits you are being used. If the field does not happen to be the upper bits, you will need it anyway, so the mask makes it more verswatile (it still requires consecutive bits, discontinous numbering will make it much less straight-forward).

edited Aug 02 '15 at 17:53

answered Aug 02 '15 at 13:53

too honest for this site

12,050
4
30
52

I fully agree about the registers access, they are `volatile` for their nature. But what I don't understand is why the first sample is more portable than the second. Taken as mandatory that the bits **must be contiguous** or it will not work anyway, could you please explain better? – Frankie_C Aug 02 '15 at 14:32
@Frankie_C: 1) Thanks for the unjustified downvote (I did answer the question!). 2) It is more readable, thus maintainable. For portability, I will edit, however, as it still depends on a magic number. – too honest for this site Aug 02 '15 at 14:42
You're welcome, but I have **not downvoted** your answer. I just asked because I really don't understand the reason, and because I having read many interesting answers by you that shows high skills, I thought that there was something that I was missing. – Frankie_C Aug 02 '15 at 14:51
@Frankie_C: sorry then! I seem to have something unpleasant under my fingernail. – too honest for this site Aug 02 '15 at 14:54

score 1 · Answer 2 · answered Aug 02 '15 at 13:47

1

It seems to me that you just want something like this:

#define GPIO_PINS      ((uint16_t)0xFF00)
#define GPIO_OFFSET    (8)

// ...

CardID = (GPIOD->IDR & GPIO_PINS) >> GPIO_OFFSET;

answered Aug 02 '15 at 13:47

Oliver Charlesworth

267,707
33
569
680

score 1 · Answer 3 · answered Aug 02 '15 at 13:49

1

Just get the full GPIOD->IDR then adjust it shifting and masking to get a 16 bits int equal to the converted value:

CardID = (GPIOD->IDR >> 8) & 0x00ff;

Your card ID is in the upper 8 bits of GPIOD->IDR.

answered Aug 02 '15 at 13:49

Frankie_C

4,764
1
13
30

Oh i was so wrong thanks for the all replies. I was thinking CardID must directly equal to decimal 6(for that example). But if cardid is equal to 0110,that means it is equal to 6 too right? I was confused, if a variable is equal to binary 0110, i dont need to convert it decimal to do operations at decimal right? They are both same. (correct me if im wrong and sorry for bad english :) ) – abdullah cinar Aug 02 '15 at 14:15
Yes the number is the same only expressed in a different radix. The way we express numbers is just a convention to represent it, so we can write `10d, 1010b, 0AH, 012` that represents always 10 respectively in decimal, binary, hex and octal. – Frankie_C Aug 02 '15 at 14:27
Dude thank you so much i was wondering that. Its all solved on my head now :) – abdullah cinar Aug 02 '15 at 14:35
@abdullahcinar: Current (and most past) digital computers store data in binary form. They have no idea about decimal in general. Conversion is done using libraries for user input/output or to store/transfer only. And then the computer can not easily proces that data as numbers. – too honest for this site Aug 02 '15 at 14:49
@olaf So i want to ask one more thing. İf i dont need to do conversions(decimal or binary etc.), isnt that enough to use: CardID = (GPIOD->IDR >> 8) Am i not able to read that 8 bits(8to15) with this? Why i need to shift? Sorry if i misunderstood you – abdullah cinar Aug 02 '15 at 16:25
I want to look at 8 bits(8to15) so i tried this part and it worked: CardID = (GPIOD->IDR >> 8) Is it ok or i need to use it with AND op? – abdullah cinar Aug 02 '15 at 16:38
@abdullahcinar: It is used because different systems may handle the shift in a wrong way. The general action is to add 0s in MSBit for Right shift and 0s to LSBit for Left shifts, so the and is superfluous. – Frankie_C Aug 02 '15 at 16:52
@abdullahcinar: You are right about `IDR`, as that is `uint16_t` for STM32F4. However, as that is `unsigned short` on ARM, it will be propagated to `int` before the shift. While this still does not make any problems on the architecture, Using the mask is still good practice to tell the compiler not to use any higher bits and to document to the reader which bits you are intereseted in. It might be very well the compiler will detect the redundancy and optimize away the masking (OTOH, this just costs one clock plus one word of code - not much of a penalty). Remove, if code is **that** critical. – too honest for this site Aug 02 '15 at 17:49
@Frankie_C: `IDR` is `uint16_t`, so the shift is very well defined, even as it is promoted to `int` (32 bits on ARM) first - if `int` was 16 bits, `IDR` would be promoted to `unsigned int`. Long prelude: Bitshifting unsigned integers is very well defined by the standard. The problem are signed integer types (and here mostly right shifts). – too honest for this site Aug 02 '15 at 17:58
Thank you i completely got it now. If my array had more than 16 bit, it could be a problem. I added the mask part though :) – abdullah cinar Aug 02 '15 at 18:48

A bitwise operation that im confused with

3 Answers3