Repository: PedalPi/PluginsManager

Issue:
Connections with sys_effect doesn't work
For test https://gist.github.com/SrMouraSilva/f7a3904f3984bcaec7e15c60039bc3c3

# Error 1

## Description
```python
> sys_effect.outputs[0].connect(reverb.inputs[0])
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/paulo/PycharmProjects/PedalPi-Raspberry/pluginsmanager/pluginsmanager/model/output.py", line 63, in connect
    self.effect.pedalboard.connections.append(Connection(self, effect_input))
AttributeError: 'NoneType' object has no attribute 'connections'
```

## Cause

This error occurs because of the fact that system_effects can not be added to pedalboards

## Other method for use

If this error is occurring with you, the following method works.

```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])
```

#

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/PedalPi/PluginsManager/pull/11?src=pr&el=h1) Report
> Merging [#11](https://codecov.io/gh/PedalPi/PluginsManager/pull/11?src=pr&el=desc) into [master](https://codecov.io/gh/PedalPi/PluginsManager/commit/749de55874f446519f7ebd01aacef38d1b810bb0?src=pr&el=desc) will **increase** coverage by `0.7%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff            @@
##           master      #11     +/-   ##
=========================================
+ Coverage   89.06%   89.76%   +0.7%     
=========================================
  Files          28       28             
  Lines         750      772     +22     
=========================================
+ Hits          668      693     +25     
+ Misses         82       79      -3
```


| [Impacted Files](https://codecov.io/gh/PedalPi/PluginsManager/pull/11?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pluginsmanager/model/system/system_output.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kZWwvc3lzdGVtL3N5c3RlbV9vdXRwdXQucHk=) | `100% <100%> (ø)` | :white_check_mark: |
| [pluginsmanager/model/updates_observer.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kZWwvdXBkYXRlc19vYnNlcnZlci5weQ==) | `100% <100%> (ø)` | :white_check_mark: |
| [pluginsmanager/model/system/system_input.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kZWwvc3lzdGVtL3N5c3RlbV9pbnB1dC5weQ==) | `100% <100%> (ø)` | :white_check_mark: |
| [pluginsmanager/model/output.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kZWwvb3V0cHV0LnB5) | `100% <100%> (ø)` | :white_check_mark: |
| [pluginsmanager/model/input.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kZWwvaW5wdXQucHk=) | `100% <100%> (ø)` | :white_check_mark: |
| [pluginsmanager/mod_host/mod_host.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvbW9kX2hvc3QvbW9kX2hvc3QucHk=) | `78.08% <100%> (-1.63%)` | :x: |
| [pluginsmanager/banks_manager.py](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...8965388b99714afb09dc03d6992ef064b8552ff4?src=pr&el=tree#diff-cGx1Z2luc21hbmFnZXIvYmFua3NfbWFuYWdlci5weQ==) | `100% <ø> (+7.14%)` | :white_check_mark: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PedalPi/PluginsManager/pull/11?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/PedalPi/PluginsManager/pull/11?src=pr&el=footer). Last update [749de55...6ad84c4](https://codecov.io/gh/PedalPi/PluginsManager/compare/749de55874f446519f7ebd01aacef38d1b810bb0...6ad84c46c9c4e1be387c3e872a07eb7149220db9?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/Readme.rst b/Readme.rst
index 4502fd9..b6ebb43 100644
--- a/Readme.rst
+++ b/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
diff --git a/pluginsmanager/mod_host/mod_host.py b/pluginsmanager/mod_host/mod_host.py
index 2933e69..8a38ff6 100644
--- a/pluginsmanager/mod_host/mod_host.py
+++ b/pluginsmanager/mod_host/mod_host.py
@@ -108,19 +108,25 @@ class ModHost(UpdatesObserver):
     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 @@ class ModHost(UpdatesObserver):
         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:
diff --git a/pluginsmanager/model/input.py b/pluginsmanager/model/input.py
index 66a8f73..00290d1 100644
--- a/pluginsmanager/model/input.py
+++ b/pluginsmanager/model/input.py
@@ -39,6 +39,8 @@ class Input(metaclass=ABCMeta):
 
         self.observer = MagicMock()
 
+        self._unique_for_all_pedalboards = False
+
     @property
     def effect(self):
         """
diff --git a/pluginsmanager/model/output.py b/pluginsmanager/model/output.py
index 3429434..2830864 100644
--- a/pluginsmanager/model/output.py
+++ b/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 @@ class Output(metaclass=ABCMeta):
 
         self.observer = MagicMock()
 
+        self._unique_for_all_pedalboards = False
+
     @property
     def effect(self):
         """
@@ -58,9 +60,21 @@ class Output(metaclass=ABCMeta):
             >>> 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 @@ class Output(metaclass=ABCMeta):
             >>> 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
diff --git a/pluginsmanager/model/system/system_input.py b/pluginsmanager/model/system/system_input.py
index a9dc781..1751756 100644
--- a/pluginsmanager/model/system/system_input.py
+++ b/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
diff --git a/pluginsmanager/model/system/system_output.py b/pluginsmanager/model/system/system_output.py
index 6e74db1..ebcdc09 100644
--- a/pluginsmanager/model/system/system_output.py
+++ b/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
diff --git a/pluginsmanager/model/updates_observer.py b/pluginsmanager/model/updates_observer.py
index 31f6da2..8769330 100644
--- a/pluginsmanager/model/updates_observer.py
+++ b/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 patch:
diff --git a/test/banks_manager_test.py b/test/banks_manager_test.py
index 9a5adb9..2b64c5e 100644
--- a/test/banks_manager_test.py
+++ b/test/banks_manager_test.py
@@ -75,3 +75,11 @@ class BanksManagerTest(unittest.TestCase):
 
         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)
diff --git a/test/example.py b/test/example.py
new file mode 100644
index 0000000..6f90063
--- /dev/null
+++ b/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])
diff --git a/test/model/output_test.py b/test/model/output_test.py
index d91b910..7efddc3 100644
--- a/test/model/output_test.py
+++ b/test/model/output_test.py
@@ -6,6 +6,9 @@ from pluginsmanager.model.lv2.lv2_effect_builder import Lv2EffectBuilder
 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 @@ class OutputTest(unittest.TestCase):
             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)
diff --git a/test/model/updates_observer_test.py b/test/model/updates_observer_test.py
new file mode 100644
index 0000000..1608639
--- /dev/null
+++ b/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)