Pin event timing (fixes #664)

Makefile

@@ -113,7 +113,7 @@ develop: tags
 	$(PIP) install -e .[doc,test]
 
 test:
-	$(COVERAGE) run -m $(PYTEST) tests -v -r sx
+	$(COVERAGE) run --rcfile coverage.cfg -m $(PYTEST) tests -v -r sx
 	$(COVERAGE) report --rcfile coverage.cfg
 
 clean:

debian/rules

@@ -17,10 +17,6 @@ override_dh_auto_install:
 override_dh_auto_test:
 	# Don't run the tests!
 
-#override_dh_installdocs:
-#	python setup.py build_sphinx -b html
-#	dh_installdocs
-
 override_dh_auto_build:
 	dh_auto_build
 	PYTHONPATH=. sphinx-build -N -bhtml docs/ build/html

gpiozero/boards.py

@@ -158,16 +158,16 @@ def __init__(self, *args, **kwargs):
                 for name, pin in kwargs.items()
                 })
         def get_new_handler(device):
-            def fire_both_events():
-                device._fire_events()
-                self._fire_events()
+            def fire_both_events(ticks, state):
+                device._fire_events(ticks, device._state_to_value(state))
+                self._fire_events(ticks, self.value)
             return fire_both_events
         for button in self:
             button.pin.when_changed = get_new_handler(button)
         self._when_changed = None
         self._last_value = None
         # Call _fire_events once to set initial state of events
-        self._fire_events()
+        self._fire_events(self.pin_factory.ticks(), None)
         self.hold_time = hold_time
         self.hold_repeat = hold_repeat
 
@@ -191,10 +191,9 @@ def _fire_changed(self):
         if self.when_changed:
             self.when_changed()
 
-    def _fire_events(self):
-        super(ButtonBoard, self)._fire_events()
-        old_value = self._last_value
-        new_value = self._last_value = self.value
+    def _fire_events(self, ticks, new_value):
+        super(ButtonBoard, self)._fire_events(ticks, new_value)
+        old_value, self._last_value = self._last_value, new_value
         if old_value is None:
             # Initial "indeterminate" value; don't do anything
             pass

gpiozero/input_devices.py

@@ -105,13 +105,16 @@ def __init__(
         try:
             self.pin.bounce = bounce_time
             self.pin.edges = 'both'
-            self.pin.when_changed = self._fire_events
+            self.pin.when_changed = self._pin_changed
             # Call _fire_events once to set initial state of events
-            self._fire_events()
+            self._fire_events(self.pin_factory.ticks(), self.is_active)
         except:
             self.close()
             raise
 
+    def _pin_changed(self, ticks, state):
+        self._fire_events(ticks, self._state_to_value(state))
+
 
 class SmoothedInputDevice(EventsMixin, InputDevice):
     """
@@ -540,10 +543,11 @@ def __init__(
         )
         try:
             self._charge_time_limit = charge_time_limit
+            self._charge_time = None
             self._charged = Event()
             self.pin.edges = 'rising'
             self.pin.bounce = None
-            self.pin.when_changed = self._charged.set
+            self.pin.when_changed = self._cap_charged
             self._queue.start()
         except:
             self.close()
@@ -553,20 +557,27 @@ def __init__(
     def charge_time_limit(self):
         return self._charge_time_limit
 
+    def _cap_charged(self, ticks, state):
+        self._charge_time = ticks
+        self._charged.set()
+
     def _read(self):
         # Drain charge from the capacitor
         self.pin.function = 'output'
         self.pin.state = False
         sleep(0.1)
         # Time the charging of the capacitor
-        start = time()
+        start = self.pin_factory.ticks()
+        self._charge_time = None
         self._charged.clear()
         self.pin.function = 'input'
         self._charged.wait(self.charge_time_limit)
-        return (
-            1.0 - min(self.charge_time_limit, time() - start) /
-            self.charge_time_limit
-        )
+        if self._charge_time is None:
+            return 0.0
+        else:
+            return 1.0 - (
+                self.pin_factory.ticks_diff(self._charge_time, start) /
+                self.charge_time_limit)
 
 LightSensor.light_detected = LightSensor.is_active
 LightSensor.when_light = LightSensor.when_activated
@@ -661,7 +672,7 @@ class DistanceSensor(SmoothedInputDevice):
     ECHO_LOCK = Lock()
 
     def __init__(
-            self, echo=None, trigger=None, queue_len=10, max_distance=1,
+            self, echo=None, trigger=None, queue_len=9, max_distance=1,
             threshold_distance=0.3, partial=False, pin_factory=None):
         if max_distance <= 0:
             raise ValueError('invalid maximum distance (must be positive)')
@@ -753,12 +764,12 @@ def echo(self):
         """
         return self.pin
 
-    def _echo_changed(self):
-        if self._echo_rise is None:
-            self._echo_rise = time()
+    def _echo_changed(self, ticks, level):
+        if level:
+            self._echo_rise = ticks
         else:
-            self._echo_fall = time()
-        self._echo.set()
+            self._echo_fall = ticks
+            self._echo.set()
 
     def _read(self):
         # Make sure the echo pin is low then ensure the echo event is clear
@@ -772,22 +783,21 @@ def _read(self):
             self._trigger.pin.state = True
             sleep(0.00001)
             self._trigger.pin.state = False
-            # Wait up to 1 second for the echo pin to rise
+            # Wait up to 1 second for the echo pin to rise and fall (35ms is
+            # the maximum pulse time, but the time before rise is unspecified
+            # in the "datasheet"; 1 second seems sufficiently long to conclude
+            # something has failed).
             if self._echo.wait(1):
-                self._echo.clear()
-                # Wait up to 40ms for the echo pin to fall (35ms is maximum
-                # pulse time so any longer means something's gone wrong).
-                # Calculate distance as time for echo multiplied by speed of
-                # sound divided by two to compensate for travel to and from the
-                # reflector
-                if self._echo.wait(0.04) and self._echo_fall is not None and self._echo_rise is not None:
-                    distance = (self._echo_fall - self._echo_rise) * self.speed_of_sound / 2.0
+                if self._echo_fall is not None and self._echo_rise is not None:
+                    distance = (
+                        self.pin_factory.ticks_diff(self._echo_fall, self._echo_rise) *
+                        self.speed_of_sound / 2.0)
                     self._echo_fall = None
                     self._echo_rise = None
                     return min(1.0, distance / self._max_distance)
                 else:
-                    # If we only saw one edge it means we missed the echo
-                    # because it was too fast; report minimum distance
+                    # If we only saw the falling edge it means we missed the
+                    # echo because it was too fast; report minimum distance
                     return 0.0
             else:
                 # The echo pin never rose or fell; something's gone horribly

gpiozero/internal_devices.py

@@ -53,7 +53,7 @@ class PingServer(InternalDevice):
     def __init__(self, host):
         self.host = host
         super(PingServer, self).__init__()
-        self._fire_events()
+        self._fire_events(self.pin_factory.ticks(), None)
 
     def __repr__(self):
         return '<gpiozero.PingServer host="%s">' % self.host
@@ -123,7 +123,7 @@ def __init__(self, sensor_file='/sys/class/thermal/thermal_zone0/temp',
         self.min_temp = min_temp
         self.max_temp = max_temp
         self.threshold = threshold
-        self._fire_events()
+        self._fire_events(self.pin_factory.ticks(), None)
 
     def __repr__(self):
         return '<gpiozero.CPUTemperature temperature=%.2f>' % self.temperature
@@ -155,6 +155,7 @@ def is_active(self):
         """
         return self.temperature > self.threshold
 
+
 class LoadAverage(InternalDevice):
     """
     Extends :class:`InternalDevice` to provide a device which is active when
@@ -209,7 +210,7 @@ def __init__(self, load_average_file='/proc/loadavg', min_load_average=0.0,
             15: 2,
         }[minutes]
         super(LoadAverage, self).__init__()
-        self._fire_events()
+        self._fire_events(self.pin_factory.ticks(), None)
 
     def __repr__(self):
         return '<gpiozero.LoadAverage load average=%.2f>' % self.load_average
@@ -281,7 +282,7 @@ def __init__(self, start_time, end_time, utc=True):
         self.start_time = start_time
         self.end_time = end_time
         self.utc = utc
-        self._fire_events()
+        self._fire_events(self.pin_factory.ticks(), None)
 
     def __repr__(self):
         return '<gpiozero.TimeOfDay active between %s and %s %s>' % (

gpiozero/mixins.py

@@ -12,7 +12,6 @@
 from functools import wraps, partial
 from threading import Event
 from collections import deque
-from time import time
 try:
     from statistics import median
 except ImportError:
@@ -168,8 +167,8 @@ def __init__(self, *args, **kwargs):
         self._inactive_event = Event()
         self._when_activated = None
         self._when_deactivated = None
-        self._last_state = None
-        self._last_changed = time()
+        self._last_active = None
+        self._last_changed = self.pin_factory.ticks()
 
     def wait_for_active(self, timeout=None):
         """
@@ -240,7 +239,8 @@ def active_time(self):
         When the device is inactive, this is ``None``.
         """
         if self._active_event.is_set():
-            return time() - self._last_changed
+            return self.pin_factory.ticks_diff(self.pin_factory.ticks(),
+                                               self._last_changed)
         else:
             return None
 
@@ -251,7 +251,8 @@ def inactive_time(self):
         When the device is active, this is ``None``.
         """
         if self._inactive_event.is_set():
-            return time() - self._last_changed
+            return self.pin_factory.ticks_diff(self.pin_factory.ticks(),
+                                               self._last_changed)
         else:
             return None
 
@@ -307,19 +308,21 @@ def _fire_deactivated(self):
         if self.when_deactivated:
             self.when_deactivated()
 
-    def _fire_events(self):
-        old_state = self._last_state
-        new_state = self._last_state = self.is_active
-        if old_state is None:
+    def _fire_events(self, ticks, new_active):
+        # NOTE: in contrast to the pin when_changed event, this method takes
+        # ticks and *is_active* (i.e. the device's .is_active) as opposed to a
+        # pin's *state*.
+        old_active, self._last_active = self._last_active, new_active
+        if old_active is None:
             # Initial "indeterminate" state; set events but don't fire
             # callbacks as there's not necessarily an edge
-            if new_state:
+            if new_active:
                 self._active_event.set()
             else:
                 self._inactive_event.set()
-        elif old_state != new_state:
-            self._last_changed = time()
-            if new_state:
+        elif old_active != new_active:
+            self._last_changed = ticks
+            if new_active:
                 self._inactive_event.clear()
                 self._active_event.set()
                 self._fire_activated()
@@ -431,7 +434,8 @@ def held_time(self):
         this is ``None``.
         """
         if self._held_from is not None:
-            return time() - self._held_from
+            return self.pin_factory.ticks_diff(self.pin_factory.ticks(),
+                                               self._held_from)
         else:
             return None
 
@@ -458,7 +462,7 @@ def held(self, parent):
                             parent._inactive_event.wait(parent.hold_time)
                             ):
                         if parent._held_from is None:
-                            parent._held_from = time()
+                            parent._held_from = parent.pin_factory.ticks()
                         parent._fire_held()
                         if not parent.hold_repeat:
                             break
@@ -508,7 +512,7 @@ def fill(self):
                 if not self.full.is_set() and len(self.queue) >= self.queue.maxlen:
                     self.full.set()
                 if (self.partial or self.full.is_set()) and isinstance(self.parent, EventsMixin):
-                    self.parent._fire_events()
+                    self.parent._fire_events(self.parent.pin_factory.ticks(), self.parent.is_active)
         except ReferenceError:
             # Parent is dead; time to die!
             pass

gpiozero/pins/__init__.py

@@ -31,8 +31,13 @@
 class Factory(object):
     """
     Generates pins and SPI interfaces for devices. This is an abstract
-    base class for pin factories. Descendents *may* override the following
-    methods, if applicable:
+    base class for pin factories. Descendents *must* override the following
+    methods:
+
+    * :meth:`ticks`
+    * :meth:`ticks_diff`
+
+    Descendents *may* override the following methods, if applicable:
 
     * :meth:`close`
     * :meth:`reserve_pins`
@@ -123,6 +128,28 @@ def spi(self, **spi_args):
         """
         raise PinSPIUnsupported('SPI not supported by this pin factory')
 
+    def ticks(self):
+        """
+        Return the current ticks, according to the factory. The reference point
+        is undefined and thus the result of this method is only meaningful when
+        compared to another value returned by this method.
+
+        The format of the time is also arbitrary, as is whether the time wraps
+        after a certain duration. Ticks should only be compared using the
+        :meth:`ticks_diff` method.
+        """
+        raise NotImplementedError
+
+    def ticks_diff(self, later, earlier):
+        """
+        Return the time in seconds between two :meth:`ticks` results. The
+        arguments are specified in the same order as they would be in the
+        formula *later* - *earlier* but the result is guaranteed to be in
+        seconds, and to be positive even if the ticks "wrapped" between calls
+        to :meth:`ticks`.
+        """
+        raise NotImplementedError
+
     def _get_pi_info(self):
         return None
 
@@ -389,7 +416,20 @@ def _set_when_changed(self, value):
         doc="""\
         A function or bound method to be called when the pin's state changes
         (more specifically when the edge specified by :attr:`edges` is detected
-        on the pin). The function or bound method must take no parameters.
+        on the pin). The function or bound method must accept two parameters:
+        the first will report the ticks (from :meth:`Factory.ticks`) when
+        the pin's state changed, and the second will report the pin's current
+        state.
+
+        .. warning::
+
+            Depending on hardware support, the state is *not guaranteed to be
+            accurate*. For instance, many GPIO implementations will provide
+            an interrupt indicating when a pin's state changed but not what it
+            changed to. In this case the pin driver simply reads the pin's
+            current state to supply this parameter, but the pin's state may
+            have changed *since* the interrupt. Exercise appropriate caution
+            when relying upon this parameter.
 
         If the pin does not support edge detection, attempts to set this
         property will raise :exc:`PinEdgeDetectUnsupported`.
@@ -693,5 +733,3 @@ def _set_bits_per_word(self, value):
 
         Several implementations do not support non-byte-sized words.
         """)
-
-