need help with project

we have taken up the project automatic room light controller with visitor counter using at89s52..we have rigged up the circuit and have written the code too..but when it is dumped into the controller and executed the kit does not produce the expected result..we are getting random numbers like 17,99,etc and there is no decrementation happening at all..could you please go thro' the below code and point out what might be the mistake..the code is as follows:

LIGHT    EQU    P2.5
DIS1      EQU    P2.7
DIS2      EQU    P2.6
SEN1     EQU   P1.0
SEN2     EQU   P1.1
ORG      00H
;************************************************************************************
;MAIN

MOV  R4,#00H
MOV  R5,#00H
MOV  P0,#0FFH   ;make P0 output
MOV  P1,#0FFH   ;make P1 input
MOV  P2,#0FFH   ;make P2 output
JNB    SEN1,UPPC
JNB    SEN2,DOWNC
AJMP DISPLAY

UPPC:
        JB  SEN2,DISPLAY
        INC   R4
        CJNE  R4,#01H,UPP1
        SETB   LIGHT
        SETB   DIS1
        SETB   DIS2
              CALL CONV
        AJMP   DISPLAY

UPP1:
        CALL   CONV
        AJMP DISPLAY


DOWNC:
        JB  SEN1,DISPLAY
              DEC  R4
        CJNE  R4,#00H,DOWN1
        CLR   LIGHT
        AJMP   DISPLAY

DOWN1:
        CALL    CONV
        AJMP    DISPLAY

;*******************************************************************************
;CONVERSION
CONV:

        MOV   A,R4
        MOV   B,#10d
        DIV      AB
        RR       A
        RR        A
        RR        A
        RR        A
        ADD     A,B ;to get both the bcd digits in one register
        MOV   R5,A
        RET
;**********************************************************************************
;DISPLAY
DISPLAY:

        MOV    P2, #80H
        MOV    A, R5
        ANL     A, #0F0H
        CALL   TRANS
        MOV   P0,A
        CALL    DELAY
        MOV    P2,#40H
        MOV    A,R5
        ANL      A,#0FH
        RR        A
        RR        A
        RR         A
        RR         A
        CALL     TRANS
        MOV    P0,A
        CALL     DELAY
        JNB  SEN1,UPPC
        JNB  SEN2,DOWNC
        AJMP  DISPLAY
;*********************************************************************************
;TO DECIDE THE NUMBER TO BE DISPLAYED
TRANS:
        CJNE    A,#00H,ONE
        MOV    A,#0C0H
        RET
ONE:
        CJNE    A,#10H,TWO
        MOV    A,#0F9H
        RET
TWO:
        CJNE   A,#20H,THREE
        MOV   A,#0A4H
        RET
THREE:
        CJNE     A,#30H,FOUR
        MOV    A,#0B0H
        RET
FOUR:
        CJNE  A,#40H,FIVE
        MOV  A,#99H
        RET
FIVE:
        CJNE   A,#50H,SIX
        MOV   A,#92H
        RET
SIX:
        CJNE   A,#60H,SEVEN
        MOV   A,#82H
        RET
SEVEN:
        CJNE  A,#70H,EIGHT
        MOV  A,#0F8H
        RET
EIGHT:
        CJNE   A,#80H,NINE
        MOV A,#80H
        RET
NINE:
        MOV   A,#98H
        RET
;************************************************************************************
;DELAY OF 1ms
DELAY:
        MOV   R6,#2
        MOV R7,#230
D1:
        DJNZ  R7,D1
        DJNZ  R6,D1
        RET
END

thanks

 what code

the below code

what you show is not code but scribbles

code is commented

Erik

 RE: what code

well this is the commented code..

LIGHT    EQU     P2.5  ;initialising variables
DIS1     EQU     P2.7
DIS2     EQU     P2.6
SEN1     EQU     P1.0
SEN2     EQU     P1.1
ORG      00H
;************************************************************************************
;MAIN

MOV  R4,#00H ;initialising count to zero
MOV  R5,#00H ;initialising counter to zero
MOV  P0,#0FFH   ;make P0 output
MOV  P1,#0FFH   ;make P1 input
MOV  P2,#0FFH   ;make P2 output
JNB    SEN1,UPPC ;sensor value is low when obstructed
JNB    SEN2,DOWNC
AJMP DISPLAY    ;loop

UPPC:
        JB  SEN2,DISPLAY ;check for sen2 obstruction
        INC   R4
        CJNE  R4,#01H,UPP1
        SETB   LIGHT ;light switched on
        SETB   DIS1  ;activate display1 transistor
        SETB   DIS2  ;activate display2 transistor
         CALL CONV    ;hex to bcd conversion
        AJMP   DISPLAY

UPP1:
        CALL   CONV
        AJMP DISPLAY


DOWNC:
        JB  SEN1,DISPLAY ;check for sen1 obstruction
         DEC  R4
        CJNE  R4,#00H,DOWN1
        CLR   LIGHT  ;switch off light when ppl leave
        AJMP   DISPLAY

DOWN1:
        CALL    CONV
        AJMP    DISPLAY

;*******************************************************************************
;CONVERSION
CONV:

        MOV   A,R4
        MOV   B,#10d
        DIV      AB ;to get the first bcd digit
        RR        A
        RR        A
        RR        A
        RR        A
        ADD     A,B ;get both digits in 1 register
         MOV     R5,A ;get the bcd number in r5
        RET
;******************************************************
;DISPLAY
DISPLAY:

        MOV    P2, #80H ;selecting display1
        MOV    A, R5    ;move count to accumulator
        ANL     A, #0F0H ;to get only first digit
        CALL   TRANS ;to display the digit
        MOV   P0,A
        CALL    DELAY
        MOV    P2,#40H ;select display2
        MOV    A,R5
        ANL      A,#0FH ;get the second digit of count
        RR         A
        RR         A
        RR         A
        RR         A
        CALL     TRANS ;to display digit
        MOV      P0,A
        CALL     DELAY
        JNB  SEN1,UPPC ;check for another entry
        JNB  SEN2,DOWNC ;check for exit
        AJMP  DISPLAY
;******************************************************
;TO DECIDE THE NUMBER TO BE DISPLAYED
TRANS:
        CJNE    A,#00H,ONE
        MOV    A,#0C0H
        RET
ONE:
        CJNE    A,#10H,TWO
        MOV    A,#0F9H
        RET
TWO:
        CJNE   A,#20H,THREE
        MOV   A,#0A4H
        RET
THREE:
        CJNE     A,#30H,FOUR
        MOV    A,#0B0H
        RET
FOUR:
        CJNE  A,#40H,FIVE
        MOV  A,#99H
        RET
FIVE:
        CJNE   A,#50H,SIX
        MOV   A,#92H
        RET
SIX:
        CJNE   A,#60H,SEVEN
        MOV   A,#82H
        RET
SEVEN:
        CJNE  A,#70H,EIGHT
        MOV  A,#0F8H
        RET
EIGHT:
        CJNE   A,#80H,NINE
        MOV A,#80H
        RET
NINE:
        MOV   A,#98H
        RET
;******************************************************
;DELAY OF 1ms
DELAY:
        MOV   R6,#2
        MOV R7,#230
D1:
        DJNZ  R7,D1
        DJNZ  R6,D1
        RET
END

will be glad if you can make sense of the comments and can help:)
thanks..

 RE: what code

Sorry, but I don't have the time to run through the code thoroughly.

This stood out though:

MOV  P0,#0FFH   ;make P0 output
MOV  P1,#0FFH   ;make P1 input
MOV  P2,#0FFH   ;make P2 output

You load all ports with the same 0FFh, but say P0 and P2 are output and P1 is an input. You may want to read the manual to get better understanding of '51 ports.

 RE: what code

oh thanks for pointing it out..i gotta look it up once..and if you could spare some time,even if not immediately, i would be happy to take any other suggestions too..thanks a lot for this reply though..will check that out:)

 About 8051 Ports

Unlike many other processors, the standard 8051 port does not have a configurable "direction" ("in" or "out")

See: http://www.8052.com/faqs/120176

 RE: About 8051 Ports

CJNE A,#00H,ONE
MOV A,#0C0H
RET
ONE:

totally misleading label

the CJNE jumps if not zero, there is no determination that the values is one.

also you have no facility for "switch debounce". YES, even a light sensor may need 'debounce', e.g. if low what about 'open legs', e.g. if higher what about swinging arms? also, I have no doubt that you may get two or more pulses if a fur is worn.
A quick look at yor code seens to show that if a person stand still in the lightpath the counter will count and count snd count ....

Erik

 RE: About 8051 Ports

@andy neil
ok then do you think we can write the code without specifying the ports as input or output??i mean we remove the following lines..

mov p1,#0ffh
mov p2,#0ffh
mov p3,#0ffh

@erik
well the concept of cjne a,#00h,one is to determine what number available at the counter..if it is not 0 then we check for one and so on upto 9..and the display in each segment does not exceed these 10 digits..so i guess that justifies my display part..coming to the second part of your reply..we have recognised these faults in the circuit at the beginning itself but right now we r jus planning a prototype for our academic curriculum..the points you stated will obviously be the disadvantages of our project:):)we only want to show that when an object obstructs the sensors the counters r incremented or decremented depending on the entry and exit.but right now the increment is happening in huge leaps like it goes from 71 to 82 or decrements from 61 to 54 etc...hope this makes it a bit clear as to what my problem is..

 RE: write the code without specifying the ports as input or output

If you read the linked FAQ, it answers that!

FAQ: http://www.8052.com/faqs/120176

"i mean we remove the following lines"

If you just remove them without understanding why, then you will almost certainly break something somewhere else in your code!

You need to understand the working of the 8051 ports;
See the above FAQ for a start;

Also: http://www.8052.com/faqs/120112

And: http://www.8052.com/tutorial

And probably also: http://www.keil.com/books/8051books.asp

 RE: write the code without specifying the ports as input or output

If you jump to the label "TWO" if you have deduced that the number is two, then the label is quote ok.

But if you jump to the label "TWO" to try to figure out if it is two or something higher, then it would be better with a label "TEST_IF_TWO" - right now, it looks more like a promise: "If I jump here, I promise that the number must have the value two."

 RE: About 8051 Ports

I stated :"A quick look at yor code seens to show that if a person stand still in the lightpath the counter will count and count snd count"
your problem right now the increment is happening in huge leaps like it goes from 71 to 82 or decrements from 61 to 54 etc...hope this makes it a bit clear as to what my problem is..
is the time "a person stand still in the lightpath"

but right now we r jus planning a prototype for our academic curriculum
so you "jus(t) r (resist) planning", that is VERY obvious. There is ZERO thought behind your code.

Erik