Drolla / WavePlus_Bridge

Airthings Wave Plus Bridge to Wifi/LAN and MQTT
MIT License
21 stars 9 forks source link

Skip CSV file setup for None value #7

Closed jmagnuson closed 2 years ago

jmagnuson commented 2 years ago

Fixes:

Read configuration file /home/pi/src/WavePlus_Bridge/waveplus_bridge.yaml
2021-10-16 03:49:56 - Opening CSV data log file None
2021-10-16 03:49:56 -   Error accessing CSV file! None : unsupported operand type(s) for /: 'str' and 'int'

when csv is omitted from the yaml config.

Drolla commented 2 years ago

Thanks for your suggestion!

The current version should work without having a CSV file specified. For example, when I comment the full 'csv:' line in my YAML configuration file, I get the following log:

2021-10-17 21:02:44 - Configuration: {...'csv': None, ...}
2021-10-17 21:02:44 - Opening CSV data log file None
2021-10-17 21:02:44 -   Done

In this case the log database (logdb) is still opened, however the data is simply not stored in a CSV file. This is the intention, because the web server still requires access to the previous data even if it was not backed up (and restored from the CSV file). If the log database is not created, also the web server will hang. I guess this is the case with your patch.

Please allow me to understand why you are getting the error if you have not defined the CSV file. The following questions may help finding the root cause:

Thanks!

jmagnuson commented 2 years ago

Derp.. I should have dug into the error message that was present, didn't realize the logdb was being used regardless.

My python version is 3.5.3. Admittedly I don't use python day-to-day, so am not aware of nuances between versions. And yes, I have all CSV configuration stripped from the config; really just using the Basic Configuration in the README.

But I honed in on the str/int mismatch error, printing the types for config.data_retention as well as config.period:

diff --git a/waveplus_bridge.py b/waveplus_bridge.py
index 10a794c..2d2f2f2 100644
--- a/waveplus_bridge.py
+++ b/waveplus_bridge.py
@@ -37,6 +37,7 @@ import argparse
 import yaml
 import json
 import fnmatch
+import traceback
 import urllib
 from http.server import BaseHTTPRequestHandler
 from libs.threadedhttpserver import ThreadedHTTPServer
@@ -709,7 +710,14 @@ if __name__ == "__main__":
         PerformanceCheck.file = logf

     # Data logging, optionally into a CSV file
-    lprint("Opening CSV data log file", config.csv, file=logf)
+    lprint(
+        "Opening CSV data log file",
+        config.csv,
+        config.data_retention,
+        type(config.data_retention),
+        config.period,
+        type(config.period),
+        file=logf)
     ldb = None
     try:
         log_labels = ["humidity", "radon_st", "radon_lt",
@@ -728,6 +736,7 @@ if __name__ == "__main__":
         if logf != sys.stdout:
             lprint("Error accessing CSV file!", config.csv, ":", err,
                     file=sys.stderr)
+        traceback.print_exc()
         sys.exit(1)

     # Start the HTTP web server

which produced

Read configuration file /home/pi/src/WavePlus_Bridge/waveplus_bridge.yaml
2021-10-18 02:54:07 - Opening CSV data log file None 2678400 <class 'str'> 600 <class 'int'>
2021-10-18 02:54:07 -   Error accessing CSV file! None : unsupported operand type(s) for /: 'str' and 'int'
Traceback (most recent call last):
  File "/home/pi/src/WavePlus_Bridge/waveplus_bridge.py", line 730, in <module>
    number_retention_records=config.data_retention/config.period)
TypeError: unsupported operand type(s) for /: 'str' and 'int'

so by doing a cast before division:

diff --git a/waveplus_bridge.py b/waveplus_bridge.py
index 10a794c..51b4d75 100644
--- a/waveplus_bridge.py
+++ b/waveplus_bridge.py
@@ -718,7 +718,7 @@ if __name__ == "__main__":
         ldb=logdb.LogDB(
                 {config.name[sn]:log_labels for sn in config.sn},
                 config.csv,
-                number_retention_records=config.data_retention/config.period)
+                number_retention_records=int(config.data_retention)/int(config.period))
         del pc
         lprint("  Done", file=logf)
     except PermissionError:

the program starts up normally:

Read configuration file /home/pi/src/WavePlus_Bridge/waveplus_bridge.yaml
2021-10-18 03:05:41 - Opening CSV data log file None
2021-10-18 03:05:41 -   Done
2021-10-18 03:05:41 - Start HTTP/Web server on port 12346
2021-10-18 03:05:41 -   Done 12346
2021-10-18 03:05:41 - Setup WavePlus device access
2021-10-18 03:05:41 -   Done
2021-10-18 03:05:41 - Start main loop. Press ctrl+C to exit program!

I am happy to make this change if it is agreeable. Not sure why no one else is seeing this issue, though.

Drolla commented 2 years ago

Thanks for debugging that, @jmagnuson. I see that you have not only removed the CVS file definition, but the entire section. After using the same basic configuration file, I see the error you are mentioning.

In general there should be no need to perform this data type casting. Integer or floating point values should have the right type if read from the YAML file or if default values are used.

The root cause of the issue is in the default value definition of the period and _dataretention configuration parameters, line 88 and 89:

        for key, value in {
                "period": str(120),
                "data_retention": str(31*24*3600) # 31 days
        }.items():
            if config[key] is None:
                config[key] = value

The type is explicitly set to string, which is incorrect. After changing the code to keep the integer format, the application starts correctly.

        for key, value in {
                "period": 120,
                "data_retention": 31*24*3600 # 31 days
        }.items():
            if config[key] is None:
                config[key] = value

If you don't mind, I prefer to resolve the issue at the root of the problem and correct the type of the predefined default values.

Drolla commented 2 years ago

Issue https://github.com/Drolla/WavePlus_Bridge/issues/8 has been created to address bug. It will be fixed by ensuring that the period and data_retention parameters have the proper integer type.