Delay implementation in STM32 using for loop

Question

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

#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;
}

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. — Clifford, Jan 30 '22 at 07: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. — Clifford, Jan 30 '22 at 07:23
Oh, and loose the macro - it is not conducive to effective debugging in a source level debugger. — Clifford, Jan 30 '22 at 07: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? — Clifford, Jan 30 '22 at 07: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](https://community.st.com/s/question/0D53W00001L08UqSAJ/nucleo-l476rg-turn-on-led) — Ashwin Sekar, Jan 30 '22 at 11:18
But your description here describes code not posted here. You should remove reference. Also the code at that link does not include the delay. You are causing a great deal of confusion by posting slightly different problems on different forums and discussing both here! Clearly changing the variable to global does not "solve" your problem; it simply changes the problem - sweeping it under the carpet so to speak. Both the `temp` declaration and the `delay` macro require stack space, possibly you have allocated insufficient stack space? — Clifford, Jan 30 '22 at 15:18
What is the value of `_Min_Stack_Size` in your project linker script? — Clifford, Jan 30 '22 at 15:25
Since you have apparently discovered an error in code not presented in this question, and arrived at a solution that no one else could possibly arrive at from the information given, I am voting to close this question. — Clifford, Jan 30 '22 at 15:40

score 2 · Answer 1 · answered Jan 29 '22 at 18:24

2

It is not a function only the macrodefinition.
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.

answered Jan 29 '22 at 18:24

0___________

60,014
4
34
74

1

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 Jan 29 '22 at 18:36
@Clifford https://godbolt.org/z/8Kc1b64K6. No stack use (volatile variable requires permanent storage), more precise delay control as it has only 2 instructions. – 0___________ Jan 29 '22 at 18:40
`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 Jan 29 '22 at 18:49
@Clifford no static does not require anything and compiler is free to do what it wants. `volatile` require an object to be created – 0___________ Jan 29 '22 at 18:51
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. – Clifford Jan 29 '22 at 19:05
@Clifford I use term permanent as not kept in registers otherwise it would not be side effects prone as 'volatile' is – 0___________ Jan 29 '22 at 19:08

Clifford · Answer 2 · 2022-01-29T22:16:34.767

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 ) ;
}

@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, Jan 29 '22 at 22:21

score 0 · Answer 3 · answered Jan 30 '22 at 11:32

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;
}

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! https://www.embedded.com/a-pox-on-globals/. The fact that this makes the code "work" is indicative of a deeper issue. — Clifford, Jan 30 '22 at 15:31
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. — Clifford, Jan 30 '22 at 15:38

score 0 · Answer 4 · answered Jan 30 '22 at 14:28

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

answered Jan 30 '22 at 14:28

Ashwin Sekar

1
1
5

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. – Clifford Jan 30 '22 at 15:36

Delay implementation in STM32 using for loop

4 Answers4