Can anyone review this arduino Motor Driver Library

Here is the main cpp code i made as an arduino library for a project i am working on. I am using two item#1457 high power motor drivers, but this library should work for all itteratons of this product.

Everything here compiles and apperars to run right but i have not yet ran it on the drivers.


/*
  Pololu_HPMD.cpp - Library for Pololu high power motor driver.
   Created by Carl Morse, Sep 11, 2012.
   
   v0.5 - released on Pololu forum for feedback.
   
 */
//TODO: Performance measurment of functions.

//Refrence of motor driver control pins 
/* 
CS -  ACS714 current sensor output (66 mV/A centered at 2.5 V).

PWMH  -  Pulse width modulation inputs.
PWML -   Pulse width modulation inputs.
DIR - LOW   - Direction input: when DIR is high current will flow from OUTA to OUTB, 
              when it is low current will flow from OUTB to OUTA.
RESET - HIGH - The reset pin, in reset when pulled low.
FF1 - LOW - Fault flag 1 
FF2 - LOW - Fault flag 2 
 
*/
#include "Arduino.h"
#include "pololu_HPMD.h"


 pololu_HPMD::pololu_HPMD(int cs_pin, int PWMH_pin, int PWML_pin, int DIR_pin, int reset_pin, int FF1_pin, int FF2_pin)
    {
        //setup pins
        pinMode(_FF1_pin, INPUT);
        pinMode(_FF2_pin, INPUT);
        pinMode(_reset_pin, OUTPUT);
    
        analogReference(DEFAULT);
        pinMode(_cs_pin, INPUT);
    
        pinMode(_PWMH_pin, OUTPUT);
        pinMode(_PWML_pin, OUTPUT);
        pinMode(_DIR_pin, OUTPUT);
        
        //place public varaibles into a known state.
        _reset = false;
        _ff1 = false;
        _ff2 = false;
        _current = 0;
        
        //place private varibles into a known state.
        _cs =0;
        _PWMH =0;
        _PWML =0;
        _DIR =0;
        _status =0;
    }
    
    void pololu_HPMD::set_reset()
     {
       digitalWrite(_reset_pin, false);
        _reset = true;
        _cs =0;
        _PWMH =0;
        _PWML =0;
        _DIR =0;
       
     }
     
     int pololu_HPMD::get_faults()
     {
         int error =0;
        _ff1 = digitalRead(_FF1_pin);
        _ff2 = digitalRead(_FF2_pin);
        
        if (_ff1 == true ) { error = 1; }
        if (_ff2 == true ) {error = error + 2;}
        return (error);
     }

    void pololu_HPMD::get_status(int arg[])
    {
        int state = 0;
        _DIR = digitalRead(_DIR_pin);
        _PWMH = digitalRead(_PWMH_pin);
        _PWML = digitalRead(_PWML_pin);
        _DIR = digitalRead(_DIR_pin);
        _cs = analogRead(_cs_pin);
        _ff1 = digitalRead(_FF1_pin);
        _ff2 = digitalRead(_FF2_pin);
        
        state = 0;
        if (_ff1 == true) {state = 1;}
        
        if (_ff2 == true) {state  = state + 2;}
        
        if (state  > 0)  {set_reset();}
        
        arg[0] =state;
        arg[1] =_DIR;
        arg[3] = _PWMH;
        arg[4] = _PWML;
        arg[5] = _DIR;
        arg[6] =_cs;
        
    }
     
     
     void pololu_HPMD::start_driver()
     {
        //put motor in a unpowered state same as coast 
        //but we are raising reset allowing the driver to 
        //operate before sending comands
        digitalWrite(_DIR_pin, false);
        digitalWrite(_PWMH_pin, 0);
        digitalWrite(_PWML_pin, 0);
        digitalWrite(_reset_pin, true);
        _reset = false;
     }
    
    void pololu_HPMD::coast()
     {
       digitalWrite(_PWMH_pin, 0);
       digitalWrite(_PWML_pin, 0);
     }
    
    //brake Low
     void pololu_HPMD::braking()
     {
        digitalWrite(_PWMH_pin, 0);
        digitalWrite(_PWML_pin, 255);
     }
   
     void pololu_HPMD::forward(int pwm)
     {
        digitalWrite(_DIR_pin, LOW);
        digitalWrite(_PWMH_pin, pwm);
        digitalWrite(_PWML_pin, pwm);
     }
     
     void pololu_HPMD::reverse(int pwm)
     {
        digitalWrite(_DIR_pin, HIGH);
        digitalWrite(_PWMH_pin, pwm);
        digitalWrite(_PWML_pin, pwm);
     }
     
    float pololu_HPMD::get_current()
    {
        //0 to 1023 analog range
        //204.8 per volt at a 5V refrece
        // 66mv per amp centered at 2.5v
        // 512 is 0 amps
        _cs = analogRead(_cs_pin);
        
        if (_cs == 512) { return (0.0);}
        
        else if (_cs > 512) {return float ((_cs - 512) / 66);}
        
        else { return float (( 512 - _cs ) / 66); }
    }

pololu_HPMD.cpp (4.04 KB)
pololu_HPMD.h (1.75 KB)

Hello.

Thank you for your efforts to write this. The first problem I see with it is that you are using members like “_FF1_pin” in the constructor before initializing them, which will result in undefined behavior.

One way to initialize _FF1_pin in C++ would be like this:

pololu_HPMD::pololu_HPMD(int cs_pin, int PWMH_pin, int PWML_pin, int DIR_pin, int reset_pin, int FF1_pin, int FF2_pin) : _FF1_pin(FF1_pin)
{
   ...
}

Github is down at the moment, but I recommend that you check out our library for the Dual MC33926 shield, which can be found here:
github.com/pololu/dual-mc33926-motor-shield

Beyond the fact that the libraries are written for different devices with difference numbers of motors, there are probably lots of little differences between your library and that one. For each of those differences you should think about which way is better. Feel free to ask us if you are not sure.

–David

Hello.

It looks like you are trying to use the driver in sign-magnitude mode, alternating between drive and coast:

void pololu_HPMD::forward(int pwm)
{
  digitalWrite(_DIR_pin, LOW);
  digitalWrite(_PWMH_pin, pwm);
  digitalWrite(_PWML_pin, pwm);
}

Generally speaking, I think that sign-magnitude alternating between drive and brake is better, as it tends to provide a more linear relationship between PWM and motor RPM and can give you more control at low speeds. Also, it only requires connecting one of the PWM pins (PWMH), which makes the driver a little easier to set up and the control code a little simpler. If you want to get fancy, you could make the control mode be selectable by a constructor parameter (or even based on whether the user specifies valid pin numbers for PWMH or PWML).

However, the bigger problem with the function I copied above is that your attempt to generate a PWM signal using digitalWrite(pin, pwm) will not work. The digitalWrite() function just sets a digital output high or low depending on the argument. What you want is:

void pololu_HPMD::forward(int pwm)
{
  digitalWrite(_DIR_pin, LOW);
  analogWrite(_PWMH_pin, pwm);
  analogWrite(_PWML_pin, pwm);
}

Note that this will only work if the user connects the driver’s PWML and PWMH pins to hardware PWM outputs on the Arduino. Otherwise, analogWrite() is basically the same as digitalWrite() (except if sets the pin high for all values above 127, else low). Another drawback to using analogWrite() is that it uses an audible PWM frequency, which means you will probably hear the PWM-induced whine from your motor. You can get around this by directly modifying the appropriate timer registers to generate an ultrasonic PWM (e.g. 20 kHz), but this can be complicated, especially if you want to be flexible in which pins the user can connect and which Arduinos your library will work with.

- Ben

My goal for this library was to try to keep it as generic as possible.

Thanks for catching that, copy paste error. :blush:

Im making a “tank steer” style outdoor robot so full power forward and reverse is required.

Using a DC load simulator and i am planning to attach a thermocouple between the driver and the heat sink, could you tell me what mosfet to place a thermocouple on for a forward direction so i can test thermals under load.

Thanks for the help.

I see what you mean, I am using multiple instances of this class so i did not want to hard code them. I’ll initialise them to -1 and check the value in the contructor that they are set to another value.

You don’t need to initialize them to -1 or hard code them. Just initialize them properly. Did you notice the thing I added in the code above that results in the initialization of _FF1_pin? What I did above is equivalent to just writing “_FF1_pin = FF1_pin;” in your constructor.

–David

I’m not sure I follow your point here. Generally speaking, you want your motor control library to have the ability to drive the motor at full power.

This motor driver supports many different control options, and the best MOSFET to monitor will depend on how you are controlling it. You might be better off placing your thermocouple directly on the PCB between the FETs to monitor some kind of average board temperature.

If you will be using a drive method that involves braking low, you should monitor the low-side FETs, as the current will circulate through them during the braking phases, which will make them generally hotter than the high-side FETs. The following picture shows the two low-side FETs:


When the red FET is on, OUTB is low; when the blue FET is on, OUTA is low. Note that “forward” is basically an arbitrary distinction from the point of view of the motor driver (it becomes meaningful once you have a particular motor on your robot connected a specific way to your motor driver), but if you are referring to the entry in the truth table on the #1457 product page with the operation label “Forward”, then you would probably want to monitor the blue FET (the one corresponding to a low OUTA).

- Ben