simple tkinter battery monitor

Rick Johnson rantingrickjohnson at gmail.com
Sun Jan 27 19:56:32 EST 2013


On Sunday, January 27, 2013 4:59:20 PM UTC-6, leonix... at gmail.com wrote:
> I tried to write a simple battery monitor for laptops
> which shows normally just the battery percentage, and when
> is clicked some more info.
> 
> If I click just one time it works, but if I click a second
> time, the additional info Label seems to be empty (but it
> holds the dimension of the StringVar content, even if it
> isn't displayed)
> 
> Where am I wrong?

Before i discover your code logic error, i want to expose style errors.

> ---------------------------------------------------
> #!/usr/bin/python3.2
> 
> from re import findall, search
> from threading import Thread
> from time import sleep
> from subprocess import Popen, call, PIPE, STDOUT
> from tkinter import *

I am wondering why you would "selectively" import from the re module, which is a quite normal sized module, but then do the global import from tkinter, of which who's namespace is terribly polluted due to lack of packaging.

> class battery_monitor:

Bad naming convention here! ALL class identifiers must (at the very least) /start/ with a capital letter. My strong opinion is to cap EVERY word. So for example:

 OptionMenu NOT Optionmenu
 ComboBox NOT Combobox
 etc...
 
Using "lowercase_with_underscores" should be reserved for interface methods and global functions.

>         def __init__(self):
>                 root=Tk()
> #                root.geometry("-0+0")
>                 root.overrideredirect(True)
>                 root.wm_attributes("-topmost", 1)

I just hate when people use 1 and 0 for True and False. I know Python allows such non-sense, but i just hate it because it can cause subtle bugs -- and most logical people agree with me. 

>                 self.battery_string=StringVar()
>                 self.battery_percent=StringVar()

Two issues here. First you use a function/interface style to define a variable. Then you go and add insult to injury by using an ambiguous name. You should have used something like: "batteryStringVar" and "batteryPercentVar".

>                 self.battery_label=Label(root, extvariable=self.battery_string, font=("fixed", 9))
>                 self.battery_icon=Label(root, textvariable=self.battery_percent, font=("fixed", 9), width=3)
>                 self.battery_icon.grid()
>                 t=Thread(target=self.update_battery_level_loop)

Don't assign variables without leaving buffer spaces around the equals sign. Only omit the spaces when passing arguments to a class, method, or function.

>                 t.start()
>                 root.bind("<Button-1>", self.display_details)
>                 self.root=root
>                 root.mainloop()
>         
>         # displays a message about details of battery status
>         # i.e. "on-line" or "charging, 20 min left" and so on

Oh gawd this is a major pet peeve of mine!!!! 

Always place a comment at the same indention level of the function or class that it references. AND NEVER, EVER, place a comment OUTSIDE of a function, method, or class like you have done here.

>         def display_details(self, event):
>                 self.battery_icon.grid_remove()
>                 self.battery_label.grid()
>                 self.root.update_idletasks()
>                 sleep(1)

You may want to look into the Universal Tkinter widget method: "w.after(ms, func)".

>                 self.battery_label.grid_remove()
>                 self.battery_icon.grid()
>                 self.root.update_idletasks()
>         
>         # dummy function used just to test the GUI
>         def read_battery_level(self):
>                 self.level=100
>                 return "battery is full"
>         
>         # threaded function, should constantly update the battery level
>         def update_battery_level_loop(self):
>                 self.read_battery_level()
>                 while True:
>                         self.battery_percent.set(self.level)
>                         self.battery_string.set(self.read_battery_level())
>                         sleep(5)
 

Finally, i don't understand why you are using such a deep indention level. Unknown Source said: "Four spaces thou shalt indent, and the number of thou indention shall be four."

Only after you clean up these style abominations by submitting a new example of your code will i offer any more help. Thanks.



More information about the Python-list mailing list