Replace HybridStepperMotor with a driver class?

Backstory: When adding DC motor support to my stepstick (note: I haven’t actually written anything about DC motors in that thread yet), I came up with the idea to create driver wrapper classes to allow DCMotor to interface with the StepperDriver and BLDCDriver classes.

There is a minor problem that the wrapper will have its own set of FOCDriver variables, so it will be confusing whether the wrapper or wrapped class’ copy is actually used. The init function can copy the values from the wrapped class, and I think only voltage_limit and voltage_power_supply are safe to change at runtime, so the worst that will happen is someone changes one of those in the wrong class and it has no effect.

Getting to the point of this thread, the same pattern could be applied to eliminate the need for the HybridStepperMotor class, instead having StepperMotor interface with BLDCDriver via a StepperDriver_from_BLDCDriver wrapper class.

The only real functionality of HybridStepperMotor is the setPhaseVoltage function, which can be moved into the wrapper’s setPwm. It doesn’t have access to the motor’s foc_modulation variable, so it will either need to be hardcoded to use the superior SpaceVectorPWM, or have its own variable to allow using SinePWM. HybridStepperMotor is currently 95 lines of header and 177 lines of cpp, whereas the driver class with SVPWM is only 24 lines of header and 24 lines of cpp.

It also eliminates the need for CurrentSense to store a modifiable driver_type variable, since the wrapper can return DriverType::Hybrid directly.

Not sure whether this is really worth doing, but worth getting the idea documented here at least.

#ifndef STEPPERDRIVER_FROM_BLDCDRIVER_H
#define STEPPERDRIVER_FROM_BLDCDRIVER_H

#include "Arduino.h"
#include "../common/base_classes/BLDCDriver.h"
#include "../common/base_classes/StepperDriver.h"

class StepperDriver_from_BLDCDriver: public StepperDriver{
    public:
        StepperDriver_from_BLDCDriver(BLDCDriver &_wrapped) : wrapped(_wrapped) {}

        int init() override;
        void enable() override { wrapped.enable(); }
        void disable() override { wrapped.disable(); }
        void setPwm(float Ua, float Ub) override;
        // Phase c set equal to phase b, so enabling or disabling all phases will work
        void setPhaseState(PhaseState sa, PhaseState sb) override { wrapped.setPhaseState(sa, sb, sb); }

        virtual DriverType type() override { return DriverType::Hybrid; }

        BLDCDriver &wrapped;
};

#endif

And the .cpp:

#include "StepperDriver_from_BLDCDriver.h"

int StepperDriver_from_BLDCDriver::init() {
  if (!wrapped.initialized)
    if (!wrapped.init())
      return 0;
  pwm_frequency = wrapped.pwm_frequency;
  voltage_power_supply = wrapped.voltage_power_supply;
  voltage_limit = wrapped.voltage_limit;
  params = wrapped.params;
  enable_active_high = wrapped.enable_active_high;
  initialized = true;
  return 1;
}

void StepperDriver_from_BLDCDriver::setPwm(float Ua, float Ub) {
  // C phase moves in order to increase max bias on coils
  float Umin = fmin(fmin(Ua, Ub), 0);
  float Umax = fmax(fmax(Ua, Ub), 0);
  float Vo = -(Umin + Umax)/2 + wrapped.voltage_limit/2;
  Ua = Ua + Vo;
  Ub = Ub + Vo;
  wrapped.setPwm(Ua, Ub, Vo);
}