Merge pull request #146 from CohenLabPrinceton/tmp

Changes as discussed

pvp/common/loggers.py

@@ -28,6 +28,7 @@
 
 
 from pvp.common import prefs
+from pvp.common.utils import get_version
 
 _LOGGERS = []
 """
@@ -158,6 +159,12 @@ class CycleData(pytb.IsDescription):
     peep             =  pytb.Float64Col()    # estimated peep pressure
     vte              =  pytb.Float64Col()    # estimated End-Tidal Volume
 
+class ProgramData(pytb.IsDescription):
+    """
+    Structure for the hdf5-table to store program data, like githash and version.
+    """
+    version          =  pytb.StringCol(32)    # Version and githash string
+
 class DataLogger:
     """
     Class for logging numerical respiration data and control settings.
@@ -172,7 +179,8 @@ class DataLogger:
         |--- derived_quantities (group)
         |    |--- (time, Cycle No, I_PHASE_DURATION, PIP_TIME, PEEP_time, PIP, PIP_PLATEAU, PEEP, VTE )
         |
-        |
+        |--- program_information (group)
+        |    |--- (version & githash)
 
     Public Methods:
         close_logfile():                      Flushes, and closes the logfile.
@@ -225,6 +233,9 @@ def __init__(self, compression_level : int = 9):
         self.h5file = pytb.open_file(self.file, mode = "a")      # Open logfile
         self.compression_level = compression_level # From 1 to 9, see tables documentation
 
+        self._open_logfile()
+        self.store_program_data() # Store githash, version et al. once after init
+
     def __del__(self):
         self.close_logfile()
 
@@ -269,13 +280,33 @@ def _open_logfile(self):
         else:
             self.derived_table = self.h5file.root.derived_quantities.readout
 
+        if "/program_information" not in self.h5file:
+            self.logger.info('Generating /program_information table in: ' + self.file )
+            group = self.h5file.create_group("/", 'program_information', 'General information about PVP-1')
+            self.program_table = self.h5file.create_table(group, 'readout', ProgramData, "Program information",
+                                                          filters = pytb.Filters(
+                                                              complevel=self.compression_level,
+                                                              complib='zlib'),
+                                                          expectedrows=1000000)
+        else:
+            self.program_table = self.h5file.root.program_information.readout
+
     def close_logfile(self):
         """
         Flushes & closes the open hdf file.
         """
         self.logger.info("Logger terminated; in..." + self.file)
         self.h5file.close() # Also flushes the remaining buffers
 
+    def store_program_data(self):
+        """Appends program metadata to the logfile: githash and version
+        """
+        if self._data_save_allowed:
+            self._open_logfile()
+            datapoint                     = self.program_table.row
+            datapoint['version']          = get_version()
+            datapoint.append()
+
     def store_waveform_data(self, sensor_values: 'SensorValues', control_values: 'ControlValues'):
         """
         Appends a datapoint to the file for continuous logging of streaming data.
@@ -400,7 +431,7 @@ def load_file(self, filename = None):
             filename (str, optional): Path to a hdf5-file. If none is given, uses currently open file. Defaults to None.
 
         Returns:
-            dictionary: Containing the data arranged as ` {"waveform_data": waveform_data, "control_data": control_data, "derived_data": derived_data}`
+            dictionary: Containing the data arranged as ` {"waveform_data": waveform_data, "control_data": control_data, "derived_data": derived_data, "program_information": program_data}`
         """
         self.close_logfile()
 
@@ -420,7 +451,10 @@ def load_file(self, filename = None):
             table = file.root.derived_quantities.readout
             derived_data = table.read()
 
-        data_dict = {"waveform_data": waveform_data, "control_data": control_data, "derived_data": derived_data}
+            table = file.root.program_information.readout
+            program_data = table.read()
+
+        data_dict = {"waveform_data": waveform_data, "control_data": control_data, "derived_data": derived_data, "program_information": program_data}
         return data_dict
 
     def log2mat(self, filename = None):
@@ -444,7 +478,8 @@ def log2mat(self, filename = None):
             ls_wv = dff['waveform_data']
             ls_dv = dff['derived_data']
             ls_ct = dff['control_data']
-            matlab_data = {'waveforms': ls_wv, 'derived_quantities': ls_dv, 'control_commands': ls_ct}
+            ls_pr = dff['program_information']
+            matlab_data = {'waveforms': ls_wv, 'derived_quantities': ls_dv, 'control_commands': ls_ct, 'program_information': ls_pr}
             sio.savemat(new_filename, matlab_data)
         except:
             print(filename + " not found.")

pvp/common/message.py

@@ -1,6 +1,5 @@
 import time
 import typing
-import pytest
 
 from pvp.common import values
 from copy import copy
@@ -14,6 +13,8 @@ class SensorValues:
     Should be instantiated with each of the :attr:`.SensorValues.additional_values`, and values for all
     :class:`.ValueName` s in :data:`.values.SENSOR` by passing them in the ``vals`` kwarg.
 
+    An ``AssertionError`` if an incomplete set of values is given.
+    
     Values can be accessed either via attribute name (``SensorValues.PIP``) or like a dictionary (``SensorValues['PIP']``)
 
     """
@@ -64,8 +65,10 @@ def __init__(self, timestamp=None, loop_counter=None, breath_count=None, vals=ty
                         # otherwise just make one
                         self.timestamp = time.time()
                     else:
-                        with pytest.raises( Exception ):
-                            raise e
+                        raise e
+
+        # insist that we have all the rest of the vals
+        assert(all([value.name in kwargs.keys() for value in values.SENSOR.keys()]))
 
         # assign kwargs as attributes,
         # don't allow any non-ValueName keys
@@ -102,8 +105,7 @@ def __getitem__(self, item):
         elif item.lower() in self.additional_values:
             return getattr(self, item.lower())
         else:
-            with pytest.raises( Exception ):
-                raise KeyError(f'No such value as {item}')
+            raise KeyError(f'No such value as {item}')
 
     def __setitem__(self, key, value):
         if key in values.ValueName:
@@ -113,8 +115,7 @@ def __setitem__(self, key, value):
         elif key.lower() in self.additional_values:
             return setattr(self, key.lower(), value)
         else:
-            with pytest.raises( Exception ):
-                raise KeyError(f'No such value as {key}')
+            raise KeyError(f'No such value as {key}')
 
 class ControlSetting:
     def __init__(self,
@@ -148,17 +149,17 @@ def __init__(self,
             else:
                 logger = init_logger(__name__)
                 logger.exception(f'Couldnt create ControlSetting with name {name}, not in values.CONTROL')
-                with pytest.raises( Exception ):
-                    raise KeyError
-
+                raise KeyError
+        elif isinstance(name, values.ValueName):
+            assert name in values.CONTROL.keys() or name in (values.ValueName.VTE, values.ValueName.FIO2)
+
         self.name = name # type: values.ValueName
 
         if (value is None) and (min_value is None) and (max_value is None):
             logger = init_logger(__name__)
             ex_string = 'at least one of value, min_value, or max_value must be set in a ControlSetting'
             logger.exception(ex_string)
-            with pytest.raises( Exception ):
-                raise ValueError(ex_string)
+            raise ValueError(ex_string)
 
         self.value = value
         self.min_value = min_value

pvp/common/utils.py

@@ -1,9 +1,13 @@
 import signal
 import time
+import subprocess
+import os
+
 from contextlib import contextmanager
-from pvp.common.loggers import init_logger
 from pvp import prefs
 
+import pvp
+
 class TimeoutException(Exception): pass
 
 _TIMEOUT = prefs.get_pref('TIMEOUT')
@@ -35,8 +39,25 @@ def wrapper(*args, **kwargs):
                 ret = func(*args, **kwargs)
                 return ret
         except TimeoutException as e:
+            from pvp.common.loggers import init_logger
             log_str = f'Method call timed out - Method: {func}'
             logger = init_logger('timeouts')
             logger.exception(log_str)
             raise e
-    return wrapper
+    return wrapper
+
+def get_version():
+    """
+    Returns PVP version, and if available githash, as a string.
+    """
+    version = pvp.__version__
+    try:
+        # get git version
+        git_version = subprocess.check_output(['git', 'describe', '--always'],
+                                                cwd=os.path.dirname(__file__)).strip().decode('utf-8')
+        version = " - ".join([version, git_version])
+    except Exception: # pragma: no cover
+        # no problem, just use package version
+        pass
+
+    return version

pvp/controller/control_module.py

@@ -8,7 +8,6 @@
 import pdb
 from itertools import count
 import signal
-import pytest
 
 import pvp.io as io
 
@@ -17,6 +16,8 @@
 from pvp.common.values import CONTROL, ValueName
 from pvp.common.utils import timeout
 from pvp.alarm import ALARM_RULES, AlarmType, AlarmSeverity, Alarm
+from pvp.common.utils import timeout, TimeoutException
+
 from pvp import prefs
 
 
@@ -788,27 +789,22 @@ def __init__(self, save_logs = True, flush_every = 10, config_file = None):
         """
         ControlModuleBase.__init__(self, save_logs, flush_every)
 
-        # Handler for HAL timeout handler for the timeout
-        def handler(signum, frame):
-            print("TIMEOUT - HAL not initialized")
-            self.logger.warning("TIMEOUT - HAL not initialized. Using MockHAL")
-            with pytest.raises( Exception ):
-                raise Exception("HAL timeout")
+        self.__get_hal(config_file)
+        self._sensor_to_COPY()
 
-        signal.signal(signal.SIGALRM, handler)
-        signal.alarm(5)
+        # Current settings of the valves to avoid unnecessary hardware queries
+        self.current_setting_ex = self.HAL.setpoint_ex
+        self.current_setting_in = self.HAL.setpoint_in
 
+    @timeout
+    def __get_hal(self, config_file):
+        """
+        Get Hal, decorated with a timeout
+        """
         try:
             self.HAL = io.Hal(config_file)
-        except Exception: 
+        except RuntimeError:
             self.HAL = io.HALMock()
-            #TODO: Raise technical alert
-
-        self._sensor_to_COPY()
-
-        # Current settings of the valves to avoid unneccesary hardware queries
-        self.current_setting_ex = self.HAL.setpoint_ex
-        self.current_setting_in = self.HAL.setpoint_in
 
     def __del__(self):
         """

pvp/gui/widgets/control_panel.py

@@ -1,7 +1,6 @@
 import time
 import os
 from collections import OrderedDict as odict
-import subprocess
 
 from PySide2 import QtWidgets, QtCore, QtGui
 
@@ -10,6 +9,8 @@
 from pvp.gui.widgets.components import QHLine, OnOffButton
 from pvp.alarm import Alarm, AlarmType
 from pvp.common import prefs, values
+from pvp.common.utils import get_version
+
 import pvp
 
 class Control_Panel(QtWidgets.QGroupBox):
@@ -98,15 +99,7 @@ def init_ui(self):
         # version indicator
         self.status_layout.addWidget(QtWidgets.QLabel('PVP Version'),
                                      2,0,alignment=QtCore.Qt.AlignLeft)
-        version = pvp.__version__
-        try:
-            # get git version
-            git_version = subprocess.check_output(['git', 'describe', '--always'],
-                                                  cwd=os.path.dirname(__file__)).strip().decode('utf-8')
-            version = " - ".join([version, git_version])
-        except Exception: # pragma: no cover
-            # no problem, just use package version
-            pass
+        version = get_version()
 
         self.status_layout.addWidget(QtWidgets.QLabel(version),
                                      2,1,alignment=QtCore.Qt.AlignRight)

pvp/main.py

@@ -49,8 +49,8 @@ def main(arg):
         coordinator = get_coordinator(single_process=args.single_process, sim_mode=args.simulation)
         app, gui = launch_gui(coordinator, args.default_controls, screenshot=args.screenshot)
         sys.exit(app.exec_())
-    finally:
-        set_valves_save_position(args)
+    finally: #Only in cases of errors; tested above
+        set_valves_save_position(args)  # pragma: no cover
 
 
     # TODO: gui.main(ui_control_module)

tests/test_common.py

@@ -13,8 +13,10 @@ def test_control_settings(control_setting_name):
     print(name)
 
     assert ControlSetting(name=name, value=np.random.random(), min_value = np.random.random(), max_value = np.random.random())
-    assert ControlSetting(name=name, value=None, min_value = None, max_value = None)
-    assert ControlSetting(name="doesnotexist", value=np.random.random(), min_value = np.random.random(), max_value = np.random.random())
+    with pytest.raises( Exception ):
+        assert ControlSetting(name=name, value=None, min_value = None, max_value = None)  # At least one has to be given
+    with pytest.raises( Exception ):
+        assert ControlSetting(name="doesnotexist", value=np.random.random(), min_value = np.random.random(), max_value = np.random.random()) # Name must exist
 
 
 
@@ -44,8 +46,10 @@ def test_sensor_values():
     sv.__setitem__('loop_counter', new_val)
     assert sv.__getitem__('loop_counter') == new_val
 
-    sv.__setitem__('bla', 12)
-    sv.__getitem__('bla')
+    with pytest.raises( Exception ):
+        sv.__setitem__('bla', 12) # SensorValue must exist
+    with pytest.raises( Exception ):
+        sv.__getitem__('bla')     # Same here, SensorValue must exists
 
 
     vals  = {   ValueName.PIP.name                  : 0,
@@ -56,7 +60,7 @@ def test_sensor_values():
                 ValueName.BREATHS_PER_MINUTE.name   : 0,
                 ValueName.INSPIRATION_TIME_SEC.name : 0,
                 ValueName.FLOWOUT.name              : 0}
-
-    sv = SensorValues(vals=vals)
+    with pytest.raises( Exception ):
+        sv = SensorValues(vals=vals)
     sv = SensorValues(timestamp=None, loop_counter=0, breath_count=0, vals=vals)
     assert sv.timestamp > 0

tests/test_logging.py

@@ -9,6 +9,7 @@
 from pvp.common.message import SensorValues, ControlValues, DerivedValues, ControlSetting
 from pvp.common.values import ValueName
 from pvp.common import values
+from pvp.common.utils import get_version
 
 from pvp import prefs
 prefs.init()
@@ -44,6 +45,12 @@ def test_control_storage(control_setting_name):
     assert control_setting.max_value == tt['control_data']['max_value'][0]
     assert control_setting.timestamp == tt['control_data']['timestamp'][0]
 
+    version_string = tt["program_information"][0][0]
+    version_from_file = version_string.decode()  # for correct unicode vs. byte string comparison
+    real_version = get_version()
+
+    assert version_from_file == real_version
+
 def test_sensor_storage():
 
     # Store stuff