Motor-Hall Sensor Alignment Tester: Problem with millis() and interval timing?

Hello all,

I am very new to the Arduino world and I am trying to write the code for a Hall Sensor to Motor phase alignment tester/display. I am using the UNO R3. My code attempts to monitor voltages (A0 only for now) and record a time ( or intervals) for every time the voltage level drops to 0V for 24 times. Once I have these intervals, I then take the average and compare to one of the other channels.

For example, if motor phase A is channel A0 and Hall Sensor 1 is channel A1, i would complete the above process and compare the averaged time interval which should tell me how far off the Hall sensor alignment is off from the motor phase alignment (the conversion can be made to electrical degrees and expressed this way). My code is shown below:

#include <Adafruit_FT6206.h>

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <gfxfont.h>
#include <Adafruit_ILI9341.h>
#include <SPI.h>



// The display also uses hardware SPI, plus #9 & #10
#define TFT_CS 10
#define TFT_DC 9
Adafruit_ILI9341 tft = Adafruit_ILI9341(TFT_CS, TFT_DC);
Adafruit_FT6206 ctp = Adafruit_FT6206();

// This is calibration data for the raw touch data to the screen coordinates
#define TS_MINX 150
#define TS_MINY 130
#define TS_MAXX 3800
#define TS_MAXY 4000

boolean CrossOverPt = false;

#define FRAME_X 60   // X coordinate for the "Press to Begin" button
#define FRAME_Y 110  // Y coordinate for the "Press to Begin" button
#define FRAME_W 200  // Width for the "Press to Begin" button
#define FRAME_H 50   // Height for the "Press to Begin" button
#define FRAME1_X 20  // X coordinate for the "???" banner
#define FRAME1_Y 5   // Y coordinate for the "???" banner
#define FRAME1_W 280 // Width for the "???" banner
#define FRAME1_H 50  // Height for the "???" banner

int SetupDelay = 3000; //Delay time (3s) for Setup Screen (??? - Hall Sensor Alignment Test) to Display.



//                                                                                             Setup()
//****************************************************************************************************************************************************************************
void setup()
{

  Serial.begin(9600); // Starting the serial monitor/plotter - Baud rate = 9600
  tft.begin();  // Start use of Adafruit TFT 2.8" Capacitive Touchscreen
  if (!ctp.begin(40)) {
    Serial.println("Unable to start touchscreen."); // If the Touchscreen is not connected, the previous message will display on the serial monitor
  }
  else {
    Serial.println("Touchscreen started."); // If the Touchscreen is connected, the previous message will display on the serial monitor
  }

  tft.fillScreen(ILI9341_BLUE); // Fills the entire screen blue
  // origin = left,top landscape (USB left upper)
  tft.setRotation(1); // Rotate the screen to use horizontally
  drawFrame(); // Calls the drawFrame function - Fills in the buttons and outlines for the setup screen
  addText();  // Calls the addText function - Adds all text to the setup screen
  //delay(SetupDelay); // Delays the next action for 3s
  tft.fillScreen(ILI9341_BLACK); // Fills the entire screen blue
  analog0();


}//                                                                                        End of Setup()
//****************************************************************************************************************************************************************************
//                                                                                             Loop()
//****************************************************************************************************************************************************************************
void loop()
{
}

//                                                                                           End of Loop()
//****************************************************************************************************************************************************************************
//                                                                                             Functions()
//************************************************************************************************************************************************************************************************************
void drawFrame()
{
  tft.fillRect(FRAME1_X, FRAME1_Y, FRAME1_W, FRAME1_H, ILI9341_WHITE);
  tft.drawRect(FRAME1_X, FRAME1_Y, FRAME1_W, FRAME1_H, ILI9341_BLACK);
  tft.drawLine(0, 75, 320, 75, ILI9341_WHITE);
  tft.drawLine(0, 58, 320, 58, ILI9341_WHITE);
  tft.fillRect(FRAME_X, FRAME_Y, FRAME_W, FRAME_H, ILI9341_BLACK);
  tft.drawRect(FRAME_X, FRAME_Y, FRAME_W, FRAME_H, ILI9341_WHITE);
  tft.drawRect(0, 0, 320, 240, ILI9341_BLACK);
}

void addText()
{
  tft.setCursor(45, 20);
  tft.setTextColor(ILI9341_BLUE);
  tft.setTextSize(3);
  tft.print("???");
  tft.setCursor(5, 60);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(2);
  tft.print("Hall Sensor Alignment Test");
  tft.setCursor(75, 125);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(2);
  tft.print("Press to Begin");
  tft.setCursor(180, 230);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(1);
  tft.print("DEVELOPED JULY 14, 2017");
}

void analog0()
{
  //Read input from analog shield Channel A0 (Phase A) and return the time it crosses over 0V.
  // Timing for phase A crossovers
  unsigned long T_PhaseAcross[23]; // Array holding the intervals for crossover (0V) times.
 /* for (int n = 1; n <= 1; n++) { // For loop to ensure that this sequence is only initiated once (only fill in the values of the array for one full cycle)
    //Serial.print("n = "); //Used for debugging purposes only
    //Serial.println(n);*/
    float VoltA0 = (analogRead(0)) / 203.8; //Modified (203.8 w/out screen - 210 with screen?? KB 07/19/17) from 204.8 for resolution tuning;
    if (VoltA0 <= 0.05) { // Ensures proper timing that first reading will fall directly on a falling/rising edge when Voltage is 0.
      delay(10);
      Serial.println("Delayed");
    }
    int i = 0;
    while (i <= 24) { // While loop to fill in the Array with the 25 interval values
      VoltA0 = (analogRead(0)) / 203.8; //Modified (203.8 w/out screen - 210 with screen?? KB 07/19/17) from 204.8 for resolution tuning;
      unsigned long currentMillis = millis(); // Starts a timer in milleseconds
      unsigned long previousMillis;
      //Serial.print("i = "); //Used for debugging purposes only
      // Serial.println(i);
      if (VoltA0 <= 0.05) {
        CrossOverPt = true;
        T_PhaseAcross[i] = currentMillis - previousMillis; // Interval timing to get 24 crossover points
        previousMillis = currentMillis;
        Serial.print("T_PhaseAcross"); //Used for debugging purposes only
        Serial.print(i);
        Serial.print(" = ");
        Serial.println(T_PhaseAcross[i]);
        i++;
        


      }

    }
    int avg = (T_PhaseAcross[4] + T_PhaseAcross[6] + T_PhaseAcross[8] + T_PhaseAcross[10] + T_PhaseAcross[12] + T_PhaseAcross[14] + T_PhaseAcross[16] + T_PhaseAcross[18] + T_PhaseAcross[20] + T_PhaseAcross[22] + T_PhaseAcross[24]) / 11;
    Serial.print("Average = ");
    Serial.print(avg);
    Serial.println(" ms");
  }
}

//                                                                                    End of Functions()
//****************************************************************************************************************************************************************************

My problem seems to be that the time intervals that are recorded and expressed aren’t right which leads me to believe i have written something wrong. The intervals for the motor that i am currently using should average around 9 ms. Does anyone see any blatantly wrong errors in my coding?

KBarnett:

    while (i <= 24) { // While loop to fill in the Array with the 25 interval values

VoltA0 = (analogRead(0)) / 203.8; //Modified (203.8 w/out screen - 210 with screen?? KB 07/19/17) from 204.8 for resolution tuning;
     unsigned long currentMillis = millis(); // Starts a timer in milleseconds
     unsigned long previousMillis;
     if (VoltA0 <= 0.05) {
       CrossOverPt = true;
       T_PhaseAcross[i] = currentMillis - previousMillis; // Interval timing to get 24 crossover points
       previousMillis = currentMillis;



Does anyone see any blatantly wrong errors in my coding?

I think that using previousMillis before you’ve assigned a value to it would fall into that category.

arduarn:
I think that using previousMillis before you've assigned a value to it would fall into that category.

I have tried assigning it to zero, and in fact retried after your suggestion. This does not seem to be the issue (at least not all of it). Thank you for your suggestion though.

KBarnett:
I have tried assigning it to zero, and in fact retried after your suggestion. This does not seem to be the issue (at least not all of it). Thank you for your suggestion though.

I had hoped that you would notice something else while following up my comment, but I suppose it may be easy to miss.
Look at where you have declared the previousMillis

Yes, i did move it out of that while loop on the last effort as well. Sorry i forgot to mention that in my last post.

KBarnett:
Yes, i did move it out of that while loop on the last effort as well. Sorry i forgot to mention that in my last post.

Groan…details are important.

Took another look at the code and noticed another (also quite blatant) bug.

  unsigned long T_PhaseAcross[23];
  int i = 0;
  while (i <= 24) {
    T_PhaseAcross[i] = currentMillis - previousMillis;
    i++;
  }

Your array has 23 elements allocated - so 0 - 22 indexable.
You write 23 and 24 in the loop. That is 8 bytes of corrupted data…

Alright, so updated code. I really do appreciate the help with this. I feel like I am learning a lot.

I know my code is still messy and i need to clean it up and separate sections.

I am also now using the Digilent analog shield. I have resolved my issue i was having as far as getting timing points for a voltage reading of zero. Now what I am trying to do is ensure that I only have one timing point recorded per one zero volt reading (along the sine wave). And preferably only when its a falling edge voltage reading (see picture) is when the timing reading should be taken. I have tried to remedy this by introducing a delay, but as I’m sure most of you are going to say, that’s not a great solution at all.

For example, on the sine wave, when the voltage goes from 3.25 down to zero: take a reading.
When the voltage goes from -3.25 to zero do nothing. The function analog0() is where I am trying to achieve this outcome.

#include <Adafruit_FT6206.h>
#include <Wire.h>
#include <Adafruit_GFX.h>
#include <gfxfont.h>
#include <Adafruit_ILI9341.h>
#include <SPI.h>
#include <analogShield.h>
#include <elapsedMillis.h>


elapsedMicros timeElapsed;

// The display also uses hardware SPI, plus #9 & #10
#define TFT_CS 10
#define TFT_DC 9
Adafruit_ILI9341 tft = Adafruit_ILI9341(TFT_CS, TFT_DC);
Adafruit_FT6206 ctp = Adafruit_FT6206();

// This is calibration data for the raw touch data to the screen coordinates
#define TS_MINX 150
#define TS_MINY 130
#define TS_MAXX 3800
#define TS_MAXY 4000
#define FSV 10  // full scale voltage (5v - -5v)
#define NBITS 65535.0  // NBITS = number of bits

boolean CrossOverPt = false;

#define FRAME_X 60   // X coordinate for the "Press to Begin" button
#define FRAME_Y 110  // Y coordinate for the "Press to Begin" button
#define FRAME_W 200  // Width for the "Press to Begin" button
#define FRAME_H 50   // Height for the "Press to Begin" button
#define FRAME1_X 20  // X coordinate for the "/" banner
#define FRAME1_Y 5   // Y coordinate for the "/" banner
#define FRAME1_W 280 // Width for the "/" banner
#define FRAME1_H 50  // Height for the "/" banner

int SetupDelay = 3000; //Delay time (3s) for Setup Screen (/ - /) to Display.



//                                                                                             Setup()
//****************************************************************************************************************************************************************************
void setup()
{

  Serial.begin(1000000); // Starting the serial monitor/plotter - Baud rate =
  /*
    tft.begin();  // Start use of Adafruit TFT 2.8" Capacitive Touchscreen
    if (!ctp.begin(40)) {
    Serial.println("Unable to start touchscreen."); // If the Touchscreen is not connected, the previous message will display on the serial monitor
    }
    else {
    Serial.println("Touchscreen started."); // If the Touchscreen is connected, the previous message will display on the serial monitor
    }

    tft.fillScreen(ILI9341_BLUE); // Fills the entire screen blue
    // origin = left,top landscape (USB left upper)
    tft.setRotation(1); // Rotate the screen to use horizontally
    drawFrame(); // Calls the drawFrame function - Fills in the buttons and outlines for the setup screen
    addText();  // Calls the addText function - Adds all text to the setup screen
    //delay(SetupDelay); // Delays the next action for 3s
    tft.fillScreen(ILI9341_BLACK); // Fills the entire screen blue*/
  analog0();


}//                                                                                        End of Setup()
//****************************************************************************************************************************************************************************
//                                                                                             Loop()
//****************************************************************************************************************************************************************************
void loop()
{
}

//                                                                                           End of Loop()
//****************************************************************************************************************************************************************************
//                                                                                             Functions()
//************************************************************************************************************************************************************************************************************
void drawFrame()
{
  tft.fillRect(FRAME1_X, FRAME1_Y, FRAME1_W, FRAME1_H, ILI9341_WHITE);
  tft.drawRect(FRAME1_X, FRAME1_Y, FRAME1_W, FRAME1_H, ILI9341_BLACK);
  tft.drawLine(0, 75, 320, 75, ILI9341_WHITE);
  tft.drawLine(0, 58, 320, 58, ILI9341_WHITE);
  tft.fillRect(FRAME_X, FRAME_Y, FRAME_W, FRAME_H, ILI9341_BLACK);
  tft.drawRect(FRAME_X, FRAME_Y, FRAME_W, FRAME_H, ILI9341_WHITE);
  tft.drawRect(0, 0, 320, 240, ILI9341_BLACK);
}

void addText()
{
  tft.setCursor(45, 20);
  tft.setTextColor(ILI9341_BLUE);
  tft.setTextSize(3);
  tft.print("/");
  tft.setCursor(5, 60);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(2);
  tft.print("/");
  tft.setCursor(75, 125);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(2);
  tft.print("Press to Begin");
  tft.setCursor(180, 230);
  tft.setTextColor(ILI9341_WHITE);
  tft.setTextSize(1);
  tft.print("DEVELOPED JULY 14, 2017");
}

void analog0()
{
  //Read input from analog shield Channel A0 (Phase A) and return the time it crosses over 0V.
  // Timing for phase A crossovers
  float T_PhaseAcross[12]; // Array holding the intervals for crossover (0V) times.
  float VoltA0 = ((((analog.read(0) / NBITS) * FSV) - FSV / 2));
  if (VoltA0 == 0) { // Ensures proper timing that first reading will fall directly on a falling/rising edge when Voltage is 0.
    delay(1);
    //Serial.println("Delayed");
  }
[color=green]  int i = 0;
  float VoltPA0 = 1;
  timeElapsed = 0;
  while (i <= 11) { // While loop to fill in the Array with the 12 interval values
    VoltA0 = ((((analog.read(0) / NBITS) * FSV) - FSV / 2));
    if (VoltA0 == VoltPA0) { // I only want one "time reading" per 0V measurement
      Serial.println("Reading was Delayed - Previous Voltage was also 0");
      delay(10);
    }
    //Serial.print("i = "); //Used for debugging purposes only
    //Serial.println(i);
    if (VoltA0 <= 0.05 && VoltA0 >= -0.05) {
      CrossOverPt = true;
      T_PhaseAcross[i] = timeElapsed; // Interval timing to get 12 crossover points
      Serial.print("T_PhaseAcross");
      Serial.print(i);
      Serial.print(" = ");
      Serial.println(T_PhaseAcross[i]);
      //Serial.print("VoltA0 = ");
      //Serial.println(VoltA0);
      //Serial.print("VoltPA0 = ");
      //Serial.println(VoltPA0);
      VoltPA0 = VoltA0;
      timeElapsed = 0;
      i++;
    }
  }
  float avg = ((T_PhaseAcross[1] + T_PhaseAcross[2] + T_PhaseAcross[3] + T_PhaseAcross[4] + T_PhaseAcross[5] + T_PhaseAcross[6] + T_PhaseAcross[7] + T_PhaseAcross[8] + T_PhaseAcross[9] + T_PhaseAcross[10] + T_PhaseAcross[11]) / 11);
  Serial.print("Average = ");
  Serial.print(avg);
  Serial.println(" microseconds");[/color]

}

//                                                                                    End of Functions()
//****************************************************************************************************************************************************************************

Other side note, my first timing reading (T_PhaseAcross[1] is always very far off from the rest and im not sure why exactly. Thoughts on that?

KBarnett:
I am also now using the Digilent analog shield.

Hey, nice bit of kit and probably a good choice.

KBarnett:
Now what I am trying to do is ensure that I only have one timing point recorded per one zero volt reading (along the sine wave). And preferably only when its a falling edge voltage reading (see picture) is when the timing reading should be taken. I have tried to remedy this by introducing a delay, but as I’m sure most of you are going to say, that’s not a great solution at all.

I would definitely be careful with delays but in this case I don’t think it is effective anyway, due to the if statement.

KBarnett:
For example, on the sine wave, when the voltage goes from 3.25 down to zero: take a reading.
When the voltage goes from -3.25 to zero do nothing.

You could remember the previous reading each time through the sample loop and then compare it with the current reading; that gives you the direction of travel. Then when it hits zero (or slightly overshoots) and is in the correct direction, you can collect the data point.
Or just notice when you hit a maximum and set a flag to take the next zero crossing as a data point.
If the signal is noisy, additional precautions might be needed, eg. taking an average of the last 5 readings to smooth things out.

It would be worth taking a little time to read some sample code for the analogue shield and pick up a few tips. This one in particular I think has some relevance.

KBarnett:
Other side note, my first timing reading (T_PhaseAcross[1] is always very far off from the rest and im not sure why exactly. Thoughts on that?

After taking a slightly more detailed look at you code, I have a little more feedback.

float VoltA0 = ((((analog.read(0) / NBITS) * FSV) - FSV / 2));
  if (VoltA0 == 0)
...
if (VoltA0 == VoltPA0)
...
if (VoltA0 <= 0.05 && VoltA0 >= -0.05)

The last if statement suggests you may already know this, but I will remind you that it usually doesn’t make any sense to compare floating point values using the == operator. It is unlikely that VoltA0 will be exactly zero, but it may be as close as makes no difference. You need to be checking within a range of values as you did in the latter case.

Which raises another question: why are you using floats at all? You are only interested in the timing intervals, so converting your analogue readings into voltage is just wasted effort. Just use the integer values directly (perhaps promoted to long for a bit of breathing space if you want to do any calculations).
Floating point operations are not supported by the Arduino hardware, so they have to be emulated in software. This means that your code ends up a bit larger and considerably slower.

      Serial.print("T_PhaseAcross");
      Serial.print(i);
      Serial.print(" = ");
      Serial.println(T_PhaseAcross[i]);

I would try to make my sample loop as deterministic and fast as possible. If you need to slow it down, you can always add controlled delays, but the important stuff should be tight.
Don’t write to serial in the loop since it will cause a bunch of interrupts to occur in the background as the serial is being written out.

  float avg = ((T_PhaseAcross[1] + T_PhaseAcross[2] + T_PhaseAcross[3]
                + T_PhaseAcross[4] + T_PhaseAcross[5] + T_PhaseAcross[6]
                + T_PhaseAcross[7] + T_PhaseAcross[8] + T_PhaseAcross[9]
                + T_PhaseAcross[10] + T_PhaseAcross[11]) / 11);
  Serial.print("Average = ");
  Serial.print(avg);
  Serial.println(" microseconds");

You can use a loop for this stuff and, in doing so, write out all the T_PhaseAcross that you remove from the sample loop.

If you haven’t done so already, for test purposes you could visualise your data readings by allocating a huge array and stuffing it with readings at an appropriate rate - perhaps as fast as possible first, then add a delay if necessary. When the array is full, print it out to serial with the indexes in a format suitable for import to a spreadsheet. Plot a chart.
A picture is supposed to be worth a thousand words.