Add support for hal overloading on stm32g4, cleanup index to injected rank util

[Moddingear]

This allows for other boards to override the default HAL behaviour for current sense on G4 (Such as Phoque 1 and 2)

Also, index to injected rank : breaks aren't necessary when returning in a case.

[runger1101001]

So I’m fine with the cleanup part…

but I’m not sure I agree with the „overloading“ part. If I understand your intent correctly then the idea is that people can use that build-time flag to disable the standard hardware API for G4, allowing you to provide your own definitions for the various hardware API functions.

Assuming „this is the way“ - why not make it a general flag that just works for all MCU types, ie. simply disables the hardware API regardless of MCU?

Bit I actually don’t think it’s the way. I think that extending or swapping current sense functionality is based on the CurrentSense class and its subclasses.

So if you want a custom current sense for G4 MCUs just make a PhoqueCurrentSense class and implement the init and getPhaseCurrents methods the way you want.

That’s a much clearer and easier to use „API“ for people than messing with build flags and diving into our hardware level APIs which are mostly undocumented and designed to be „under the hood“ from the user POV…

[Moddingear]

Well yes, but that means that anyone who want to use my board will have to change functions from _readADCVoltageLowSide to whatever I make, same for the MX functions (which have standard names).

I made it this way so that it opens the way for more hardware-specific implementations, without adding && !defined(ARDUINO_B_G431B_ESC1) or other to the list, so that any board anyone makes can profit from the overloading by adding a define in the board's json defines.

It could be expanded to cover all MCUs, that's true.

As for using the GenericCurrentSense, that means i'd have to recode the LowSideCurrentSense logic. Not optimal.

Another way to do this would be to make the default function definitions in generic_mcu.cpp even weaker so that the hardware-specific definitions can be weak. That would probably be the best way to do it honestly, but it might be a bit ugly :

//in generic_mcu.cpp
__attribute__((weak)) 
void* _configureADCLowSide_fallback(const void* driver_params, const int pinA,const int pinB,const int pinC){
  SIMPLEFOC_DEBUG("ERR: Low-side cs not supported!");
  return SIMPLEFOC_CURRENT_SENSE_INIT_FAILED;
}

//in stm32g4_mcu.cpp
void* _configureADCLowSide_fallback(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)
  };
  if(_adc_gpio_init(cs_params, pinA,pinB,pinC) != 0) return SIMPLEFOC_CURRENT_SENSE_INIT_FAILED;
  if(_adc_init(cs_params, (STM32DriverParams*)driver_params) != 0) return SIMPLEFOC_CURRENT_SENSE_INIT_FAILED;
  return cs_params;
}

//in generic_mcu.cpp
__attribute__((weak))
void* _configureADCLowSide(const void* driver_params, const int pinA, const int pinB, const int pinC) {
  return _configureADCLowSide_fallback(driver_params, pinA, pinB, pinC);
}

//in custom_mcu.cpp
void* _configureADCLowSide(const void* driver_params, const int pinA, const int pinB, const int pinC) {
  //custom code here
}

This doesn't solve the MX function calls though.

[runger1101001]

No no, anyone who wanted to use your board would just use the PhoqueCurrentSense you provide…

Doing it using GenericCurrentSense is definitely an option, but I think it’s cleaner and more elegant to make one’s own subclass of CurrentSense, or subclass LowSideCurrentSense if you like things it does.

I don’t think there’s any real difference in the amount of work, but there is a difference in ease of use:

In the way you suggest you have to provide implementations for all the hardware API methods, and then use either lowside or inline correctly configured for your board.

In my suggestion you have to implement a class that overrides the virtual methods of CurrentSense (or one of its existing subclasses), and which is coded pre-configured to match the Phoque board setup. The hardware API can be ignored.

The even weaker binding idea I like even less, it forces people to mess with the hardware API and is even more confusing.

Basically I’d like to keep people away from the hardware API level. We don’t plan to document that, and unless you’re supporting a new MCU type for the existing generic Driver and CurrentSense layer then you shouldn’t have to touch it or see it.

The B-G431-ESC1 implementation should not be taken as a model of how it should work. It was done a long time ago, and I would not do it that way today. Today I would make a BG431ESC1CurrentSense class which lives in the drivers repository. Perhaps in future we’ll refactor that, but for now we have this „legacy“ code, but please don’t copy the pattern.

The better pattern is like with the sensors: yes, there are generic cross-platform sensor classes in the base library, but if you can it’s better to use a hardware specific sensor class from the drivers repository.
Think of current-sense in the same way.