Fixing the endlessly loading devices and printers tray

Question

I work for a service desk in a Windows 7 environment. Recently we have had an issue where (do to an update) when users are attempting to map printers to themselves they are not able to open the devices and printers tray, it just loads endlessly. I've figured out a solution for this via the following:

• Open up services.msc as an administrator
• Find the bluetooth service (it's usually disabled)
• Set the service to manual
• Stop the print spoolers
• Delete any stuck print jobs (people tend to want to keep trying to print when it's not working)
• Restart the spoolers
• Start the bluetooth service

I said to myself, "Self, this is a whole crap ton of work", so I decided to create a program to do it for me..

How this program works:

• First things first, it checks if the user is running an elevated command prompt and fails if they are not
• Secondly it stops the spoolers and checks for stuck print jobs, if they're there, it deletes all of them, and then restarts the spoolers
• Thirdly, it opens the registry key bthserv checks if the value of the Start key is 4 (disabled) if it is, it will edit the value to a 3 (manual). If the value is already three, it will skip this step, because that could mean that it was just stuck jobs in the spoolers. If the value is anything else, I had it exit because there's probably a reason why that value is the way it is.

Unfortunately I have not figured out a way to restart the bluetooth service, you have to restart the bluetooth service manually, because for some reason when you edit the registry SCM doesn't notice so net start bthserv doesn't recognize the change, but when you go into the services.msc tray it will recognize the change. (Kinda weird to be honest.. I think it's a bug)

I also added an options for you to use your own value, in case you know something that I don't.

What I would like to do is get some critique on my coding, how's my styling? Is there anything I could run differently or do differently? Etc.. The reason it points to that exe is that it runs off a virtual environment using virtualenv that way I don't have to install python on everyone's system.

#! bin/Scripts/python.exe
import os
import ctypes
import subprocess
import platform
import optparse
import _winreg as wreg

# values of the startups and what they mean
START_UP_SETTINGS = {
    0: "Boot - (Loaded by kernel loader at start up time)",
    1: "System - (Loaded by I/O subsystem after start up)",
    2: "Automatic - (Loaded by Service Control Manager at login)",
    3: "Manual - (The service does not start until the user starts it manually)",
    4: "Disabled - (Specifies that the service should not be started, usually the cause of the problem)"
}

# version number <major-update>.<minor-update>.<patch-count>.<commit-count>
VERSION = "1.0.0"

# usage to be displayed
USAGE = r"bin\Scripts\python.exe fdap.py [Optional Arguments]"

# default value to change the registry to
DEFAULT_VALUE = 3


def check_version_type(version=VERSION):
    """ check the version type of the program
    :type version: version number of the program """
    amount = 0
    vtypes = {
        2: "stable",
        3: "dev",
    }
    for c in version:
        if c == ".":
            amount += 1
    return vtypes[amount]


def check_if_admin():
    """ check if you're running as administrator """
    is_admin = ctypes.windll.shell32.IsUserAnAdmin()
    if is_admin == 1:
        return True
    else:
        raise EnvironmentError("Must be administrator to run this application. Run a CMD as admin and try again.")


def verify_os(arc_tup=platform.architecture()):
    """ make sure you're on Windows
    :type arc_tup: tuple that will contain the systems architecture (32 or 64 bit)"""
    if "Windows" in arc_tup[1]:
        return True


def restart_spool():
    """ Restart the spoolers and delete the stuck print jobs """
    stop_spool = ["net", "stop", "spooler"]
    start_spool = ["net", "start", "spooler"]
    print_job_path = r"C:\Windows\System32\spool\PRINTERS"

    stuck = len(os.listdir(print_job_path))
    subprocess.call(stop_spool)
    if stuck != 0:
        print("\nFound {} stuck print jobs, deleting..\n".format(stuck))
        for item in os.listdir(print_job_path):
            os.remove(print_job_path + "\\" + item)
    subprocess.call(start_spool)


def edit_key(value, reg_key="Start", bluetooth_reg_key=r"SYSTEM\CurrentControlSet\services\bthserv", default=DEFAULT_VALUE):
    """ reset the bluetooth key from the registry and restart the services
    :type value: integer that defines what value to give the key
    :type reg_key: the key to edit in the registry
    :type bluetooth_reg_key: path to the bluetooth registry key
    :type default: default value of the key """
    try:
        root_reg = wreg.ConnectRegistry(None, wreg.HKEY_LOCAL_MACHINE)  # Connect to the registry
        root_key = wreg.OpenKey(root_reg, bluetooth_reg_key, 0, wreg.KEY_ALL_ACCESS)  # Open all the keys for write access
        reg_val, _ = wreg.QueryValueEx(root_key, "Start")  # Find the value of the Start key

        if reg_val == 4:
            info_msg = "\nBluetooth 'Start' key is currently assigned a value of {} meaning {}. "
            info_msg += "It is most likely that this is the cause of the Devices and Printers issue. "
            if value is None:
                info_msg += "By default if you do not specify a value, the default value is {} {}...\n"
            else:
                info_msg += "You have specified to user {}. {} for the value of the registry key...\n"

            print(info_msg.format(reg_val, START_UP_SETTINGS[reg_val], default, START_UP_SETTINGS[default]))

            if value is not None:
                print("\nUsing {}. {} as specified..\n".format(value, START_UP_SETTINGS[value]))
                wreg.SetValueEx(root_key, reg_key, 0, wreg.REG_DWORD, value)
            else:
                print("\nDefaulting to {}. {}\n".format(default, START_UP_SETTINGS[default]))
                wreg.SetValueEx(root_key, reg_key, 0, wreg.REG_DWORD, default)
        elif reg_val == 3:
            print("\nBluetooth 'Start' key is already on {}. {} no need to edit the 'Start' key, skipping...\n".format(
                default, START_UP_SETTINGS[default]))
        else:
            print(
                "\nBluetooth 'Start' key is assigned a value of {}. {}\nPlease edit this value manually. Exiting..\n"
            ).format(reg_val, START_UP_SETTINGS[reg_val])

    except (WindowsError, Exception) as e:
        return "Failed with: {}".format(e)


def main(value):
    """ main function of the program
    :type value: integer that will be used to set the value of the key
    """
    if verify_os(platform.architecture()) is True:
        print("\nVerifying if user is an administrator..\n")
        check_if_admin()
        print("\nChecking the registry..\n")
        edit_key(value)
        print("\nRestarting spoolers..\n")
        restart_spool()


if __name__ == '__main__':
    parser = optparse.OptionParser(usage=USAGE)
    parser.add_option("--version", dest="versionNum", help="Show the version number and exit",
                      action="store_true")
    parser.add_option("-v", "--value", dest="keyValue",
                      help="Specify a key value to use instead of default value {}. {}".format(
                          DEFAULT_VALUE, START_UP_SETTINGS[DEFAULT_VALUE]),
                      type="int")
    opts, _ = parser.parse_args()

    if opts.versionNum is True:
        print("fdap v{}({})\nCreated by: Thomas Perkins\n04/13/2017".format(VERSION, check_version_type()))
        exit(0)
    else:
        if opts.keyValue:
            if opts.keyValue < 0:
                raise WindowsError("Value cannot be under 0.")
        main(opts.keyValue)

Answer 1

First things first: Nice write up.

Second thing, in general, this code looks pretty good. Most (all?) of the suggestions below are just python idioms.

Now down to business. I have recast your code, find it at the end of this post. I will discuss various changes individually...

Pep8:

So most of the changes below were for formatting. Python has a strong idea of how the code should be styled, and it is expressed in pep8.

I suggest you get a style/lint checker. I use the pycharm ide which will show you style and compile issues right in the editor.

If you are going to throw an exception, you don't need a return

So I recast this:

def check_if_admin():
    is_admin = ctypes.windll.shell32.IsUserAnAdmin()
    if is_admin == 1:
        return True
    else:
        raise EnvironmentError("Must be administrator to run this application. Run a CMD as admin and try again.")

To:

def check_if_admin():
    if 1 != ctypes.windll.shell32.IsUserAnAdmin():
        raise EnvironmentError(
            "Must be administrator to run this application. "
            "Run a CMD as admin and try again.")

Two changes:

1. The intermediate variable is_admin seemed redundant since the entire point of the function is check_if_admin.
2. Promoted the exception from else to if, and removed the else since the if throws an exception.

Consume bool evaluations directly:

If you need a bool and are performing a test to determine the bool state, you rarely need an if. I redid this:

def verify_os(arc_tup=platform.architecture()):
    if "Windows" in arc_tup[1]:
        return True

as this:

def verify_os(arc_tup=platform.architecture()):
    return "Windows" in arc_tup[1]

So there is no reason for the if, the result of the test can be returned (or assigned to a variable) directly.

I find end of line comments less useful:

So I changed:

root_reg = wreg.ConnectRegistry(None, wreg.HKEY_LOCAL_MACHINE)  # Connect to the registry

To:

# Connect to the registry
root_reg = wreg.ConnectRegistry(None, wreg.HKEY_LOCAL_MACHINE)

This makes it easier to stay under a pep8 80 columns, and introduces the line of code it is commenting. There are some uses for end of line comments, but I personally find that if the comment can be subjugated to the far reaches of the screen, there is a good chance it can just be removed, or the code can be improved to not need it.

You don't need the is True

if verify_os(platform.architecture()) is True:

Should be:

if verify_os(platform.architecture()):

Add change announcement:

And as the bonus review item. I think what you need to get the registry change to be noticed by net start bthserv is:

# notify about registry change
win32gui.SendMessageTimeout(
    win32con.HWND_BROADCAST, win32con.WM_SETTINGCHANGE, 0,
    'bthserv', win32con.SMTO_ABORTIFHUNG, 1000)

(Source)

Recast Code:

#! bin/Scripts/python.exe
import os
import ctypes
import subprocess
import platform
import optparse
try:
    import _winreg as wreg
except ImportError:
    # this has been renamed in python 3
    import winreg as wreg
import win32con
import win32gui

# values of the startups and what they mean
START_UP_SETTINGS = {
    0: "Boot - (Loaded by kernel loader at start up time)",
    1: "System - (Loaded by I/O subsystem after start up)",
    2: "Automatic - (Loaded by Service Control Manager at login)",
    3: "Manual - (The service does not start until the user starts it manually)",
    4: "Disabled - (Specifies that the service should not be "
       "started, usually the cause of the problem)"
}

# version number <major-update>.<minor-update>.<patch-count>.<commit-count>
VERSION = "1.0.0"

# usage to be displayed
USAGE = r"bin\Scripts\python.exe fdap.py [Optional Arguments]"

# default value to change the registry to
DEFAULT_VALUE = 3


def check_version_type(version=VERSION):
    """ check the version type of the program
    :type version: version number of the program """
    amount = 0
    vtypes = {
        2: "stable",
        3: "dev",
    }
    for c in version:
        if c == ".":
            amount += 1
    return vtypes[amount]


def check_if_admin():
    """ check if you're running as administrator """
    if 1 != ctypes.windll.shell32.IsUserAnAdmin():
        raise EnvironmentError(
            "Must be administrator to run this application. "
            "Run a CMD as admin and try again.")


def verify_os(arc_tup=platform.architecture()):
    """ make sure you're on Windows
    :type arc_tup: tuple tcontaining the systems architecture (32 or 64 bit)"""
    return "Windows" in arc_tup[1]


def restart_spool():
    """ Restart the spoolers and delete the stuck print jobs """
    stop_spool = ["net", "stop", "spooler"]
    start_spool = ["net", "start", "spooler"]
    print_job_path = r"C:\Windows\System32\spool\PRINTERS"

    stuck = len(os.listdir(print_job_path))
    subprocess.call(stop_spool)
    if stuck != 0:
        print("\nFound {} stuck print jobs, deleting..\n".format(stuck))
        for item in os.listdir(print_job_path):
            os.remove(os.path.join(print_job_path, item))
    subprocess.call(start_spool)


def edit_key(value, reg_key="Start",
             bluetooth_reg_key=r"SYSTEM\CurrentControlSet\services\bthserv",
             default=DEFAULT_VALUE):
    """ reset the bluetooth key from the registry and restart the services
    :type value: integer that defines what value to give the key
    :type reg_key: the key to edit in the registry
    :type bluetooth_reg_key: path to the bluetooth registry key
    :type default: default value of the key """
    try:
        # Connect to the registry
        root_reg = wreg.ConnectRegistry(None, wreg.HKEY_LOCAL_MACHINE)

        # Open all the keys for write access
        root_key = wreg.OpenKey(
            root_reg, bluetooth_reg_key, 0, wreg.KEY_ALL_ACCESS)

        # Find the value of the Start key
        reg_val, _ = wreg.QueryValueEx(root_key, "Start")

        if reg_val == 4:
            info_msg = "\nBluetooth 'Start' key is currently assigned " \
                       "a value of {} meaning {}. It is most likely that " \
                       "this is the cause of the Devices and Printers issue. "
            if value is None:
                info_msg += "By default if you do not specify a value, " \
                            "the default value is {} {}...\n"
            else:
                info_msg += "You have specified to user {}. " \
                            "{} for the value of the registry key...\n"

            print(info_msg.format(reg_val, START_UP_SETTINGS[reg_val],
                                  default, START_UP_SETTINGS[default]))

            if value is not None:
                print("\nUsing {}. {} as specified..\n".format(
                    value, START_UP_SETTINGS[value]))
                wreg.SetValueEx(root_key, reg_key, 0, wreg.REG_DWORD, value)
            else:
                print("\nDefaulting to {}. {}\n".format(
                    default, START_UP_SETTINGS[default]))
                wreg.SetValueEx(root_key, reg_key, 0, wreg.REG_DWORD, default)

            # notify about registry change
            win32gui.SendMessageTimeout(
                win32con.HWND_BROADCAST, win32con.WM_SETTINGCHANGE, 0,
                'bthserv', win32con.SMTO_ABORTIFHUNG, 1000)

        elif reg_val == 3:
            print("\nBluetooth 'Start' key is already on {}. "
                  "{} no need to edit the 'Start' key, skipping...\n".format(
                default, START_UP_SETTINGS[default]))
        else:
            print("\nBluetooth 'Start' key is assigned a value of {}. {}\n"
                  "Please edit this value manually. Exiting..\n"
            ).format(reg_val, START_UP_SETTINGS[reg_val])

    except (WindowsError, Exception) as e:
        return "Failed with: {}".format(e)


def main(value):
    """ main function of the program
    :type value: integer that will be used to set the value of the key
    """
    if verify_os(platform.architecture()):
        print("\nVerifying if user is an administrator..\n")
        check_if_admin()
        print("\nChecking the registry..\n")
        edit_key(value)
        print("\nRestarting spoolers..\n")
        restart_spool()


if __name__ == '__main__':
    parser = optparse.OptionParser(usage=USAGE)
    parser.add_option("--version", dest="versionNum",
                      help="Show the version number and exit",
                      action="store_true")
    parser.add_option("-v", "--value", dest="keyValue",
                      help="Specify a key value to use instead of "
                           "default value {}. {}".format(
                          DEFAULT_VALUE, START_UP_SETTINGS[DEFAULT_VALUE]),
                      type="int")

    opts, _ = parser.parse_args()

    if opts.versionNum is True:
        print("fdap v{}({})\nCreated by: Thomas Perkins\n"
              "04/13/2017".format(VERSION, check_version_type()))
        exit(0)
    else:
        if opts.keyValue:
            if opts.keyValue < 0:
                raise WindowsError("Value cannot be under 0.")
        main(opts.keyValue)