What are the "rules" on function calls from an ISR/Callback Function?

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
KyTr_1955226
Level 6
Level 6
250 sign-ins 10 likes given 50 solutions authored

Hi all,

This is maybe more of an embedded C question than a PSoC specific question.

I've got something I'm working on where I'm using a single timer to count timeouts for 3 separate peripherals.  A Keyboard, Keypad, and Mouse.  Normally I would just make all the timeout counters and flags extern and just drop the counter code right into the ISR, however I've recently started to try and take an approach of minimizing my global variables/externs and I'm wondering how to go about maintaining 3 separate counters, in a single timer ISR, while limiting the scope of the counter variables and flags to their respective C files.

I would think I would do something like this:

In mouse.c:

#define MSE_TIMEOUT_COUNT    50

static volatile bool MSE_Timeout    = false;

static volatile bool MSE_Timeout_En = false;

static volatile uint8_t MSE_Timeout_Counter = 0;

inline void Mouse_TimeoutTick (void){

   

    if (MSE_Timeout_En){

       

        if (++MSE_Timeout_Counter >= MSE_TIMEOUT_COUNT){

            MSE_Timeout = true;

        }

       

    }

}

in keypad.c:

static volatile bool KP_Timeout = false;

static volatile bool KP_Timeout_En = false;

static volatile uint8_t KP_Timeout_Counter = 0;

#define KP_TIMEOUT_COUNT     50

inline void Keypad_TimeoutTick(void){

   

    if (KP_Timeout_En){

       

        if (++KP_Timeout_Counter >= KP_TIMEOUT_COUNT){

            KP_Timeout = true;

        }

       

    }

}

in keyboard.c:

volatile bool KBD_Timeout = false;

volatile bool KBD_Timeout_En = false;

volatile uint8_t KBD_Timeout_Counter = 0;

#define KBD_TIMEOUT_COUNT     50

inline void Keyboard_TimeoutTick (void){

   

    if (KBD_Timeout_En){

       

        if (++KBD_Timeout_Counter >= KBD_TIMEOUT_COUNT){

            KBD_Timeout = true;

        }

       

    }

}

in timers.c:

void ISR_100MS_Interrupt_InterruptCallback (void){

    Keypad_TimeoutTick();

    Keyboard_TimeoutTick();

    Mouse_TimeoutTick();

}

My question here is basically: Is this "Safe?" and is this the best way to go about what I'm looking to do?  I'd think declaring the functions inline as well as not having these "Tick" functions being called anywhere else but from inside the ISR Callback should do the trick right?

Thanks in advance for any input!

[EDIT] This code doesn't actually compile, "inline function is declared but never defined" for all 3 "Tick" functions.  So I'm definitely missing something about how this is supposed to be done (if it's supposed to be done at all, that is).  I don't use inline often, and I've never tried to do so in this fashion, so I'm probably misunderstanding it. 

0 Likes
1 Solution
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Hi,

> Would it be safe to make the calls to the _tick functions through normal means

> (I.E. don't even worry about inline, just declare the _tick functions normally in my header

> and define them in their respective .c file and just call them from my ISR callback)?

Probably, I will go with this strategy.

In the timers.h I will have

================

#ifndef _TIMERS_H_

#define _TIMERS_H_

CY_ISR_PROTO(_tick) ; // your PIT isr function

/* xxx_Timer_enable functions */

/* clears xxx_Timeout flag       */

/* reset and start the timeout count for each entry */

void KP_Timer_enable(void) ;

void KBD_Timer_enable(void) ;

void MSE_Timer_enable(void) ;

/* the only variables accessible from other part of the program */

/* or may be you could have int is_KP_Timeout() ; function to protect the variable */

extern int KP_Timeout ;

extern int KBD_Timeout ;

extern int MSE_Timeout ;

#endif /* _TIMERS_H_ */

================

> I suppose another option would be to just set a flag in the ISR

> and tick the timeouts in my main program.

> That might be the "safest" route.

Since the objects are HID type, this will also work fine.

But for a faster timing staff this can not guarantee

the time between the "real" timeout to the time it will be served.

moto

View solution in original post

7 Replies
odissey1
Level 9
Level 9
First comment on KBA 1000 replies posted 750 replies posted

Cypress redefined "inline" somewhere as "__INLINE" (notice double "_")

__INLINE void Keyboard_TimeoutTick (void){      

0 Likes
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Hi,

__INLINE was inline, so I hope that we can use inline just like usual.

in cmsis_gcc.h

===============

#ifndef   __INLINE

  #define __INLINE                  inline

#endif

===============

It seems that the inline definition must be in the header. (May be I'm wrong)

About the "safety", if we keep these variables only within

xxx.h xxx.c, timers.h and timers.c

it will be reasonably safer than scatter them around the project.

But as the definition is in the header file, those variable won't be protected

so may be not as safe as you expected.

As far as I tried I could "compile" with following setup.

=== mouse.h ===

#ifndef _MOUSE_H_

#define _MOUSE_H_

#include "project.h"

#define MSE_TIMEOUT_COUNT 50

 

extern volatile int MSE_Timeout_En ;

extern volatile int MSE_Timeout ;

extern volatile uint8_t MSE_Timeout_Counter ;

inline void mouse_timeout_tick(void)

{

    if (MSE_Timeout_En) {

        if (++MSE_Timeout_Counter >= MSE_TIMEOUT_COUNT) {

            MSE_Timeout = 1 ;

        }

    }

}

#endif /* _MOUSE_H_ */

=== mouse.c ===

#include "project.h"

#include "mouse.h"

#define MSE_TIMEOUT_COUNT 50

volatile int MSE_Timeout =  0 ;

volatile int MSE_Timeout_En = 0 ;

volatile uint8_t MSE_Timeout_Counter = 0 ;

=== keypad.h ===

#ifndef _KEYPAD_H_

#define _KEYPAD_H_

#include "project.h"

 

#define KP_TIMEOUT_COUNT 50

 

extern volatile int KP_Timeout_En ;

extern volatile int KP_Timeout ;

extern volatile uint8_t KP_Timeout_Counter ;

inline void keypad_timeout_tick(void)

{

    if (KP_Timeout_En) {

        if (++KP_Timeout_Counter >= KP_TIMEOUT_COUNT) {

            KP_Timeout = 1 ;

        }

    }

}

#endif /* _KEYPAD_H_ */

=== keypad.c ===

#include "project.h"

#include "keypad.h"

#define KP_TIMEOUT_COUNT 50

volatile int KP_Timeout =  0 ;

volatile int KP_Timeout_En = 0 ;

volatile uint8_t KP_Timeout_Counter = 0 ;

=== keyboard.h ===

#ifndef _KEYBOARD_H_

#define _KEYBOARD_H_

#include "project.h"

 

#define KBD_TIMEOUT_COUNT 50

 

extern volatile int KBD_Timeout_En ;

extern volatile int KBD_Timeout ;

extern volatile uint8_t KBD_Timeout_Counter ;

inline void keyboard_timeout_tick(void)

{

    if (KBD_Timeout_En) {

        if (++KBD_Timeout_Counter >= KBD_TIMEOUT_COUNT) {

            KBD_Timeout = 1 ;

        }

    }

}

#endif /* _KEYBOARD_H_ */

=== keyboard.c ===

#include "project.h"

#include "keyboard.h"

#define KBD_TIMEOUT_COUNT 50

volatile int KBD_Timeout =  0 ;

volatile int KBD_Timeout_En = 0 ;

volatile uint8_t KBD_Timeout_Counter = 0 ;

=== timers.h ===

#ifndef _TIMERS_H_

#define _TIMERS_H_

#include "project.h"

CY_ISR_PROTO(tick_timer_isr) ;

#endif /* _TIMERS_H_ */

=== timers.c ===

#include "project.h"

#include "timers.h"

#include "mouse.h"

#include "keypad.h"

#include "keyboard.h"

CY_ISR(tick_timer_isr)

{

    mouse_timeout_tick() ;

    keypad_timeout_tick() ;

    keyboard_timeout_tick() ;

}

=== main.c ===

#include "project.h"

#include "timers.h"

int main(void)

{

    CyGlobalIntEnable; /* Enable global interrupts. */

    UART_Start() ;

    tick_INT_ClearPending() ;

    tick_INT_StartEx(tick_timer_isr) ;

    Counter_Start() ;

    for(;;)

    {

        /* Place your application code here. */

    }

}

=======

moto

0 Likes

Yes I think you're right, this is the only way I was able to get this to "work", having the inline function written in the header.  Like you said though, this places the definition in the header, which is included in a whole bunch of other .c files.  If I were to do it this way, what would even be the point?  I could just write the ISR without any calls and just have all the counter flags and variables declared extern in their respective headers, which I was trying to avoid.

Would it be safe to make the calls to the _tick functions through normal means (I.E. don't even worry about inline, just declare the _tick functions normally in my header and define them in their respective .c file and just call them from my ISR callback)?

I suppose another option would be to just set a flag in the ISR and tick the timeouts in my main program.  That might be the "safest" route.

0 Likes
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Hi,

> Would it be safe to make the calls to the _tick functions through normal means

> (I.E. don't even worry about inline, just declare the _tick functions normally in my header

> and define them in their respective .c file and just call them from my ISR callback)?

Probably, I will go with this strategy.

In the timers.h I will have

================

#ifndef _TIMERS_H_

#define _TIMERS_H_

CY_ISR_PROTO(_tick) ; // your PIT isr function

/* xxx_Timer_enable functions */

/* clears xxx_Timeout flag       */

/* reset and start the timeout count for each entry */

void KP_Timer_enable(void) ;

void KBD_Timer_enable(void) ;

void MSE_Timer_enable(void) ;

/* the only variables accessible from other part of the program */

/* or may be you could have int is_KP_Timeout() ; function to protect the variable */

extern int KP_Timeout ;

extern int KBD_Timeout ;

extern int MSE_Timeout ;

#endif /* _TIMERS_H_ */

================

> I suppose another option would be to just set a flag in the ISR

> and tick the timeouts in my main program.

> That might be the "safest" route.

Since the objects are HID type, this will also work fine.

But for a faster timing staff this can not guarantee

the time between the "real" timeout to the time it will be served.

moto

Yeah, the time may be a little "off" from my expected with the flagging method, but for a long (5 second) timeout like this, a little bit of swing in the actual timeout time is acceptable.

Thanks for all the advice!

KTrenholm,

You may check also SysTimers component by MarkH, which accomplishes same goal (creating repeated events at pre-defined intervals)

SysTimers component

/odissey1

I use timer components for recurring events all the time, so I could see how this SysTimers component would be really good to have if I'm ever hurting for hardware space.  This could really come in handy to free some up by offloading some of my periodic events from their dedicated timers onto this.  Good one to have in the toolkit for sure. 

Thanks!

0 Likes