Yet Another Combat Program (Damage Timer) Question

[Wrenow]

OK, I now have some 20M's to play with, and it looks like I can bypass the need for 74HC595's etc.

The setup:
Inputs: 5 targets that can be shot down, triggering switches.
1 firing switch
1 reset switch
Outputs: 5 individual target indicator LEDs
1 disabled, not ready to fire LED
1 output to gas solenoid for firing circuit.
Function:
When you fire the shore battery, a timing cycle starts, disabling the firing switch until time has run. If undamaged (no targets down), the delay is about 8 seconds. About 10 seconds delay is added for each target knocked down until all 5 are down, at which time the shore battery is fully disabled until reset.

The code looks pretty oogly so far (I kept fiddling until I got something without syntax errors), and I am certainly open to any suggestions to optimize or make it better/more flexible.

The preliminary code is:

; 20M
; Shot Timer with 5 damage delays
; Inputs 0-4 (pins 10-6) are damage level triggers, 1=target hit
; Input 5 = (pin 5) fire button
; Input 6 (pin 4) = reset
; Outputs 7-3 (pins 11-15) are damage indicators
; Output 2 = (pin 16) fire output
; Output 1 = (pin 17) Ready to fire status (Green - on, armed, ready to fire)
; Output 0 = (pin 18) Ready to fire status (Red - flashing = waiting, not ready to fire, steady = disabled)

Main:


Firing:
    high 1                ; armed led on
    if pin5 = 0 then goto main    ; cycle until a fire command is read (add debounce?)
    high 2                ; trigger solenoid
    low 1                    ; armed led off
    pause 500                ; Pause for solenoid on to enable firing - adjust as necessary
    low 2                    ; turn solenoid off

Damage:

    b2 = 0        ; set damage to 0
    if pin0 = 1 then gosub Damagecount    ; to add up damage factor 
    if pin0 = 1 then high 7 endif

    if pin1 = 1 then gosub Damagecount
    if pin1 = 1 then high 6 endif

    if pin2 = 1 then gosub Damagecount
    if pin2 = 1 then high 5 endif 

    if pin3 = 1 then gosub Damagecount
    if pin3 = 1 then high 4 endif

    if pin4 = 1 then gosub Damagecount
    if pin4 = 1 then high 3 endif

    if b2 = 5 then goto Disabled


Firedelay:

    b3=0
    for b3 = 1 to 8
    high 0
    pause 500        ; Base 8 second pause
    low 0
    pause 500
    next b3

damagedelay:

    if b2 = 0 then goto Firing
    b3 = 0
    for b3 = 1 to 10    ;Base 10 second damage pause with flashing damage pause
        high 0        
        pause 500        
        low 0
        pause 500
    next b3
    b2=b2-1

Damagecount:
    
    b2=b2+1    ;Increment Damage Counter
    return

Disabled:

    High 0                ; Locks Disabled indicator on solid
    if pin3 = 1 then goto Main    ; Reset enabled (debounce?)
    pause 1000
goto disabled                 ;loop until reset hit

Any thoughts or gotchas noted?

Cheers,

Wreno

 

[hippy]

In terms of understandability I'd argue this is much easier to get to grips with than your current "IF pinX=" code ...

  If TARGET_1 Is DAMAGED Then : High TARGET_1_LED : Gosub DamageCount : End If
  If TARGET_2 Is DAMAGED Then : High TARGET_2_LED : Gosub DamageCount : End If
  If TARGET_3 Is DAMAGED Then : High TARGET_3_LED : Gosub DamageCount : End If
  If TARGET_4 Is DAMAGED Then : High TARGET_4_LED : Gosub DamageCount : End If

You're already having to document what each pin is in the code anyway so it's just as easy to turn those into SYMBOL definitions.

The big advantage is that people can get a good gist of what the code does without having to fully understand it ... "If Pin1 = 1 Then High 6" ... What's pin 1 what's pin 6 ? It's an advantage which will also serve you well when you come back to the code in a few weeks time

When people can understand most of the code by scanning it they don't have to worry about how it works but can concentrate on what it does and are far more likely to see flaws and improvements in logic and algorithm. When they find a "huh?" they can then look at the implementation, although a source comment should explain anything which isn't immediately understandable.

Other useful tricks are using DO-LOOP which can make things muc clearer; aim for GOTO-less programming whenever possible. The Disabled: code at the end would perhaps be more easily understood with ...

Disabled:
  High DISABLED_LED
  Do While RESET_BUTTON Is NOT_PUSHED
    Pause 1000
  Loop
  Goto Main

One rule I try to follow is that if you can write code which others can generally follow without understanding it you'll have little problem understanding it, debugging it or altering it yourself. If you find yourself asking "what am I trying to do with this piece of code", "what did I use 'b2' for" then others will be having an even harder job of it.

This is where Programming switches from being pure Engineering or Science and into an Art. It's about aesthetics; if the program looks good, feels impressive, then it probably is. The difference between craftsman and builder.

Don't, and I'm sure you won't, take it as personal criticism of your efforts because half the battle is getting something to work which is far better than the 'perfect program' which doesn't work !

Getting it perfect comes with time and experience and it's easy to say, "it works, that's it, job done", but working towards getting it right from the start can actually pay dividends and make program development far easier in the long term, especially when it comes to complicated code and algorithms. Getting it right doesn't always happen ( I know, I'm by no means perfect ) but striving to be is a good foot to start off on and will help rescue code problems rather than see them spin out of control.

And a final rule; rules are there to be broken - I'm usually against one-line IF statements but, as in the hit target detection, if it makes more sense and is more understandable use them. In this case it makes it clear that four events cause very similar things to happen.

On the actual program code front - There seems to be a possibility that execution can fall into FireDelay, into DamageDelay, into DamageCount from where it does a RETURN ... except there's been no GOSUB !

 

[Wrenow]

Thanks, Hippy,

Good comments all. Now for some editing (and rereading/rethinking to look into your code comment)....
And then, putting it on the breadboard, of course, to see if it really, really, works.

Again, thanks for the tips.

Cheers,

Wreno

 

[Wrenow]

Hi Hippy,

Thanks again for your help. I think I fixed the error you found. I still have some gotos, but have narrowed them down a bit. And, yeah, for one of the quick timing loops, I haven't changed the symbol name - yet. But, it is up and running in the simulator and seems to be doing pretty much as expected.

If you don;t mind glancing at the new iteration, my question is: Does anything jump out where I have obviously made it substantially worse anywhere? Or is it looking a bit closer to something new eyes can grasp and maintain?

One issue that the simulator didn't like was having a structure of:

Gosub SUB1
Gosub SUB2

SUB1:
do something
return

SUB2:
do something else
If a condition > 0 gosub SUB3
Return

SUB3:
Yet something else to do
Return

Anyway, the latest iteration is:

; 20M
; Shot Timer with 5 damage delays
; Inputs 0-4 (physical legs 10-6) are damage level triggers, 1=target hit
; Input 5 = (physical leg 5) fire button
; Input 6 (physical leg 4) = reset
; Outputs 7-3 (physical legs 11-15) are damage indicators
; Output 2 = (physical leg 16) fire output
; Output 1 = (physical leg 17) Ready to fire status (Green - on, armed, ready to fire)
; Output 0 = (physical leg 18) Ready to fire status (Red - flashing = waiting, not ready to fire, steady = disabled)

; Inputs
    Symbol TARGET_1 = Pin0
    Symbol TARGET_2 = Pin1
    Symbol TARGET_3 = Pin2
    Symbol TARGET_4 = Pin3
    Symbol TARGET_5 = Pin4

    Symbol FIRING_TRIGGER = Pin5
    Symbol RESETSWITCH = Pin6

; Outputs
    Symbol TARGET_1_LED = 7
    Symbol TARGET_2_LED = 6
    Symbol TARGET_3_LED = 5
    Symbol TARGET_4_LED = 4
    Symbol TARGET_5_LED = 3

    Symbol FIRING_SOLENOID = 2
    Symbol READY_TO_FIRE_LED = 1
    Symbol WAITING_DISABLED_LED = 0

; Conditions, factors, ETC.
    Symbol DAMAGED = 1
    Symbol NOT_PUSHED = 0
    Symbol DAMAGE_FACTOR = b2

Main:

Do While FIRING_TRIGGER is 0                            ; cycle until a fire command is read (add debounce?)
    high READY_TO_FIRE_LED                            ; armed led on
    pause 100
Loop    
                    
    gosub Firing
    gosub Damage_Calculation
    gosub ROF_delay
    gosub Damage_delay
    goto main

Damage_Calculation:

    DAMAGE_FACTOR = 0        ; reset damage to 0
    if TARGET_1 is DAMAGED Then : high TARGET_1_LED : gosub Damagecount :endif    ; to add up damage factor 
    if TARGET_2 is DAMAGED Then : high TARGET_2_LED : gosub Damagecount :endif
    if TARGET_3 is DAMAGED Then : high TARGET_3_LED : gosub Damagecount :endif
    if TARGET_4 is DAMAGED Then : high TARGET_4_LED : gosub Damagecount :endif 
    if TARGET_5 is DAMAGED Then : high TARGET_5_LED : gosub Damagecount :endif 

    if DAMAGE_FACTOR = 5 then goto Disabled             ; Break out to disabled

    
    Return

ROF_delay:

        b3 = 0            ; Base 8 second pause
            for b3 = 1 to 8
                high WAITING_DISABLED_LED
                pause 500        
                low WAITING_DISABLED_LED
                pause 500
            next b3
    Return


Damage_delay:

    Do while DAMAGE_FACTOR > 0                      ;if no damage dlay is completed, arm the system
    
        b3 = 0
            for b3 = 1 to 10                         ;Base 10 second damage / target pause with flashing damage pause
                high WAITING_DISABLED_LED        
                pause 500        
                low WAITING_DISABLED_LED
                pause 500
            next b3

        DAMAGE_FACTOR = DAMAGE_FACTOR - 1              ;Decrements Damage counter as damage delay is passed
    Loop
Return

Damagecount:
    
    DAMAGE_FACTOR = DAMAGE_FACTOR + 1                     ;Increment Damage Counter
    return

Disabled:

    High 0                                     ; Locks Disabled indicator on solid
    do while RESETSWITCH = NOT_PUSHED                     ; Reset enabled (debounce?)
        pause 1000
    Loop                                          ;loop until reset hit
    goto Main

Firing:
    high FIRING_SOLENOID                            ; trigger solenoid
    low READY_TO_FIRE_LED                            ; armed led off
    pause 500                                    ; Pause for solenoid on to enable firing - adjust as necessary
    low FIRING_SOLENOID                            ; turn solenoid off
Return

Thanks again for all your suggestions,

Wreno