[BUG] Potential for memory leaks

[txf-]

Unless I'm mistaken I've noticed a potential for a memory leak when instantiating current sense parameters for example looking at

stm32g4_mcu.cpp I see:

void* _configureADCLowSide(const void* driver_params, const int pinA, const int pinB, const int pinC){

  Stm32CurrentSenseParams* cs_params= new Stm32CurrentSenseParams {
    .pins={(int)NOT_SET, (int)NOT_SET, (int)NOT_SET},
    .adc_voltage_conv = (_ADC_VOLTAGE_G4) / (_ADC_RESOLUTION_G4)
  };
  _adc_gpio_init(cs_params, pinA,pinB,pinC);
  if(_adc_init(cs_params, (STM32DriverParams*)driver_params) != 0) return SIMPLEFOC_CURRENT_SENSE_INIT_FAILED;
  return cs_params;
}

Stm32CurrentSenseParams are instantiated on the heap and the pointer is assigned to a void* member in the calling class. I don't see any way that if the caller is destructed that this memory will be freed appropriately. If the top level class is only instantiated once and has a lifetime of the entire program then it's not really a issue, but if one would happen to want to use these functions dynamically then this would be bad.

Presumably the most workable solution would be for these drivers to implement a function that frees the memory, to be called in the destructor of the top level class?

something like:

_deinitADCLowSide(void* cs_params)
{
   delete static_cast<Stm32CurrentSenseParams*>(cs_params);
}

and then _deinitADCLowSide() would be called in the LowsideCurrentSense destructor.

Describe the hardware setup
For us it is very important to know what is the hardware setup you're using in order to be able to help more directly

• Which motor
  NA
• Which driver
  LowSideCurrentSense (maybe others?)
• Which microcontroller
  STM32G431RB
• Which position sensor
  NA
• Current sensing used?
  -Yes

IDE you are using
Platformio

[runger1101001]

Hey, thank you for reading the code so carefully, and you are of course 100% correct!

however, you’re also correct in your hypothesis that the intent is that this only happens once, globally.

while of course we can’t see exclude the possibility of other use cases, we do expect that normally current sensing and PWM are configured once, during Initialization, and then never changed. This is also because they are closely linked to the hardware used.

but in the longer term we should probably clean this up, so thank you for reporting it!