Fix #9 - sys_effect can't connect with other effects (#11)

* Fix sys_effect. Improve banksmanager tests
* Update Readme

Readme.rst

@@ -136,10 +136,7 @@ Connecting *mode one*:
 
 .. code-block:: python
 
-    # For connect output system effects, use
-    pedalboard.connections.append(Connection(sys_effect.outputs[0], reverb.inputs[0]))
-    # Instead of
-    #sys_effect.outputs[0].connect(reverb.inputs[0])
+    sys_effect.outputs[0].connect(reverb.inputs[0])
 
     reverb.outputs[0].connect(fuzz.inputs[0])
     reverb.outputs[1].connect(fuzz.inputs[0])
@@ -163,6 +160,13 @@ Connecting *mode two*:
     pedalboard.connections.append(Connection(reverb2.outputs[0], sys_effect.inputs[0]))
     pedalboard.connections.append(Connection(reverb2.outputs[0], sys_effect.inputs[1]))
 
+.. warning::
+
+    If you need connect system_output with system_input directly (for a bypass, as example), only the
+    second mode will works::
+
+        pedalboard.connections.append(Connection(sys_effect.outputs[0], sys_effect.inputs[0]))
+
 Set effect status (enable/disable bypass) and param value
 
 .. code-block:: python

pluginsmanager/mod_host/mod_host.py

@@ -108,19 +108,25 @@ def pedalboard(self):
     def pedalboard(self, pedalboard):
         self.on_current_pedalboard_changed(pedalboard)
 
+    def __del__(self):
+        self._remove_pedalboard(self.pedalboard)
+
     ####################################
     # Observer
     ####################################
     def on_current_pedalboard_changed(self, pedalboard):
         if self.pedalboard is not None:
-            for effect in self.pedalboard.effects:
-                self.on_effect_updated(effect, UpdateType.DELETED)
+            self._remove_pedalboard(self.pedalboard)
 
         self._pedalboard = pedalboard
 
         for effect in pedalboard.effects:
             self.on_effect_updated(effect, UpdateType.CREATED)
 
+    def _remove_pedalboard(self, pedalboard):
+        for effect in pedalboard.effects:
+            self.on_effect_updated(effect, UpdateType.DELETED)
+
     def on_bank_updated(self, bank, update_type, **kwargs):
         if self.pedalboard is not None \
         and bank != self.pedalboard.bank:
@@ -162,7 +168,7 @@ def on_param_value_changed(self, param):
         self.host.set_param_value(param)
 
     def on_connection_updated(self, connection, update_type):
-        if connection.input.effect.pedalboard != self.pedalboard:
+        if connection not in self.pedalboard.connections:
             return
 
         if update_type == UpdateType.CREATED:

pluginsmanager/model/input.py

@@ -39,6 +39,8 @@ def __init__(self, effect):
 
         self.observer = MagicMock()
 
+        self._unique_for_all_pedalboards = False
+
     @property
     def effect(self):
         """

pluginsmanager/model/output.py

@@ -1,6 +1,6 @@
 from abc import ABCMeta, abstractmethod
 
-from pluginsmanager.model.connection import Connection
+from pluginsmanager.model.connection import Connection, ConnectionError
 
 from unittest.mock import MagicMock
 
@@ -39,6 +39,8 @@ def __init__(self, effect):
 
         self.observer = MagicMock()
 
+        self._unique_for_all_pedalboards = False
+
     @property
     def effect(self):
         """
@@ -58,9 +60,21 @@ def connect(self, effect_input):
             >>> Connection(driver_output, reverb_input) in driver.effect.connections
             True
 
+        .. note::
+
+            This method does not work for all cases.
+            class:`SystemOutput` can not be connected with class:`SystemInput` this way.
+            For this case, use ::
+
+                >>> pedalboard.connections.append(Connection(system_output, system_input))
+
         :param Input effect_input: Input that will be connected with it
         """
-        self.effect.pedalboard.connections.append(Connection(self, effect_input))
+        if self._unique_for_all_pedalboards and effect_input._unique_for_all_pedalboards:
+            error = "Isn't possible connect this way. Please use pedalboard.connect(Connection(output, input))"
+            raise ConnectionError(error)
+
+        effect_input.effect.pedalboard.connections.append(Connection(self, effect_input))
 
     def disconnect(self, effect_input):
         """
@@ -74,8 +88,20 @@ def disconnect(self, effect_input):
             >>> Connection(driver_output, reverb_input) in driver.effect.connections
             False
 
+        .. note::
+
+            This method does not work for all cases.
+            class:`SystemOutput` can not be disconnected with class:`SystemInput` this way.
+            For this case, use ::
+
+                >>> pedalboard.connections.remove(Connection(system_output, system_input))
+
         :param Input effect_input: Input that will be disconnected with it
         """
+        if self._unique_for_all_pedalboards and effect_input._unique_for_all_pedalboards:
+            error = "Isn't possible connect this way. Please use pedalboard.connect(Connection(output, input))"
+            raise ConnectionError(error)
+
         self.effect.pedalboard.connections.remove(Connection(self, effect_input))
 
     @property

pluginsmanager/model/system/system_input.py

@@ -6,6 +6,7 @@ class SystemInput(Input):
     def __init__(self, effect, input):
         super(SystemInput, self).__init__(effect)
         self._input = input
+        self._unique_for_all_pedalboards = True
 
     def __str__(self):
         return self._input

pluginsmanager/model/system/system_output.py

@@ -6,6 +6,7 @@ class SystemOutput(Output):
     def __init__(self, effect, output):
         super(SystemOutput, self).__init__(effect)
         self._output = output
+        self._unique_for_all_pedalboards = True
 
     def __str__(self):
         return self._output

pluginsmanager/model/updates_observer.py

@@ -12,6 +12,17 @@ class UpdatesObserver(metaclass=ABCMeta):
     example of a manager is :class:`BanksManager`.
     """
 
+    def __init__(self):
+        self.manager = None
+
+    def __enter__(self):
+        if self.manager is not None:
+            self.manager.enter_scope(self)
+
+    def __exit__(self, type, value, traceback):
+        if self.manager is not None:
+            self.manager.exit_scope()
+
     @abstractmethod
     def on_bank_updated(self, bank, update_type, **kwargs):
         """

test/banks_manager_test.py

@@ -75,3 +75,11 @@ def test_observers(self):
 
         manager.banks.remove(bank2)
         observer.on_bank_updated.assert_called_with(bank2, UpdateType.DELETED, index=0, origin=manager)
+
+    def test_initial_banks(self):
+        banks = [Bank('Bank 1'), Bank('Bank 1')]
+
+        manager = BanksManager(banks)
+
+        for original_bank, bank_managed in zip(banks, manager.banks):
+            self.assertEqual(original_bank, bank_managed)

test/example.py

@@ -0,0 +1,51 @@
+from pluginsmanager.banks_manager import BanksManager
+from pluginsmanager.mod_host.mod_host import ModHost
+
+from pluginsmanager.model.bank import Bank
+from pluginsmanager.model.pedalboard import Pedalboard
+from pluginsmanager.model.connection import Connection
+
+from pluginsmanager.model.lv2.lv2_effect_builder import Lv2EffectBuilder
+
+from pluginsmanager.model.system.system_effect import SystemEffect
+
+
+if __name__ == '__main__':
+    manager = BanksManager()
+
+    bank = Bank('Bank 1')
+    manager.append(bank)
+
+
+    mod_host = ModHost('raspberrypi.local')
+    mod_host.connect()
+    manager.register(mod_host)
+
+
+    pedalboard = Pedalboard('Rocksmith')
+    bank.append(pedalboard)
+
+    mod_host.pedalboard = pedalboard
+
+    builder = Lv2EffectBuilder()
+
+    reverb = builder.build('http://calf.sourceforge.net/plugins/Reverb')
+    fuzz = builder.build('http://guitarix.sourceforge.net/plugins/gx_fuzz_#fuzz_')
+    reverb2 = builder.build('http://calf.sourceforge.net/plugins/Reverb')
+
+    pedalboard.append(reverb)
+    pedalboard.append(fuzz)
+    pedalboard.append(reverb2)
+
+    sys_effect = SystemEffect('system', ['capture_1'], ['playback_1', 'playback_2'])
+
+    pedalboard.connections.append(Connection(sys_effect.outputs[0], reverb.inputs[0]))
+
+    reverb.outputs[0].connect(fuzz.inputs[0])
+    reverb.outputs[1].connect(fuzz.inputs[0])
+    fuzz.outputs[0].connect(reverb2.inputs[0])
+    reverb.outputs[0].connect(reverb2.inputs[0])
+
+    # Causes error
+    reverb2.outputs[0].connect(sys_effect.inputs[0])
+    reverb2.outputs[0].connect(sys_effect.inputs[1])

test/model/output_test.py

@@ -6,6 +6,9 @@
 from pluginsmanager.model.pedalboard import Pedalboard
 
 from pluginsmanager.model.update_type import UpdateType
+from pluginsmanager.model.system.system_effect import SystemEffect
+
+from pluginsmanager.model.connection import ConnectionError
 
 
 class OutputTest(unittest.TestCase):
@@ -81,3 +84,21 @@ def test_disconnect_connection_not_created(self):
             reverb.outputs[1].disconnect(reverb2.inputs[0])
 
         pedalboard.observer.on_connection_updated.assert_not_called()
+
+    def test_connect_system_effect(self):
+        sys_effect = SystemEffect('system', ['capture_1'], ['playback_1', 'playback_2'])
+
+        effect_output = sys_effect.outputs[0]
+        effect_input = sys_effect.inputs[0]
+
+        with self.assertRaises(ConnectionError):
+            effect_output.connect(effect_input)
+
+    def test_disconnect_system_effect(self):
+        sys_effect = SystemEffect('system', ['capture_1'], ['playback_1', 'playback_2'])
+
+        effect_output = sys_effect.outputs[0]
+        effect_input = sys_effect.inputs[0]
+
+        with self.assertRaises(ConnectionError):
+            effect_output.disconnect(effect_input)

test/model/updates_observer_test.py

@@ -0,0 +1,52 @@
+import unittest
+from unittest.mock import MagicMock
+
+from pluginsmanager.banks_manager import BanksManager
+from pluginsmanager.model.bank import Bank
+from pluginsmanager.model.update_type import UpdateType
+from pluginsmanager.model.updates_observer import UpdatesObserver
+
+
+class Observer(UpdatesObserver):
+
+    def on_bank_updated(self, bank, update_type, **kwargs):
+        pass
+
+    def on_pedalboard_updated(self, pedalboard, update_type, **kwargs):
+        pass
+
+    def on_effect_updated(self, effect, update_type, **kwargs):
+        pass
+
+    def on_effect_status_toggled(self, effect):
+        pass
+
+    def on_param_value_changed(self, param):
+        pass
+
+    def on_connection_updated(self, connection, update_type):
+        pass
+
+
+class UpdatesObserverTest(unittest.TestCase):
+
+    def test_notify(self):
+        observer1 = Observer()
+        observer1.on_bank_updated = MagicMock()
+        observer2 = Observer()
+        observer2.on_bank_updated = MagicMock()
+        observer3 = Observer()
+        observer3.on_bank_updated = MagicMock()
+
+        manager = BanksManager()
+        manager.register(observer1)
+        manager.register(observer2)
+        manager.register(observer3)
+
+        bank = Bank('Bank 1')
+        with observer1:
+            manager.banks.append(bank)
+
+        observer1.on_bank_updated.assert_not_called()
+        observer2.on_bank_updated.assert_called_with(bank, UpdateType.CREATED, index=0, origin=manager)
+        observer3.on_bank_updated.assert_called_with(bank, UpdateType.CREATED, index=0, origin=manager)