Stepper Motor Not Stopping, Need Some Help

Go To Last Post
16 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi guys. So im basically trying to start and stop a stepper motor using the atmega16 mcu. I have a GUI with a start and stop button and im using usart to transmit the signal. 

 

This is basically the c# code for the start button (Excluding all the port configs):

private void BtnStart_Click(object sender, EventArgs e)
        {
            serialPort1.Write(",");
        }

And for the stop button:

 private void BtnStop_Click(object sender, EventArgs e)
        {
            serialPort1.Write("/");
        }

This is my MCU code:

 

#ifndef F_CPU
#define F_CPU 8000000UL
#endif

#define BAUD 9600                           // define baud
#define BAUDRATE ((F_CPU)/(BAUD*16UL)-1)

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

uint16_t t = 50535;         // Time Delay Value For Delay Loop
uint8_t d = 0, i = 0;          //Initialise Direction Flag
char rx = ' ';

void USARTInitialize();
void Clockwise(void);
uint8_t r = 0; 

int main(void)
{
	sei();
	USARTInitialize();
	
	DDRC = 0xFF;        // Initialize Ports As An Input or Output
	DDRB = 0x03;
	DDRD = 0x03;
	DDRA = 0x00;
	PORTC = 0x00;
	PORTB = 0x00;
	PORTD = 0x00;
	PORTA = 0xFF;
	while (1)
	{
	}
}

void Clockwise(void)               // Rotates The Stepper Motor Clockwise
{
	while(PINA == 0xFF)        // Apply Pulse Sequence
	{
		PORTC = 0x01;          // Apply Pulse To 1st Coil
		_delay_loop_2(t);     // Generate Delay

		PORTC = 0x02;
		_delay_loop_2(t);

		PORTC = 0x04;
		_delay_loop_2(t);

		PORTC = 0x08;
		_delay_loop_2(t);
	}
}

void USARTInitialize()
{
	UBRRL = BAUDRATE; //Set baud rate
	UCSRB |= (1<<TXEN)|(1<<RXEN)|(1<<RXCIE); // Enable transmitter, receiver
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1); //8 bit data
}

ISR(USART_RXC_vect)
{
	rx = UDR;
	if(rx==',') //Check keycode sent from gui
	{
		PORTD = 0x01;
		//KeyDelay();
		PORTD = 0x00;
		r = 1;                        // Set Run Flag
		d = 0;
		Clockwise(); //Start stepper motor

	}
	if(rx=='/') //Check keycode sent from gui
		{
			PORTC = 0x00;              // Stop The Stepper Motor
			r = 0;
		}
	reti();
}

So when I press the start button, the motor rotates just fine, but when I press the stop button, it just continues to rotate. There must be something i'm doing wrong but i'm not sure what. What could be the problem ?

This topic has a solution.
Last Edited: Wed. Nov 2, 2016 - 10:49 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Well, to start you'll never get out of the first ISR when you're looping in the Clockwise function (which is called in ISR and the loop never exits).

 

Secondly, I assume you are trying to use the variable r as a flag for start/stop of the stepper. You set it but you never check it. You should do this in your main while loop.

 

Simplify your ISR and do the heavy lifting in the main while loop.

 

[Edit] Changed looping/not looping to start/stop

My digital portfolio: www.jamisonjerving.com

My game company: www.polygonbyte.com

Last Edited: Wed. Nov 2, 2016 - 05:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Jamison wrote:

Well, to start you'll never get out of the first ISR when you're looping in the Clockwise function (which is called in ISR and the loop never exits).

 

Secondly, I assume you are trying to use the variable r as a flag for start/stop of the stepper. You set it but you never check it. You should do this in your main while loop.

 

Simplify your ISR and do the heavy lifting in the main while loop.

 

[Edit] Changed looping/not looping to start/stop

 

So the problem is in that loop in the clockwise() method which doesnt stop ?, but wouldn't the interrupt fire regardless of the infinite loop since it takes preference ?, which is when I press the stop button

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Sgt123 wrote:

So the problem is in that loop in the clockwise() method which doesnt stop ?, but wouldn't the interrupt fire regardless of the infinite loop since it takes preference ?, which is when I press the stop button

 

Exactly that. And no, until the ISR has finished no other interrupts can be triggered. This is the main reason why ISRs should be kept as short as possible, what Jamison said about setting a flag in the ISR then checking for it and doing the heavy lifting in the while(1) loop is a pretty typical set up 

 

Edit: typo

 

PS: If you're working with stepper motors you may be interested in some of STMicroelectronics stepper driver IC's such as the L6480, the L6482, and the PowerSTEP01. I've used all three of these devices myself and they're pretty clever. You use SPI to control them, so you don't need to worry about step sequence algorithms, you just send them commands over SPI, then wait for a pin (the /BUSY pin) to go high to signify that it's finished moving - simples! Of course controlling stepper motors the way you are currently is just fine, I'm just throwing in my two scents :)

Last Edited: Wed. Nov 2, 2016 - 06:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Howard_Smith wrote:
PS: If you're working with stepper motors you may be interested in ...

 

And for real work, you don't just "start" and "stop" a stepper motor.  You need to ramp the step rate up and down.  I've posted my more-or-less complete example of that.

See https://www.avrfreaks.net/comment... for overview and code fragments.

 

 

If no-load or very light load, you might get by with it.  And I guess your long (in microcontroller terms) delay between steps means you never get to a fast step rate anyway.

 

 

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Ok, I shortened the interrupt by just setting a flag but now the motor does not turn on

Here's my code:

 

#ifndef F_CPU
#define F_CPU 8000000UL
#endif

#define BAUD 9600                           // define baud
#define BAUDRATE ((F_CPU)/(BAUD*16UL)-1)

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

uint16_t t = 50535;         // Time Delay Value For Delay Loop
uint8_t d = 0, i = 0;          //Initialise Direction Flag
char rx = ' ';
int flag = 0;

void USARTInitialize();
void Clockwise(void);
void stopMotor(void);
uint8_t r = 0; 


int main(void)
{
	
	sei();
	USARTInitialize();
	
	DDRC = 0xFF;        // Initialize Ports As An Input or Output
	DDRB = 0x03;
	DDRD = 0x03;
	DDRA = 0x00;
	PORTC = 0x00;
	PORTB = 0x00;
	PORTD = 0x00;
	PORTA = 0xFF;
	while (1)
	{
		if(flag=1)
		{
			rx = UDR;
			if(rx==',') //Check keycode
			{
				flag = 0;
				Clockwise(); //Start stepper motor
			}
		}
	}
}

void Clockwise(void)               // Rotates The Stepper Motor Clockwise
{
	while(1)        // Apply Pulse Sequence
	{
		PORTC = 0x01;          // Apply Pulse To 1st Coil
		_delay_loop_2(t);     // Generate Delay

		PORTC = 0x02;
		_delay_loop_2(t);

		PORTC = 0x04;
		_delay_loop_2(t);

		PORTC = 0x08;
		_delay_loop_2(t);
		
		if(flag=1)
		{
			rx = UDR;	
			if(rx=='/') //Check keycode
			{
				flag=0;
				stopMotor();
				break;
			}
		}
	}
}

void stopMotor(void)
{
	PORTC = 0x00;              // Stop The Stepper Motor
	//r = 0;
}


ISR(USART_RXC_vect)
{
	flag = 1;
	reti();
}

void USARTInitialize()
{
	UBRRL = BAUDRATE; //Set baud rate
	UCSRB |= (1<<TXEN)|(1<<RXEN)|(1<<RXCIE); // Enable transmitter, receiver
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1); //8 bit data
}

What could be the problem ?

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

You aren't storing the received data in your ISR, just setting your flag. EDIT: Just noticed you store the data inside the while(1) inside main(). This could still be causing the problem though - I can't remeber off hand but .NET's SerialPort class may well send 2 byes for the .Write method; one being the byte you want to send, and the other being a delimiter (typically null, IOW 0x00). Now depending on the speed of your microcontroller, by the time you get out of the ISR and back into main where you're reading UDR, the first byte (i.e the byte you actually wanted to send) may have been overwritten by the delimiting byte, meaning that  if (rx == ',') would never be true, thus your motor would never turn.  

(Be careful using the .NETs SerialPort class, it's a little hit n miss. This link explains some of its pitfalls ad how to get around them)

 

Maybe not why your motor is never turning, but I also have a few points to consider:

 

   - Once you call your Clockwise() function, your code will ever get out of that loop. You have a break; in one of the if statements in the while loop, but that if statement is nested inside another which does not have a break, so your code will continue to loop. Also it's generally not good practice to have never ending loops inside functions, even with break; sure you may get away with it, but its not sound as far as code goes. You already have a never ending loop inside main() anyway so you don't need it.

 

Also in your ISR you're setting your flag regardless of what you've been sent. What I was thinking (and I'm sure Jamison was thinking something along these lines too) was something like this (in pseudo code form)

 

In your ISR:

 

ISR(USART_RXC_vect)
{
    rx = UDR

    if (rx == ',')
        flag = 1;
    else if (rx == '/')
        flag = 0
    // else error, but you would have to do something about flagging that
}

 

Then in main()

 

int main(void)
{
    InitPorts();
    InitUART();
    sei();

    while(1)
    {
        if (flag == 1)
            Clockwise();
        else
            stopMotor();

        // If you flag an error you can check the flag and do something
        // about it here
    }
}

Edit: Correction

Edit 2: spotted that you are in fact reading UDR into rx

Last Edited: Wed. Nov 2, 2016 - 08:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Sgt123 wrote:
if(flag=1)
Sgt123 wrote:
What could be the problem ?

Do you see the typo where flag=1 will always be true, and what you wanted to use is flag==1 ?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Sgt123 wrote:
reti();
Hmmm--won't that be dangerous, and the stack will immediately be messed up?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

 

int flag = 0;

 

should be

volatile int flag = 0;

because flag is modified in the ISR.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks guys for all the suggestions yes, got it working and even managed to add speed up and speed down buttons smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

Sgt123 wrote:
reti();
Hmmm--won't that be dangerous, and the stack will immediately be messed up?

 

Can I ask why that'd be dangerous?  Unnecessary sure, but what problems would it cause?  Even if the compiler did as requested and put two reti instructions in a row, wouldn't it just hit the first one and return to where it was before the ISR fired, never hitting the (implied) reti?

 

Edit:  Oh, I think I see where you're going with that now...would that actually cause it to skip the rest of the epilogue?  I thought avr-gcc automatically added it even if you have multiple return paths within an ISR.

Last Edited: Thu. Nov 3, 2016 - 05:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rezer wrote:
Can I ask why that'd be dangerous? Unnecessary sure, but what problems would it cause? Even if the compiler did as requested and put two reti instructions in a row, wouldn't it just hit the first one and return to where it was before the ISR fired, never hitting the (implied) reti?

The C startup files will handle the reti, stack, and register saves and restores. By prematurely forcing an reti, you're skipping over those commands to restore the registers and stack. If you understand assembly, take a look at the generated code for that function. I don't have access to Atmel Studio on this machine so unfortunately I can't post an example of this.

 

Rezer wrote:
wouldn't it just hit the first one and return to where it was before the ISR fired, never hitting the (implied) reti?

But you're using an assembly level command (not the normal C return command). When the compiler sees the return command, it will decide how to treat the return appropriately (e.g. what registers need to be restored from the stack, thus keeping the stack preserved). Otherwise you force the compiler to step over that code and thus causing the stack to be trashed by not restoring any used registers in the ISR.

 

[Edit] More clarification

My digital portfolio: www.jamisonjerving.com

My game company: www.polygonbyte.com

Last Edited: Thu. Nov 3, 2016 - 12:02 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah, ok...I didn't realize reti() is just an assembly macro and avr-gcc won't decide to cram in the epilogue anyway.  So then the only appropriate usage of reti() is within a naked ISR?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rezer wrote:

Ah, ok...I didn't realize reti() is just an assembly macro and avr-gcc won't decide to cram in the epilogue anyway.  So then the only appropriate usage of reti() is within a naked ISR?

Personally I would say in C or C++, one should never use reti unless you fully understand what it does, why it does it, and why you want to use it over a normal return (again, in C, probably never).

 

The reti command is useful when hand-crafting assembly for precise clock cycles and you have full control over saving and restoring the registers and how the stack is used.

My digital portfolio: www.jamisonjerving.com

My game company: www.polygonbyte.com

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rezer wrote:
Ah, ok...I didn't realize reti() is just an assembly macro and avr-gcc won't decide to cram in the epilogue anyway.

No "realize" needed, just a bit of cut-and-paste:

 

#include <avr/interrupt.h>
volatile unsigned char flag;

ISR(USART_RXC_vect)
{
  76:	1f 92       	push	r1
  78:	0f 92       	push	r0
  7a:	0f b6       	in	r0, 0x3f	; 63
  7c:	0f 92       	push	r0
  7e:	11 24       	eor	r1, r1
  80:	8f 93       	push	r24
	flag = 1;
  82:	81 e0       	ldi	r24, 0x01	; 1
  84:	80 93 65 00 	sts	0x0065, r24
	reti();
  88:	18 95       	reti
}
  8a:	8f 91       	pop	r24
  8c:	0f 90       	pop	r0
  8e:	0f be       	out	0x3f, r0	; 63
  90:	0f 90       	pop	r0
  92:	1f 90       	pop	r1
  94:	18 95       	reti

"You make the call..."  https://www.youtube.com/watch?v=...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.