1
\$\begingroup\$

I built an allsky camera with a temperature sensor and a heater. The heater is supposed to turn when the sensor (measure_temp()) return a temperature below 8, which is checked every 15 minutes. However, about every 4th measurement fails and so I included an error handler that returns 7, so that the heater turns on if the sensor returns an error, but it seems like a bad solution. Is there a better way to handle the error like run the function until no error occurs?

import RPi.GPIO as GPIO
import time
import datetime
import time
import board
import adafruit_dht

def measure_temp():
    dhtDevice = adafruit_dht.DHT22(board.D18)

    try:
        temperature = dhtDevice.temperature
        error_test = ""
    except RuntimeError as error:
        temperature = 7
    except:
        temperature = 7        
    return temperature


if __name__ == '__main__':

    while True:
        if  measure_temp() < 8:
            GPIO.setmode(GPIO.BCM)
            RELAIS_1_GPIO = 17
            GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
            GPIO.output(RELAIS_1_GPIO, GPIO.HIGH)
            print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
            print('Heating on')
            print()
            time.sleep(900)
            GPIO.cleanup()
        else:
            GPIO.setmode(GPIO.BCM)
            RELAIS_1_GPIO = 17
            GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
            GPIO.output(RELAIS_1_GPIO, GPIO.LOW)
            print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
            print('Heating off')
            time.sleep(900)
            GPIO.cleanup()
\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

Every 4th measurement fails is a pretty high error rate. The preferred course of action would be to root cause the error and fix it. Meanwhile, since the error seems to be transient, you may want to read the sensor again, and again, until it reads cleanly. Another strategy is to read more often, and maintain the running average (discarding the failures).

For a proper review, DRY. The GPIO setup shall be lifted out of the if/else:

while True:
    GPIO.setmode(GPIO.BCM)
    RELAIS_1_GPIO = 17
    GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
    if  measure_temp() < 8:
        GPIO.output(RELAIS_1_GPIO, GPIO.HIGH)
        print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
        print('Heating on')
        print()
    else:
        GPIO.output(RELAIS_1_GPIO, GPIO.LOW)
        print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
        print('Heating off')
    time.sleep(900)
    GPIO.cleanup()

I also recommend to factor the printing out to the function, e.g.

def log_heating_status(status):
    print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
    print('Heating {%s}'.format(status))

to be called as log_heating_status('on') and log_heating_status('off').

It is also very unclear why do you setup/cleanup GPIO on every iteration. I'd expect it to be done once, outside the loop.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for you feedback @vnp! \$\endgroup\$
    – VBA Pete
    Commented Dec 20, 2020 at 8:28
2
\$\begingroup\$

Constants should be defined at module level

RELAIS_1_GPIO = 17 doesn't seem to change. Define it right after the import statements.

Keep implementation details out of your main loop

You currently know what GPIO.output(RELAIS_1_GPIO, GPIO.HIGH) does, but will you know in a couple of months? In addition to what @vnp recommended, better extract that into a function turn_heater_on(), the same goes for turning the heater off.

if measure_temp() < 8:
    turn_heater_on()
else:
    turn_heater_off()

Don't use bare except

See e.g. this Q&A for reasons why.

Handle exceptions where you can handle them

If I understood you correctly, your logic is:

  • Read temperature
  • If temperature is below threshold, turn heater on
  • If temperature is above threshold, turn heater off
  • If there was an error reading the temperature, turn heater on

But that's not what your main loop's logic does because part of the decision is hidden in measure_temp. This may become a problem if your script grows bigger and in a couple of months you decide that 6 °C are actually acceptable, changing

if  measure_temp() < 8:

to

if  measure_temp() < 6:

...and all of a sudden your heater erroneously turns off once every hour (i.e. every fourth measurement) despite -10°C, only because you forgot to change the return value of measure_temp in case of an error.

While using an unrealistically small value like -99999 would prevent that, there are two conceptual problems with this approach:

  • You are using a return value to indicate an error. The proper way to signal an exception to the caller is, well, raising an exception.
  • The measure_temp function implicitly decides what action to take in case of an error.

By the way, documenting your functions is a good way to find problems like this. Even if the function is trivial, it can help if you at least imagine what the docstring for measure_temp would look like:

def measure_temp():
    """Returns the temperature of sensor XYZ in °C, or 7 if reading the sensor fails.
    """

That's no function you'd want to work with.

measure_temp should raise an exception if the measurement fails, so you can decide what to do in your main loop instead. If you don't know which exceptions the adafruit library may raise (which unfortunately is very common in python), and if the only information you're interested in is whether the measurement failed, it is acceptable to catch all Exceptions. In order to meaningfully indicate a failed measurement to the caller, you can define a custom exception class:

class MeasurementError(Exception):
    pass

def measure_temp():
    try:
        return adafruit_dht.DHT22(board.D18).temperature
    except Exception as error:
        # raising your custom exception "from" the original one helps when debugging because you can also see where it originated from
        raise MeasurementError("Error reading sensor XYZ") from error

Now your main loop can be written like this:

while True:
    try:
        if measure_temp() < 8:
            turn_heater_on()
        else:
            turn_heater_off()
        time.sleep(900)
    except MeasurementError:
        print("Error reading temperature sensor. Retrying measurement.")

In fact, in small scripts like this I usually write the logic (i.e. the snippet above) first using functions that don't yet exist, and implement those functions later.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.