8

in my (personal) embedded project global variables are piling up. I need those variables to be acessible from the ISR (interrupt service routine) and/or from a menu system (so that they can be modified by the user) but at the same time I'd like to avoid having too many globals.

Since they can be grouped for modules I thought that I can encapsulate them in their own .c file, declared as static or static volatile , and expose to the outside world some functions to handle them.

Something along the line of:

in module1.c

#include module1.h

static volatile int module1_variable;

int getModule1Var(void){
    return module1_variable;
}

void setModule1Var(int i){
    //some code to disable interrupts for atomic operations
    module1_variable = i;
    //some other code to re-enable interrupts
    return;
}

module1.h would contain the function prototypes, structs and all the other elements for making the module work except for the static variable definition of course

in main.c

#include module1.h

void main(){
    //setting the variable value, could be done from a menu 
    setModule1Var(15);
    //remaining application code
}

void generic_ISR(){
    //trivial usage example
    doSomething(getModule1Var());
    return;
}

This scheme would naturally be extended to the other modules.

Now my questions are:
is this a good approach? Is it the same to simply have a bunch of globals? Any major drawbacks?

I also thought I could use some sort of mix like still having the global variables to allow direct manipulation by the ISR (since function calls from the ISR are sometimes frown upon) and the functions in every other case. Would this be better?

zakkos
  • 163
  • 1
  • 11
  • You could also make a `struct` to capture your necessary globals. I'd say the approach itself is better solely because you can have control of the input given (*such as not allowing certain values*). You may want to re-think the program itself though, see if for some of the gobals if they really need to be global or not. – Spencer Wieczorek Apr 19 '17 at 13:10
  • Why are you afraid of globals? – Fredrik Apr 19 '17 at 13:14
  • 3
    Globals are like `goto` - it is better to avoid but they are not 100% evil. – i486 Apr 19 '17 at 13:15
  • 2
    Using functions instead of variables directly is better, but consider how you name those functions. If `module1_variable` is simply an implementation detail, maybe function shouldn't be called `setModule1Var`. – user694733 Apr 19 '17 at 13:24
  • 1
    Also, don't feel too bad if have to use globals. In embedded you just sometimes have to do so. And remember that `const` globals are not nearly as bad as regular global variables. – user694733 Apr 19 '17 at 13:30
  • @Fredrik I'm not afraid of globals, in fact I'm ready to implement an all globals or a mixed way, but they're there for everyone to modify without notice. I modify it, you modify it, everyone modifies it! Said that if I have a couple globals and it works I wouldn't bother, especially for a personal project, but I'm asking here for generic understanding and broad application – zakkos Apr 19 '17 at 14:21
  • @user694733 true (and +1)! In my actual program variables and functions are (hopefully) properly named. I generalized in this question to get a, well...general answer that can be applied not only to my special little program but to get a more general understanding of how people solve this problem (also how much is perceived as a problem!) and a generic use case. Incidentally I also like to see how professionals write their code, best practices etc and compare to how I'd do it. – zakkos Apr 19 '17 at 14:32
  • I like this question but probably https://softwareengineering.stackexchange.com is more appropriate site to ask it. – Alex Lop. Apr 19 '17 at 15:30
  • @AlexLop. when referring other sites, it is often helpful to point that [cross-posting is frowned upon](https://meta.stackexchange.com/tags/cross-posting/info) – gnat Apr 19 '17 at 15:53
  • Avoiding globals at all cost...is primarily opinion based...When to use them or not, primarily opinion based, alternatives, primarily opinion based. Not that there isnt any room to discuss them, but perhaps the question should have been done a bit differently or asked as several questions with each one of the alternatives. This is one of those problem things on SO, you ask it wrong start a flame war and it gets closed, you ask again slightly differently you get closed as a duplicate... – old_timer Apr 19 '17 at 17:23
  • Just to be clear I'm not asking whether my "opinion" on globals is shared among the users or not. That is my problem and I don't need any preaching about it. I'm asking a technical advice if my solution is ok or not. That is if it would work, if there would be some problems in doing so (for example undefined behaviour would be a problem) and if there is a better solution with the same outcome. I'm not asking to be convinced on changing idea. :) – zakkos Apr 19 '17 at 17:35
  • I really can't think of this question as "primarily opinion based". As I stated in my previous comment I'm not asking to judge if I should or not use global variables. I don't want to and I'm asking HOW to achieve that. It would be like asking how to make a dish and being told that the recipe is an opinion, only because someone doesn't like that dish and asks why would I do it in the first place. I don't have the time nor the reputation to oppose closing this question so this is everything I can do. I'd really love, tho, that an honest explanation be given. – zakkos Apr 19 '17 at 22:55
  • This kind of boils down to what you mean with "globals", which is not a formal term. If by globals you mean "variable at file scope with external linkage" then they are 100% bad and there exists no reason to use them ever, there's nothing opinion-based about that. If by "globals" you mean "variable at file scope with internal linkage" (static), then it may turn a bit opinion-based - these can be considered good practice in some cases and bad practice in other cases. – Lundin Apr 20 '17 at 11:20
  • That being said, if you are calling printf from inside an ISR, globals is the least of your problems. – Lundin Apr 20 '17 at 11:21
  • @Lundin I should really change that `printf()`. The whole program Is a Mock-up, I put the `printf()` in there just to have something inside the isr. – zakkos Apr 20 '17 at 11:43

1 Answers1

3

There are several questions in there:

is this a good approach?

Yes. There is perhaps an argument for placing the ISR in the same module as the access functions and data it shares, treating it as an access function itself allowing it to read the data directly without the access wrapper overhead.


Is it better/worse/equal to simply have a bunch of globals?

Much worse. With a global where are you going to place your access mutex, data validation or breakpoints? It increases module coupling, and that should always be minimised.


Any major drawbacks?

Not really. There is a function call overhead which may be critical in an ISR; but if that were an issue you would not be calling printf() in an ISR! You can mitigate any possible performance penalty applying my earlier suggestion about placing the ISR in the data module, or by in-lining the code - but your compiler is free to ignore that, and may do so if debugging is enabled, and may also inline regardless if optimisation is enabled. Some compilers have a "forced" inline extension that the compiler is not allowed to ignore; but it is not portable.

One significant benefit is that if the variable is non-atomic, you need some access protection mechanism to ensure consistent access - and that is most easily and safely performed by having access functions. In this case you might have get / set functions that disable and re-enable the interrupt around the access for example, or use a spin-lock (suitable for where the ISR writes and the normal context reads - not the other way around as you have here - don't lock the ISR indefinitely!) .


I also thought I could use some sort of mix like still having the global variables to allow direct manipulation by the ISR (since function calls from the ISR are sometimes frown upon) and the functions in every other case. Would this be better?

Not really - see my point above about function call overhead and placement of the ISR with the data. The problem with calling functions in an ISR is seldom a time-overhead issue, but rather that the implementer of the function may not have designed it to be called safely and efficiently from an ISR and it may use excessive stack, non-deterministic execution time, busy-wait or not be re-entrant or thread-safe. Calling third-party or standard library functions for example may be ill-advised; calling functions you have specifically written and designed for the purpose and conforming to your specific constraints is not necessarily a problem.


All you need to know about why globals are a bad idea and appropriate ways to avoid them can be found in Jack Ganssle's article "A Pox on Globals" - it is also an entertaining read.

Clifford
  • 88,407
  • 13
  • 85
  • 165
  • I edited my question a tiny bit, hoping it wont get closed. It's not that it makes your answer invalid but since you're quoting I thought you would know. – zakkos Apr 19 '17 at 23:06
  • As a side note the `printf()` function is there just to have something easily recognizable! About the answer I'd like a bit of clarification on this: "[place] the ISR in the data module". – zakkos Apr 19 '17 at 23:14