CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to CCS Technical Support

Need help with efficiency

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Vefa



Joined: 13 Feb 2007
Posts: 9

View user's profile Send private message

Need help with efficiency
PostPosted: Wed May 16, 2007 3:49 pm     Reply with quote

Hello,

I have been working on this code that reads the duty cycle of 7 serial PWM signals that is hacked from a 7 channel Futaba receiver and then adjusts 3 of them with some control input signal and sends 5 PWM signals to the servos of a T-REX heli (servo1, servo2, servo3, servo4, servo6). The code checks a switch (channel 7) to determine if the PWMs that come from the transmitter should be adjusted or not. 5 I/Os are used to send PWM to servos. 7 I/Os are used to detect what the receiver pins are getting. 1 CCP4 is used to capture the 7 serial PWM signals that is hacked from the receiver. There is also an encoder data that is being captured but that does not take a whole lot of time.
At this point the code works but the servos are too noisy to fly the heli. I will move towards using CCPs to generate PWM signals and using a second PIC (PICF877) to capture the 7 serial PWM receiver signals. However, before moving towards that direction I wanted to get feedback about my code to see if there is any room to improve it or if I did any MUST NOT DOs of PIC coding.


Code:
//FILE: antiSway5.c
//Author: V. Narli
//Date: 04/25/07 - WORKS
//Desc: Captures encoder and PWM signals, switches from manual to autonomous control
//Sends PWM signals by adjusting the pilots commands with anti-sway control input
//Modified Last: 05/16/07 Servos are too noisy!!

#include <18F8722>
#include <stdlib.h>
#include <math.h>

//Define EEPROM pins
#define EEPROM_SDA PIN_C4
#define EEPROM_SCL PIN_C3
#include <24515.c>

#define LED PIN_C0

#define bitP0 PIN_D0
#define bitP1 PIN_D1
#define bitP2 PIN_D2
#define bitP3 PIN_D3
#define bitP4 PIN_D4
#define bitP5 PIN_D5
#define bitP6 PIN_D6
#define resetEncoder PIN_D7

#define ch1 PIN_E0
#define ch2 PIN_E1
#define ch3 PIN_E2
#define ch4 PIN_E3
#define ch6 PIN_E5
#define ch7 PIN_E6

#define servo1 PIN_F3
#define servo2 PIN_F4
#define servo3 PIN_F5
#define servo4 PIN_F6
#define servo6 PIN_F7

#define detectPWM PIN_G3

#fuses HS,NOWDT,NOPROTECT,PUT,NOLVP
#use delay(clock=20000000)
#use rs232(baud=57600, xmit=PIN_H0, rcv=PIN_H1, stream=PCzigbee, DISABLE_INTS, ERRORS)


//Global Variables
boolean manualFlag=false;
int i=0, k=0, l=0, m=0, n=0, reset=0, angle=0;
int bit0d=0, bit1d=0, bit2d=0, bit3d=0, bit4d=0, bit5d=0, bit6d=0;
signed long uL=0, j=0, angleL=0, angleP=0, angleInt=0, angleDer=0, kP=20, kD=4, sign1uL=0, sign2uL=0;
unsigned long pulseLength[8]={0,0,0,0,0,0,0,0}, address=0;
signed long duty1=0, duty2=0, duty6=0, servo1Time=0, servo2Time=0, servo6Time=0, timing1=0, timing2=0, timing6=0;


#INT_TIMER0
void encoder(void)
{

   set_timer0(0);

   bit0d = (input(bitP0));
   bit1d = (input(bitP1));
   bit2d = (input(bitP2));
   bit3d = (input(bitP3));
   bit4d = (input(bitP4));
   bit5d = (input(bitP5));
   bit6d = (input(bitP6));

   angle = (bit0d*1+bit1d*2+bit2d*4+bit3d*8+bit4d*16+bit5d*32+bit6d*64)*2;

   angleL = ((signed long) angle)/2;

   m = m+1;
   if(m==4)
   {
      m = 0;
      angleDer = angleL-angleP;
      angleP = angleL;
   }

   //Anti-Sway Control Input
   uL = angleL*kP+angleDer*kD;

   if(uL>0)
   {
      sign1uL = 1;
      sign2uL = 0;
   }
   else
   {
      uL= -uL;
      sign1uL = 0;
      sign2uL = 1;
   }

   duty1 = ((signed long)pulseLength[0]+(2000));
   duty2 =   ((signed long)pulseLength[1]+(2000));
   duty6 = ((signed long)pulseLength[5]-(2000));

   if(address<65535) // still have memory left to write to
   {
      init_ext_eeprom();

      WRITE_EXT_EEPROM(address, (signed int) angleL);
      address = address + 1;
   }

}


#INT_CCP4 // hardware interrupt to detect pwm
void manualOR_isr()
{
   reset=0;
   set_timer1(0);

   while(input(detectPWM)&&(pulseLength[i]<10000>9000)
   {
      i = 0;
   }
   else
   {
      i = i+1;
   }
      
   if(i==8)
   {
      i = 0;
   }
   
}


void main(void)
{
   
   setup_timer_0(RTCC_INTERNAL);
   setup_timer_1(T1_INTERNAL);
   setup_ccp4(CCP_CAPTURE_RE); // on rising edge

   set_timer0(0);

   enable_interrupts(INT_TIMER0);
   enable_interrupts(INT_CCP4);
   enable_interrupts(GLOBAL);

   output_high(resetEncoder);
   output_low(resetEncoder);

   
   while(1)
   {

      reset = 1;
   
      //fprintf(PCzigbee, "%Ld\r\n", timing1);

      //Detect manual override switch -B-
      if(pulseLength[6]>4000)
      {
         output_low(LED);
         manualFlag=true;

         output_bit(servo1, input(ch1));
         output_bit(servo2, input(ch2));
         output_bit(servo3, input(ch3));
         output_bit(servo4, input(ch4));
         output_bit(servo6, input(ch6));
      }
      else
      {
         manualFlag=false;
      
         output_bit(servo1, input(ch1));
         output_bit(servo2, input(ch2));
         output_bit(servo3, input(ch3));
         output_bit(servo4, input(ch4));
         output_bit(servo6, input(ch6));
         
         output_high(LED);
/*
         

         if(input(ch1))
         {
            output_high(servo1);
            output_low(servo2);
            output_low(servo6);
            servo1Time = get_timer0();
            do
            {
            timing1 = get_timer0()-servo1Time;
            if(timing1<0) timing1 = timing1+65535;
            }while(timing1<duty1);
            
         }

         else if(input(ch2))
         {
            output_low(servo1);
            output_high(servo2);
            output_low(servo6);
            servo2Time = get_timer0();
            do
            {
            timing2 = get_timer0()-servo2Time;
            if(timing2<0) timing2 = timing2+65535;
            }while(timing2<duty2);
         }
         else if(input(ch6))
         {
            output_low(servo1);
            output_low(servo2);
            output_high(servo6);
            servo6Time = get_timer0();
            do
            {
            timing6 = get_timer0()-servo6Time;
            if(timing6<0) timing6 = timing6+65535;
            }while(timing6<duty6);
         }
         else
         {
            output_low(servo1);
            output_low(servo2);
            output_low(servo6);
         }
   
         */
      }

      
   }

} // end of main


I commented out the part of the code where PWM signals are sent using the adjusted values to see if the servos would still be noisy by the simple output_bit command and they were still noisy.

Thanks,


Vefa
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Wed May 16, 2007 4:01 pm     Reply with quote

Code:
while(input(detectPWM)&&(pulseLength[i]<10000>9000)

Your code was garbled during posting. To fix this:
Please edit your post, delete the code, paste in a fresh copy of it,
and select the tickbox for Disable HTML in this post, which is
just below the posting window. Then it will post OK.
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

Re: Need help with efficiency
PostPosted: Fri May 18, 2007 12:10 pm     Reply with quote

Vefa wrote:
I wanted to get feedback about my code to see if there is any room to improve it
Code:
   bit0d = (input(bitP0));
   bit1d = (input(bitP1));
   bit2d = (input(bitP2));
   bit3d = (input(bitP3));
   bit4d = (input(bitP4));
   bit5d = (input(bitP5));
   bit6d = (input(bitP6));

   angle = (bit0d*1+bit1d*2+bit2d*4+bit3d*8+bit4d*16+bit5d*32+bit6d*64)*2;

   angleL = ((signed long) angle)/2;


This is the same as

angleL=(input_d() && 0b00111111);
angle = angleL*2;

But if you look at the list file you will see that what I typed generates far less assembly code than what you typed.

I think you would benifit a lot from using explicit variable types like.

Int1
Int8
Int16
Signed Int16

You have declared

signed long angleL=0
that is the same as
signed Int16 angleL=0

Also if you can use unsigned numbers that will make code that runs faster. I doubt you ever have any values that are actually negative.
jma_1



Joined: 08 Feb 2005
Posts: 147
Location: Wisconsin

View user's profile Send private message

PostPosted: Fri May 18, 2007 12:59 pm     Reply with quote

You can also use bit shifts for mulitiplication and divide. >>2 = divide by 2
<< 2 == multiply by 2

Cheers,
JMA
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

PostPosted: Fri May 18, 2007 3:15 pm     Reply with quote

jma_1 wrote:
You can also use bit shifts for mulitiplication and divide. >>2 = divide by 2
<< 2 == multiply by 2

Cheers,
JMA


Thats mostly correct.

angle = angleL*2;
is the same as
angle = angleL<<1;

When you code for multiply or divide by a power of 2 the compiler will often preform a bit shift operation in the assembly code that is generated. It's a lot easier to read the multiply statement than it is to read the shift statement. If you have a choice, it's better to write the statement as a multiply operation and let the compiler substitute the shift for you. This will keep you from thinking <<2 is the same as *2. I know that I have made that mistake before a few times but not anymore.
jma_1



Joined: 08 Feb 2005
Posts: 147
Location: Wisconsin

View user's profile Send private message

PostPosted: Fri May 18, 2007 3:21 pm     Reply with quote

Thanks Neutone.

I was typing in a hurry and wasn't thinking.

Cheers,
JMA_1
ckielstra



Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

View user's profile Send private message

PostPosted: Fri May 18, 2007 7:20 pm     Reply with quote

Code:
#INT_TIMER0
void encoder(void)
{

   set_timer0(0);
Why are you resetting the timer to zero? The program will only enter the interrupt on timer overflow, so the timer will already be reset to zero. Another problem is that you never know exaclty how long it has taken to enter this interrupt routine, it is possible another interrupt was already active introducing some delay. Ressetting the timer here will cause calculation errors and jitter.

Try adding #use fast_io. Doing so you only have to set the TRIS registers once at program startup, instead of having the compiler set these registers at each I/O access, this will save memory and is faster.

As a general design note: your program will be much easier to maintain when you try to use as few global variables as possible, i.e. declare the variables on top of the functions where they are used instead of at the global level.
Now I have to scan your whole program to see where each variable is used in order to check that there is not some special nitty-gritty backdoor function modifying that same variable in a way I didn't anticipate. A waste of my time and error prone.
Another benefit of making the variables local is that the compiler will be able to re-use the memory locations, you will save memory.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group