unreliable timing using metro library

Status
Not open for further replies.
D

dimammx

Member
I am using metro library to make delays in order to do a PWM ramp-up for the motor.
The timer is set for 30 ms and i have a delay in code for 2, counter goes up to 266 so 32*266= 8192
so the whole start up should take around 8 sec. And it does it in about 40% of cases other 60% it takes just 1 sec to speed up the motor.
I cant understand what is wrong, i even added 2nd timer to see if it will trigger the check but it never does so i assume that whatever the cause is it causes all the timers to malfunction in a same way .
Board is Teensy LC
PHP: 
#include <Encoder.h>

#include <Metro.h> // Include the Metro library

	
	Metro tbutton = Metro(30);  // Instantiate an instance with 30ms timer 1000 is 1 sec
Metro ttest = Metro(29);
Encoder lefts(3, 4);
Encoder rights(6, 5); //encoders
//   avoid using pins with LEDs attached

void setup() {
  analogWriteFrequency(17, 1000); //Set PWM Freq to 1khz
  Serial.begin(230400);
  Serial.println("Table Startup:");
    pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(16, OUTPUT);
  pinMode(17, OUTPUT);
  pinMode(13, OUTPUT);
  pinMode(24, INPUT_PULLUP);
   pinMode(25, INPUT_PULLUP);
digitalWrite(11, HIGH);
digitalWrite(12,HIGH);
}
  volatile int pwml=0; //Speed can use byte
  volatile int pwmr=0; //Speed 
volatile int testl=0; //test var
  volatile int testr=0; //

void loop() {

 if (digitalRead(24)==LOW )
 
  {
		delay(2);
  if (digitalRead(24)==LOW && tbutton.check()==1)
  {
    Serial.println("Up button Pressed");
    digitalWrite(12, LOW);
    
    go(); //call go function that does motor speed up
  }
  }
  else if (digitalRead(24)==HIGH)
  {
    delay(2);
    if (digitalRead(24)==HIGH)
    {
    Serial.print(".");
    pwml=pwmr=0;
    testl=testr=0;
    digitalWrite(12, HIGH);
    delay(150);
    }
    }
  
  
 }

void level_up()
{
  if(lefts.read()-rights.read()>10) 
  {
  pwml--;
  }
  if(rights.read()-lefts.read()>10) 
  {
    pwmr--;
  }
}

void go()
{
	if (pwml<256 and pwmr<256)
	{
  
  Serial.println("Spooling UP");
	analogWrite(16, pwml++);
	analogWrite(17, pwmr++);
if (ttest.check()==1)
{
  testl++;
  testr++;
  }
  if (pwml-testl>1)
  {
    Serial.println("////////////////////////////Corruption occured UP");
    delay(2000);
    }
    if (pwmr-testr>1)
  {
    Serial.println("////////////////////////////Corruption occured UP");
    delay(20000);
    }
   prpos(); //function to print out info to serial
   // very fast spool up need to solve without delay
	}
 else 
 {
    Serial.println("////////////////////Full Speed Reached/////////////////////////////////");
    prpos();
 }
}

void prpos()
{
  Serial.print("Current Position Left = ");
    Serial.print(lefts.read());
    Serial.print(",  Speed Left = ");
    Serial.print(pwml);
    Serial.print(", Right = ");
    Serial.print(rights.read());
     
    Serial.print(",Speed Right = ");
    Serial.print(pwmr);
    Serial.println();
}
 
Last edited:
This is not a good way to read pushbuttons! Mixing delay() and Metro with complex program logic will at best be confusing, when/if it does work properly. Even someone very skilled at programming would likely not get this delay-based approach right every time.

Use the Bounce library. Click File > Examples > Teensy > USB_Keyboard > Buttons for an example to help get started. With Bounce, you can update() the object for each button, and then check for fallingEdge() to detect when the buttons are pressed. The updates and checks take almost no time, so your program can do other things in loop() like checking those encoders more often.

Please, do yourself a favor and switch to Bounce. It will let you keep your program structure much simpler, and you can avoid using delay() which causes so much trouble for this sort of application.
 
Thank you for advice, i was actually planning to use some kind of de-bounce library as a next step. But i didnt want to include extra stuff before i figured out what causing the Metro library not to work reliably.
I know delay for de-bouncing is pretty bad practice BUT i dont understand how 1 delay can cause Metro to fail. Should i use something else instead?

I am in process of switching to Bounce library but i have a problem with the current logic as it needs to know if button is still being pressed. And bounce library only indicates change in state.
 
Last edited:
Ok i reworked the code to use bounce library, now metro is more predictable but still in some cases motor will speed up to 100% (256 pwm in just 1 second ). For example if program left alone for 15 seconds and then button is pressed there is 99% chance that motor will speed up instantly, however the next button press it will ramp up as it suppose to do. Or if button pressed for the first time after program upload, Metro will not function.

I am going to try to use elapsedMillis instead to see if it fixes the issue, i need to have reliable timers in order to have stable leveling algorithm.
Is there a better way to do it them Metro or elapsedMillis?

PHP: 
#include <Encoder.h> //Encoder library
#include <Bounce.h> //Debounce library
#include <Metro.h> // Include the Metro library

	
  Metro tbutton = Metro(30);  // Instantiate an instance with 30ms timer 1000 is 1 sec
  Metro ttest = Metro(31);
  Encoder lefts(3, 4);
  Encoder rights(6, 5); //encoders

  Bounce buttonup = Bounce(24, 10);
  Bounce buttondown = Bounce(25, 10); 


void setup() {
  analogWriteFrequency(17, 1000); //Set PWM Freq to 1khz
  Serial.begin(230400);
  Serial.println("Table Startup:");
    pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(16, OUTPUT);
  pinMode(17, OUTPUT);
  pinMode(13, OUTPUT);
  pinMode(24, INPUT_PULLUP);
   pinMode(25, INPUT_PULLUP);
digitalWrite(11, HIGH);
digitalWrite(12,HIGH);
}
  volatile int pwml=0; //Speed can use byte
  volatile int pwmr=0; //Speed 
volatile int testl=0; //test var
  volatile int testr=0; //
  bool btnu=0;
  bool btnd=0;

void loop() {
buttonup.update();
if (buttonup.fallingEdge()) btnu=1;
if (buttonup.risingEdge()) btnu=0;
 
buttondown.update();

 if (btnu==1)
 
  {
	
  if (tbutton.check()==1)
  {
    Serial.println("Up button Pressed");
    digitalWrite(12, LOW);
    
    go();
    
  }
  }
  else if (btnu==0)
  {
    
    if (tbutton.check()==1)
    {
    Serial.print(".");
    pwml=pwmr=0;
    testl=testr=0;
    digitalWrite(12, HIGH);
    delay(150);
    }
    }
  
  
 }

void level_up()
{
  if(lefts.read()-rights.read()>10) 
  {
  pwml--;
  Serial.println("Leveling Left speed -1");
  }
  if(rights.read()-lefts.read()>10) 
  {
    pwmr--;
      Serial.println("Leveling Right speed -1");
  }
}
void limiter()
{
if (lefts.read()>30 or lefts.read()<3000 or rights.read()>30 or rights.read()<3000 )
  {
    
  }
}
void go()
{
	if (pwml<256 and pwmr<256)
	{
  
  Serial.println("Spooling UP");
	analogWrite(16, pwml++);
	analogWrite(17, pwmr++);
if (ttest.check()==1)
{
  testl++;
  testr++;
  }
  if (pwml-testl>1)
  {
    Serial.println("////////////////////////////Corruption occured UP");
    delay(2000);
    }
    if (pwmr-testr>1)
  {
    Serial.println("////////////////////////////Corruption occured UP");
    delay(20000);
    }
   prpos();
   // very fast spool up need to solve without delay
	}
 else 
 {
    Serial.println("////////////////////Full Speed Reached/////////////////////////////////");
    prpos();
 }
}

void prpos()
{
  Serial.print("Current Position Left = ");
    Serial.print(lefts.read());
    Serial.print(",  Speed Left = ");
    Serial.print(pwml);
    Serial.print(", Right = ");
    Serial.print(rights.read());
     
    Serial.print(",Speed Right = ");
    Serial.print(pwmr);
    Serial.println();
}
 
Last edited:
Your code still has problematic delays. Get rid of them!

There is at least one error with the btnu variable. This variable starts as false:

Code: 
bool btnu = 0;

You're doing this:

Code: 
  if (buttonup.fallingEdge()) btnu = 1;
  if (buttonup.risingEdge()) btnu = 0;

The risingEdge case is identical to the default value.

Then you test it like this:

Code: 
  else if (btnu == 0) {
    if (tbutton.check() == 1) {
      Serial.print(".");
      pwml = pwmr = 0;
      testl = testr = 0;
      digitalWrite(12, HIGH);
      delay(150);
    }
  }

This code will run every loop(), even when the button doesn't change. Worse yet, it has a 150 ms delay, which will cause every part of your program to respond slowly. These types of delays will kill your program's performance!

When you write code, it's best to keep things simple. The easier it is to read your code, the easier you can understand what it is really doing.

Before you do anything else, click Tools > Auto Format. This will help your code instantly become easier to read.

I highly recommend eliminating variables like btnu. Instead, write this:

Code: 
  else if (buttonup.risingEdge()) {

This style is much clearer. You can easily see when that block of code is supposed to run. Adding unnecessary variables only makes reading and understanding what it will really do much harder.

Whatever you do, get rid of all those delay() functions. When you have a motor running, the absolute last thing your program should do is ignore it for any length of time. Every delay() in your program is an opportunity for it to allow the motor to get out of control.
 
Thanks Paul,
Yes i missed that delay probably due to bad formatting. All corrected now,
The btnu and btnd variables are the way for me to see if button is still pressed, i only want to run the code while button is pressed down. Is there a way to do it without use of extra variables?

Code: 
 else if (btnu == 0) {
    if (tbutton.check() == 1) {
      Serial.print(".");
      pwml = pwmr = 0;
      testl = testr = 0;
      digitalWrite(12, HIGH);
      delay(150);
    }
  }
Probably i can move to rising edge since running it once is sufficient as all it does is stopping the motor and there is no need to run it all the time.
Also i have moved away from Metro and using elapsedMillis it works in predictable way! Right now i am trying to figure out balancing of 2 motors (cant find a good way to adjust the speed it always overshoots and then has to correct that and overshoots again each time making it worse )
 
Wow that is a lot to learn about PID implementation. I probably just stick with good enough liner adjustments for now.
I got another question for Paul about encoder library. Since i expect that my project might experience power downs from time to time, On the start up in approximately 94% of cases the encoders will read (Hall sensors will start in pull up state unless magnet is directly under it, which you can see from image doesn't happen very often )
1 1 . Now depending on the actual position of the magnet in relation to sensors i might miss up to 0.75 of a rotation. So my question is is it possible to pass up the last state to the Encoder library so it will have a same info as it did before power down? I know the design of encoder "Wheel" is less then ideal but i had to retrofit it on the existing motor and thats the best i could do to fit in there to get close to 90 degree phase.
newsensor.png
 
Last edited:
dimammx said:
So my question is is it possible to pass up the last state to the Encoder library so it will have a same info as it did before power down?
Click to expand...

This is the sort of question which really depends on the meaning of "possible". Certainly this is possible with some amount of work. But perhaps that will be more than you wish to do?

First, you're going to need somewhere to store this data. The 2 basic choices are EEPROM and battery backed RAM. EEPROM has a limited write endurance, and the write speed is slow, so you really can't expect to write every encoder change to EEPROM. If you go the EEPROM route, you'll need to build hardware that detects power loss and gives you an early warning interrupt. Then you could back up the data to EEPROM right before the power fails.

Writing to RAM is probably easier, since it's fast and you can write the RAM an infinite number of times. There's no need to build circuitry for early detection of power loss. You will of course need to add a coin cell to the VBAT pin on Teensy. There's a special 32 byte area for storing battery supported data. The RTC startup uses the last 4 bytes. You could use the other 28 bytes to write copies of the stuff you want to back up.

If you want to get the internal state from the Encoder library, then you'll have to do even more work to get it to return that data. But maybe just the 32 bit count is enough? Then you just read the count at startup and initialize the library.
 
Status
Not open for further replies.
Back
Top