[Fix #49] Rework MonitorObserver API

Deprecate "event_handler" in favour of "callback" argument.

CHANGES.rst

@@ -7,12 +7,15 @@
   :attr:`pyudev.Monitor.poll`.
 - #47: Deprecate :attr:`pyudev.Monitor.enable_receiving` in favor of
   :attr:`pyudev.Monitor.start`.
+- #49: Deprecate ``event_handler`` to :class:`pyudev.MonitorObserver` in favour
+  of ``callback`` argument.
 - #46: Continuously test pyudev on Travis-CI.
 - Add :attr:`pyudev.Device.ancestors`.
 - Add :attr:`pyudev.Device.action`.
 - #10: Add :attr:`pyudev.Device.sequence_number`.
 - #47: Add :meth:`pyudev.Monitor.poll`.
 - #47: Add :attr:`pyudev.Monitor.started`.
+- #49: Add ``callback`` argument to :class:`pyudev.Monitor`.
 - :meth:`pyudev.Monitor.start` can be called repeatedly.
 - #45: Get rid of 2to3
 - #43: Fix test failures on Python 2.6

pyudev/monitor.py

@@ -454,32 +454,45 @@ class MonitorObserver(Thread):
        :meth:`Monitor.start()` is implicitly called when the thread is started.
     """
 
-    def __init__(self, monitor, event_handler, *args, **kwargs):
+    def __init__(self, monitor, event_handler=None, callback=None, *args,
+                 **kwargs):
         """
         Create a new observer for the given ``monitor``.
 
-        ``monitor`` is the :class:`Monitor` to observe.  ``event_handler`` is a
-        callable with the signature ``event_handler(action, device)``, where
-        ``action`` is a string describing the event (see
-        :attr:`Device.action`), and ``device`` is the :class:`Device` object
-        that caused this event.  This callable is invoked for every device
-        event received through ``monitor``.
+        ``monitor`` is the :class:`Monitor` to observe. ``callback`` is the
+        callable to invoke on events, with the signature ``callback(device)``
+        where ``device`` is the :class:`Device` that caused the event.
 
         .. warning::
 
-           ``event_handler`` is always invoked in this background thread, and
-           *not* in the calling thread.
+           ``callback`` is invoked in the observer thread, hence the observer
+           is blocked while callback executes.
 
-        ``args`` and ``kwargs`` are passed unchanged to the parent constructor
-        of :class:`~threading.Thread`.
+        ``args`` and ``kwargs`` are passed unchanged to the constructor of 
+        :class:`~threading.Thread`.
+
+        .. deprecated:: 0.16
+           The ``event_handler`` argument will be removed in 1.0. Use
+           ``callback`` instead.
+        .. versionchanged:: 0.16
+           Add ``callback`` argument.
         """
-        Thread.__init__(self, *args, **kwargs)
+        if callback is None and event_handler is None:
+            raise ValueError('callback missing')
+        elif callback is not None and event_handler is not None:
+            raise ValueError('Use either callback or event handler')
 
+        Thread.__init__(self, *args, **kwargs)
         self.monitor = monitor
         # observer threads should not keep the interpreter alive
         self.daemon = True
         self._stop_event_source, self._stop_event_sink = os.pipe()
-        self._handle_event = event_handler
+        if event_handler is not None:
+            import warnings
+            warnings.warn('"event_handler" argument will be removed in 1.0. '
+                          'Use Monitor.poll() instead.', DeprecationWarning)
+            callback = lambda d: event_handler(d.action, d)
+        self._callback = callback
 
     def run(self):
         self.monitor.start()
@@ -498,7 +511,7 @@ def run(self):
                     else:
                         device = self.monitor.poll(timeout=0)
                         if device:
-                            self._handle_event(device.action, device)
+                            self._callback(device)
 
     def send_stop(self):
         """

tests/test_monitor.py

@@ -298,13 +298,22 @@ def test_iter(self, monitor):
 
 class TestMonitorObserver(object):
 
-    def receive_event(self, action, device):
+    def callback(self, device):
+        self.events.append(device)
+        if len(self.events) >= 2:
+            self.observer.send_stop()
+
+    def event_handler(self, action, device):
         self.events.append((action, device))
         if len(self.events) >= 2:
             self.observer.send_stop()
 
-    def make_observer(self, monitor):
-        self.observer = MonitorObserver(monitor, self.receive_event)
+    def make_observer(self, monitor, use_deprecated=False):
+        if use_deprecated:
+            self.observer = pytest.deprecated_call(
+                MonitorObserver, monitor, event_handler=self.event_handler)
+        else:
+            self.observer = MonitorObserver(monitor, callback=self.callback)
         return self.observer
 
     def setup(self):
@@ -313,19 +322,30 @@ def setup(self):
     def teardown(self):
         self.events = None
 
+    def test_deprecated_handler(self, fake_monitor, fake_monitor_device):
+        observer = self.make_observer(fake_monitor, use_deprecated=True)
+        observer.start()
+        fake_monitor.trigger_event()
+        fake_monitor.trigger_event()
+        # wait a second for the tests to finish, and kill the observer if
+        # it is still alive then
+        observer.join(1)
+        if observer.is_alive():
+            observer.stop()
+        assert self.events == [(None, fake_monitor_device)] * 2
+
     def test_fake(self, fake_monitor, fake_monitor_device):
         observer = self.make_observer(fake_monitor)
         observer.start()
         fake_monitor.trigger_event()
         fake_monitor.trigger_event()
-        # fake one second for the tests to finish
+        # wait a second for the tests to finish
         observer.join(1)
         # forcibly quit the thread if it is still alive
         if observer.is_alive():
             observer.stop()
         # check that we got two events
-        assert self.events == [(None, fake_monitor_device),
-                               (None, fake_monitor_device)]
+        assert self.events == [fake_monitor_device] * 2
 
     @pytest.mark.privileged
     def test_real(self, context, monitor):
@@ -339,7 +359,6 @@ def test_real(self, context, monitor):
         observer.join(2)
         if observer.is_alive():
             observer.stop()
-        assert [e[0] for e in self.events] == ['add', 'remove']
-        assert [e[1].action for e in self.events] == ['add', 'remove']
-        for _, device in self.events:
+        assert [d.action for d in self.events] == ['add', 'remove']
+        for device in self.events:
             assert device.device_path == '/devices/virtual/net/dummy0'