On 19Aug2018 09:32, richard lucassen <mailingli...@lucassen.org> wrote:
This is a working script I made. It initializes the I/O expanders, then
it waits for an INT from these I/O expanders on GPIO23, reads the
contents and sends which bit on which chip went up or down to a fifo
(and stdout for logging)
As I'm new to Python, just this question: are there any unPythony
things in this code?
There are always unPythonic bits. Even after you've cleaned them all up, since
people will disagree about the finer points of Pythonicism there will be bits
both over and under cleaned.
##############################################################
#!/usr/bin/env python3
list_pcf = [0x38, 0x39, 0x3a, 0x3b]
list_pcf_value = []
import sys
from smbus import SMBus
from time import sleep
import RPi.GPIO as GPIO
Imports should come before any other code.
GPIO.setmode(GPIO.BCM)
bus = SMBus(1)
# initialisation of the input devices:
print ("[INFO] initialisation input devices")
for i in range(len(list_pcf)):
This is better written as:
for i, pcf in enumerate(list_pcf):
then you can replace every mention of "list_pcf[i] with "pcf" within the loop.
And in fact, since you're not using "i" for anything else you can just go:
for pcf in list_pcf:
try:
bus.write_byte(list_pcf[i], 0xff) # set device to 0xff
output = bus.read_byte(list_pcf[i])
list_pcf_value.append(output) # append value to list
print ("found: %d, input value: 0x%x" % (list_pcf[i], output))
except IOError:
It is best to put try/except around the smallest possible piece of code. If
there is an I/O error in the above you have no idea what caused it. Even the
print() could emit one, and in that case your except clause's action will be
misguided. Example:
try:
bus.write_byte(list_pcf[i], 0xff) # set device to 0xff
output = bus.read_byte(list_pcf[i])
except IOError as e:
...
else:
list_pcf_value.append(output) # append value to list
print ("found: %d, input value: 0x%x" % (list_pcf[i], output))
print ("[ALERT] I/O problem device 0x%x (init)" % pcf)
You should always include the exception itself in this kind of message,
example:
print ("[ALERT] I/O problem device 0x%x (init): %s" % (pcf, e))
sys.stdout.flush()
# GPIO 23 set up as input. It is pulled up to stop false signals
GPIO.setup(23, GPIO.IN, pull_up_down=GPIO.PUD_UP)
loopcntr = 0 # detects if INT is kept low
while True:
if GPIO.input(23) == 1: # if still 0, another event has occurred
GPIO.wait_for_edge(23, GPIO.FALLING)
print ('-------')
while GPIO.input(23) == 0:
for i in range(len(list_pcf)):
You can write thos as:
for i, (pcf, pcf_value) in enumerate(zip(list_pcf, list_pcf_value)):
and then just talk about "pcf" and "pcf_value" where you have "list_pcf[i]" and
"list_pcf_value[i]". Then you just want the "i" when you update
"list_pcf_value[i]". (Assigning to pcf_value won't affect that, so you need the
index.)
try:
output = bus.read_byte(list_pcf[i])
Again, this try/except covers too much code. It should only cover bus I/O
actions.
if output != list_pcf_value[i]:
xor = list_pcf_value[i] ^ output
for l in range(8):
You might want a more meaningful name than "l", such as "bit_pos".
if xor & 0x1:
updown = (output >> l) & 0x1
print ("%d bit %d: to %d" % (list_pcf[i],l,updown))
print("%d %d %d" % (list_pcf[i],l,updown),
file=open('/mnt/ramdisk/var/lib/ha/events.fifo', 'w'))
This print is better written:
with open('/mnt/ramdisk/var/lib/ha/events.fifo', 'w') as fifo:
print("%d %d %d" % (list_pcf[i],l,updown), file=fifo)
This ensures timely closing of the fifo when you exit the with suite.
Your "print(...., file=open(...))" _will_ close the fifo in a timely fashion in
CPython because the reference counting will trigger the file's delete action,
but in other Python implementations the deletion may happen an arbitrarily
large amount of time later (because they may garbage collect unreferenced
objects differently).
Importantly, your print() won't get flushed to the fifo until the file closes,
so timely closing is important. The with is reliable. And also the common
idiom.
xor = xor >> 1
You can write this:
xor >>= 1
list_pcf_value[i] = output
except IOError:
print ("[ALERT] I/O problem device 0x%x" % list_pcf[i])
And again, you should include the exception value in the message.
if GPIO.input(23) == 1:
loopcntr = 0
break
else:
You don't need this else, because the break will exit the loop.
loopcntr += 1
if loopcntr >=20:
print ('[ALERT] possible INT loop, disable 10 seconds')
sleep (10)
sys.stdout.flush()
You need a flush before the sleep, otherwise the flush won't occur for 10
seconds. Untimely.
GPIO.cleanup()
Cheers,
Cameron Simpson <c...@cskk.id.au>
--
https://mail.python.org/mailman/listinfo/python-list
