New to programming and am having a few issues with some code

[Lestat]

Hi

I'm having a few issues with some code I have written, it compiles with no errors but I am not getting the response that I expect.

All of the outputs are being simulated with LEDs and are wired in with 470ohm resistors. I also have 5k ohm potentiometors for the target and current position inputs.

When I first turn the ciruit on the direction LEDs come on but the power LED never turns on. However much I turn the current position potentiometer I can not seem to get the LEDs to turn on or off as expected.

If anybody can help me with any errors with my code or can point me in the right direction to solve the issues I am getting.

Many thanks in advance.
My current code is below:

DEFINE OSC 8

'GP2 (AN2) IS FOR TARGET POSITION (INPUT)
'GP4 DIRECTION SIGNAL CW (OUTPUT)
'GP5 DIRECTION SIGNAL CCW (OUTPUT)
'GP0 POWER (OUTPUT)
'GP1 (AN1) CURRENT POSITION (INPUT)

'REGISTERS
OPTION_REG = %11000000
INTCON = %11000000
PIE1 = %01000000
PIR1 = %00000000
OSCCON = %01110001
TRISIO = %00000111
ANSEL = %01010110
CCP1CON = %00000000

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts

;---------------------------------------------------------------------------
wsave VAR BYTE $20 SYSTEM ' location for W if in bank0
;wsave VAR BYTE $70 SYSTEM ' alternate save location for W
' if using $70, comment wsave1-3

' --- IF any of these three lines cause an error ?? ------------------------
' Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using --
wsave1 VAR BYTE $A0 SYSTEM ' location for W if in bank1
'wsave2 VAR BYTE $120 SYSTEM ' location for W if in bank2
'wsave3 VAR BYTE $1A0 SYSTEM ' location for W if in bank3
' --------------------------------------------------------------------------

'OUTPUT PINS
DIRCW VAR GPIO.4
DIRCCW VAR GPIO.5
PWMOUT VAR GPIO.0

'VARIABLES
TARGET VAR WORD bank0
TARGET_L VAR Target.BYTE0
TARGET_H VAR TARGET.byte1
CURRENT VAR WORD bank0
CURRENT_L VAR current.byte0
CURRENT_H VAR current.byte1

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler AD_INT, _CurrentPos, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM


'READ TARGET POSITION FROM POT
ADCON0 = %00001011 'TARGET POSITION READ
REPEAT
PAUSE 100
UNTIL ADCON0.1 = 0

asm
MOVF ADRESL,W
MOVWF _TARGET_L
MOVF ADRESH,W
MOVWF _TARGET_H
endasm

ADCON0 = %00000111 'CURRENT POSITION ADC SETUP

'MAIN PROG
MAIN:
IF CURRENT=TARGET THEN
goto motorbreak
else
if current > target then
goto motorcw
else
goto motorccw
endif
endif
GOTO MAIN

'SUB PROGRAM
MotorBreak:
GPIO = %00110001
return

MotorCW:
GPIO = %00010001
Return

MotorCCW:
GPIO = %00100001
Return

CurrentPos:
ASM
MOVF ADRESL,W ;COPY LOW BYTE OF ADC TO W
MOVWF _CURRENT_L ;COPY LOW BYTE OF ADC TO CURRENT POS FROM W
MOVF ADRESH,W ;COPY HIGH BYTE OF ADC TO W
MOVWF _CURRENT_H ;COPY HIGH BYTE OF ADC TO CURRENT POS FROM W
ENDASM
@ INT_RETURN ;RETURN

END


I am using a PIC12F683 microchip.

Many thanks

[mister_e]

those variant of

asm
MOVF ADRESL,W
MOVWF _TARGET_L
MOVF ADRESH,W
MOVWF _TARGET_H
endasm

shoud be replaced with
current.lowbyte = ADRESL
Current.HighByte = ADRESH

That's the first thing I see here on a lazy sunday while watching TV

[Demon]

Use GOSUB to go to subroutines.

Robert

[mister_e]

motion seconded... back to basics... and pillow

[Dave]

I don't see where you are re-reading the commanded position within the main loop. I only see where you are reading the feedback position in the interrupt routine... Maybe my eyes are going bad...

[Lestat]

Thank you for the feedback so far.

I am work at the moment so I can not impliment the suggestions until later today,I will give feedback on any results in due course.

Dave - at the moment it only should read the target position at the start of program and only update the target position if the PIC is reset, I intend to write in a routine later in the programs evolution that will allow the target position to be updated while the program is running.

Once again thank you for the help.

[Lestat]

I have made the suggested changes to the program. depending on the position of the potentiometers when the power is applied to the circuit, one of the direction LEDs lights up. If both of the Potentiometers have equal resistances, both of the direction LEDs light up. The power LED never seems to light. I have checked the LEDs to ensure they are working as expected. I have also checked that the resistance of the potentiometers functions as expected.

The reading of the ADC appears to work, though the current position does not seem to update. I believe that either the If statements do not work properly or the main loop is not cycling, or the program is stopping as it is turning the LEDs on.

Is there anything that I am missing?

I have copied the latest form of the code bellow:

DEFINE OSC 8

'GP2 (AN2) IS FOR TARGET POSITION (INPUT)
'GP4 DIRECTION SIGNAL CW (OUTPUT)
'GP5 DIRECTION SIGNAL CCW (OUTPUT)
'GP0 POWER (OUTPUT)
'GP1 (AN1) CURRENT POSITION (INPUT)

'REGISTERS
OPTION_REG = %11000000
INTCON = %11000000
PIE1 = %01000000
PIR1 = %00000000
OSCCON = %01110001
TRISIO = %00000111
ANSEL = %01010110
CCP1CON = %00000000

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts

;---------------------------------------------------------------------------
wsave VAR BYTE $20 SYSTEM ' location for W if in bank0
;wsave VAR BYTE $70 SYSTEM ' alternate save location for W
' if using $70, comment wsave1-3

' --- IF any of these three lines cause an error ?? ------------------------
' Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using --
wsave1 VAR BYTE $A0 SYSTEM ' location for W if in bank1
'wsave2 VAR BYTE $120 SYSTEM ' location for W if in bank2
'wsave3 VAR BYTE $1A0 SYSTEM ' location for W if in bank3
' --------------------------------------------------------------------------

'OUTPUT PINS
DIRCW VAR GPIO.4
DIRCCW VAR GPIO.5
PWMOUT VAR GPIO.0

'VARIABLES
TARGET VAR WORD bank0
TARGET_L VAR Target.BYTE0
TARGET_H VAR TARGET.byte1
CURRENT VAR WORD bank0
CURRENT_L VAR current.byte0
CURRENT_H VAR current.byte1

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler AD_INT, _CurrentPos, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM

'READ TARGET POSITION FROM POT
ADCON0 = %00001011 'TARGET POSITION READ
REPEAT
PAUSE 100
UNTIL ADCON0.1 = 0

TARGET.lowbyte = ADRESL
Target.HighByte = ADRESH

ADCON0 = %00000111 'CURRENT POSITION ADC SETUP

'MAIN PROG
MAIN:
IF CURRENT=TARGET THEN
gosub motorbreak
else
if current > target then
gosub motorcw
else
gosub motorccw
endif
endif
GOTO MAIN

'SUB PROGRAM
MotorBreak:
GPIO = %00110001
return

MotorCW:
GPIO = %00010001
Return

MotorCCW:
GPIO = %00100001
Return

CurrentPos:
current.lowbyte = ADRESL
Current.HighByte = ADRESH
@ INT_RETURN ;RETURN

END

Many thanks

[Dave]

How is the power Led wired?
I can only assume it is wired so that the Pic is sourcing the current to the led. Am I correct?

[Lestat]

Yes the LED is sourcing the current to the LED, which is exactly the same as the other LEDs. which do light up depending on the position of the potentiometers.

many thanks

[mister_e]

well... maybe not a bad idea to read the ADC again once in a while, as it is right now you read it only once ;)

[Demon]

About your POWER LED not coming on:

How about taking a copy of that last code, deleting all the lines except the absolute minimum to get the power LED on.

Just keep the config, variable, ports, etc. Get rid of everything else, it's cluttering your view.

I bet you have a config set up wrong, or your wiring is not exactly as you think it is.

Robert


Dumb question #1: Did you try to light the POWER LED and resistor directly to confirm it does indeed work?

#2: Maybe you blew the pin on the PIC, try another one.

#3: PWMOUT VAR GPIO.0 <-- Is this your POWER LED? You never turn it on, you only define it (or maybe I'm missing something).

#4: TRISIO = %00000111 <-- port 0 is set as input, it should be TRISIO = %00000110.

[Dave]

I also notice that you are not setting the CMCON0 register. If not then it is defaulting to GP0 and GP1 as analog inputs and GP2 as digital. If you are not using the comparator then you should be setting it to %00000111. This will allow port GP0 to be digital. It's all in the data sheet.....

[Lestat]

I tried the resistor and the LED separately, with your help and looking through the datasheet again it does seem that it is the configuration settings.

Demon

#3: PWMOUT VAR GPIO.0 <-- Is this your POWER LED? You never turn it on, you only define it (or maybe I'm missing something) - I was initally going to use these labels instead of the binary code for the pins, but I ended up using the binary numbers as I wanted to be able to turn more than one pin on and off at a time. Though the pin names are left in incase I wish to use them in the future.

Dave
I never realised that I needed to set the CMCON0 register as I decided to not look into the comparator section of the datasheet as this is not a function I am using at this time. I'll spend more time reading the rest of the datasheet now to see if there is any other bits I have missed.

I shall look at the registers later tonight to see if there is anything else I have missed.

Many thanks for your help.

[mackrackit]

http://www.picbasic.co.uk/forum/showthread.php?t=561

[Dave]

Actually Robert, He is comanding the power led to light every time he is sending the commands to the other led's with any of the GPIO statements like: GPIO = %00010001

[Demon]

Ah, thanks Dave, now I see it as plain as the nose on my face.

[Lestat]

Mackrackit - thanks for the link its very useful reading.



DEFINE OSC 8

'GP2 (AN2) IS FOR TARGET POSITION (INPUT)
'GP4 DIRECTION SIGNAL CW (OUTPUT)
'GP5 DIRECTION SIGNAL CCW (OUTPUT)
'GP0 POWER (OUTPUT)
'GP1 (AN1) CURRENT POSITION (INPUT)

'REGISTERS
OPTION_REG = %11000000
INTCON = %11000000
PIE1 = %01000000
PIR1 = %00000000
OSCCON = %01110001
TRISIO = %00001110
ANSEL = %01010110
CCP1CON = %00000000
CMCON0 = %00000111

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts

;---------------------------------------------------------------------------
wsave VAR BYTE $20 SYSTEM ' location for W if in bank0
;wsave VAR BYTE $70 SYSTEM ' alternate save location for W
' if using $70, comment wsave1-3

' --- IF any of these three lines cause an error ?? ------------------------
' Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using --
wsave1 VAR BYTE $A0 SYSTEM ' location for W if in bank1
'wsave2 VAR BYTE $120 SYSTEM ' location for W if in bank2
'wsave3 VAR BYTE $1A0 SYSTEM ' location for W if in bank3
' --------------------------------------------------------------------------

'OUTPUT PINS
DIRCW VAR GPIO.4
DIRCCW VAR GPIO.5
PWMOUT VAR GPIO.0

'VARIABLES
TARGET VAR WORD bank0
TARGET_L VAR Target.BYTE0
TARGET_H VAR TARGET.byte1
CURRENT VAR WORD bank0
CURRENT_L VAR current.byte0
CURRENT_H VAR current.byte1

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler AD_INT, _CurrentPos, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM

'READ TARGET POSITION FROM POT
ADCON0 = %00001011 'TARGET POSITION READ
REPEAT
PAUSE 100
UNTIL ADCON0.1 = 0

TARGET.lowbyte = ADRESL
Target.HighByte = ADRESH

ADCON0 = %00000111 'CURRENT POSITION ADC SETUP

'MAIN PROG
MAIN:
IF CURRENT=TARGET THEN
gosub motorbreak
else
if current > target then
gosub motorcw
else
gosub motorccw
endif
endif
GOTO MAIN

'SUB PROGRAM
MotorBreak:
GPIO = %00110001
return

MotorCW:
GPIO = %00010001
Return

MotorCCW:
GPIO = %00100001
Return

CurrentPos:
current.lowbyte = ADRESL
Current.HighByte = ADRESH
@ INT_RETURN ;RETURN

END

I appear to be getting close now.

The LEDs now light as expected. Which is good news. I temporary added a piece of code to turn the Power LED off everytime main section of code cycled through to prove that the PIC wasn't hanging.

The power LED was flashing as expected.

My only question is with the interupts, when the main program keeps running through the main loop it doesn't appear to update the current position when the current postion potentiometer is changed. With DT Interupts do I have to reset the ADCON0 GO/DONE bit or does the DT interupts automatically.

Thank you for your time.

[mister_e]

http://www.picbasic.co.uk/forum/showthread.php?t=16433&p=113311#post113311

[Lestat]

http://www.picbasic.co.uk/forum/showthread.php?t=16433&p=113311#post113311

My apologies, I thought that previous comment was about the initial target potentiometer and not the current (feedback) potentiometer

So I'll have to reset the go/done bit. Thank you very much

[Lestat]

Success.

Thank you to everyone for all your help.

I've posted the working code as an example for anyone who wants to use it as an example.


clear

DEFINE OSC 8

'GP2 (AN2) IS FOR TARGET POSITION (INPUT)
'GP4 DIRECTION SIGNAL CW (OUTPUT)
'GP5 DIRECTION SIGNAL CCW (OUTPUT)
'GP0 POWER (OUTPUT)
'GP1 (AN1) CURRENT POSITION (INPUT)

'REGISTERS
OPTION_REG = %11000000
INTCON = %11000000
PIE1 = %01000000
PIR1 = %00000000
OSCCON = %01110001
TRISIO = %00001110
ANSEL = %01010110
CCP1CON = %00000000
CMCON0 = %00000111

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts

;---------------------------------------------------------------------------
wsave VAR BYTE $20 SYSTEM ' location for W if in bank0
;wsave VAR BYTE $70 SYSTEM ' alternate save location for W
' if using $70, comment wsave1-3

' --- IF any of these three lines cause an error ?? ------------------------
' Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using --
wsave1 VAR BYTE $A0 SYSTEM ' location for W if in bank1
'wsave2 VAR BYTE $120 SYSTEM ' location for W if in bank2
'wsave3 VAR BYTE $1A0 SYSTEM ' location for W if in bank3
' --------------------------------------------------------------------------

'OUTPUT PINS
DIRCW VAR GPIO.4
DIRCCW VAR GPIO.5
PWMOUT VAR GPIO.0

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler AD_INT, _CurrentPos, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM

'Variables
Target var word
Current VAR WORD

ADCON0 = %00001011 'TARGET POSITION READ
pause 100

TARGET.lowbyte = ADRESL
Target.HighByte = ADRESH

'MAIN PROG
MAIN:
pause 100
GPIO = %00000000
pause 100
IF CURRENT = TARGET THEN
gosub motorbreak
else
if current > target then
gosub motorcw
else
gosub motorccw
endif
endif

ADCON0 = %00000111 'CURRENT POSITION ADC SETUP

GOTO MAIN

'SUB PROGRAM
MotorBreak:
GPIO = %00110001
return

MotorCW:
GPIO = %00010001
Return

MotorCCW:
GPIO = %00100001
Return

CurrentPos:
pause 10
GPIO.0 = 0
current.lowbyte = ADRESL
Current.HighByte = ADRESH
'ENDIF
@ INT_RETURN ;RETURN

END

Right onto its next stage of its evolution.