How to Implement Universal Setter/Getter Functions for Interrupt-Driven Variables in Embedded C?

Question

I had an interrupt functionality implementation in my embedded project with global variables. The interrupt checks PWM signal duty cycle and sets the value of a bool var to true if the value is correct. The main code then uses this variable to output HIGH or LOW on a specified pin.

I wanted to modularize this functionality and limit the scope of the variables. I saw an approach of making an "object" in a separate .c file. This way I defined static variables and methods to access and change values of the variables. I have a couple bool variables, methods to get their value and two universal methods to change the value of an input variable (to reduce code duplication).

The resulting implementation works but I would like some feedback if my approach is appropriate.

My module .c file looks like this (the number of variables could be higher than in this example):

static bool var1 = false;
static bool var2 = false;

bool *var1_get(void) {
  return &var1;
}

bool *var2_get(void){
  return &var2;
}

void var_set(bool *var_ok) {
  /* signal PWM is good */
  *var_ok = true;
}
void var_reset(bool *var_ok) {
  /* signal PWM is bad */
  *var_ok = false;
}

The interrupt uses the set and reset functions in the main.c for each variable at a time (different interrupts for different input signals):

// capture_val is the PWM signal duty cycle value
if ((capture_val > 88) && (capture_val < 92)) {
  var_set(var1_get());
} else {
  var_reset(var1_get());
}

and in the main while(1) loop the value of the variable to output on a specified pin is checked:

if (*var1_get() == false) {
  HAL_GPIO_WritePin(GPIOC, OUT_OK_Pin, GPIO_PIN_RESET);
} else {
  HAL_GPIO_WritePin(GPIOC, OUT_OK_Pin, GPIO_PIN_SET);
}

That means that with _get function I get the address of the variable to change its value in the interrupt and in the while loop I check its value with the same method. As I understand it I used a pointer function, but I saw it is usually defined differently to use functions dynamically (which is not what I want to do here). I don't really have a lot of experience with pointer functions and never saw an implementation like this which leads me to a couple of questions:

• Is my approach of implementation fine or should I just use static top-level variables in the main.c and ditch the module I made?

• Are the *var_get() functions fine to use as I defined them or is there a better standard of defining such functions? Is there a different approach that would have the same functionality of the module but with more appropriate implementation?

I'm mostly self-taught, so I appreciate any feedback on my approach.

Answer 1

Returning pointers isn't good because it breaks encapsulation of allowing only the module to read/write the actual variable values. As is, your code allows things like this:

bool *var1 = var1_get();
print("var1=%d\n", *var1);
*var1 = false;
print("var1=%d\n", *var1);

So only return the values and have a separate setter function for each variable that takes the value to set:

bool var1_get(void) {
  return var1;
}

bool var2_get(void){
  return var2;
}

void var1_set(bool val) {
  var1 = val;
}
void var2_set(bool val) {
  var2 = val;
}

And to use:

if ((capture_val > 88) && (capture_val < 92)) {
  var1_set(true);
} else {
  var1_set(false);
}

...

if (!var1_get()) {
  HAL_GPIO_WritePin(GPIOC, OUT_OK_Pin, GPIO_PIN_RESET);
} else {
  HAL_GPIO_WritePin(GPIOC, OUT_OK_Pin, GPIO_PIN_SET);
}