Question

I am working on some firmware for an embedded device that uses a 16 bit PIC operating at 40 MIPS and programming in C. The system will control the position of two stepper motors and maintain the step position of each motor at all times. The max position of each motor is around 125000 steps so I cannot use a 16bit integer to keep track of the position. I must use a 32 bit unsigned integer (DWORD). The motor moves at 1000 steps per second and I have designed the firmware so that steps are processed in a Timer ISR. The timer ISR does the following:

1) compare the current position of one motor to the target position, if they are the same set the isMoving flag false and return. If they are different set the isMoving flag true.

2) If the target position is larger than the current position, move one step forward, then increment the current position.

3) If the target position is smaller than the current position, move one step backward, then decrement the current position.

Here is the code:

void _ISR _NOPSV _T4Interrupt(void)
{
    static char StepperIndex1 = 'A';    

    if(Device1.statusStr.CurrentPosition == Device1.statusStr.TargetPosition)
    {
        Device1.statusStr.IsMoving = 0;
        // Do Nothing
    }   
    else if (Device1.statusStr.CurrentPosition > Device1.statusStr.TargetPosition)
    {
        switch (StepperIndex1)      // MOVE OUT
        {
            case 'A':
                SetMotor1PosB();
                StepperIndex1 = 'B';
                break;
            case 'B':
                SetMotor1PosC();
                StepperIndex1 = 'C';
                break;
            case 'C':
                SetMotor1PosD();
                StepperIndex1 = 'D';
                break;
            case 'D':
                default:
                SetMotor1PosA();
                StepperIndex1 = 'A';
                break;      
        }
        Device1.statusStr.CurrentPosition--;    
        Device1.statusStr.IsMoving = 1;
    }   
    else
    {
        switch (StepperIndex1)      // MOVE IN 
        {
            case 'A':
                SetMotor1PosD();
                StepperIndex1 = 'D';
                break;
            case 'B':
                SetMotor1PosA();
                StepperIndex1 = 'A';
                break;
            case 'C':
                SetMotor1PosB();
                StepperIndex1 = 'B';
                break;
            case 'D':
                default:
                SetMotor1PosC();
                StepperIndex1 = 'C';
                break;      
        }
        Device1.statusStr.CurrentPosition++;
        Device1.statusStr.IsMoving = 1;
    }   
    _T4IF = 0;          // Clear the Timer 4 Interrupt Flag.
}

The target position is set in the main program loop when move requests are received. The SetMotorPos lines are just macros to turn on/off specific port pins.

My question is: Is there any way to improve the efficiency of this code? The code functions fine as is if the positions are 16bit integers but as 32bit integers there is too much processing. This device must communicate with a PC without hesitation and as written there is a noticeable performance hit. I really only need 18 bit math but I don't know of an easy way of doing that! Any constructive input/suggestions would be most appreciated.

Was it helpful?

Solution

Warning: all numbers are made up...

Supposing that the above ISR has about 200 (likely, fewer) instructions of compiled code and those include the instructions to save/restore the CPU registers before and after the ISR, each taking 5 clock cycles (likely, 1 to 3) and you call 2 of them 1000 times a second each, we end up with 2*1000*200*5 = 2 millions of clock cycles per second or 2 MIPS.

Do you actually consume the rest 38 MIPS elsewhere?

The only thing that may be important here and I can't see it, is what's done inside of the SetMotor*Pos*() functions. Do they do any complex calculations? Do they perform some slow communication with the motors, e.g. wait for them to respond to the commands sent to them?

At any rate, it's doubtful that such simple code would be noticeably slower when working with 32-bit integers than with 16-bit.

If your code is slow, find out where time is spent and how much, profile it. Generate a square pulse signal in the ISR (going to 1 when the ISR starts, going to 0 when the ISR is about to return) and measure its duration with an oscilloscope. Or do whatever is easier to find it out. Measure the time spent in all parts of the program, then optimize where really necessary, not where you have previously thought it would be.

OTHER TIPS

The difference between 16 and 32 bits arithmetic shouldn't be that big, I think, since you use only increment and comparision. But maybe the problem is that each 32-bit arithmetic operation implies a function call (if the compiler isn't able/willing to do inlining of simpler operations).

One suggestion would be to do the arithmetic yourself, by breaking the Device1.statusStr.CurrentPosition in two, say, Device1.statusStr.CurrentPositionH and Device1.statusStr.CurrentPositionL. Then use some macros to do the operations, like:

#define INC(xH,xL) {xL++;if (xL == 0) xH++;}

I would get rid of the StepperIndex1 variable and instead use the two low-order bits of CurrentPosition to keep track of the current step index. Alternately, keep track of the current position in full rotations (rather than each step), so it can fit in a 16 bit variable. When moving, you only increment/decrement the position when moving to phase 'A'. Of course, this means you can only target each full rotation, rather than every step.

Sorry, but you are using bad program design.

Let's check the difference between 16 bit and 32 bit PIC24 or PIC33 asm code...

16 bit increment

inc    PosInt16               ;one cycle

So 16 bit increment takes one cycle

32bit increment

clr    Wd                     ;one cycle
inc    low PosInt32           ;one cycle
addc   high PosInt32, Wd      ;one cycle

and 32 increment takes three cycles. The total difference is 2 cycles or 50ns (nano seconds).

Simple calcolation will show you all. You have 1000 steps per second and 40Mips DSP so you have 40000 instructions per step at 1000 steps per second. More than enough!

When you change it from 16bit to 32bit do you change any of the compile flags to tell it to compile as a 32bit application instead.

have you tried compiling with the 32bit extensions but using only 16bit integers. do you still get such a performance drop?

It's likely that just by changing from 16bit to 32bit that some operations are compiled differently, perhaps do a Diff between the two sets of compiled ASM code and see what is actually different, is it lots or is it only a couple of lines.

Solutions would be maybe instead of using a 32bit integer, just use two 16bit integers, when the valueA is int16.Max then set it to 0 and then increment valueB by 1 otherwise just incriment ValueA by 1, when value B is >= 3 you then check valueA >= 26696 (or something similar depending if you use unsigned or signed int16) and then you have your motor checking at 12500.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top