Help to improve and make it stable this code

nanogear

Member
Hello,

And thank you in advance to all. This code works fine but in some cases fail. An expert who know how to improve some details like add some pauses or something to make it stable that want to help me?.

I am learning a lot but I know that there are people who knows the pros and cons or tricks that the picaxe have.

One of the issues is that sometimes when the pic is running outside of the loop and then the condition b0 < 9 is fulfilled, the code don't jump to sub1...

Thank you guys!!!



Code:
Code:
#picaxe 08m2
symbol lightlevel = b0 

inmain:
       pause 500                                   ; make a label called inmain
       do                                          ; start loop just at start up and while b0 > 9  
          high B.2                                 ; switch on 2
          readadc C.1, b0                          ; read ADC1 into variable b0
          debug b0 
            if b0 < 9 then                         
            exit                                   ; exit the loop when b0 < 9
            end if
       loop                                        ; loop
 
main:                                              ; make a label called main
    readadc C.1, lightlevel                        ; read ADC1 into variable b0 named lightlevel
    debug b0                                       ; transmit value to computer screen
    pause 500                                      ; short delay
    
    do
       select case lightlevel
         case < 9 : if time > 5 and time < 8 then
                       gosub Sub1 
                    end if
         case > 48 : if time > 5 and time < 8 then
                       gosub sub1 
                     end if
         else      : time = 0 
       end select

    high B.2                                       ; else switch on 2
    goto main                                      ; jump back to start
   loop

sub1:                                              ; make a label
    low B.2                                        ; switch off 2
    pause 2000                                     ; wait 2 seconds to simulate switch
    high B.2                                       ; switch on 2
    pause 500
    goto main                                      ; jump back to start
 

SAborn

Senior Member
This code works fine but in some cases fail
That made me laugh:D

Reminds me of a friend telling me about a dead deer on the road side, and how it looked to be in "very good condition", well i would hate to have seen it in a bad condition, as the poor thing was dead.
 

nanogear

Member
That made me laugh:D

Reminds me of a friend telling me about a dead deer on the road side, and how it looked to be in "very good condition", well i would hate to have seen it in a bad condition, as the poor thing was dead.
hehehehe :D
 

inglewoodpete

Senior Member
I haven't tried to work out what the code does but it has some structural problems.

1. The Do: Loop under Main: does not work: there is a Goto Main immediately before the Loop statement. (It's my personal choice to (almost) never use the Goto statement. A program like this does not need them.)

2. The subroutine must terminate with a Return statement. With its current structure, the return stack will corrupt after a few calls.
 

hippy

Ex-Staff (retired)
One of the issues is that sometimes when the pic is running outside of the loop and then the condition b0 < 9 is fulfilled, the code don't jump to sub1...
If b0 is < 9 and it's not jumping to sub1 the only reason can be that time is <= 5 or time >= 8, so what do your DEBUG commands reveal ?

If it's not a program style error, my guess is that lightLevel is not stable and occasionally rises to >= 9 resetting time to zero before 5 seconds have elapsed.
 

nanogear

Member
If b0 is < 9 and it's not jumping to sub1 the only reason can be that time is <= 5 or time >= 8, so what do your DEBUG commands reveal ?

If it's not a program style error, my guess is that lightLevel is not stable and occasionally rises to >= 9 resetting time to zero before 5 seconds have elapsed.
hippy thank you!!!

I am going to post a little video to show you the debug, the circuit and a osciloscope in real time to show you that sometimes it doesn't jump to sub1. :)
 

nanogear

Member
I haven't tried to work out what the code does but it has some structural problems.

1. The Do: Loop under Main: does not work: there is a Goto Main immediately before the Loop statement. (It's my personal choice to (almost) never use the Goto statement. A program like this does not need them.)

2. The subroutine must terminate with a Return statement. With its current structure, the return stack will corrupt after a few calls.
Thank you inglewoodpete,

I am sorry, I did not understand the first part :S ... but in other hand, then I need to use a return statement instead of a "goto main", right?

like this:

Code:
sub1:                                              ; make a label
    low B.2                                        ; switch off 2
    pause 2000                                     ; wait 2 seconds to simulate switch
    high B.2                                       ; switch on 2
    pause 500
    return                                        ; jump back to start
thanks!!
 

nanogear

Member
If b0 is < 9 and it's not jumping to sub1 the only reason can be that time is <= 5 or time >= 8, so what do your DEBUG commands reveal ?

If it's not a program style error, my guess is that lightLevel is not stable and occasionally rises to >= 9 resetting time to zero before 5 seconds have elapsed.

Here is the video but this is strange... I tried to show you when the code crash or when not doing what I want without success. So, the video show the debug, the circuit and the oscilloscope doing all good.



 

hippy

Ex-Staff (retired)
C'mon guys!! Some help
It looks to be doing what is expected; the pulse happens five seconds or so after light is applied or light is removed, so perhaps you could explain what it's not doing you expected ?

I think I've twigged it.

The program assumes that in transitioning between light and dark or vice-versa the reading will always pass through the values 9..48 which resets the time variable, when that's not guaranteed.

The program behaves as specified, but the specification is wrong. The main problem is in asking 'how do I make the program do this' when the real question should have been 'what should a program look like to achieve this functionality'.

You need to note when it's light or dark and then reset the time when it's no longer light or dark. Reading values of 9..48 is not a guaranteed way of doing that.
 
Last edited:

nanogear

Member
The program assumes that in transitioning between light and dark or vice-versa the reading will always pass through the values 9..48 which resets the time variable, when that's not guaranteed.

The program behaves as specified, but the specification is wrong. The main problem is in asking 'how do I make the program do this' when the real question should have been 'what should a program look like to achieve this functionality'.

You need to note when it's light or dark and then reset the time when it's no longer light or dark. Reading values of 9..48 is not a guaranteed way of doing that.
Hippy,

The values of 9 and 48 are values physically measured outside during all day. For the purposes of the project, it is necessary that when <9 or >48 the code execute the subrutine. <9 means that is dark enough to activate lights(for example), and >48 means that is bright enough to deactivate lights.

The first loop means that when the pic is powered and instantly the adc reads values greater than 9 then stay in the loop and do nothing until the adc reads values under 9.

Technically my code does all I want to do but the help I am asking for is that someone check if I need to use a better syntax or, for example, change the gotosub to return or something to benefit the stability of the code execution and the picaxe during all day.

Thank you very much to all
 

hippy

Ex-Staff (retired)
Technically my code does all I want to do but the help I am asking for is ... something to benefit the stability of the code execution and the picaxe during all day.
I think what you really need to do is clarify what the instability issue is, describe exactly how it materialises itself and under what conditions.

Basically; how is the device you are building meant to operate and in what circumstances, and how does it not do what it is meant to. If the instability cannot be defined it's going to be hard to cure it.

I suspect you want it to do something when light levels have reliably gone above one level and then do something ( or something else ) when light levels have reliably fallen below another level. If that's all you need to achieve your code can be greatly simplified and the instability will probably go way.

Perhaps something like this would work ...

Code:
#Picaxe 08M2

Symbol lightlevel = b0 

High B.2

Do
  ReadAdc C.1, lightlevel
Loop Until lightLevel < 9

Do

  Do
    ReadAdc C.1, lightlevel
    If lightLevel < 48 Then
      time = 0
    End If
  Loop Until time > 5
  Gosub Sub1

  Do
    ReadAdc C.1, lightlevel
    If lightLevel > 9 Then
      time = 0
    End If
  Loop Until time > 5
  Gosub Sub1

Loop

Sub1:
  Low B.2
  Pause 2000
  High B.2
  time = 0
  Return
 

nanogear

Member
I think what you really need to do is clarify what the instability issue is, describe exactly how it materialises itself and under what conditions.

Basically; how is the device you are building meant to operate and in what circumstances, and how does it not do what it is meant to. If the instability cannot be defined it's going to be hard to cure it.

I suspect you want it to do something when light levels have reliably gone above one level and then do something ( or something else ) when light levels have reliably fallen below another level. If that's all you need to achieve your code can be greatly simplified and the instability will probably go way.

Perhaps something like this would work ...
Hippy thank you a lot,

You gave me an idea; The instability go away just by reseting the time just after the exit of the first loop, the elimination of all the pauses except the ones of sub1, and I guess by simplifying the way the loops are executed...

code:
Code:
high B.2                                           ; switch on 2
Do
   ReadAdc C.1, lightlevel                         ; read ADC1 into variable b0 named lightlevel
   Loop Until lightLevel < 9                       ; loop
   time = 0
Well this results were given using the simulator. Now let me check with the real hardware...

Thank you!!!
 

nanogear

Member
Hello,

I think I missed something in my logic to achieve this.

The code works very well now. If lightlevel(b0) go down to < 9 for the 5 seconds stipulated then jumps to sub1. As same with b0 > 48. Perfectly until there.

But, for example, the code executed the subrutine because lightlevel are below 9 for x time. if lightlevel are below 9 and then instantly go up to 10 or more and then go down to < 9 for more than 5 seconds again, a jump to subrutine will be executed. That is a mistake in my logic.

I need that the code when the subrutine be executed, because lightlevel read levels below 9 for the period of time, not execute subrutine again if lightlevel go up to more than 9 to 48 and then go down below 9.

In simple words, lightlevel can be oscillating from 0 to 48 once the first execution of subrutine(lightlevel < 9 and time > 5 and time < 8) without executing subrutine again. The new execution of subrutine will be made when lightlevel go > 48 for more than 5 seconds as stipulated and equally lightlevel can be oscillating from 255 to 9 without executing subrutine again.

I think is something simply to achieve but using a select-case is not working. How can I make this??

Thank you in advance for your help
 

hippy

Ex-Staff (retired)
What you need is some sort of flag which is set and cleared when dark or light so you don't enter Sub1 when dark if you haven't been light again since last dark etc.

Or you write your program more as a State Machine than a continuous loop, which is what the code in post #12 does.
 

nanogear

Member
What you need is some sort of flag which is set and cleared when dark or light so you don't enter Sub1 when dark if you haven't been light again since last dark etc.

Or you write your program more as a State Machine than a continuous loop, which is what the code in post #12 does.
hippy I'm sorry,

Your code do exactly what I want in a very easy way and very stable. I had not realized it. I just simply change the second loop to the first place and the first loop to the second one and it works beautiful!!!

Thank you so much!!!
 
Top