Address Issue #81

[roscoe81]

I've raised this PR to partially address Issue #81 with the following comments:

1. I moved update_time = time.time() to before send_to_luftdaten so the data sending function has the full 145 seconds to resend the values.
2. I added python logging, to log success/failure states and other info with timestamps. I didn't send this to a file, since it might be more valuable to print the log, given that this is just an example. Logging to a file would be an easy addition.
3. I added exception handling when posting data to luftdaten as raised in Requests to luftdaten should be in try except #79. I've been using this exception handling for Luftdaten over many months on my project and it has worked very well. I've included a Timeout Exception, since I've found that necessary in some circumstances. I haven't attempted to send data multiple times (with a small delay), since I've found that the Luftdaten site is often down and a missed update isn't a big problem - a new one will be attempted in 145 seconds. I have added a log entries to record detiails of the exceptions.
4. I added PMS5003 Checksum exception handing.
5. I haven't included a systemd in the PR.

Hope that helps

[Gadgetoid]

Oh wow! This is brilliant, thank you. I'd raised this to pick up... eventually... but I'm stretched thin and really appreciate you giving it a shot.

I'll try and test this today!

[Gadgetoid]

Seems to work okay. (although the screen isn't working for me, I think that's a new Pi 4 kernel problem or a hardware problem)

But I'd recommend a few changes:

1. All non-fatal errors should use logging.warning
2. We can initialise resp to None and use a not None check on that instead of tracking resp2_exception as a separate variable
3. print(values) should probably be within the update portion of the loop, or it gets quite spammy- some people might want this though? But it'd blow up a log file pretty quickly
4. Should output the response reason if the status is not ok

Here's an attempt to put those changes into a patch:

diff --git a/examples/luftdaten.py b/examples/luftdaten.py
index 7701a4a..38c47aa 100755
--- a/examples/luftdaten.py
+++ b/examples/luftdaten.py
@@ -124,10 +124,12 @@ def send_to_luftdaten(values, id):

     pm_values_json = [{"value_type": key, "value": val} for key, val in pm_values.items()]
     temp_values_json = [{"value_type": key, "value": val} for key, val in temp_values.items()]
-    resp1_exception = False
-    resp2_exception = False
+
+    resp_pm = None
+    resp_bmp = None
+
     try:
-        resp_1 = requests.post(
+        resp_pm = requests.post(
             "https://api.luftdaten.info/v1/push-sensor-data/",
             json={
                 "software_version": "enviro-plus 0.0.1",
@@ -142,17 +144,14 @@ def send_to_luftdaten(values, id):
             timeout=5
         )
     except requests.exceptions.ConnectionError as e:
-        resp1_exception = True
-        logging.info('Luftdaten PM Connection Error: ' + str(e))
+        logging.warning('Luftdaten PM Connection Error: ' + str(e))
     except requests.exceptions.Timeout as e:
-        resp1_exception = True
-        logging.info('Luftdaten PM Timeout Error: ' + str(e))
+        logging.warning('Luftdaten PM Timeout Error: ' + str(e))
     except requests.exceptions.RequestException as e:
-        resp1_exception = True
-        logging.info('Luftdaten PM Request Error: ' + str(e))
+        logging.warning('Luftdaten PM Request Error: ' + str(e))

     try:
-        resp_2 = requests.post(
+        resp_bmp = requests.post(
             "https://api.luftdaten.info/v1/push-sensor-data/",
             json={
                 "software_version": "enviro-plus 0.0.1",
@@ -167,19 +166,18 @@ def send_to_luftdaten(values, id):
             timeout=5
         )
     except requests.exceptions.ConnectionError as e:
-        resp2_exception = True
-        logging.info('Luftdaten Climate Connection Error: ' + str(e))
+        logging.warning('Luftdaten Climate Connection Error: {}'.format(e))
     except requests.exceptions.Timeout as e:
-        resp2_exception = True
-        logging.info('Luftdaten Climate Timeout Error: ' + str(e))
+        logging.warning('Luftdaten Climate Timeout Error: {}'.format(e))
     except requests.exceptions.RequestException as e:
-        resp2_exception = True
-        logging.info('Luftdaten Climate Request Error: ' + str(e))
+        logging.warning('Luftdaten Climate Request Error: {}'.format(e))

-    if not resp1_exception and not resp2_exception:
-        if resp_1.ok and resp_2.ok:
+    if resp_pm is not None and resp_bmp is not None:
+        if resp_pm.ok and resp_bmp.ok:
+            logging.info('Luftdaten Climate Success', values)
             return True
         else:
+            logging.warning('Luftdaten Error PM: {}, Climate: {}'.format(resp_pm.reason, resp_bmp.reason))
             return False
     else:
         return False
@@ -210,12 +208,14 @@ update_time = time.time()
 while True:
     try:
         values = read_values()
-        logging.info(values)
         time_since_update = time.time() - update_time
-        if time_since_update > 145:
-            resp = send_to_luftdaten(values, id)
+        if time_since_update > 145:
+            logging.info(values)
             update_time = time.time()
-            logging.info("Luftdaten Response: {}\n".format("ok" if resp else "failed"))
+            if send_to_luftdaten(values, id):
+                logging.info("Luftdaten Response: OK")
+            else:
+                logging.warning("Luftdaten Response: Failed")
         display_status()
     except Exception as e:
         logging.info("Main Loop Exception: " + str(e))

[roscoe81]

Your changes make it much better. I've had problems applying the patch to my PR and it doesn't seem to apply the @@ -124,10 +124,12 @@ changes. I suspect that it's a line numbering problem and I'll try again tomorrow when I have some more time. I also think that it might be better to use your "{}'.format(e)" Climate Error logging changes to the PM Errors and Main Loop Exception. They still have "+ str(e)". To be consistent, the Main Loop Exception should probably also be a warning, rather that an info.

[roscoe81]

Still couldn't apply the patch but I've now updated the code with your suggestions and my previous comment. Testing has shown that it's working OK but that "logging.info('Luftdaten Climate Success', values)" was redundant. Happy to put that back in if you think it's essential.

[Gadgetoid]

Sorry this has flown somewhat under my radar due to juggling so many projects! It has not been forgotten though.

[Gadgetoid]

@sandyjmacdonald is merging this going to cause a major catastrophe with any Luftdaten setup guides or anthing? I can't see any reason why it might.

[Gadgetoid]

Thanks @helgibbons for taking this for a spin so I could merge.

Thank you @roscoe81 for all your effort here, much appreciated!