Code Improvement

[gadelhas]

Hi everyone, once again!

Hi made this code for controlling a DC motor with a PIC12F683.
The PIC reads the ADC value, and then controls the duty of the PWM output.

Everything is working good, however, i like to know your opinion about the code.

Is it possible to make any improvement, or other change to put the code better?

Here is the code

'************************************************* ***************
'* Name : PWM.BAS *
'* Author : Gadelhas *
'* Notice : Copyright (c) 2010 *
'* : All Rights Reserved *
'* Date : 28-11-2010 *
'* Version : 1.0 *
'* Notes : *
'* : PIC12F683 + MICROCODE + EASYPICV6 *
'************************************************* ***************
' INCLUDE's e FUSES
' ================================================== ==================
@ __config _INTRC_OSC_NOCLKOUT & _WDT_OFF & _MCLRE_OFF & _CP_OFF & _BOD_OFF

' ================================================== ==================
Define OSC 8
' VARIABLES
' ================================================== =========
Duty var WORD: DUTY=0
AdcValue var word: AdcValue=0
I var word: I=0
Sample var word: sample=0

' REGISTERS & PINOUT 1 = IN; 0 = OUT
' ================================================== ==================
'76543210
TRISIO = %00000001

'76543210
GPIO = %00000000

ANSEL = 1
VRCON = 0
CMCON0 = 7
ADCON0 = %10000000

' PINOUT NAMES
' ================================================== =========
POT1 VAR GPIO.0

' DEFINITIONS
' ================================================== =========
DEFINE ADC_BITS 10 ' Set number of bits in result
DEFINE ADC_CLOCK 3 ' Set clock source (rc = 3)
DEFINE ADC_SAMPLEUS 50 ' Set sampling time in microseconds

' MAIN PROGRAM
' ================================================== =========
Main:
gosub ADC
DUTY=(AdcValue*64)/256
HPWM 1, DUTY, 5000
GOTO MAIN

' SUB-ROTINAS
' ================================================== =========
ADC:
for I = 1 to 15
ADCON0.1 = 1
Not_Done:
if ADCON0.1 = 1 then not_done
adcin 0,AdcValue
sample = AdcValue + sample
next I
AdcValue = sample/15
sample=0
return
END


Thanks

[mackrackit]

Short, sweet, and works...
Looks good!!!

[Charles Linquis]

Muliplying by 64 and dividing by 256 is equivalent to dividing by 4.
Dividing by powers of 2 is easily done with shifts. To divide by 4, right shift two places (>> 2)

And.. the HPWM command in PBP is limiting. If you write directly to CCPRxL, and CCPxCON4:5 you can get PWM frequencies that are easy to filter with an L-C network + Schottky diode (above 100Khz is easily obtainable). That way, your motor sees nearly pure DC. A lot of brushless motors really prefer not to have a "chopped" power input.

[aratti]

You can remove all the division if:

loop 16 times

for I = 0 to 15

Since Charles has suggested to simplify deviding by 4 , then 16 x 4 = 64, so if you devide by 64 you will do only one division.

AdcValue = sample/64

or

AdcValue = sample >> 6


Hence, you don't need to change variable:

HPWM 1, AdcValue, 5000

Naturaly writing to CCPRxL, and CCPxCON4:5, as suggested in the previuos post is an additional improvement.



Cheers

Alberto

[gadelhas]

Hi Everyone, and many thanks for the answers to my post!!!

Hi follow your opinions and i change the code to this;



'************************************************* ***************
'* Name : PWM.BAS *
'* Author : Gadelhas *
'* Notice : Copyright (c) 2010 *
'* : All Rights Reserved *
'* Date : 28-11-2010 *
'* Version : 1.0 *
'* Notes : *
'* : PIC12F683 + MICROCODE + EASYPICV6 *
'************************************************* ***************
' INCLUDE's e FUSES
' ================================================== ==================
@ __config _INTRC_OSC_NOCLKOUT & _WDT_OFF & _MCLRE_OFF & _CP_OFF & _BOD_OFF

' ================================================== ==================
Define OSC 8
' VARIÁVEIS
' ================================================== ==================
AdcValue var word: AdcValue=0
I var word: I=0
Sample var word: sample=0

' REGISTOS E PINOUT 1 = IN; 0 = OUT
' ================================================== ==================
'76543210
TRISIO = %00000001

'76543210
GPIO = %00000000

ANSEL = 1 'GP0 AANALOG
VRCON = 0 'VREF OFF
CMCON0 = 7 'COMPARATOR OFF
ADCON0 = %10000000 'RIGHT JUSTIFY - 128

PR2 = %00011000 'SET 24 - 5000Hz - 8Bits
T2CON = %00000110 'TIMER2 ON - PRESCALER 1:16 - Max Duty Value 100 - 1% Step
CCPR1L = %00000000
CCP1CON = %00001100 'PWM ON

' NOMES PINOUT
' ================================================== ==================
POT1 VAR GPIO.0

' DEFINIÇÕES
' ================================================== ==================
DEFINE ADC_BITS 10 ' Set number of bits in result
DEFINE ADC_CLOCK 3 ' Set clock source (rc = 3)
DEFINE ADC_SAMPLEUS 50 ' Set sampling time in microseconds

' INICIO PROGRAMA
' ================================================== ==================
Main:
gosub ADC
CCPR1L.7=AdcValue.9
CCPR1L.6=AdcValue.8
CCPR1L.5=AdcValue.7
CCPR1L.4=AdcValue.6
CCPR1L.3=AdcValue.5
CCPR1L.2=AdcValue.4
CCPR1L.1=AdcValue.3
CCPR1L.0=AdcValue.2
CCP1CON.5=AdcValue.1
CCP1CON.4=AdcValue.0
GOTO MAIN

' SUB-ROTINAS
' ================================================== ==================
ADC:
for I = 1 to 15
ADCON0.1 = 1
Not_Done:
if ADCON0.1 = 1 then not_done
adcin 0,AdcValue
sample = AdcValue + sample
next I
AdcValue = (sample/163)
sample=0
return
END


The code works great. Initially it had 288 words, after change went to 213 words.
Is it possible to improve this piece of code?;



CCPR1L.7=AdcValue.9
CCPR1L.6=AdcValue.8
CCPR1L.5=AdcValue.7
CCPR1L.4=AdcValue.6
CCPR1L.3=AdcValue.5
CCPR1L.2=AdcValue.4
CCPR1L.1=AdcValue.3
CCPR1L.0=AdcValue.2
CCP1CON.5=AdcValue.1
CCP1CON.4=AdcValue.0


Is this correct?

>>1 is the same as devide by 2
>>2 is the same as devide by 4
>>3 is the same as devide by 8
>>4 is the same as devide by 16
>>5 is the same as devide by 32
>>6 is the same as devide by 64
>>7 is the same as devide by 128
>>8 is the same as devide by 256

and

<<1 is the same as multiply by 2
<<2 is the same as multiply by 4
<<3 is the same as multiply by 8
<<4 is the same as multiply by 16
<<5 is the same as multiply by 32
<<6 is the same as multiply by 64
<<7 is the same as multiply by 128
<<8 is the same as multiply by 256

Thanks once again to everyone!!

[Charles Linquis]

One thing you could do is -

CCPR1L = AdcValue >> 2

CCP1CON.5=AdcValue.1
CCP1CON.4=AdcValue.0