Let's say I have a main function as follows:

int pwmPeriod = 10000;
void main(void)
{
    while(1)
    {
    float duty = 0.5;
    applyDutyFactor(duty);
    }
}

The "applyDutyFactor" function is defined in a header file called "PWM.h" as follows:

void applyDutyFactor(float duty)
{
    extern int pwmPeriod;
    PWM_COMPARE_REGISTER_NAME = duty * pwmPeriod;
    return;
}

I have a few similar situations to this, where I need a function defined in a header file to be able to "see" a variable which is declared elsewhere. I have been getting round this by using the "extern" keyword to let the compiler know that the variable does indeed exist somewhere. However, I have heard it said that using too many global variables like this is bad practice. I now have around 20 global variables in my program because of the above issue.

Question: Is what I am doing really bad practice? If so, what is the preferred way to access a variable that is declared elsewhere? Should I pass every variable into the function as an argument instead?

 

The biggest problem with C is that all extern symbols are in a single namespace. If two files both declare a global variable, they might well be mapped to the same memory location without any error message, and one variable will “mysteriously” change when we write to a what we think is a different variable.

You can overcome many of the problems by using a few simple conventions:

1. Use a unique prefix for each C file for all if its global names. For example, if there is a PWM module, start any globals it defines with PWM_.

2. Never declare a variable as extern in a C file. Each module should have an H file that defines all the externally visible symbols.

Bob

 

Is pwmPeriod used in the main file? If not you could declare it in the PWM file.
Is pwmPeriod ever changed at run time?

 

Let's say I have a main function as follows:

int pwmPeriod = 10000;
void main(void)
{
    while(1)
    {
    float duty = 0.5;
    applyDutyFactor(duty);
    }
}

The "applyDutyFactor" function is defined in a header file called "PWM.h" as follows:

void applyDutyFactor(float duty)
{
    extern int pwmPeriod;
    PWM_COMPARE_REGISTER_NAME = duty * pwmPeriod;
    return;
}

I have a few similar situations to this, where I need a function defined in a header file to be able to "see" a variable which is declared elsewhere. I have been getting round this by using the "extern" keyword to let the compiler know that the variable does indeed exist somewhere. However, I have heard it said that using too many global variables like this is bad practice. I now have around 20 global variables in my program because of the above issue.

Question: Is what I am doing really bad practice? If so, what is the preferred way to access a variable that is declared elsewhere? Should I pass every variable into the function as an argument instead?

The rule that I always used is that you should never use global variables unless you have a specific and damn good reason. The corollary to this rule is that damn good reasons do exist.

Working with resource-starved embedded microcontroller and applications that have real-time performance constraints can easily rise to the damn good reason threshold.

One thing that C suffers from is poor namespace management.

One way to deal with this issue is to put all of the variables that you current have as globals into a structure and then pass a pointer to that structure to the functions that need to access any of them. This is a relatively clean way to package everything. There is some access overhead and that might be too much for your application.

 

You should not define a function in .h header file.
Header files should be used for #define and function prototypes for cross .c declarations.

For example, instead of defining the function, just declare the function prototype.
void applyDutyFactor(float duty);

The actual definition of applyDutyFactor(float duty) goes in your PWM.c file.
You should pass the parameter pwmPeriod in the function argument in the input argument list. This avoids having to call it as global.

Comment of variable names
PWM_COMPARE_REGISTER_NAME = duty * pwmPeriod;

I choose to adhere fairly strictly on how I name constants and variables.

Local variables are all lower case, e.g. duty
Global variables begin with upper case, e.g. PWM_Period
Constants are all upper case, e.g. PWM_MAX_LIMIT

 

To answer a couple of your follow up points:

1. @BobTPH I am not too worried about losing track of which globals "belong" to each peripheral as they all have pretty meaningful names, although prefixing them with something like "PWM_" may be useful as the program increases in complexity. Moving all of the "extern" declarations into the .h files above the function prototypes would be very easy for me to do.
2. @AlbertHall This was a somewhat arbitrary example I provided to make the problem more concise, although I do happen to have a variable for the "pwmPeriod". I have declared it as a global variable as I need the PWMconfiguration() function to be able to see it, but also the currentControl() function, which are both defined in their own source files. Most of the global variables I am using at the moment can and do change at runtime. For example, I have a variable "DCvoltage" which needs to be visible to my ADC ISR so that it can be updated with new samples but also be visible to the currentControl() function so that it can act on the newest samples.
3. @WBahn I am working on a Texas Instruments C2000 which is quite a hefty microcontroller for what I am doing. For this reason, I am not too worried about efficiency or memory usage. Readability, reliability and ease of alteration by future-me are my main concerns. I had considered putting certain groups of related measurements into structs, although there would likely only be one instance of each struct which may call into question the usefulness of doing so. For example, I could put Ia, Ib, Ic and Vdc into a struct which would wrap up all of the latest samples in one object.
4. @MrChips I have, in fact, put the fuction prototypes and #defines in a .h file then the definitions in a .C file of the same name. I should have made this clear in my original post. I appreciate your advice on naming conventions as I am trying to improve the standard of my code.

A final point: When I was taught programming, by the instructor's own admission, the class was about "problem solving and getting the thing to do what you want it to do. This is not the place to learn how to write good code". I am now trying to improve the standard of my programming and build my knowledge, so if my use of terminology is clumsy or imprecise, please call me out on it.

 

A final point: When I was taught programming, by the instructor's own admission, the class was about "problem solving and getting the thing to do what you want it to do. This is not the place to learn how to write good code". I am now trying to improve the standard of my programming and build my knowledge, so if my use of terminology is clumsy or imprecise, please call me out on it.

I beg to differ from what your instructor taught you. I teach programming and one of the first thing I teach is Structured Programming. I teach sound engineering problem solving methodology, not how to get to a working solution as quickly as possible.

If you do not follow sound programming techniques from the first get go it will come back to haunt you.

 

I beg to differ from what your instructor taught you. I teach programming and one of the first thing I teach is Structured Programming. I teach sound engineering problem solving methodology, not how to get to a working solution as quickly as possible.

If you do not follow sound programming techniques from the first get go it will come back to haunt you.

I completely agree with you and it has indeed caused me a lot of problems further down the line. Unfortunately, complaints about quality of teaching are routinely ignored, so long as the academic in question keeps the journal papers coming. There's no sign of that changing in the near future, if anything it's getting worse.