printing to stdout

[Cameron Simpson]

On 19Aug2018 09:32, richard lucassen <mailinglists at 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 <cs at cskk.id.au>