Can't work out what's wrong with my 'if' statement

I have a variable called pcsBiK which is a 1 or a 0
I want to create a new variable called pcsB1Ks which is either an asterisk or a dash depending on whether pcsB1K is a 1 or a 0
My first step (just the * if it’s a 1) was to add an if statement like this

            elif ScoreType == "B1K":                                        # Batsman 1 On Strike?
                pcsB1K = ScoreData
                if pcsB1K == "1":
                    pcsB1Ks = "*"

But it doesn’t work
It’s bound to be something simple, help appreciated please.

(Here’s the full code if needed)

import serial, time
import sys
import dbus, dbus.mainloop.glib
import json

from gi.repository import GLib
from example_advertisement import Advertisement
from example_advertisement import register_ad_cb, register_ad_error_cb
from example_gatt_server import Service, Characteristic
from example_gatt_server import register_app_cb, register_app_error_cb

BLUEZ_SERVICE_NAME =           'org.bluez'
DBUS_OM_IFACE =                'org.freedesktop.DBus.ObjectManager'
LE_ADVERTISING_MANAGER_IFACE = 'org.bluez.LEAdvertisingManager1'
GATT_MANAGER_IFACE =           'org.bluez.GattManager1'
GATT_CHRC_IFACE =              'org.bluez.GattCharacteristic1'
UART_SERVICE_UUID =            '5a0d6a15-b664-4304-8530-3a0ec53e5bc1'
UART_RX_CHARACTERISTIC_UUID =  'df531f62-fc0b-40ce-81b2-32a6262ea440'
LOCAL_NAME =                   'FoSCC-Scoreboard-TGT'

mainloop = None

# Set all the Scoreboard variables to starting positions

Wickets = "0"
Overs = "-0"
BatTotal = "--0"
BatAscore = "--0"
BatBscore = "--0"
Target = "---"
pcsCOV = "-"
pcsOVB = "-"
pcsOVR = "-"
pcsRRQ = "-"
pcsRRR = "-"
pcsBTN = "-"
pcsFTN = "-"
pcsFTS = "-"
pcsBAN = "-"
#pcsB1S = "0"
pcsB1B = "-"
pcsB1K = "-"
pcsBBN = "-"
#pcsB2S = "0"
pcsB2B = "-"
pcsB2K = "-"
pcsF1N = "-"
pcsF1S = "-"
pcsF2N = "-"
pcsF2S = "-"
pcsLWK = "-"
pcsDLT = "-"
pcsDLP = "-"
PSHIP = "-"
PshipTOT = "-"

def update_scoreboard():
    Scoreboard = "4," + BatTotal + "," + Wickets + "," + Overs + "," + Target + "#"
    print(Scoreboard)
    with serial.Serial("/dev/ttyACM0", 57600, timeout=1) as arduino:
        # time.sleep(0.1) #wait for serial to open
        if arduino.isOpen():
            print("{} connected!".format(arduino.port))
            arduino.write(Scoreboard.encode())
            print("Data Sent: " + Scoreboard)
            arduino.close
    score_data = {
        "total": BatTotal,
        "wickets": Wickets,
        "overs": Overs,
        "OversBowled": pcsOVB,
        "OversRem": pcsOVR,
        "target": Target,
        "BatTeamName": pcsBTN,
        "RunsReq": pcsRRQ,
        "ReqRunRate": pcsRRR,
        "DLSTarget": pcsDLT,
        "DLSPar": pcsDLP,
        "Bat1Name": pcsBAN,
        "BatAscore": BatAscore,
#        "Bat1Score": pcsB1S,
        "BatABallsFaced": pcsB1B,
        "Bat1onStrike": pcsB1K,
        "Bat2Name": pcsBBN,
        "BatBscore": BatBscore,
#        "Bat2Score": pcsB2S,
        "BatBBallsFaced": pcsB2B,
        "Bat2onStrike": pcsB2K,
        "LastWicket": pcsLWK,
        "PshipTOT": PshipTOT,
        "PshipLWK": pcsLWK,
        "FieldTeamName": pcsFTN,
        "FieldTeamScore": pcsFTS,
        "CurrBowlName": pcsF1N,
        "CurrBowlFigs": pcsF1S,
        "CurrentOver": pcsCOV,
        "PrevBowlName": pcsF2N,
        "PrevBowlFigs": pcsF2S,
    }
    
    
    
    print("score_data =", score_data)
    ordered_score_data = dict(sorted(score_data.items()))
    with open('/var/www/html/scorePCS.json', 'w') as jsonf:
        json.dump(ordered_score_data, jsonf)


Scoreboard = "4," + BatTotal + "," + Wickets + "," + Overs + "," + Target + "#"

# Open the serial port and set the Scoreboard to starting positions

#with serial.Serial("/dev/ttyACM0", 57600, timeout=1) as arduino:
    # time.sleep(0.1) #wait for serial to open
#    if arduino.isOpen():
#            print("{} connected!".format(arduino.port))
#            arduino.write(Scoreboard.encode())
#            print("Data Sent: " + Scoreboard)
#            arduino.close
update_scoreboard()

class RxCharacteristic(Characteristic):
    def __init__(self, bus, index, service):
        Characteristic.__init__(self, bus, index, UART_RX_CHARACTERISTIC_UUID,
                                ['write'], service)

    def WriteValue(self, value, options):
        global Wickets
        global Overs
        global BatTotal
        global BatAscore
        global BatBscore
        global Target
        global Scoreboard
        # New code for enhanced Spectator board START
        global pcsCOV #Current Over
        global pcsOVB #Need this for spectator board in addition to 'overs' as overs is truncated but we want the scoreboard to display the decimal point
        global pcsOVR #Overs Remaining
        global pcsRRQ #Runs Required
        global pcsRRR #Run Rate Required
        global pcsBTN #Batting Team Name
        global pcsBTS #not needed as already in code
        global pcsFTN #Fielding Team Name
        global pcsFTS #Fielding Team Score (1 is added to this to generate 'target' as target is not a PCS variable
        global pcsBAN #Batsman 1 name
        #global pcsB1S #Batsman 1 score not needed as already in code
        global pcsB1B #Batsman 1 Balls Faced
        global pcsB1K #Batsman 1 on strike? (Yes - 1, No - 0)
        global pcsBBN #Batsman 2 name
        #global pcsB2S #Batsman 2 score not needed as already in code
        global pcsB2B #Batsman 2 Balls Faced
        global pcsB2K #Batsman 2 on strike? (Yes - 1, No - 0)
        global PSHIP 
        global PshipTOT
        global PshipLWK
        global pcsF1N #Current Bowler Name
        global pcsF1S #Current Bowler Figures
        global pcsF2N #Previous Bowler Name
        global pcsF2S #Previous Bowler Figures
        global pcsLWK #Last Wicket
        global pcsDLT #Duckworth Lewis target
        global pcsDLP #Duckworth Lewis Par at end of current over
        # New code for enhanced Spectator board END
        try:
            WriteReturn = '{}'.format(bytearray(value).decode())
            ScoreType = WriteReturn[0:3]
            ScoreData = WriteReturn[3:]
	    #if the bluetooth data is of type OVB, B1S, B2S, BTS, FTS then we need to update the scoreboard.  Other messages can be ignore.
            UpdateScoreboard = True
            if ScoreType == "OVB":                                          # Let's deal with Overs Bowled!
                pcsOVB = ScoreData
                if ScoreData.find(".") == -1:                               # If there's no dot because it's the end of the over, just format the number and use it
                    Overs = ScoreData.rjust(2,"-")
                else:
                    Overs = ScoreData.split(".",1)[0].rjust(2,"-")          # If there's a dot, take whatever's before it.  Cheap way of rounding down without using a float
            elif ScoreType == "B1S":                                        # Batsman 1 score formatted with leading dashes for blanks
                BatAscore = ScoreData.rjust(3,"-")
            elif ScoreType == "B2S":                                        # Batsman 2 score formatted with leading dashes for blanks
                BatBscore = ScoreData.rjust(3,"-")
            elif ScoreType == "BTS":                                        # Batting Team Score
                TempSplit = ScoreData.split(" &",1)[0]                      # Current Inning Score only (split everything before the ampersand)
                PshipTOT = TempSplit.split("/",1)[0]
                BatTotal = TempSplit.split("/",1)[0].rjust(3,"-")           # Everything before the dash is the score, with leading dashes for blanks
                Wickets = TempSplit.split("/",1)[1].replace("10","-")       # Everything after the dash is the wickets, and if it's all out (10) then make it blank because we only have 1 digit
            elif ScoreType == "FTS":
                TempSplit = ScoreData.split(" &",1)[0]                      # Current Inning Score only
                Target = TempSplit.split("/",1)[0]		            # Everything before the dash is the score, with leading dashes for blanks
                Target = str(int(Target) + 1).rjust(3,"-")
                
            # New code for enhanced Spectator board START
            elif ScoreType == "COV":                                        # Current Over Details
                pcsCOV = ScoreData
#            elif ScoreType == "OVB":                                        # Overs Bowled for spectator board as overs is truncated & we want the full number
#                pcsOVB = ScoreData                # MOVED TO LINE 186
            elif ScoreType == "OVR":                                        # Overs Remaining for spectator board as overs is truncated & we want the full number
                pcsOVR = ScoreData
            elif ScoreType == "RRQ":                                        # Runs Required for spectator board
                pcsRRQ = ScoreData
            elif ScoreType == "RRR":                                        # Run Rate Required for spectator board
                pcsRRR = ScoreData   
            elif ScoreType == "BTN":                                        # Batting Team Name
                pcsBTN = ScoreData
                pcsBTN = pcsBTN[:20]
            elif ScoreType == "FTN":                                        # Fielding Team Name
                pcsFTN = ScoreData
                pcsFTN = pcsFTN[:20]
            elif ScoreType == "FTS":                                        # Fielding Team Score (1 is added to this to generate 'target' as target is not a PCS variable
                pcsFTS = ScoreData
            elif ScoreType == "B1N":                                        # Batsman 1 Name
                pcsBAN = ScoreData
                pcsBAN = pcsBAN[:20]
            elif ScoreType == "B1S":                                        # Batsman 1 score formatted with leading dashes for blanks
                Bat1Score = ScoreData.rjust(3,"-")
            elif ScoreType == "B1B":                                        # Batsman 1 Balls Faced
                pcsB1B = ScoreData
            elif ScoreType == "B1K":                                        # Batsman 1 On Strike?
                pcsB1K = ScoreData
                if pcsB1K == "1":
                    pcsB1Ks = "*"
            elif ScoreType == "B2N":                                        # Batsman 2 Name
                pcsBBN = ScoreData
                pcsBBN = pcsBBN[:20]
            elif ScoreType == "B2S":                                        # Batsman 1 score formatted with leading dashes for blanks
                Bat2Score = ScoreData.rjust(3,"-")
            elif ScoreType == "B2B":                                        # Batsman 2 Balls Faced
                pcsB2B = ScoreData
            elif ScoreType == "B2K":                                        # Batsman 2 On Strike?
                pcsB2K = ScoreData
            elif ScoreType == "F1N":                                        # Current Bowler Name
                pcsF1N = ScoreData
                pcsF1N = pcsF1N[:20]
            elif ScoreType == "F1S":                                        # Current Bowler  Figures
                pcsF1S = ScoreData
            elif ScoreType == "F2N":                                        # Current Bowler Name
                pcsF2N = ScoreData
                pcsF2N = pcsF2N[:20]
            elif ScoreType == "F2S":                                        # Previous Bowler Figures
                pcsF2S = ScoreData
            elif ScoreType == "LWK":                                        # Last Wicket
                pcsLWK = ScoreData
                PshipLWK = ScoreData
            elif ScoreType == "DLT":                                        # Duckworth Lewis Target
                pcsDLT = ScoreData
            elif ScoreType == "DLP":                                        # Duckworth Lewis Par Score at end of current over
                pcsDLP = ScoreData                
            # New code for enhanced Spectator board END  
            else:
                print(WriteReturn + " (not used)")
	        #lets not send an update to the scoreboard
            UpdateScoreboard = False

            if UpdateScoreboard:
                Scoreboard = "4," + BatTotal + "," + Wickets + "," + Overs + "," + Target + "#"
                print(Scoreboard)
                #with serial.Serial("/dev/ttyACM0", 57600, timeout=1) as arduino:
                #    # time.sleep(0.1) #wait for serial to open
                #    if arduino.isOpen():
                #        print("{} connected!".format(arduino.port))
                #        arduino.write(Scoreboard.encode())
                #        print("Data Sent: " + Scoreboard)
                #        arduino.close
            update_scoreboard()
        except:
            print(sys.exc_info()[0], "occurred")
        
        
class UartService(Service):
    def __init__(self, bus, index):
        Service.__init__(self, bus, index, UART_SERVICE_UUID, True)
        self.add_characteristic(RxCharacteristic(bus, 1, self))

class Application(dbus.service.Object):
    def __init__(self, bus):
        self.path = '/'
        self.services = []
        dbus.service.Object.__init__(self, bus, self.path)

    def get_path(self):
        return dbus.ObjectPath(self.path)

    def add_service(self, service):
        self.services.append(service)

    @dbus.service.method(DBUS_OM_IFACE, out_signature='a{oa{sa{sv}}}')
    def GetManagedObjects(self):
        response = {}
        for service in self.services:
            response[service.get_path()] = service.get_properties()
            chrcs = service.get_characteristics()
            for chrc in chrcs:
                response[chrc.get_path()] = chrc.get_properties()
        return response

class UartApplication(Application):
    def __init__(self, bus):
        Application.__init__(self, bus)
        self.add_service(UartService(bus, 0))

class UartAdvertisement(Advertisement):
    def __init__(self, bus, index):
        Advertisement.__init__(self, bus, index, 'peripheral')
        self.add_service_uuid(UART_SERVICE_UUID)
        self.add_local_name(LOCAL_NAME)
        self.include_tx_power = True

def find_adapter(bus):
    remote_om = dbus.Interface(bus.get_object(BLUEZ_SERVICE_NAME, '/'),
                               DBUS_OM_IFACE)
    objects = remote_om.GetManagedObjects()
    for o, props in objects.items():
        if LE_ADVERTISING_MANAGER_IFACE in props and GATT_MANAGER_IFACE in props:
            return o
        print('Skip adapter:', o)
    return None

def main():
    global mainloop
    dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
    bus = dbus.SystemBus()
    adapter = find_adapter(bus)
    if not adapter:
        print('BLE adapter not found')
        return

    service_manager = dbus.Interface(
                                bus.get_object(BLUEZ_SERVICE_NAME, adapter),
                                GATT_MANAGER_IFACE)
    ad_manager = dbus.Interface(bus.get_object(BLUEZ_SERVICE_NAME, adapter),
                                LE_ADVERTISING_MANAGER_IFACE)

    app = UartApplication(bus)
    adv = UartAdvertisement(bus, 0)

    mainloop = GLib.MainLoop()

    service_manager.RegisterApplication(app.get_path(), {},
                                        reply_handler=register_app_cb,
                                        error_handler=register_app_error_cb)
    ad_manager.RegisterAdvertisement(adv.get_path(), {},
                                     reply_handler=register_ad_cb,
                                     error_handler=register_ad_error_cb)
    try:
        mainloop.run()
    except KeyboardInterrupt:
        adv.Release()

if __name__ == '__main__':
    main()

My wild guess is the pcsB1K is not a string. Try add this line:

print("The type of pcsB1K is", type(pcsB1K))

before the if and tell us what it prints.

Besides, your code is veeery hard to read and reason. My advice would be:

  • try to split the code into some files
  • use meaningful names for variables that describe their function in code
  • try to avoid global variables as they introduce uncertainty

Thank you for the response.
FYI I didn’t write the code, I just added to it.
I have added the line you said but could you please let me know where I’m looking for the result?

In terminal? E.g. the command-line where you execute your program?

I don’t run it in that sense, it sits in the background & takes data from the input from an app (on a phone connected over bluetooth) & saves that data in to a text (json) file.
I ran the code manually in terminal but didn’t see anything different from when the line wasn’t present

ScoreData is a string, so pcsB1K is, too:

What is value in the above?

I think I’ve sorted it with your original reply pointing me in the right direction.
I hadn’t defined it as a variable.
Thank you for your help.

Unfortunately that doesn’t save you from the consequences of its poor state :upside_down_face: Refactoring it and adding tests is likely to do a lot to improve maintainability and make it easier to add behaviour to in future (not to mention you might catch bugs)

1 Like

I wouldn’t disagree with you but I wouldn’t know where to start.

I would start with an end to end test. You can patch out anything completely untestable with unittest.mock.patch, but do that as little as you can. That’ll give you a safely you can leverage to start fixing things.

Next, prioritise getting rid of those globals. Worst case, wrap the whole thing in a function and change global to nonlocal. The key is to make it so you can run > 1 test without them interfering!

For background, Tony’s not a programmer. He’s inherited this code from
someone and is adapting it. He’s new to all this: Python, programming,
all the terminology.

There are several things I’d change about this programme if I had a free
hand, but there’s no point to advice which Tony can’t use with his
present skills. At present, that means incremental change, usually to
solve an immediate problem.

In particular, keep in mind that the only person who can run and test
the code is Tony, with hardware available only to him. Some massive code
refactor would doubtless introduce some shiny new bug which would then
be impossible to debug.

3 Likes

That sounds like a great way to stay stuck exactly where you are and sinking further into the mire. Good luck!

This looks like a good job for classic IIDPIO debugging: If In Doubt, Print It Out! Add some print() calls in various places, showing you what’s in various variables.