Mitt första "Riktiga" program till pic16f648

[Bosen]

Nu é jag glad!!!!
Jag har lyckats för första gången att få ett program som jag har konstruerat helt efter eget huvud att fungera som jag vill!
Nu vill jag ha kritik på koden!
Vad kunde jag gjort bättre???? Labels??? Kommentarer??

Här é koden:

Kod: Markera allt

;**********************************************************************
;   En kod för att testa att få bort kontakt-studs samt att skilja    *
;   på långa eller korta knapptryckningar.							  *
;	RB0 är kopplad till en knapp som vid nedtryckning jordar.		  *
;	RA0 och RA1 är kopplade till varsin lysdiod som lyser vid en 	  *
;	digital 0:a på motsvarande pinne.								  *
;	vid kort tryck på knappen så släcks eller tänds RA0 och 		  *
;	vid långt tryck (2sek) så tänds eller släcks RA1.				  *
;**********************************************************************
;                                                                     *
;    Filename:      project1.asm                                           *
;    Date:                                                            *
;    File Version:                                                    *
;                                                                     *
;    Author:	Andreas "Bösen" Torstensson                                                          *
;                                                                     *
;                                                                     *
;**********************************************************************
;                                                                     *
;    Files required: P16F648A.INC                                     *
;                                                                     *
;                                                                     *
;                                                                     *
;**********************************************************************

    list      p=16F648A           ; list directive to define processor
    #include <p16F648A.inc>       ; processor specific variable definitions

    errorlevel  -302              ; suppress message 302 from list file


    __CONFIG   _CP_OFF & _DATA_CP_OFF & _LVP_OFF & _BOREN_OFF & _MCLRE_ON & _WDT_OFF & _PWRTE_ON & _INTOSC_OSC_NOCLKOUT 


;***** VARIABLE DEFINITIONS (examples)

; example of using Shared Uninitialized Data Section
INT_VAR     UDATA_SHR     
w_temp      RES     1       ; variable used for context saving 
status_temp RES     1       ; variable used for context saving
pclath_temp RES     1       ; variable used for context saving
delay_var	RES		1		; variable used for delay counter
knapp_var	RES		1		; variabel för knappen
knapp1_var	RES		1		; variabel för tiden knappen är intryckt
;**********************************************************************
RESET_VECTOR    CODE    0x0000    ; processor reset vector
        goto    start             ; go to beginning of program


INT_VECTOR      CODE    0x0004    ; interrupt vector location

INTERRUPT

        movwf   w_temp            ; save off current W register contents
        movf    STATUS,w          ; move status register into W register
        movwf   status_temp       ; save off contents of STATUS register
        movf    PCLATH,w          ; move pclath register into W register
        movwf   pclath_temp       ; save off contents of PCLATH register


; isr code can go here or be located as a call subroutine elsewhere
; ______________________________________________________________________


 		banksel	INTCON
		bcf		INTCON, T0IF

 		btfss	knapp_var, 0
		goto	ej_flagga
		
		btfss	PORTB, 0
		goto	knapp_tryckt
		btfsc	knapp_var, 1
		goto	knapp_kort

knapp_kort
		clrf	knapp_var
		clrf	knapp1_var
;om man bara har tryckt ett kort tryck på knappen så hamnar man här

		btfsc	PORTA, 0
		goto 	knapp_kort1
		goto	knapp_kort2
knapp_kort1
		bcf		PORTA, 0
		goto	isr_slut
knapp_kort2
		bsf		PORTA, 0		
		goto	isr_slut



knapp_tryckt
		bsf		knapp_var, 1
		incfsz	knapp1_var
		goto	isr_slut
		clrf	knapp_var
		clrf	knapp1_var
; Om knappen har varit intryckt mer än 2sek så hamnar du här

		btfsc	PORTA, 1
		goto 	knapp_long1
		goto	knapp_long2
knapp_long1
		bcf		PORTA, 1
		goto	knapp_long3
knapp_long2
		bsf		PORTA, 1		
knapp_long3	
		btfss	PORTB, 0
		goto	knapp_long3	
		goto	isr_slut


ej_flagga
		btfss	PORTB, 0
		bsf		knapp_var, 0

isr_slut

; ______________________________________________________________________
        movf    pclath_temp,w     ; retrieve copy of PCLATH register
        movwf   PCLATH            ; restore pre-isr PCLATH register contents
        movf    status_temp,w     ; retrieve copy of STATUS register
        movwf   STATUS            ; restore pre-isr STATUS register contents
        swapf   w_temp,f
        swapf   w_temp,w          ; restore pre-isr W register contents
        retfie                    ; return from interrupt

MAIN_PROG       CODE

start
; remaining code goes here
; ______________________________________________________________________

		banksel	cmcon			
		movlw	0x07		;stäng av komparatorerna
		movwf	cmcon							
;       _____________
		
		banksel	TRISA
		clrf	TRISA			;sätt hela porta till utgångar
	
		banksel	TRISB	
		movlw	b'00000001'		;Sätt RB0 till ingång	
		movwf	TRISB
		
		banksel	OPTION_REG
		movlw	b'01010011'		;sätt weak pullups till portb
		movwf	OPTION_REG		;sätt på timer0, 1:32(?) prescaler
		bcf		INTCON, T0IF
		bsf		INTCON, GIE
		bsf		INTCON, PEIE
		bsf		INTCON, T0IE
		movlw	b'11111100'
		movwf	PORTA
		clrf	knapp_var
		clrf	knapp1_var
loop	
		goto	loop




; ______________________________________________________________________
; initialize eeprom locations

EE      CODE    0x2100
        DE  0x00, 0x01, 0x02, 0x03

        END                       ; directive 'end of program'

Jag har använt ett template som följde med MPLAB, därför är kommentarerna både på engelska och svenska.
Igentligen tycker jag att det hadde känts bättre/snyggare att sätta ISR:en efter MAIN-koden men eftersom templaten va uppbyggd på detta sättet så gjorde jag såhär... Hur brukar ni göra???

[bearing]

Jag har försökt förstå vad programmet gör, tycker det är krångligt (jag programmerar visserligen mest i C). Angående variabelnamn blir det mer lättläst med namn som beskriver funktion, exempelvis knapp_old/knapp_counter istället för knapp_var/knapp_var1. För att spara plats kan knapp_old vara en bit i en generell flaggvariabel - då tar den bara en bit istället för en byte.

Fungerar programmet så att korta tryck växlar en lysdiod? långa tryck växlar först samma lysdiod och sedan en annan?

Kod: Markera allt

knapp_long3	
		btfss	PORTB, 0
		goto	knapp_long3	

Det här gör att interuptet inte returnerar om knappen hålls inne. Då stannar huvudprogrammet medans knappen är intryckt.

[sodjan]

Visst är det härligt !

Men visst, eftersom du bad om det. Det är igentligen inget
som stör fukntionen, och mycket handlar om tycke och smak,
men i alla fall...

Notera att jag inte har studerat funktionen alls, utan bara hur det är skrivet.
Spontant så verkar bearing i alla fall ha rätt i sina kommenterer, och jag
håller med. Jag tycker att det saknas kommenterar för att det ska vara
lätt att sätta sig in i själva funktionen. Kommenterarna har även den nyttan
att de berättar vad du *tror* att koden gör, och det blir då enklare att se
logiska tankevurpor. Nu så har man bara koden och får anta att det är
så som du tänkte att det skulle vara, men det är ju inte säkert...

Labels. Jag har lite svårt för labels typ "knapp_long1", jag
skulle nog ha kallat den "switch_long1". Alltså p.g.a av språkblandningen.

Samma sak med variablerna, att blanda "delay_var" med "knapp_var"
känns lite onödigt, på något sätt.

Kommenterar har jag personligen mindre problem med, det kan man
skriva på det språk som passar en själv bäst. Och den "målgrupp"
man har, om det finns någon.

Om man ånda har en "Files required" i början, så kan man gärna
ta med LKR filen också. Många skulle missa det eftersom många
är vana vid absolut mode.

EE delen antar jag åkte med från kodmallen, men i ditt fall används den
väl inte. Kan plockas bort för att städa upp lite.

Var man lägger själva ISR koden är mest en utseende fråga.

En annan sak...

Detta är alltså en PIC modell som har 4K flash uppdelat i 2 "pages".
Notera att PCLATH har en viktig funktion i alla dessa PIC16 !

Jag ska inte ta alla detaljer här, men *om* du hade haft mycket mer
kod, och interruptet inträffar när processorn kör kod i andra "pagen", så
kommer din ISR inte att fungera. Dina GOTO kommer att hoppa helt fel !

Du kan lägga en PAGESEL i början av din ISR för att sätta PCLATH rätt.
Det finns alltså en anledning till att PCLATH sparas och återställs först
och sist i ISR'en !

Sen, när det gäller OPTION_REG och INTCON, så är ju båda register med
bitar med olika betydelse. Jag skulle nog ha satt dom med samma metod,
antingen med ett binärt värde som du gör med OPTION_REG, eller med
separata BCF/BSF (vilket jag föredrar) som du gör med INTCON. INTCON
fallet är lite tydligare, bit-namnen säger vad det handlar om. Och som du
kanske märker så behöver INTCON fallet knappt kommenteras alls...

[Bosen]

bearing:
om du läser dom alra första raderna (kommentarerna) i koden så står det vad programmet gör...
korta tryck växlar en lysdiod, långa tryck växlar en annan lysdiod!

du har helt rätt om det sista... det är bara för om man håller in knappen och släpper den så växlar lysdioden på RA0 också, och det vill jag ju inte.... så knappen måste släppas upp för att programmet ska gå vidare.... men det kan ju kanske finnas andra sätt att lös det.... inte vet jag...

[sodjan]

Du kan "minnas" att knappen är intryckt så att du vet det när
du kommer in till ISR'en nästa gång. Generellt är det inte en bra
lösning att ISR'en kan "fastna" p.g.a något som användaren gör.

[Icecap]

Min erfarenhet: Ska man ha olika funktioner på knappar och/eller ett flertal knappar gör jag som följer:

1: Startar en timer-interrupt, det behövs i nästa alla fall. Ofta tar jag 10Hz men ska vissa knappar "säkerställas" behövs det 2 avläsningar efter varandra och det ger 20Hz (eller snabbare om man behöver det). Följande görs då i timerns ISR.

2: Debounce tid behövs EJ, man ska bara se till att läsa knapp-porten EN gång per interrupt! Ska man fibbla med värden sparar man det lästa värde i en minneslokation (RAM) och jobbar med den.

3: Vill du ha kort tryck kontra långt tryck? Räkna upp en räknarelokation så länge knappen är intryckt, när knappen är släppt OCH tiden inte är noll kollar man kort/lång tid och utför den funktion. Man kan även göra det direkt en viss max. tid är uppnådd. Tidsräknaren nollställs sedan om knappen är släppt.

4: Vill man bara avkänna nya knapptryckningar? Använd "n-key rollover" då. Man har en variabel som håller "förra avläsningen" (kallas "Previous_Keys"), vi har den nuvarande avläsningen ("Now_Keys") och resultatet ("New_Keys").
New_Keys = (Previous_Keys ^ Now_Keys) & Now_Keys;
(^ betyder EXOR, & betyder AND)

På detta vis har man ingen vänttid på debounce, ISR'n "faller igenom" snabbt, inget stoppar och allt fungerar.

[Bosen]

Sodjan:
Du har rätt, givetvis kan jag sätta en flagga för att knappen är tryckt också.

Icecap:
Jag hänger faktiskt inte med i dina förklaringar....
1. det är väl ungefär vad jag har gjort?
2. Vad menar du med att debounce inte behövs... vad händer nästa gång ISR startar då? du menar att jag inte ska spara i en variabel som jag har gjort?
3. det är väl ungefär vad jag har gjort?
4. Hmm... detta får jag leta vidare lite information om... det verkar smart... men lite avancerat...

[Icecap]

1: På ett mycket ungefär ja.
2: Om man avläser knappstatus 10 gg per sekund och man i en ISR enbart läser 1 gg då räcker det fint som debounce! Eller är det vad "debounce" är som du inte är med på?
3: Nja... du setter en variabel om man trycker på knappen, du räknar aldrig upp den och du koller inte heller om den är över eller under ett visst värde. Om vi utgår ifrån att ISR'n anropas med 10Hz är en kort knapptryckning mellan 100 och 199ms i det fall och en lång är 200ms+, du kan inte välja att kort ska vara < 400ms t.ex. eller att lång ska vara >= 3000ms.
4: Inte alls avancerat... men patentregistrerat ... vilket förvisso inte stoppar en hobbyist från att använde det.

Min beskrivning var inte menad enkom som en kommentar till din kod men menad "generellt" för att visa hur man kan göra en avläsning av knappar helt utan att ha någon form av väntande i ISR'n.

[Bosen]

ok... tack för beskrivningen