Requests to luftdaten should be in try except

[ulrischa]

Requests to luftdaten should be in try except block. If script runs in background and and network (e.g. DNS) error occured, the script stops running.

[Gadgetoid]

I must preface this with: I didn't write the luftdaten script, and I'm not familiar with their API beyond what I can glean from brushing up on the documentation.

While I agree in principle, a bare Except clause is an antipattern in Python- it should trap the specific network error and do something useful such as retry sending the data a limited number of times, just in case it's a temporary exception.

While a hard fail is pretty nasty, it's probably better than the script quietly failing indefinitely while squashing any useful debug output.

I can see a few improvements to this script that might go nicely with your proposal, not least of all:

1. Moving update_time = time.time() to before send_to_luftdaten so the data sending function has the full 145??? seconds to retry sending the values. Though I can't see any fearture for sending a timestamp to luftdaten.
2. Adding python logging, so we can log success/failure states and other info with timestamps, this is illustrated nicely here:

   enviroplus-python/examples/all-in-one-no-pm.py

   Lines 22 to 31 in 7452b88

   	import logging
   	logging.basicConfig(
   	format='%(asctime)s.%(msecs)03d %(levelname)-8s %(message)s',
   	level=logging.INFO,
   	datefmt='%Y-%m-%d %H:%M:%S')
   	logging.info("""all-in-one.py - Displays readings from all of Enviro plus' sensors
   	Press Ctrl+C to exit!
   	""")

I don't expect you to make all of these changes, but if you could at least update the try/catch to produce some useful debug output it would be a good start that I could merge and promote the rest of this to an Issue.

Thank you for taking the time to make a PR!

[ulrischa]

:) won't do the work for a commercial poduct. I Think I helped enought that it dont silently crash 

…

-------- Ursprüngliche Nachricht --------Von: Philip Howard <notifications@github.com> Datum: 05.08.20 11:50 (GMT+01:00) An: pimoroni/enviroplus-python <enviroplus-python@noreply.github.com> Cc: ulrischa <uli@ulrischa.de>, Author <author@noreply.github.com> Betreff: Re: [pimoroni/enviroplus-python] Requests to luftdaten should be in try except (#79) I must preface this with: I didn't write the luftdaten script, and I'm not familiar with their API beyond what I can glean from brushing up on the documentation. While I agree in principle, a bare Except clause is an antipattern in Python- it should trap the specific network error and do something useful such as retry sending the data a limited number of times, just in case it's a temporary exception. While a hard fail is pretty nasty, it's probably better than the script quietly failing indefinitely while squashing any useful debug output. I can see a few improvements to this script that might go nicely with your proposal, not least of all: Moving update_time = time.time() to before send_to_luftdaten so the data sending function has the full 145??? seconds to retry sending the values. Though I can't see any fearture for sending a timestamp to luftdaten. Adding python logging, so we can log success/failure states and other info with timestamps, this is illustrated nicely here: https://github.com/pimoroni/enviroplus-python/blob/7452b88562c25344ecf9e8eed9b13325d4aef4e6/examples/all-in-one-no-pm.py#L22-L31 I don't expect you to make all of these changes, but if you could at least update the try/catch to produce some useful debug output it would be a good start that I could merge and promote the rest of this to an Issue. Thank you for taking the time to make a PR! —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.

[Gadgetoid]

Well it's MIT code for an open data service, but I understand.

I can't merge a PR that exchanges one kind of broken for another, however, so I'll promote this to an issue and find time to give this example a proper once over.

Thanks again!

[ulrischa]

Can't see what is so bad with the except: pass. It just skips the current request to luftdaten and tries again in the time specified in the main loop.Logging would be nice,  but to be honest the whole script has no logging and is kind of beta. I also commented out line 186. display_status crashed the script also from time to time. Also no config variables defined in one place (WIDTH, HEIGHT, comp_value). -------- Ursprüngliche Nachricht --------Von: Philip Howard <notifications@github.com> Datum: 05.08.20 13:30 (GMT+01:00) An: pimoroni/enviroplus-python <enviroplus-python@noreply.github.com> Cc: ulrischa <uli@ulrischa.de>, Author <author@noreply.github.com> Betreff: Re: [pimoroni/enviroplus-python] Requests to luftdaten should be in try except (#79) Well it's MIT code for an open data service, but I understand. I can't merge a PR that exchanges one kind of broken for another, however, so I'll promote this to an issue and find time to give this example a proper once over. Thanks again! —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.