Merge pull request #146 from wehr-lab/jackd_triggers

Jackd triggers

Improvements to sound timing and stability!!! The major change here is in the sound modules
- Changed the way that continuous sounds work. Rather than cycling through an array, which was easy to drop, now pass a sound object that can generate its own samples on the fly using the `hydration` module. 
- More accurate timing of sound ending callbacks. Before, the event would be called immediately on buffering the sounds into the jack ports, but that was systematically too early. Instead, use jack timing methods to account for delay from blocksize and n_periods to wait_until a certain delay to `set()` the event. See `_wait_for_end`

# New
- `hydration` module for creating and storing autopilot objects between processes and computers!
- `@Introspect` made and added to sound classes. Will be moved to root class. Allows storing the parameters given on instantiation.
- minor - added exceptions module

# Improvements
- Made `ALSA_NPERIODS` its own pref
- More debugging flags!
- Changed `repeat` to false by default in Nodes because it is largely unnecessary in normal use and causes a lot of overhead!
- Made a method to debug timing within the sound server, pass `debug_timing=True` to `jackclient`
- `Noise` now can generate noise continuously as a model for refactoring sound classes to do that as the main  way of doing so in the future.
- Incremental improvements to `requires` classes.

# Bugfixes
- Fixed several lists that would grow indefinitely and cause hard to diagnose memory issues, particularly with continuously operating things like the sound server or streaming data. Using a deque 
- `__del__` methods cause more problems than they solve, need to start moving towards registering signal handles explicitly rather than trying to clean up as a last resort when removing from memory. Removed from hardware and networking classes or at least wrapped in try blocks for now.
- use `str(Path)` rather than passing a `Path` instance to `pkgutil.iter_modules`
- use `importlib-metadata` for python3.7

# Deprecations
- Removed `jack_apt` because it only causes problems. `jackd_source` works every time and doesn't take that long.
-

Author: sneakers-the-rat

autopilot/__init__.py

@@ -2,4 +2,5 @@
 __version__ = '0.4.3'
 
 from autopilot.setup import setup_autopilot
-from autopilot.utils.registry import get, get_task, get_hardware, get_names
+from autopilot.utils.registry import get, get_task, get_hardware, get_names
+from autopilot.utils.hydration import dehydrate, hydrate

autopilot/core/pilot.py

@@ -791,6 +791,7 @@ def run_task(self, task_class, task_params):
                 self.task.end()
             except Exception as e:
                 self.logger.exception(f'got exception while stopping task: {e}')
+            del self.task
             self.task = None
             row.append()
             table.flush()

autopilot/exceptions.py

@@ -0,0 +1,11 @@
+"""
+Custom warnings and exceptions for better testing and diagnosis!
+"""
+
+import warnings
+
+class DefaultPrefWarning(UserWarning):
+    """
+    Warn that a default pref value is being accessed
+    """
+

autopilot/external/__init__.py

@@ -110,6 +110,10 @@ def start_jackd():
     if prefs.get('FS'):
         jackd_string = jackd_string.replace('-rfs', f"-r{prefs.get('FS')}")
 
+    # replace string nperiods with number
+    if prefs.get('ALSA_NPERIODS'):
+        jackd_string = jackd_string.replace('-nper', f"-n{prefs.get('ALSA_NPERIODS')}")
+
     # construct rest of launch string!
     # if JACKD_MODULE:
     #     jackd_path = os.path.join(autopilot_jack.__path__._path[0])

autopilot/hardware/__init__.py

@@ -82,9 +82,6 @@ class Hardware(object):
 
     Primarily for the purpose of defining necessary attributes.
 
-    Also defines `__del__` to call `release()` so objects are always released
-    even if not explicitly.
-
     Attributes:
         name (str): unique name used to identify this object within its group.
         group (str): hardware group, corresponds to key in prefs.json ``"HARDWARE": {"GROUP": {"ID": {**params}}}``
@@ -250,7 +247,3 @@ def calibration(self, calibration):
             self.logger.info(f'Calibration saved to {cal_fn}: \n{calibration}')
 
         self._calibration = calibration
-
-    def __del__(self):
-        self.release()
-

autopilot/hardware/gpio.py

@@ -24,7 +24,7 @@
 import itertools
 import typing
 import warnings
-
+from collections import deque as dq
 
 from autopilot import prefs
 from autopilot.hardware import Hardware, BOARD_TO_BCM
@@ -310,6 +310,7 @@ def release(self):
         Note:
             the Hardware metaclass will call this method on object deletion.
         """
+        self.logger.debug('releasing')
         try:
             self.pull = None
         except:
@@ -697,6 +698,7 @@ def release(self):
         """
         Stops and deletes all scripts, sets to :attr:`~.Digital_Out.off`, and calls :meth:`.GPIO.release`
         """
+        self.logger.debug('releasing')
         try:
 
             self.delete_all_scripts()
@@ -723,6 +725,7 @@ class Digital_In(GPIO):
             set this event whenever the callback is triggered. Can be used to handle
             stage transition logic here instead of the :class:`.Task` object, as is typical.
         record (bool): Whether all logic transitions should be recorded as a list of ('EVENT', 'Timestamp') tuples.
+        max_events (int): Maximum size of the :attr:`.events` deque
         **kwargs: passed to :class:`GPIO`
 
     Sets the internal pullup/down resistor to :attr:`.Digital_In.off` and
@@ -733,19 +736,18 @@ class Digital_In(GPIO):
         pull and trigger are set by polarity on initialization in digital inputs, unlike other GPIO classes.
         They are not mutually synchronized however, ie. after initialization if any one of these attributes are changed, the other two will remain the same.
 
-
     Attributes:
         pig (:meth:`pigpio.pi`): The pigpio connection.
         pin (int): Broadcom-numbered pin, converted from the argument given on instantiation
         callbacks (list): A list of :meth:`pigpio.callback`s kept to clear them on exit
         polarity (int): Logic direction, if 1: off=0, on=1, pull=low, trigger=high and vice versa for 0
-        events (list): if :attr:`.record` is True, a list of ('EVENT', 'TIMESTAMP') tuples
+        events (list): if :attr:`.record` is True, a deque of ('EVENT', 'TIMESTAMP') tuples of length ``max_events``
     """
     is_trigger=True
     type = 'DIGI_IN'
     input = True
 
-    def __init__(self, pin, event=None, record=True, **kwargs):
+    def __init__(self, pin, event=None, record=True, max_events=256, **kwargs):
         """
 
         """
@@ -763,8 +765,7 @@ def __init__(self, pin, event=None, record=True, **kwargs):
         self.callbacks = []
 
         # List to store logic transition events
-        # FIXME: Should be a deque
-        self.events = []
+        self.events = dq(maxlen=max_events)
 
         self.record = record
         if self.record:
@@ -853,6 +854,7 @@ def release(self):
         """
         Clears any callbacks and calls :meth:`GPIO.release`
         """
+        self.logger.debug('releasing')
         self.clear_cb()
         super(Digital_In, self).release()
 
@@ -983,6 +985,7 @@ def release(self):
 
         """
         # FIXME: reimplementing parent release method here because of inconsistent use of self.off -- unify API and fix!!
+        self.logger.debug('releasing')
         try:
             self.delete_all_scripts()
             self.set(0) # clean values should handle inversion, don't use self.off
@@ -1257,6 +1260,7 @@ def release(self):
         """
         Release each channel and stop pig without calling superclass.
         """
+        self.logger.debug('releasing')
         for chan in self.channels.values():
             chan.release()
         self.pig.stop()

autopilot/networking/node.py

@@ -234,7 +234,7 @@ def send(self, to: Optional[Union[str, list]] = None,
              key:str=None,
              value:typing.Any=None,
              msg:Optional['Message']=None,
-             repeat:bool=True, flags = None, force_to:bool = False):
+             repeat:bool=False, flags = None, force_to:bool = False):
         """
         Send a message via our :attr:`~.Net_Node.sock` , DEALER socket.
 

autopilot/networking/station.py

@@ -126,7 +126,7 @@ def __init__(self,
             self.ip = ""
 
 
-        self.file_block = threading.Event() # to wait for file transfer
+        self.file_block = multiprocessing.Event() # to wait for file transfer
 
         # number messages as we send them
         self.msg_counter = count()
@@ -146,7 +146,10 @@ def __init__(self,
 
 
     def __del__(self):
-        self.release()
+        try:
+            self.release()
+        except:
+            pass
 
     def run(self):
         """
@@ -289,7 +292,6 @@ def send(self, to=None, key=None, value=None, msg=None, repeat=True, flags=None)
         msg_enc = msg.serialize()
 
         if not msg_enc:
-            #set_trace(term_size=(80,40))
             self.logger.exception('Message could not be encoded:\n{}'.format(str(msg)))
             return
 

autopilot/prefs.py

@@ -82,6 +82,7 @@
 
 #from autopilot.core.loggers import init_logger
 from collections import OrderedDict as odict
+from autopilot.exceptions import DefaultPrefWarning
 
 class Scopes(Enum):
     """
@@ -395,10 +396,17 @@ class Scopes(Enum):
         'depends': 'AUDIOSERVER',
         "scope": Scopes.AUDIO
     },
+    'ALSA_NPERIODS': {
+        'type': 'int',
+        'text': 'number of buffer periods to use with ALSA sound driver',
+        'default': 3,
+        'depends': 'AUDIOSERVER',
+        'scope': Scopes.AUDIO
+    },
     'JACKDSTRING': {
         'type': 'str',
         'text': 'Arguments to pass to jackd, see the jackd manpage',
-        'default': 'jackd -P75 -p16 -t2000 -dalsa -dhw:sndrpihifiberry -P -rfs -n3 -s &',
+        'default': 'jackd -P75 -p16 -t2000 -dalsa -dhw:sndrpihifiberry -P -rfs -nper -s &',
         'depends': 'AUDIOSERVER',
         "scope": Scopes.AUDIO
     },
@@ -475,7 +483,7 @@ def get(key: typing.Union[str, None] = None):
                 default_val = globals()['_DEFAULTS'][key]['default']
                 if key not in globals()['_WARNED']:
                     globals()['_WARNED'].append(key)
-                    warnings.warn(f'Returning default prefs value {key} : {default_val} (ideally this shouldnt happen and everything should be specified in prefs', UserWarning)
+                    warnings.warn(f'Returning default prefs value {key} : {default_val} (ideally this shouldnt happen and everything should be specified in prefs', DefaultPrefWarning)
                 return default_val
 
             # if you still can't find a value, None is an unambiguous signal for pref not set
@@ -594,15 +602,24 @@ def init(fn=None):
         cal_raw = os.path.join(prefs['BASEDIR'], 'port_calibration.json')
 
         if os.path.exists(cal_path):
-            with open(cal_path, 'r') as calf:
-                cal_fns = json.load(calf)
-            prefs['PORT_CALIBRATION'] = cal_fns
+            try:
+                with open(cal_path, 'r') as calf:
+                    cal_fns = json.load(calf)
+                prefs['PORT_CALIBRATION'] = cal_fns
+            except json.decoder.JSONDecodeError:
+                warnings.warn(f'calibration file was malformed. Renaming to avoid using in the future')
+                os.rename(cal_path, cal_path + '.bak')
         elif os.path.exists(cal_raw):
             # aka raw calibration results exist but no fit has been computed
-            luts = compute_calibration(path=cal_raw, do_return=True)
-            with open(cal_path, 'w') as calf:
-                json.dump(luts, calf)
-            prefs['PORT_CALIBRATION'] = luts
+            try:
+                luts = compute_calibration(path=cal_raw, do_return=True)
+                with open(cal_path, 'w') as calf:
+                    json.dump(luts, calf)
+                prefs['PORT_CALIBRATION'] = luts
+            except json.decoder.JSONDecodeError:
+                warnings.warn(f'processed calibration file was malformed. Renaming to avoid using in the future')
+                os.rename(cal_raw, cal_raw + '.bak')
+
 
     ###########################
 

autopilot/setup/__init__.py

@@ -0,0 +1 @@
+from autopilot.setup.scripts import SCRIPTS