0

I am using a NUCLEO-L476RG development board, I am learning to write GPIO drivers for STM32 family I have implementing a simple logic in which I need to turn on an LED when a push button is pressed.

I have a strange issue: Edit 1:The Bread board LED turns ON when the line temp=10 is commented, it doesn't turn ON when the delay issue called. Assuming if I add any line of code into that while loop the LED does not turn ON

The Bread board LED turns ON when the delay() function is commented, it doesn't turn ON when the delay issue called.

What could be the issue? I have powered the board using the mini usb connector on the board, and the clock is configured at MSI with 4MHz Connection Details

#define delay() for(uint32_t i=0; i<=50000; i++);

int main(void)
{
    GPIO_Handle_t NucleoUserLED,NucleoUserPB,BreadBoardLED,BreadBoardPB;
    uint8_t inputVal,BBinpVal;
    uint32_t temp;

    //User green led in the nucleo board connected to PA5
    NucleoUserLED.pGPIO                             = GPIOA;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinNumber       = GPIO_PIN_5;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinMode         = GPIO_MODE_OP;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinPuPdControl  = GPIO_IP_NO_PUPD;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinOpType       = GPIO_OP_TYPE_PP;

    //User blue button in the nucleo connected to PC13
    NucleoUserPB.pGPIO                              = GPIOC;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinNumber        = GPIO_PIN_13;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinMode          = GPIO_MODE_IP;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinPuPdControl   = GPIO_IP_NO_PUPD;

    //User led in the bread board connected to PC8
    BreadBoardLED.pGPIO                             = GPIOC;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber       = GPIO_PIN_8;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinMode         = GPIO_MODE_OP;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinPuPdControl  = GPIO_IP_NO_PUPD;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinOpType       = GPIO_OP_TYPE_PP;

    //User DPDT connected in the breadboard connected to PC6
    BreadBoardPB.pGPIO                              = GPIOC;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinNumber        = GPIO_PIN_6;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinMode          = GPIO_MODE_IP;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinPuPdControl   = GPIO_IP_PU;


    GPIO_PeriClkCtrl(GPIOA, ENABLE);
    GPIO_PeriClkCtrl(GPIOC, ENABLE);

    GPIO_Init(&NucleoUserLED);
    GPIO_Init(&NucleoUserPB);
    GPIO_Init(&BreadBoardLED);
    GPIO_Init(&BreadBoardPB);

    while(1)
    {

        /*****************************************************************
         *       Controlling the IO present in the nucleo board          *
         *****************************************************************/
        inputVal = GPIO_ReadInputPin(NucleoUserPB.pGPIO, NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinNumber);
        BBinpVal = GPIO_ReadInputPin(BreadBoardPB.pGPIO, BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinNumber);




        if(inputVal == 0)
        {

            GPIO_ToggleOutputPin(NucleoUserLED.pGPIO, NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinNumber);
        }

        /*****************************************************************
         *       Controlling the IO present in the bread board           *
         *****************************************************************/


        if (BBinpVal == 0 )
        {
            GPIO_WriteOutputPin(BreadBoardLED.pGPIO, BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber, 1);
        }
        else
        {
            GPIO_WriteOutputPin(BreadBoardLED.pGPIO, BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber, 0);
        }
        delay();
    }

    return 0;
}
Improve this question
8
  • There is no line temp = 10 - temp is unused. That leads me to suspect that the code posted is not necessarily the code that exhibits the problem. Start by switching off any an all compiler optimisation. Nucleo boards integrate an STLink debug interface, you can for example step the code line-by-line to understand what is going on. That should generally be your first course before _debugging by SO question. Commented Jan 30, 2022 at 7:14
  • If you hold the BB button down whilst applying the power (or press reset if there is one) does the LED light then? That would suggest the delay loop is not terminating so the while loop is not iterating. Again you can determine that (and why) in a debugger. Commented Jan 30, 2022 at 7:23
  • Oh, and loose the macro - it is not conducive to effective debugging in a source level debugger. Commented Jan 30, 2022 at 7:28
  • What is the delay actually for? What happens if you move it inside the BBinpVal == 0 block (after the LED on) so that it only occurs on a button press? Does the LED then stick on? Commented Jan 30, 2022 at 7:53
  • @Clifford 1: To diagnose the issue I have removed the function and placed a dummy line temp=10; and unfortunately the LED did not turn ON when the the line was commented.. As per the solution given on this thread declaring the temp variable as global solved the issue https://community.st.com/s/question/0D53W00001L08UqSAJ/nucleo-l476rg-turn-on-led Commented Jan 30, 2022 at 11:18

4 Answers 4

Reset to default
2
  1. It is not a function only the macrodefinition.
  2. Your loop is likely to be optimized out

define it as

void inline __attribute__((always_inline)) delay(uint32_t delay)
{
   while(delay--) __asm("");
}

Bear in mind that 50000 can be quite long if you run on low clock settings.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Would have to be very low clock speed - STM32 MCUs start up on the HSI (internal) oscillator at 16MHz. A more likely issue is the time being rather short, at typical clock speeds. Is there any real benefit in in-lining a delay? I'd leave it to the compiler to make that decision. Is there any advantage in the dummy in-line assembler over simply declaring the counter volatile? The latter would be more portable since in-line assembly syntax is not standardised. Genuinely interested in your solution, in no way a criticism.
@Clifford godbolt.org/z/8Kc1b64K6. No stack use (volatile variable requires permanent storage), more precise delay control as it has only 2 instructions.
static would require permanent storage, but not volatile, but I take your point, it requires stack usage. I see no real or significant compelling advantage that would make me jump through such compiler specific hoops. A "premature optimisation" in most cases I think. But a sound answer nonetheless - even if not what I'd do - SO is better with different solutions, but for a novice to choose amongst them it is well to have some explanation.
@Clifford no static does not require anything and compiler is free to do what it wants. volatile require an object to be created
You miss my point I think; it was the term permanent (which I took to mean for the lifetime of the execution) that I was questioning. Stack space is reusable not permanently assigned to a single object. Semantically a static lifetime is for the duration of the execution whether or not any actual storage is allocated or optimised-out. I think perhaps we are in violent agreement - and straying into semantic issues that are not really relevant to the question.
|
0

Not sure what the issue is because "it is not working" is not very specific.

However there are "quality" issues:

  • That is an inappropriate use of a macro - there is no benefit over using a function. The function call overhead argument does not hold - it is a delay, it is supposed to take time!
  • The empty-loop counter is not declared volatile - the compiler at any optimisation level other then the minimum is likely to remove the loop altogether.
  • A for-loop for a delay is a crude and generally non-deterministic solution, with a period that will change between compilers, with different compiler options and on different targets or with different clock speeds. STM32 is a Cortex-M device and given that you should use the SYSTICK counter for this. For example, as a minimum something like:
volatile uint32_t tick = 0 ;
  
void SysTick_Handler(void)  
{
    tick++ ;
}

void delayms( uint32_t millisec )
{
    static bool init = false ;
    if( !init )
    {
        SysTick_Config( SystemCoreClock / 1000 ) ;
        init = true ;
    }

    uint32_t start = tick ;
    while( tick - start < millisec ) ;
}
Improve this answer

3 Comments

static not needed
@0___________ The use of static was intentional, as an initialise on first use, but it is only semantically correct in C++ . It is not specified, but this was clearly a C not C++ question. Fixed it, but the C code is somewhat cumbersome by comparison. More commonly perhaps you'd do that separately at system initialisation.
@Clifford "Its not working" I meant the LED does not blink
0

The issue was solved by declaring the iterator as a global variable. Now the LED turns on when the Push button is pressed

Previous implementation

#define delay() for(uint32_t i=0; i<=50000; i++);

Working implementation

   uint32_t temp;
    
        void delay(void)
        {
            for(temp = 0;temp<=50000;temp++)
            {
                ;
            }
        }

Can any one tell me how declaring the variable as global solves the issue?

Find the working implementation below

#include <stdint.h>
#include "stm32l476xx.h"
#include "stm32l476xx_gpoi_driver.h"
#if !defined(__SOFT_FP__) && defined(__ARM_FP)
  #warning "FPU is not initialized, but the project is compiling for an FPU. Please initialize the FPU before use."
#endif



uint32_t temp;

void delay(void)
{
    for(temp = 0;temp<=50000;temp++)
    {
        ;
    }
}


int main(void)
{
    GPIO_Handle_t NucleoUserLED,NucleoUserPB,BreadBoardLED,BreadBoardPB;
    volatile uint8_t inputVal,BBinpVal;


    //User green led in the nucleo board connected to PA5
    NucleoUserLED.pGPIO                             = GPIOA;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinNumber       = GPIO_PIN_5;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinMode         = GPIO_MODE_OP;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinPuPdControl  = GPIO_IP_NO_PUPD;
    NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinOpType       = GPIO_OP_TYPE_PP;

    //User blue button in the nucleo connected to PC13
    NucleoUserPB.pGPIO                              = GPIOC;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinNumber        = GPIO_PIN_13;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinMode          = GPIO_MODE_IP;
    NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinPuPdControl   = GPIO_IP_NO_PUPD;

    //User led in the bread board connected to PC8
    BreadBoardLED.pGPIO                             = GPIOC;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber       = GPIO_PIN_8;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinMode         = GPIO_MODE_OP;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinPuPdControl  = GPIO_IP_NO_PUPD;
    BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinOpType       = GPIO_OP_TYPE_PP;

    //User DPDT connected in the breadboard connected to PC6
    BreadBoardPB.pGPIO                              = GPIOC;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinNumber        = GPIO_PIN_6;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinMode          = GPIO_MODE_IP;
    BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinPuPdControl   = GPIO_IP_PU;


    GPIO_PeriClkCtrl(GPIOA, ENABLE);
    GPIO_PeriClkCtrl(GPIOC, ENABLE);

    GPIO_Init(&NucleoUserLED);
    GPIO_Init(&NucleoUserPB);
    GPIO_Init(&BreadBoardLED);
    GPIO_Init(&BreadBoardPB);

    while(1)
    {

        /*****************************************************************
         *       Controlling the IO present in the nucleo board          *
         *****************************************************************/
        inputVal = GPIO_ReadInputPin(NucleoUserPB.pGPIO, NucleoUserPB.GPIO_Pin_Cfg.GPIO_PinNumber);
        BBinpVal = GPIO_ReadInputPin(BreadBoardPB.pGPIO, BreadBoardPB.GPIO_Pin_Cfg.GPIO_PinNumber);




        if(inputVal == 0)
        {

            GPIO_ToggleOutputPin(NucleoUserLED.pGPIO, NucleoUserLED.GPIO_Pin_Cfg.GPIO_PinNumber);
        }

        /*****************************************************************
         *       Controlling the IO present in the bread board           *
         *****************************************************************/

        temp = 10;
        if (BBinpVal == 0 )
        {
            GPIO_WriteOutputPin(BreadBoardLED.pGPIO, BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber, 1);
        }
        else
        {
            GPIO_WriteOutputPin(BreadBoardLED.pGPIO, BreadBoardLED.GPIO_Pin_Cfg.GPIO_PinNumber, 0);
        }

        delay();
    }

    return 0;
}
Improve this answer

2 Comments

Only by the loosest definition of the word "solved". You have simply created a latent bug. If changing the allocation/location of a variable changes the problem, then you clearly have a memory configuration issue - in this case perhaps insufficient stack allocation. This observation should posted as part of the question since it is no in any manner an acceptable answer. You cannot reasonably resort to making all your variable global! embedded.com/a-pox-on-globals. The fact that this makes the code "work" is indicative of a deeper issue.
I see in any case you have chosen to ignore all advice about using volatile or using a hardware timer to implement delays. This code will break in other ways sooner or later. Also if you appear to ignore advice on SO, people may stop giving it.
0

The issue is solved,

There was an bug in the driver layer I have written

Whenever an GPIO is configured as Input, the registers related to Output for that GPIO pin should be set to their reset value or the driver should not implement the API related to the Output

Improve this answer

1 Comment

I am not sure how changing a local variable to a global relates to that solution. I'd be very cautious about declaring this solved if I were you. Also SO is a Q&A not a discussion forum. If you post an answer it should include a solution, and it should an an solution that can be arrived at from the the information in the question. Your question does not include the driver code you have "corrected" and this answer does not include the solution you arrived at. As such it has no community benefit, and should if at all be posted as a comment, or simply delete the question.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.