Repository: missionpinball/mpf

Issue:
Add ball routing device
Add a device which can route a ball when it has been captured by a certain device to a certain other device or playfield.

Acceptance Criteria:
a) This logic should trigger when a defined ball device captures a ball and nobody claims it
b) This should work at ball or player turn start
c) It should be possible to eject into a lock (normally the lock would not consider the ball as unclaimed)

Discussion:

Patch:
diff --git a/mpf/config_spec.yaml b/mpf/config_spec.yaml
index 9772cb31c..91bebd9d6 100644
--- a/mpf/config_spec.yaml
+++ b/mpf/config_spec.yaml
@@ -176,6 +176,12 @@ ball_locks:
     reset_events: event_handler|str:ms|machine_reset_phase_3, ball_starting, ball_will_end, service_mode_entered
     release_one_events: event_handler|str:ms|None
     release_one_if_full_events: event_handler|str:ms|None
+ball_routings:
+    __valid_in__: mode
+    source_devices: list|machine(ball_devices)|
+    target_device: single|machine(ball_devices)|
+    enable_events: event_handler|str:ms|None
+    disable_events: event_handler|str:ms|None
 ball_saves:
     __valid_in__: machine, mode
     source_playfield: single|machine(ball_devices)|playfield
@@ -1232,8 +1238,6 @@ snux:
     platform: single|str|None
     console_log: single|enum(none,basic,full)|none
     file_log: single|enum(none,basic,full)|basic
-    prefer_a_side_event: single|str|game_ended
-    prefer_c_side_event: single|str|game_will_start
 smartmatrix:
     __valid_in__: machine
     port: single|str|
diff --git a/mpf/core/mode_device.py b/mpf/core/mode_device.py
index 47c7c6ee1..4178e8fc1 100644
--- a/mpf/core/mode_device.py
+++ b/mpf/core/mode_device.py
@@ -54,7 +54,7 @@ class ModeDevice(Device, metaclass=abc.ABCMeta):
         del config
         raise AssertionError("Device {} cannot be overloaded.".format(self))
 
-    def enable(self, **kwarg) -> None:
+    def enable(self, **kwargs) -> None:
         """Enable handler."""
         pass
 
diff --git a/mpf/devices/ball_device/ball_device.py b/mpf/devices/ball_device/ball_device.py
index e92737299..8621ddfb8 100644
--- a/mpf/devices/ball_device/ball_device.py
+++ b/mpf/devices/ball_device/ball_device.py
@@ -143,7 +143,9 @@ class BallDevice(SystemWideDevice):
     def expected_ball_received(self):
         """Handle an expected ball."""
         # post enter event
-        yield from self._post_enter_event(unclaimed_balls=0)
+        unclaimed_balls = yield from self._post_enter_event(unclaimed_balls=0)
+        # there might still be unclaimed balls (e.g. because of a ball_routing)
+        self._balls_added_callback(0, unclaimed_balls)
 
     @asyncio.coroutine
     def unexpected_ball_received(self):
diff --git a/mpf/devices/ball_routing.py b/mpf/devices/ball_routing.py
new file mode 100644
index 000000000..ebd83183e
--- /dev/null
+++ b/mpf/devices/ball_routing.py
@@ -0,0 +1,96 @@
+"""Routes balls from one device to another when captured."""
+from collections import defaultdict
+
+import asyncio
+
+from mpf.devices.ball_device.ball_device import BallDevice
+
+from mpf.core.machine import MachineController
+from mpf.core.mode import Mode
+from mpf.core.mode_device import ModeDevice
+
+
+class BallRouting(ModeDevice):
+
+    """Route balls from one device to another when captured."""
+
+    config_section = 'ball_routings'
+    collection = 'ball_routings'
+    class_label = 'ball_routing'
+
+    def __init__(self, machine: "MachineController", name: str) -> None:
+        """Initialise device."""
+        super().__init__(machine, name)
+        self._enabled = False
+        self._routing_queue = defaultdict(int)
+        self._balls_at_target = 0
+
+    @asyncio.coroutine
+    def _initialize(self):
+        for device in self.config['source_devices']:
+            self.machine.events.add_handler(
+                'balldevice_' + device.name + '_ball_enter',
+                self._claim_balls, device=device)
+            self.machine.events.add_handler(
+                'balldevice_' + device.name + '_ball_entered',
+                self._route_ball, device=device)
+
+        self.machine.events.add_handler(
+            'balldevice_' + self.config['target_device'].name + '_ball_enter',
+            self._add_ball)
+
+    def enable(self, **kwargs):
+        """Enable routing."""
+        del kwargs
+        if self._enabled:
+            return
+        self._enabled = True
+        self.debug_log("Enabling")
+
+    def _claim_balls(self, device: BallDevice, unclaimed_balls, **kwargs):
+        """Claim balls to route them to destination later."""
+        del kwargs
+        if not self._enabled:
+            return {}
+        # remember how many balls were captured
+        self._routing_queue[device] += unclaimed_balls
+        # claim all balls
+        return {"unclaimed_balls": 0}
+
+    def _route_ball(self, device: BallDevice, **kwargs):
+        """Route balls to destination."""
+        del kwargs
+        self._balls_at_target += self._routing_queue[device]
+        for _ in range(self._routing_queue[device]):
+            device.eject(target=self.config['target_device'])
+
+        self._routing_queue[device] = 0
+
+    def _add_ball(self, unclaimed_balls, new_balls, **kwargs):
+        """Mark balls as unclaimed at destination."""
+        del kwargs
+        claimed_balls = new_balls - unclaimed_balls
+        if self._balls_at_target and claimed_balls:
+            if claimed_balls <= self._balls_at_target:
+                new_unexpected = unclaimed_balls + self._balls_at_target
+                self._balls_at_target = 0
+                return {"unclaimed_balls": new_unexpected}
+            else:
+                self._balls_at_target -= claimed_balls
+                return {"unclaimed_balls": new_balls}
+
+        return {}
+
+    def disable(self, **kwargs):
+        """Disable routing."""
+        del kwargs
+        if not self._enabled:
+            return
+        self.debug_log("Disabling")
+        self._enabled = False
+
+    def device_removed_from_mode(self, mode: Mode) -> None:
+        """Disable ball save when mode ends."""
+        super().device_removed_from_mode(mode)
+        self.debug_log("Removing")
+        self.disable()
diff --git a/mpf/devices/ball_save.py b/mpf/devices/ball_save.py
index 0e56f1831..5a5712ba8 100644
--- a/mpf/devices/ball_save.py
+++ b/mpf/devices/ball_save.py
@@ -301,7 +301,7 @@ class BallSave(SystemWideDevice, ModeDevice):
 
     def device_removed_from_mode(self, mode: Mode) -> None:
         """Disable ball save when mode ends."""
-        del mode
+        super().device_removed_from_mode(mode)
         self.debug_log("Removing...")
 
         self.disable()
diff --git a/mpf/mpfconfig.yaml b/mpf/mpfconfig.yaml
index ca085d7ef..9120da70f 100644
--- a/mpf/mpfconfig.yaml
+++ b/mpf/mpfconfig.yaml
@@ -87,6 +87,7 @@ mpf:
         - mpf.devices.stepper.Stepper
         - mpf.devices.state_machine.StateMachine
         - mpf.devices.score_queue.ScoreQueue
+        - mpf.devices.ball_routing.BallRouting
 
     plugins:
         mpf.plugins.auditor.Auditor
diff --git a/mpf/platforms/snux.py b/mpf/platforms/snux.py
index ce17d8026..d92e3c81a 100644
--- a/mpf/platforms/snux.py
+++ b/mpf/platforms/snux.py
@@ -47,11 +47,6 @@ class SnuxHardwarePlatform(DriverPlatform):
         self.c_side_enabled = False
 
         self.ac_relay_in_transition = False
-        # Specify whether the AC relay should favour the A or C side when at rest.
-        # Typically during a game the 'C' side should be preferred, since that is
-        # normally where the flashers are which need a quick response without having to wait on the relay.
-        # At game over though, it should prefer the 'A' side so that the relay isn't permanently energised.
-        self.prefer_a_side = True
 
     def stop(self):
         """Stop the overlay. Nothing to do here because stop is also called on parent platform."""
@@ -67,16 +62,6 @@ class SnuxHardwarePlatform(DriverPlatform):
         """Return if C side cannot be switches off right away."""
         return self.drivers_holding_c_side or self.c_side_done_time > self.machine.clock.get_time()
 
-    @property
-    def c_side_busy(self):
-        """Return if C side cannot be switches off right away."""
-        return self.drivers_holding_c_side or self.c_side_done_time > self.machine.clock.get_time() or self.c_side_queue
-
-    @property
-    def a_side_active(self):
-        """Return if A side cannot be switches off right away."""
-        return self.drivers_holding_a_side or self.a_side_done_time > self.machine.clock.get_time()
-
     def _null_log_handler(self, *args, **kwargs):
         pass
 
@@ -120,22 +105,6 @@ class SnuxHardwarePlatform(DriverPlatform):
         self.machine.events.add_handler('init_phase_5',
                                         self._initialize_phase_2)
 
-        self.machine.events.add_handler(self.snux_config['prefer_a_side_event'], self._prefer_a_side)
-        self.log.info("Configuring Snux driver to prefer A side on event %s", self.snux_config['prefer_a_side_event'])
-
-        self.machine.events.add_handler(self.snux_config['prefer_c_side_event'], self._prefer_c_side)
-        self.log.info("Configuring Snux driver to prefer C side on event %s", self.snux_config['prefer_c_side_event'])
-
-    def _prefer_a_side(self, **kwargs):
-        del kwargs
-        self.prefer_a_side = True
-        self._enable_a_side()
-
-    def _prefer_c_side(self, **kwargs):
-        del kwargs
-        self.prefer_a_side = False
-        self._enable_c_side()
-
     def _initialize_phase_2(self, **kwargs):
         del kwargs
         self.machine.clock.schedule_interval(self._flash_diag_led, 0.5)
@@ -152,20 +121,12 @@ class SnuxHardwarePlatform(DriverPlatform):
 
         Called based on the timer_tick event
         """
-        if self.prefer_a_side:
-            if self.a_side_queue:
-                self._service_a_side()
-            elif self.c_side_queue:
-                self._service_c_side()
-            elif self.c_side_enabled and not self.c_side_active:
-                self._enable_a_side()
-        else:
-            if self.c_side_queue:
-                self._service_c_side()
-            elif self.a_side_queue:
-                self._service_a_side()
-            elif self.a_side_enabled and not self.a_side_active:
-                self._enable_c_side()
+        if self.a_side_queue:
+            self._service_a_side()
+        elif self.c_side_queue:
+            self._service_c_side()
+        elif self.c_side_enabled and not self.c_side_active:
+            self._enable_a_side()
 
     def _flash_diag_led(self):
         """Flash diagnosis LED."""
@@ -255,22 +216,13 @@ class SnuxHardwarePlatform(DriverPlatform):
 
         This action will be serviced immediately if it can, or ASAP otherwise.
         """
-        if self.prefer_a_side:
-            if driver in self.a_drivers:
-                self.a_side_queue.add((driver, pulse_settings, hold_settings))
-                self._service_a_side()
-            elif driver in self.c_drivers:
-                self.c_side_queue.add((driver, pulse_settings, hold_settings))
-                if not self.ac_relay_in_transition and not self.a_side_busy:
-                    self._service_c_side()
-        else:
-            if driver in self.c_drivers:
-                self.c_side_queue.add((driver, pulse_settings, hold_settings))
+        if driver in self.a_drivers:
+            self.a_side_queue.add((driver, pulse_settings, hold_settings))
+            self._service_a_side()
+        elif driver in self.c_drivers:
+            self.c_side_queue.add((driver, pulse_settings, hold_settings))
+            if not self.ac_relay_in_transition and not self.a_side_busy:
                 self._service_c_side()
-            elif driver in self.a_drivers:
-                self.a_side_queue.add((driver, pulse_settings, hold_settings))
-                if not self.ac_relay_in_transition and not self.c_side_busy:
-                    self._service_a_side()
 
     def _enable_ac_relay(self):
         self.system11_config['ac_relay_driver'].enable()
@@ -293,46 +245,27 @@ class SnuxHardwarePlatform(DriverPlatform):
     # -------------------------------- A SIDE ---------------------------------
 
     def _enable_a_side(self):
-        if self.prefer_a_side:
-            if not self.a_side_enabled and not self.ac_relay_in_transition:
-
-                if self.c_side_active:
-                    self._disable_all_c_side_drivers()
-                    self._disable_ac_relay()
-                    self.delay.add(ms=self.system11_config['ac_relay_delay_ms'],
-                                   callback=self._enable_a_side,
-                                   name='enable_a_side')
-                    return
-
-                elif self.c_side_enabled:
-                    self._disable_ac_relay()
-
-                else:
-                    self._a_side_enabled()
-        else:
-            if (not self.ac_relay_in_transition and
-                    not self.a_side_enabled and
-                    not self.c_side_busy):
+        if not self.a_side_enabled and not self.ac_relay_in_transition:
+
+            if self.c_side_active:
+                self._disable_all_c_side_drivers()
                 self._disable_ac_relay()
+                self.delay.add(ms=self.system11_config['ac_relay_delay_ms'],
+                               callback=self._enable_a_side,
+                               name='enable_a_side')
+                return
 
-            elif self.a_side_enabled and self.a_side_queue:
-                self._service_a_side()
+            elif self.c_side_enabled:
+                self._disable_ac_relay()
+
+            else:
+                self._a_side_enabled()
 
     def _a_side_enabled(self):
         self.ac_relay_in_transition = False
-        if self.prefer_a_side:
-            self.a_side_enabled = True
-            self.c_side_enabled = False
-            self._service_a_side()
-        else:
-
-            if self.c_side_queue:
-                self._enable_c_side()
-                return
-
-            self.c_side_enabled = False
-            self.a_side_enabled = True
-            self._service_a_side()
+        self.a_side_enabled = True
+        self.c_side_enabled = False
+        self._service_a_side()
 
     def _service_a_side(self):
         if not self.a_side_queue:
@@ -367,47 +300,24 @@ class SnuxHardwarePlatform(DriverPlatform):
     # -------------------------------- C SIDE ---------------------------------
 
     def _enable_c_side(self):
-        if self.prefer_a_side:
-            if not self.c_side_enabled and not self.ac_relay_in_transition:
-
-                if self.a_side_active:
-                    self._disable_all_a_side_drivers()
-                    self._enable_ac_relay()
-                    self.delay.add(ms=self.system11_config['ac_relay_delay_ms'],
-                                   callback=self._enable_c_side,
-                                   name='enable_c_side')
-                    return
-
-                elif self.a_side_enabled:
-                    self._enable_ac_relay()
-
-                else:
-                    self._c_side_enabled()
-        else:
-            if (not self.ac_relay_in_transition and
-                    not self.c_side_enabled and
-                    not self.a_side_busy):
-                self._enable_ac_relay()
+        if (not self.ac_relay_in_transition and
+                not self.c_side_enabled and
+                not self.a_side_busy):
+            self._enable_ac_relay()
 
-            elif self.c_side_enabled and self.c_side_queue:
-                self._service_c_side()
+        elif self.c_side_enabled and self.c_side_queue:
+            self._service_c_side()
 
     def _c_side_enabled(self):
         self.ac_relay_in_transition = False
 
-        if self.prefer_a_side:
-            self.c_side_enabled = True
-            self.a_side_enabled = False
-            self._service_c_side()
-        else:
-
-            if self.a_side_queue:
-                self._enable_a_side()
-                return
+        if self.a_side_queue:
+            self._enable_a_side()
+            return
 
-            self.a_side_enabled = False
-            self.c_side_enabled = True
-            self._service_c_side()
+        self.a_side_enabled = False
+        self.c_side_enabled = True
+        self._service_c_side()
 
     def _service_c_side(self):
         if not self.c_side_queue:
@@ -449,14 +359,6 @@ class SnuxHardwarePlatform(DriverPlatform):
             self.c_side_done_time = 0
             self.c_side_enabled = False
 
-    def _disable_all_a_side_drivers(self):
-        if self.a_side_active:
-            for driver in self.drivers_holding_a_side:
-                driver.disable()
-            self.drivers_holding_a_side = set()
-            self.a_side_done_time = 0
-            self.a_side_enabled = False
-
     def validate_coil_section(self, driver, config):
         """Validate coil config for platform."""
         return self.platform.validate_coil_section(driver, config)


Test patch:
diff --git a/mpf/tests/machine_files/ball_routing/config/config.yaml b/mpf/tests/machine_files/ball_routing/config/config.yaml
new file mode 100644
index 000000000..4b88097d9
--- /dev/null
+++ b/mpf/tests/machine_files/ball_routing/config/config.yaml
@@ -0,0 +1,61 @@
+#config_version=5
+modes:
+    - mode1
+
+playfields:
+    playfield:
+        default_source_device: test_trough
+        tags: default
+
+coils:
+    c_trough:
+        number:
+    c_device1:
+        number:
+    c_device2:
+        number:
+    c_device3:
+        number:
+    c_device4:
+        number:
+
+switches:
+    s_ball_switch1:
+        number:
+    s_ball_switch2:
+        number:
+    s_device1:
+        number:
+    s_device2:
+        number:
+    s_device3:
+        number:
+    s_device4:
+        number:
+
+ball_devices:
+    test_trough:
+        eject_coil: c_trough
+        ball_switches: s_ball_switch1, s_ball_switch2
+        eject_targets: playfield
+        tags: trough, drain, home
+    test_device1:
+        eject_coil: c_device1
+        ball_switches: s_device1
+        debug: True
+        eject_targets: test_device2, test_device3
+    test_device2:
+        eject_coil: c_device2
+        ball_switches: s_device2
+        debug: True
+        eject_targets: playfield
+    test_device3:
+        eject_coil: c_device3
+        ball_switches: s_device3
+        debug: True
+        eject_targets: test_device4
+    test_device4:
+        eject_coil: c_device4
+        ball_switches: s_device4
+        debug: True
+        eject_targets: playfield
diff --git a/mpf/tests/machine_files/ball_routing/modes/mode1/config/mode1.yaml b/mpf/tests/machine_files/ball_routing/modes/mode1/config/mode1.yaml
new file mode 100644
index 000000000..26ee6200d
--- /dev/null
+++ b/mpf/tests/machine_files/ball_routing/modes/mode1/config/mode1.yaml
@@ -0,0 +1,14 @@
+#config_version=5
+ball_routings:
+    route_to_ball_device2:
+      source_devices: test_device1
+      target_device: test_device2
+      debug: True
+      enable_events: route_to_2
+      disable_events: route_to_4
+    route_to_ball_device4:
+      source_devices: test_device1
+      target_device: test_device4
+      debug: True
+      enable_events: route_to_4
+      disable_events: route_to_2
diff --git a/mpf/tests/test_BallRouting.py b/mpf/tests/test_BallRouting.py
new file mode 100644
index 000000000..ed2057f6a
--- /dev/null
+++ b/mpf/tests/test_BallRouting.py
@@ -0,0 +1,73 @@
+from mpf.tests.MpfGameTestCase import MpfGameTestCase
+
+
+class TestBallRouting(MpfGameTestCase):
+
+    def getConfigFile(self):
+        return 'config.yaml'
+
+    def getMachinePath(self):
+        return 'tests/machine_files/ball_routing/'
+
+    def get_platform(self):
+        return 'smart_virtual'
+
+    def test_routing(self):
+        self.hit_switch_and_run("s_ball_switch1", 1)
+        self.hit_switch_and_run("s_ball_switch2", 1)
+        self.start_game()
+        self.advance_time_and_run(10)
+
+        self.assertBallsOnPlayfield(1)
+
+        # by default the ball is routed back to the playfield
+        self.mock_event("balldevice_test_device1_ball_enter")
+        self.mock_event("balldevice_test_device2_ball_enter")
+        self.hit_switch_and_run("s_device1", 1)
+        self.assertBallsOnPlayfield(0)
+        self.assertAvailableBallsOnPlayfield(1)
+        self.advance_time_and_run(20)
+        self.assertBallsOnPlayfield(1)
+        self.assertAvailableBallsOnPlayfield(1)
+        self.assertEventCalledWith("balldevice_test_device1_ball_enter",
+                                   device=self.machine.ball_devices["test_device1"], new_balls=1, unclaimed_balls=1)
+        self.assertEventCalledWith("balldevice_test_device2_ball_enter",
+                                   device=self.machine.ball_devices["test_device2"], new_balls=1, unclaimed_balls=0)
+
+        self.start_mode("mode1")
+        # route ball via device2
+        self.post_event("route_to_2")
+        self.mock_event("balldevice_test_device1_ball_enter")
+        self.mock_event("balldevice_test_device2_ball_enter")
+        self.mock_event("balldevice_test_device3_ball_enter")
+        self.mock_event("balldevice_test_device4_ball_enter")
+        self.hit_switch_and_run("s_device1", 1)
+        self.assertBallsOnPlayfield(0)
+        self.advance_time_and_run(20)
+        self.assertBallsOnPlayfield(1)
+        self.assertAvailableBallsOnPlayfield(1)
+        self.assertEventCalledWith("balldevice_test_device1_ball_enter",
+                                   device=self.machine.ball_devices["test_device1"], new_balls=1, unclaimed_balls=0)
+        self.assertEventCalledWith("balldevice_test_device2_ball_enter",
+                                   device=self.machine.ball_devices["test_device2"], new_balls=1, unclaimed_balls=1)
+        self.assertEventNotCalled("balldevice_test_device3_ball_enter")
+        self.assertEventNotCalled("balldevice_test_device4_ball_enter")
+
+        # route ball via device4
+        self.post_event("route_to_4")
+        self.mock_event("balldevice_test_device1_ball_enter")
+        self.mock_event("balldevice_test_device2_ball_enter")
+        self.mock_event("balldevice_test_device3_ball_enter")
+        self.mock_event("balldevice_test_device4_ball_enter")
+        self.hit_switch_and_run("s_device1", 1)
+        self.assertBallsOnPlayfield(0)
+        self.advance_time_and_run(20)
+        self.assertBallsOnPlayfield(1)
+        self.assertAvailableBallsOnPlayfield(1)
+        self.assertEventCalledWith("balldevice_test_device1_ball_enter",
+                                   device=self.machine.ball_devices["test_device1"], new_balls=1, unclaimed_balls=0)
+        self.assertEventNotCalled("balldevice_test_device2_ball_enter")
+        self.assertEventCalledWith("balldevice_test_device3_ball_enter",
+                                   device=self.machine.ball_devices["test_device3"], new_balls=1, unclaimed_balls=0)
+        self.assertEventCalledWith("balldevice_test_device4_ball_enter",
+                                   device=self.machine.ball_devices["test_device4"], new_balls=1, unclaimed_balls=1)
diff --git a/mpf/tests/test_Snux.py b/mpf/tests/test_Snux.py
index 46bda22aa..f21149f55 100644
--- a/mpf/tests/test_Snux.py
+++ b/mpf/tests/test_Snux.py
@@ -1,9 +1,10 @@
 """Test snux platform."""
-from mpf.tests.MpfFakeGameTestCase import MpfFakeGameTestCase, MagicMock
 from mpf.platforms.interfaces.driver_platform_interface import PulseSettings, HoldSettings
 
+from mpf.tests.MpfTestCase import MpfTestCase, MagicMock
 
-class TestSnux(MpfFakeGameTestCase):
+
+class TestSnux(MpfTestCase):
     def getConfigFile(self):
         return 'config.yaml'
 
@@ -29,22 +30,6 @@ class TestSnux(MpfFakeGameTestCase):
                 return driver
         return False
 
-    def test_ac_relay_default(self):
-        # outside game it should be default off
-        c_ac_relay = self.machine.coils["c_ac_relay"]
-        self.assertEqual("disabled", c_ac_relay.hw_driver.state)
-
-        self.start_game()
-        # during a game it should be default on to allow fast flashers
-        self.assertEqual("enabled", c_ac_relay.hw_driver.state)
-
-        # after the game ended it should turn back to default off
-        self.drain_all_balls()
-        self.drain_all_balls()
-        self.drain_all_balls()
-        self.assertGameIsNotRunning()
-        self.assertEqual("disabled", c_ac_relay.hw_driver.state)
-
     def test_ac_switch_and_pulse(self):
         # test diag led flashing. otherwise snux is not running
         c_diag_led_driver = self.machine.coils.c_diag_led_driver