Why is this function being skipped?

Using Arduino Programming Questions
#1

I am currently running this main program on my arduino uno.

#include "speed_profile.h"


void setup() {
  // put your setup code here, to run once:
  output_pin_setup();
  cli();
  timer1_setup();
  sei();
}

void loop() 
{
      int motor_steps = 23400;//-9600; //23400 -  100%
      // Accelration to use.
      int motor_acceleration = 500;//500;
      // Deceleration to use.
      int motor_deceleration = 500;//500;
      // Speed to use.
      int motor_speed = 1000; // 1000
      init_tipper_position();
      compute_speed_profile(motor_steps, motor_acceleration, motor_deceleration, motor_speed); 
}

For some weird reason is the init_tipper_position(); not being executed, it is rather skipped from the whole program for some weird reason.

The function looks like this

void init_tipper_position()
{
  digitalWrite(en_pin, HIGH);
  delay(1);
  
  digitalWrite(dir_pin, LOW);
  delay(1);

  int decrement = 0;
  while((PINB & (1 << 2)))  
  {
    decrement++;
    digitalWrite(step_pin, HIGH);
    delayMicroseconds(600);
    digitalWrite(step_pin, LOW);
    delayMicroseconds(600);
  }

  digitalWrite(en_pin, LOW);

}

I know for fact it being skipped as it doesn’t perform the task it should be performing which is,
move the motor until you hit a sensor. The motor doesn’t move, that could be due to me providing it a PWM
with an to high frequency. The other things it that the enable signal is never set. It would be impossible to
move the motor, if the en_pin was set, but as the motor can be easily moved, i guess that the en_pin isn’t being setted.

I own a logic analyzer, and have currently tested the signal of the en_pin and confirmed that it never becomes high?

So why is the program skipping the init_tipper_position(); and goes directly to compute_speed_profile()?..

All other part of the program works as it should. The sensor is not defect, i’ve tested it with a dummy program, it is capable outputting HIGH or LOW.

So something is very wrong with my program?

I’ve attached .cpp files and .h being used in the program

speed_profile.cpp (9.63 KB)

speed_profile.h (2.25 KB)

main.ino (516 Bytes)

main.ino (516 Bytes)

#2

215_215:
I know for fact it being skipped

As far as I can see that is the only function in loop() and there is no conditional code that might operate to prevent it being called therefore my conclusion is that if loop() is called so is your function.

Is there something in your other code that is preventing loop() from being called?

ISR(TIMER1_COMPA_vect) in your .cpp file has a crazy amount of code in it. How long does it take to execute? I try to keep my ISRs to 4 or 5 lines of code.

...R

#3

If PINB & 0b100 is 0, how many times will the while loop iterate? Which pin, specifically, is that reading? Is that pin defined as an INPUT_PULLUP pin?

Is there some reason you can't be bothered to:
A) Post all of your code every time?
B) Stick to one of the other 47 threads you have going?

#4

why are you starting a new thread? you had an another one with similar code… was that solved? or that is still a consequence of the other issue not really being under control?

You did not even bother to change the unsigned int stuff we mentioned to you… good luck.

#5

J-M-L:
why are you starting a new thread? you had an another one with similar code... was that solved? or that is still a consequence of the other issue not really being under control?

You did not even bother to change the unsigned int stuff we mentioned to you... good luck.

The other thread is solved, by using direct pin acess. I didn't change it , but removed the variables that were used to call the function. The function does use signed int and so on.

The function works, but doesn't work if the other one isn't commented.

I am not sure if this is related to the problem, but i am currently on 949 bytes memory left for local variables?..
I read somewhere that low memory could cause weird things like this? is this a low memory or ok?

Robin2:
As far as I can see that is the only function in loop() and there is no conditional code that might operate to prevent it being called therefore my conclusion is that if loop() is called so is your function.

Is there something in your other code that is preventing loop() from being called?

ISR(TIMER1_COMPA_vect) in your .cpp file has a crazy amount of code in it. How long does it take to execute? I try to keep my ISRs to 4 or 5 lines of code.

...R

I am would not say that is anything that prevent it from being called.
the ISR routine is basically a switch case, so not all of the code get executed in a ISR call, the only thing that is being executed is the state itself. The ISR routine is a copy of an atmel application note. http://www.atmel.com/images/doc8017.pdf which states that the ISR depending on the state will max take approx 200µs.

PaulS:
If PINB & 0b100 is 0, how many times will the while loop iterate? Which pin, specifically, is that reading? Is that pin defined as an INPUT_PULLUP pin?

Is there some reason you can't be bothered to:
A) Post all of your code every time?
B) Stick to one of the other 47 threads you have going?

The pin it is reading from is pin 10, and the while loop should iterate as long the pin is low, which it would be until the sensor is being activated. All the code is posted, i've attached it as files? I guess posting the full code using the code tags, would just make the post a bit unstructured, and hard to read.. Just my oppinion.

#6

The pin it is reading from is pin 10, and the while loop should iterate as long the pin is low, which it would be until the sensor is being activated.

Not if the pin mode is INPUT_PULLUP.

Just my oppinion.

You are in the minority (all by yourself, actually).

You posted the header and source files, but NOT the (complete) sketch.

#7

The pin it is reading from is pin 10, and the while loop should iterate as long the pin is low, which it would be until the sensor is being activated.

 while((PINB & (1 << 2)))

no the loop iterate as long as pin 10 is high.

#8

Ohh...
I changed it to zero, but that but that still does make it move.
And enable is still LOW

#9

PaulS:
Not if the pin mode is INPUT_PULLUP.
You are in the minority (all by yourself, actually).

You posted the header and source files, but NOT the (complete) sketch.

Ahh.. Sorry.. I guess i might have miscommunicated something. The main i've posted in first first is the sketch, or the *.ino file. I've for completeness sake attached all three files. All the files should be attached in the first post now.

#10

I just confirmed using serial that it does indeed skip the sequence?
But why though?

#11

215_215:
which states that the ISR depending on the state will max take approx 200µs.

Considering that the Arduino system expects micros() to be updated every 4µsecs don't you think that 200µsecs might be a little long?

...R

#12

Well.. having the while loop delayed don't matter to me as it only provides me with debug info. The ISR routine is based on atmel application note http://www.atmel.com/images/doc8017.pdf

its basically a copy of the code.

#13

If, at any point, you think it necessary to post your code, please, feel free.

#14

Here is the full code

main.ino

#include "speed_profile.h"


void setup() {
  // put your setup code here, to run once:
  output_pin_setup();
  cli();
  timer1_setup();
  sei();
}

void loop() 
{
      //int motor_steps = 23400;//-9600; //23400 -  100%
      // Accelration to use.
      //int motor_acceleration = 500;//500;
      // Deceleration to use.
      //int motor_deceleration = 500;//500;
      // Speed to use.
      //int motor_speed = 1000; // 1000
      init_tipper_position();
      compute_speed_profile(23400, 500, 500, 1000); 
}
[\code]
#15

.cpp

#include "speed_profile.h"


volatile speed_profile profile;

ros::NodeHandle_<ArduinoHardware, 1, 2, 125, 125> nh;
//ros::NodeHandle nh;

std_msgs::Int8 status_step_count;
std_msgs::Int8 status_tipper_availability;
ros::Publisher chatter_status("tipper_status", &status_step_count);
ros::Publisher chatter_availabilty("tipper_availability", &status_tipper_availability);

volatile bool global_state = false;
int received_data = 0;


void call_back_control( const std_msgs::Empty & msg)
{
  global_state = true;
  // For HMI
  received_data  = (10 *23400.0)/100.0; // Converts input to motor_steps.
}

ros::Subscriber<std_msgs::Empty> sub_control("tipper_control", &call_back_control );

void output_pin_setup()
{
  pinMode(en_pin, OUTPUT);
  pinMode(dir_pin, OUTPUT);
  pinMode(step_pin, OUTPUT);
  pinMode(slot_sensor_pin,INPUT_PULLUP);
  nh.initNode();
  nh.advertise(chatter_status);
  nh.advertise(chatter_availabilty);
  nh.subscribe(sub_control);
  //nh.subscribe(sub_command);
  profile.current_step_position = 0;
  delay(10);
  nh.getHardware()->setBaud(57600);
}

void init_tipper_position()
{
  digitalWrite(en_pin, HIGH);
  delay(1);
  
  digitalWrite(dir_pin, LOW);
  delay(1);

  
  while((PINB & (0 << 2)))  
  {
    digitalWrite(step_pin, HIGH);
    delayMicroseconds(100);
    digitalWrite(step_pin, LOW);
    delayMicroseconds(100);

  }

  digitalWrite(en_pin, LOW);

}
void timer1_setup()
{
  // Tells what part of speed ramp we are in.
  profile.run_state = STOP;

  // Timer/Counter 1 in mode 4 CTC (Not running).
  TCCR1B = (1 << WGM12);

  // Timer/Counter 1 Output Compare A Match Interrupt enable.
  TIMSK1 = (1 << OCIE1A);
}


void compute_speed_profile(signed int motor_steps, unsigned int motor_accel, unsigned int motor_decel, unsigned int motor_speed)
{
  while (global_state == false)
  {
    //do nothing
    status_step_count.data = ((float)(profile.current_step_position / 23400.0)) * 100.0; //map(profile.current_step_position,0,23400,0,100); // could be expensive -- Nice flowing is only available with float
    status_tipper_availability.data = 1;
    
    chatter_status.publish( &status_step_count);
    chatter_availabilty.publish(&status_tipper_availability);
    nh.spinOnce();
  }

  digitalWrite(en_pin, HIGH);
  delay(1);

  signed int move_steps;
  //if (motor_steps < 0)
  if(received_data > profile.current_step_position) // Compares whether received percentage (converted to motor_steps) is greater or smaller than current_step_position.
  {
    profile.dir = CCW;
    //motor_steps = -motor_steps;
    move_steps =  profile.current_step_position - received_data;
    digitalWrite(dir_pin, HIGH);                          
  }
  else
  {
    profile.dir = CW;
    move_steps =  profile.current_step_position - received_data;
    digitalWrite(dir_pin, LOW);
  }

  delay(1);
  // received_data = percentage converted to a step number received from tipper_control topic
  
  computation_of_speed_profile(move_steps, motor_accel, motor_decel, motor_speed); // function should be called with the correct motor_steps value

  OCR1A = 10;
  // Set Timer/Counter to divide clock by 8
  TCCR1B |= ((0 << CS12) | (1 << CS11) | (0 << CS10));


  while (global_state == true)
  { 
    status_step_count.data = ((float)(profile.current_step_position / 23400.0)) * 100.0; //map(profile.current_step_position,0,23400,0,100); // could be expensive -- Nice flowing is only available with float
    status_tipper_availability.data = 0;

    chatter_availabilty.publish(&status_tipper_availability);
    chatter_status.publish( &status_step_count);
    nh.spinOnce();
    //delay(100);
  }
}

ISR(TIMER1_COMPA_vect)
{
  // Holds next delay period.
  unsigned int new_first_step_delay;

  // Remember the last step delay used when accelrating.
  static int last_accel_delay;

  // Counting steps when moving.
  static unsigned int step_count = 0;

  // Keep track of remainder from new_step-delay calculation to incrase accurancy
  static unsigned int rest = 0;
  OCR1A = profile.first_step_delay;
  switch (profile.run_state)
  {

    case STOP:
      step_count = 0;
      global_state = false;
      rest = 0;
      TCCR1B &= ~((1 << CS12) | (1 << CS11) | (1 << CS10)); // Stop the timer,  No clock source
      break;

    case ACCEL:
      //digitalWrite(step_pin, !digitalRead(step_pin));
      PORTB ^= _BV(PB3);  // Toggles the step_pin
      step_count++;
      
      if (profile.dir == CCW)
      {
        profile.current_step_position++;
      }
      else
      {
        profile.current_step_position--;
      }  
      
      profile.accel_count++;
      new_first_step_delay = profile.first_step_delay - (((2 * (long)profile.first_step_delay) + rest) / (4 * profile.accel_count + 1));
      rest = ((2 * (long)profile.first_step_delay) + rest) % (4 * profile.accel_count + 1);

      // Chech if we should start decelration.
      if (step_count >= profile.decel_start)
      {
        profile.accel_count = profile.decel_length;
        profile.run_state = DECEL;
      }

      // Chech if we hitted max speed.
      else if (new_first_step_delay <= profile.min_time_delay)
      {
        last_accel_delay = new_first_step_delay;
        new_first_step_delay = profile.min_time_delay;
        rest = 0;
        profile.run_state = RUN;
      }
      break;
    case RUN:
      //digitalWrite(step_pin, !digitalRead(step_pin));
      PORTB ^= _BV(PB3); // Toggles the step_pin
      step_count++;
      
      if (profile.dir == CCW)
      {
        profile.current_step_position++;
      }
      else
      {
        profile.current_step_position--;
      }  
      new_first_step_delay = profile.min_time_delay;
      // Chech if we should start decelration.
      if (step_count >= profile.decel_start)
      {
        profile.accel_count = profile.decel_length;
        // Start decelration with same delay as accel ended with.
        new_first_step_delay = last_accel_delay;
        profile.run_state = DECEL;
      }
      break;
    case DECEL:
      //digitalWrite(step_pin, !digitalRead(step_pin));
      PORTB ^= _BV(PB3); // Toggles the step_pin
      step_count++;
      if (profile.dir == CCW)
      {
         profile.current_step_position++;
      }
      else
      {
        profile.current_step_position--;
      }  
      profile.accel_count++;
      new_first_step_delay = profile.first_step_delay - (((2 * (long)profile.first_step_delay) + rest) / (4 * profile.accel_count + 1));
      rest = ((2 * (long)profile.first_step_delay) + rest) % (4 * profile.accel_count + 1);
      // Check if we at last step
          
      if (profile.accel_count >= 0)
      {
        digitalWrite(en_pin, !digitalRead(en_pin));
        //PORTB &= ~_BV(PB3);

        profile.run_state = STOP;
      }
      break;

  }
   
  profile.first_step_delay = new_first_step_delay;
  
}

code without one one functions that basically does a lot math an sets the profile variables.

#16 
 The ISR routine is based on atmel application note http://www.atmel.com/images/doc8017.pdf

You understand that your arduino execution environment is not equivalent at coding a raw processor where you control everything right? there are timers ticking (timer0 to control delay(), millis(), …)

for example for your motor you do

  delayMicroseconds(600);

delayMicroseconds() does not disable interrupts and just does a busy active wait loop based on knowing how long it takes to execute assembly language code on your 16MHz arduino

// busy wait
 __asm__ __volatile__ (
 "1: sbiw %0,1" "\n\t" // 2 cycles
 "brne 1b" : "=w" (us) : "0" (µs) // 2 cycles
 );

so this will get interrupted by timer0 but also your timer a number of time. and 200 µsecs in your timer is putting a dent clearly in the stability of the pulse you generate

I’ve not looked - How often does your timer tick?

#17

.h

/*
 * Contains the class concerning with calculating the proper speed profile 
 * for accelerating and decelerating the stepper motor.
 * 
 */

#ifndef speed_profile_h
#define speed_profile_h


#include <Arduino.h> 
#include <ros.h>
#include <std_msgs/Int8.h>
#include <std_msgs/Empty.h>

// Timer/Counter 1 running on 3,686MHz / 8 = 460,75kHz (2,17uS). (T1-FREQ 460750)
//#define T1_FREQ 460750
#define T1_FREQ 1382400

//! Number of (full)steps per round on stepper motor in use.
#define FSPR 1600

// Maths constants. To simplify maths when calculating in compute_speed_profile().
#define ALPHA (2*3.14159/FSPR)                    // 2*pi/spr
#define A_T_x100 ((long)(ALPHA*T1_FREQ*100))     // (ALPHA / T1_FREQ)*100
#define T1_FREQ_148 ((int)((T1_FREQ*0.676)/100)) // divided by 100 and scaled by 0.676
#define A_SQ (long)(ALPHA*2*10000000000)         // ALPHA*2*10000000000
#define A_x20000 (int)(ALPHA*20000)              // ALPHA*20000

// Speed ramp states
#define STOP  0
#define ACCEL 1
#define DECEL 2
#define RUN   3

// Pin numbering
#define en_pin 13
#define dir_pin 12
#define step_pin 11
#define slot_sensor_pin 10

// Motor direction 
#define CW  0
#define CCW 1

typedef struct 
{
  unsigned char run_state : 3; // Determining the state of the speed profile
  unsigned char dir: 1; // Determining the direction the motor has to move - Start being CCW 
  unsigned int first_step_delay; // Period time for the next timer_delay, determines the acceleration rate. 
  unsigned int decel_start; //  Determines at which step the deceleration should begin. 
  signed int decel_length; // Set the deceleration length
  signed int min_time_delay; //minium time required time_delay for achieving the wanted speed.
  signed int accel_count; // counter used when computing step_delay for acceleration/decelleration. 
  volatile signed int current_step_position; // contains the current step_position

}speed_profile;


void compute_speed_profile(signed int motor_steps, unsigned int motor_accel, unsigned int motor_decel, unsigned int motor_speed);
void computation_of_speed_profile(signed int motor_steps, unsigned int motor_accel, unsigned int motor_decel, unsigned int motor_speed);
void init_tipper_position();
void timer1_setup(void);
void output_pin_setup(void);
#endif
#18

without one one functions that basically does a lot math

are you doing all your math like this?

[color=red]int[/color] received_data = 0;
...

void call_back_control( const std_msgs::Empty & msg)
{
  global_state = true;
  // For HMI
  received_data  =[color=red] (10 *23400.0)/100.0[/color]; // Converts input to motor_steps.
}

This will work (because of the .0 to go to float) - but why can't you just do received_data = 2340; without letting some guess work to the pre-processor?

#19

215_215:
Well.. having the while loop delayed don't matter to me as it only provides me with debug info.

Why don't you move all of the code into your .ino file and treat it as a standard Arduino program?

Or else just write a completely non-Arduino program using the Atmel programming system.

What you are doing at the moment seems to me like someone trying to write a book in a mixture of Russian and Japanese when they are not familiar with either language.

...R

#20

J-M-L:

 The ISR routine is based on atmel application note http://www.atmel.com/images/doc8017.pdf

You understand that your arduino execution environment is not equivalent at coding a raw processor where you control everything right? there are timers ticking (timer0 to control delay(), millis(), …)

for example for your motor you do

  delayMicroseconds(600);

delayMicroseconds() does not disable interrupts and just does a busy active wait loop based on knowing how long it takes to execute assembly language code on your 16MHz arduino

// busy wait

asm volatile (
“1: sbiw %0,1” “\n\t” // 2 cycles
“brne 1b” : “=w” (us) : “0” (µs) // 2 cycles
);




so this will get interrupted by timer0 but also your timer a number of time. and 200 µsecs in your timer is putting a dent clearly in the stability of the pulse you generate

I've not looked - How often does your timer tick?

The timer has only be setup at that point, and no actual interrupt would occur inside the function as to clock has been not been given to the timer, and is first is given in the function under.

you might be right about the dent…

I don’t have a specific number of the number of ticks that occurs, as everything is calculated inside the ISR. OCR1A is set to 10 at for the tick. And then are the next ticks calculated inside (that a time consuming one)

example

ISR(TIMER1_COMPA_vect)
{
  // Holds next delay period.
  unsigned int new_first_step_delay;

  // Remember the last step delay used when accelrating.
  static int last_accel_delay;

  // Counting steps when moving.
  static unsigned int step_count = 0;

  // Keep track of remainder from new_step-delay calculation to incrase accurancy
  static unsigned int rest = 0;
  OCR1A = profile.first_step_delay;
  switch (profile.run_state)
  {

    case STOP:
      step_count = 0;
      global_state = false;
      rest = 0;
      TCCR1B &= ~((1 << CS12) | (1 << CS11) | (1 << CS10)); // Stop the timer,  No clock source
      break;

    case ACCEL:
      PORTB ^= _BV(PB3);  // Toggles the step_pin
      step_count++;
      
      if (profile.dir == CCW)
      {
        profile.current_step_position++;
      }
      else
      {
        profile.current_step_position--;
      }  
      
      profile.accel_count++;
      new_first_step_delay = profile.first_step_delay - (((2 * (long)profile.first_step_delay) + rest) / (4 * profile.accel_count + 1)); // delay calculations
      rest = ((2 * (long)profile.first_step_delay) + rest) % (4 * profile.accel_count + 1);

      // Chech if we should start decelration.
      if (step_count >= profile.decel_start)
      {
        profile.accel_count = profile.decel_length;
        profile.run_state = DECEL;
      }

      // Chech if we hitted max speed.
      else if (new_first_step_delay <= profile.min_time_delay)
      {
        last_accel_delay = new_first_step_delay;
        new_first_step_delay = profile.min_time_delay;
        rest = 0;
        profile.run_state = RUN;
      }
      break;
   
/Some more......
  }
   
  profile.first_step_delay = new_first_step_delay;
  
}