debugpythonMinor
Fixing the endlessly loading devices and printers tray
Viewed 0 times
theendlesslydevicestrayfixingloadingprintersand
Problem
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:
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:
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
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
- Open up
services.mscas 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
bthservchecks if the value of theStartkey 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
Solution
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:
To:
Two changes:
Consume bool evaluations directly:
If you need a
as this:
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:
To:
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
Should be:
Add change announcement:
And as the bonus review item. I think what you need to get the registry change to be noticed by
(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 ...
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, dele
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:
- The intermediate variable
is_adminseemed redundant since the entire point of the function ischeck_if_admin.
- Promoted the exception from
elsetoif, and removed theelsesince theifthrows 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 Trueas 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 registryTo:
# 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 Trueif 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 ...
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, dele
Code Snippets
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.")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.")def verify_os(arc_tup=platform.architecture()):
if "Windows" in arc_tup[1]:
return Truedef verify_os(arc_tup=platform.architecture()):
return "Windows" in arc_tup[1]root_reg = wreg.ConnectRegistry(None, wreg.HKEY_LOCAL_MACHINE) # Connect to the registryContext
StackExchange Code Review Q#160766, answer score: 2
Revisions (0)
No revisions yet.