I recently got a new laptop and the battery wasn't the greatest in the world, so I decided on a little project to save some battery:
Make a screen dimmer for Linux distros that use Gnome Shell, changes brightness based on battery level and charging status, and runs from command line.
Requirements:
argparse
(Python module)- Linux distro (tested on Ubuntu 17.10)
- Gnome Shell (tested on gnome-shell 3.26.2)
Usage:
Normal usage:
python3 main.py -v
Help usage:
(
-v
is the only one needed for output to shell.)python3 main.py -h
Can anyone tell me how I did with this project? (Only tested with Gnome Shell 3.26.2, would be greatly appreciated if anyone can tell me if this can be ran on previous versions)
Are there things that I could possibly do to shorten the code, or make it more efficient?
Battery.py
import os
import subprocess
class Battery:
'''A basic wrapper around the files in
/sys/class/power_supply/BAT0 in Linux to get
battery percent and charging status on a laptop.'''
def __init__(self):
if not os.path.exists("/sys/class/power_supply/BAT0"):
raise Exception("No Battery Present")
def percent(self):
return int(self.get("capacity"))
def is_charging(self):
status = self.get("status")
return status != "Discharging"
def get(self, file):
f = os.path.join("/sys/class/power_supply/BAT0", file)
cmd = "cat {}".format(f)
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
result = proc.stdout.read().decode().split("\n")[0]
proc.stdout.close()
return result
BrightnessManager.py
import os
import subprocess
import re
class BrightnessManager:
'''Used to get and set system brightness on Linux based systems.'''
def __init__(self):
self.brightness_regex = re.compile(r'([0-9]*)')
def set_brightness(self, value):
'''Make a system call to gdbus to change the screen brightness.
The value passed in must be a number between 0 - 100.'''
if not 0 <= value <= 100:
raise Exception("Brightness value must be between 0 and 100")
cmd = 'gdbus call --session --dest org.gnome.SettingsDaemon.Power '\
'--object-path /org/gnome/SettingsDaemon/Power '\
'--method org.freedesktop.DBus.Properties.Set '\
'org.gnome.SettingsDaemon.Power.Screen Brightness "<int32 {}>" '\
'> /dev/null'.format(value)
os.system(cmd)
def get_brightness(self):
'''Make a system call to gdbus and get the system brightness.'''
cmd = "gdbus call --session --dest org.gnome.SettingsDaemon.Power "\
"--object-path /org/gnome/SettingsDaemon/Power "\
"--method org.freedesktop.DBus.Properties.Get "\
"org.gnome.SettingsDaemon.Power.Screen Brightness"
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
result = self.brightness_regex.findall(proc.stdout.read().decode())
return int(''.join(result))
Settings.py
import json
class Settings:
'''A simple wrapper around the json module to read a settings file.'''
def __init__(self, file):
self.file = file
self.contents = self.read()
def read(self):
with open(self.file, 'r') as f:
contents = json.load(f)
return contents
main.py
import os
import time
from Settings import Settings
from BrightnessManager import BrightnessManager
from Battery import Battery
class PowerSaver:
def __init__(self, args=None):
self.setup(args)
def setup(self, args=None):
'''Set up arguments to be used, and initialize Battery and Brightness mangager.'''
arguments = {
"verbose": False,
"manual": False,
"fade": .25,
"time": 2,
"profile": None
}
if args is not None:
for arg in args.keys():
if arg in arguments:
arguments[arg] = args[arg]
self.arguments = arguments
if self.arguments["verbose"]:
print("Arguments", flush=True)
print("=====================")
for key, value in self.arguments.items():
print(key, ":", value, flush=True)
print("=====================\n")
self.brightness_manager = BrightnessManager()
self.battery = Battery()
self.brightness = self.brightness_manager.get_brightness()
self.charging = self.battery.is_charging()
self.percent = self.battery.percent()
self.level = None
self.min_percent = None
self.max_percent = None
if self.arguments["profile"] is None:
cur_dir = os.path.abspath(os.path.dirname(__file__))
if self.arguments["verbose"]:
print("Default settings loaded", flush=True)
self.settings = Settings(os.path.join(cur_dir, "settings.json"))
else:
self.settings = Settings(arguments["profile"])
def poll(self):
'''Poll the battery and brightness. If the battery level defined in settings
has changed, update the screen brightness.'''
poll_time = self.arguments["time"]
while True:
time.sleep(poll_time)
update = False
# Get percent, charge status, and brightness
self.percent = self.battery.percent()
charging = self.battery.is_charging()
brightness = self.brightness_manager.get_brightness()
# Close the program if the brightness
# was changed manually and not set in
# command line args.
if brightness != self.brightness:
if not self.arguments["manual"]:
if self.arguments["verbose"]:
print("Brightness Manually Changed, Exiting")
exit(1)
# If the battery level ("low", "medium", "high") is None,
# then initialize it. and set the brightness to the
# brightness value corresponding to the level
# of the battery's percent is currently at
if self.level is None:
if self.arguments["verbose"]:
print("Battery Level Initializing.", flush=True)
update = True
# If the battery percent has moved out of the range of the
# battery level, then update to change the brightness.
elif self.percent not in range(self.min_percent, self.max_percent + 1):
if self.arguments["verbose"]:
print("Battery level changed.", flush=True)
update = True
# If the battery's charging status has changed,
# determine if the screen should brighten for charging
# or dim for discharging.
elif charging != self.charging:
if self.arguments["verbose"]:
print("Charging status changed:", charging, flush=True)
update = True
# Print out the battery percent if verbose was set.
if self.arguments["verbose"]:
print(self.percent, flush=True)
# Only update the brightness if one of the
# above requirements are met.
if update:
self.charging = charging
# Check what level the battery percent is ("low", "medium", "high")
# and cache the range that level is in.
for battery_level, battery_range in self.settings.contents["levels"].items():
# If the current percent of the battery is in the range specified in the
# battery level, then that is the level needed to get brightness values.
if self.percent in range(battery_range[0], battery_range[1] + 1):
self.level = battery_level
self.min_percent, self.max_percent = battery_range
if self.arguments["verbose"]:
print("Battery Level: ", self.level, flush=True)
break
# If the battery is charging, handle brightness settings
# for charging in the settings file.
if self.charging:
target_brightness = self.settings.contents["on_charge_brightness"][self.level]
if target_brightness != self.brightness:
if target_brightness < self.brightness:
levels = reversed(range(target_brightness, self.brightness + 1))
else:
levels = range(self.brightness, target_brightness + 1)
for brightness_level in levels:
self.brightness_manager.set_brightness(brightness_level)
if self.arguments["verbose"]:
print("Setting Brightness:", brightness_level, flush=True)
time.sleep(self.arguments["fade"])
# Otherwise, handle brightness settings
# for battery usage in the settings file
else:
target_brightness = self.settings.contents["on_battery_brightness"][self.level]
if target_brightness != self.brightness:
if target_brightness < self.brightness:
levels = reversed(range(target_brightness, self.brightness + 1))
else:
levels = range(self.brightness, target_brightness + 1)
for brightness_level in levels:
self.brightness_manager.set_brightness(brightness_level)
if self.arguments["verbose"]:
print("Setting Brightness:", brightness_level, flush=True)
time.sleep(self.arguments["fade"])
# Get the brightness after everything has changed.
self.brightness = self.brightness_manager.get_brightness()
if __name__ == "__main__":
import argparse
parser = argparse.ArgumentParser()
parser.add_argument(
"-v",
"--verbose",
help="Display messages in the terminal each time the battery is polled.\n"
"Default: Off",
action="store_true"
)
parser.add_argument(
"-m",
"--manual",
help="Keep the program open if the brightness is manually changed.\n"
"Default: Off",
action="store_true"
)
parser.add_argument(
"-f",
"--fade",
help="The speed to fade the brightness in or out.\n"
"Higher is slower. Default: .25",
type=float,
default=.25
)
parser.add_argument(
"-t",
"--time",
help="The time to sleep between each poll on the battery. (in seconds)\n"
"Default: 2",
type=float,
default=2
)
parser.add_argument(
"-p",
"--profile",
help="The json file to use for battery levels and percentages.",
type=str
)
args = parser.parse_args()
arguments = {
"verbose": args.verbose,
"manual": args.manual,
"fade": args.fade,
"time": args.time,
"profile": None if not args.profile else args.profile,
}
powersaver = PowerSaver(arguments)
powersaver.poll()
settings.json
{
"on_battery_brightness": {
"high": 75,
"medium": 50,
"low": 10
},
"on_charge_brightness": {
"high": 100,
"medium": 75,
"low": 50
},
"levels": {
"low": [0, 30],
"medium": [31, 74],
"high": [75, 100]
}
}
2 Answers 2
Let's start with the obvious, coding best practices:
You should strive to make variable names easy to remember and meaningful. I think you could improve a bit on this, e.g.
percent
\$ \rightarrow \$percentage_remaining
,get
\$ \rightarrow \$_read_battery_descriptor
.brightness_regex
isn't related to the class and should probably be a global variable:import os import subprocess import re BRIGHTNESS_REGEX = re.compile(r'(\d*)')
You use the magic variable
"/sys/class/power_supply/BAT0"
. Maybe call thatPOWER_SUPPLY_PATH
?Try to be as specific as possible about exceptions. If you raise a bare
Exception
, other people have to catch it asexcept Exception:
, which also catches all other subclasses ofException
. In case the path to the battery descriptor doesn't exist, it makes sense to raise aFileNotFoundError
:POWER_SUPPLY_PATH = "/sys/class/power_supply/BAT0" class Battery: ... def __init__(self): if not os.path.exists(POWER_SUPPLY_PATH): raise FileNotFoundError( "Default path to battery '{}' does not exist.".format( POWER_SUPPLY_PATH ) )
And similarly,
class BrightnessManager: ... def set_brightness(self, value): '''Make a system call to gdbus to change the screen brightness. The value passed in must be a number between 0 - 100.''' if not 0 <= value <= 100: raise ValueError("Brightness value must be between 0 and 100.")
Battery.percent
andBattery.is_charging
could both be properties of the class:class Battery: ... @property def percent(self): return int(self.get("capacity")) @property def is_charging(self): status = self.get("status") return status != "Discharging"
Now some of my other concerns:
In my opinion, checking if an argument
n
is within a certain range is more Pythonic than checking ifa < n < b
:class BrightnessManager: ... def set_brightness(self, value): '''Make a system call to gdbus to change the screen brightness. The value passed in must be a number between 0 - 100.''' if value not in range(101): # Slightly more readable? raise ValueError("Brightness value must be between 0 and 100.")
As @jonrsharpe pointed out, you may not need classes here. Functions would do the job perfectly and are perhaps less of a hassle for these types of scripts. Personally, I would go with a compromise, namely cramping battery-related stuff into a class of its own and work with functions for power-saving functionality.
Here's my own take on it:
import subprocess
import os
import re
brightness_regex = re.compile(r"(\d*)")
POWER_SUPPLY_PATH = "/sys/class/power_supply/BAT0"
class Battery:
"""A basic wrapper around the files in
/sys/class/power_supply/BAT0 in Linux to get
the remaining battery capacity and charging status.
"""
def __init__(self):
if not os.path.exists(POWER_SUPPLY_PATH):
raise FileNotFoundError(
"Default path to battery '{}' does not exist.".format(
POWER_SUPPLY_PATH
)
)
@staticmethod
# _read_battery_descriptor may be a staticmethod
def _read_battery_descriptor(self, file):
file = os.path.join(POWER_SUPPLY_PATH, file)
if not os.path.isfile(file):
# Make sure the file exists before using subprocess.
raise FileNotFoundError(
"Invalid path '{}': File not found".format(
file
)
)
command = "cat {}".format(file)
try:
process = subprocess.Popen(
command,
stdout=subprocess.PIPE,
shell=True
)
result = process.stdout.read().decode().split("\n")[0]
finally:
# Wrap 'subprocess' calls into try: finally: statements
# to assure the file descriptors are closed after communication
process.stdout.close()
return result
@property
def percentage_remaining(self):
"""Return the battery percentage remaining, as an integer."""
return int(_Battery._read_battery_descriptor("capacity"))
@property
def is_charging(self):
"""Returns a boolean indicating whether or not the battery
is charging.
"""
return _Battery._read_battery_descriptor("status") != "Discharging"
def get_brightness():
"""Make a system call to gdbus and get the system brightness."""
command = "gdbus call --session --dest org.gnome.SettingsDaemon.Power "\
"--object-path /org/gnome/SettingsDaemon/Power "\
"--method org.freedesktop.DBus.Properties.Get "\
"org.gnome.SettingsDaemon.Power.Screen Brightness"
try:
process = subprocess.Popen(
command,
stdout=subprocess.PIPE,
shell=True
)
result = BRIGHTNESS_REGEX.findall(process.stdout.read().decode())
finally:
process.stdout.close()
return int("".join(result))
def set_brightness(value):
"""Make a system call to gdbus to change the screen brightness.
The value passed in must be a number between 0 - 100.
"""
if value not in range(101):
raise ValueError("Brightness value must be between 0 and 100.")
command = 'gdbus call --session --dest org.gnome.SettingsDaemon.Power '\
'--object-path /org/gnome/SettingsDaemon/Power '\
'--method org.freedesktop.DBus.Properties.Set '\
'org.gnome.SettingsDaemon.Power.Screen Brightness "<int32 {}>" '\
'> /dev/null'.format(value)
subprocess.call(
command,
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE
)
# Replaced 'os.system' with 'subprocess.call'.
I've added some comments describing what I changed and why. One last thing, you should probably check the return code of the subprocess
calls before reading from stdout
, to check if the command failed. I'll leave that as an exercise to the reader.
-
1\$\begingroup\$ Thanks for the tips. All of your points were clear. I will definitely keep this in mind for future work. \$\endgroup\$Jebby– Jebby2017年12月10日 02:31:43 +00:00Commented Dec 10, 2017 at 2:31
-
\$\begingroup\$ Instead of
range(100)
you should userange(101)
. \$\endgroup\$Roland Illig– Roland Illig2017年12月11日 07:15:36 +00:00Commented Dec 11, 2017 at 7:15
Except for the PowerSaver.poll
method I can follow your code pretty well. And in constrast to @jonrsharpe and @Coal_ I think the BrightnessManager
class is a good abstraction. Here are three points to consider:
1) Some of the config values are stored in the json file while others are command line options. I see no advantage to keep this apart. For the user passing command line options and changing a config file is nearly the same effort. Moreover, I find it a bit confusing to have two places to pass config values. And if you store them in only one place (i favour the json file) your code can become simpler.
2) Your PowerSaver.poll
method is a canditate for method extraction. I.e. almost each code block provided with comment lines in that method may become a method of their own. As a result the poll
method will be more expressive.
3) I like to see the class properties at the very first glance, i.e. in the constructor, especially if there are many. In contrast you define the properties of the PowerSaver
class only in the setup
method and its hard to keep track of them. So I would do it like this:
class PowerSaver:
def __init__(self, args=None):
self.arguments = None
self.settings = None
self.level = None
self.min_percent = None
self.max_percent = None
self.brightness_manager = BrightnessManager()
self.battery = Battery()
self.brightness = self.brightness_manager.get_brightness()
self.charging = self.battery.is_charging()
self.percent = self.battery.percent()
self.setup(args)
That is even if you assign them a value later I would define them with None
in the constructor. (Does anyone see a reason not to do that?) Alternativly you can list all properties in the constructor' doc string.
-
\$\begingroup\$ Thanks for the pointers! The reason I had setup the json and the command line arguments, is to not clutter the command line with unnecessary flags when most of the time I would be only using
-v
. \$\endgroup\$Jebby– Jebby2017年12月11日 20:55:00 +00:00Commented Dec 11, 2017 at 20:55 -
\$\begingroup\$ And I like the point of number 3. Kind of like declaring variables before usage like in Java. Hadn't thought about it like that. \$\endgroup\$Jebby– Jebby2017年12月11日 21:00:10 +00:00Commented Dec 11, 2017 at 21:00
-
\$\begingroup\$ Yes you're right, it's kind of java-like and I think it increases the readability. Though there may be reasons not doing that in python. \$\endgroup\$char bugs– char bugs2017年12月11日 21:30:10 +00:00Commented Dec 11, 2017 at 21:30
-
\$\begingroup\$ Well I've been learning Java as of recently, so this will help keep the same style of code between the languages. \$\endgroup\$Jebby– Jebby2017年12月11日 21:39:40 +00:00Commented Dec 11, 2017 at 21:39
os.listdir()
which will definitely be easier and faster than starting a subprocess to usecat
\$\endgroup\$os.listdir()
do what I'm doing withcat
? I'm reading from system files (with a known directory). I'm not listing folders. \$\endgroup\$