STM8 library bug - GPIO_ReadInputPin()

[swhite2]

Posted on June 11, 2010 at 14:45

STM8 library bug - GPIO_ReadInputPin()

[mozra272]

Posted on May 17, 2011 at 15:09

Hi StuartMW,

 

The GPIO_ReadInputPin() function use a BitStatus value cast, so when casting the bitmask value with the BitStause type we should have SET or RESET.

BitStatus GPIO_ReadInputPin(GPIO_TypeDef* GPIOx, GPIO_Pin_TypeDef GPIO_Pin)

{

    return ((BitStatus)(GPIOx->IDR & (vu8)GPIO_Pin));

}

I don't see any issue in this function...

regards

mozra

[swhite2]

Posted on May 17, 2011 at 15:09

Using a cast from an unsigned byte to an enumeratied type is not guaranteed to produce the desired result. It may work with some compilers but it does NOT with IAR's EWSTM8. I know because I looked at the value returned  and it was NOT 0/1.

The safe, and IMO proper, way to code this is

return ((GPIOx->IDR & (vu8)GPIO_Pin) != 0 ? SET : RESET);

That will work with any compiler.

BTW if you believe that a cast should work consider this

typedef enum

            {

                ZERO,

                ONE,

                TWO,

                THREE

            } mytype_t;

Lets say I have a u8 type variable i which has the value 0x40.

What would be the result of ''(mytype_t) i''? Obviously a compiler can't  divine the programmers intent so the result would be undefined.

[Clifford1]

Did you report this bug with ST Support, or only here?

Astonishingly this remains uncorrected at https://documentation.help/STM8S-STM8A/stm8s__gpio_8c_source.html

 

Suggest:

return (GPIOx->IDR & (vu8)GPIO_Pin) == 0 ? 
RESET :
SET;

rather than the undefined behaviour cast.  

Discussed on StackOverflow at https://stackoverflow.com/questions/77727600/evaluation-if-conditional-statement-on-st-visual-developer-with-cosmic-for-stm8/77729742#77729742 more than 12 years after reporting it here.

 

In my experience (albeit on STM32) if you report a bug  (and do the analysis for them as you have) it gets fixed.  My experience is also that there are an unfortunate number of bugs, and some rather dubious code and design decisions.

 

[Clifford1]

On reflection I think the better resolution would be to change the return type to GPIO_Pin_Type  since these are bitmasks, and it is possible to call the function with a combination of bits e.g. GPIO_PIN_4 | GPIO_PIN_3, where a single result SET/RESET would make no sense.

The problem with that is that GPIO_Pin_TypeDef is an enum, so OR'ed combinations result in values that are not defined enumeration values.  Overall it is a rather dubious design.