CLI improvements

11 files changed, 66 insertions, 123 deletions

diff --git a/Makefile b/Makefile
index 1288113..74a7976 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,5 @@
 test: all
 	cd solo-tool-project/test && ../../venv/bin/pytest *test.py
-	cd cli-project/test && ../../venv/bin/pytest *test.py
 
 all: venv .git/hooks/pre-commit
 
diff --git a/cli-project/pyproject.toml b/cli-project/pyproject.toml
index 7d31a09..3e2c855 100644
--- a/cli-project/pyproject.toml
+++ b/cli-project/pyproject.toml
@@ -16,7 +16,6 @@ dynamic = ["version"]
 
 [project.optional-dependencies]
 dev = [
-    "pytest"
 ]
 
 [project.scripts]
diff --git a/cli-project/src/solo_tool_cli.py b/cli-project/src/solo_tool_cli.py
index 1c8a7d6..5cc1537 100644
--- a/cli-project/src/solo_tool_cli.py
+++ b/cli-project/src/solo_tool_cli.py
@@ -1,57 +1,33 @@
 import sys
 import time
-import threading
 
 from solo_tool import SoloTool
 from solo_tool.midi_controller_launchpad_mini import MidiController
 
-class SoloToolCLI:
-    def __init__(self, sessionJson, soloToolOverride=None, midiOverride=None, tickEnable=True):
-        self._soloTool = SoloTool() if soloToolOverride is None else soloToolOverride
-        self._soloTool.loadSession(sessionJson)
-        self._midiController = MidiController(self._soloTool) if midiOverride is None else midiOverride
-        self._commands = {
-            "song" : self._song,
-            "midi" : self._midi
-        }
-        if tickEnable:
-            self._tick()
-
-    def input(self, commandString):
-        split = commandString.strip().split(" ")
-        if split[0] in self._commands:
-            self._commands[split[0]](split[1:])
-
-    def _song(self, args):
-        if len(args) > 0:
-            self._soloTool.setSong(int(args[0]))
-        else:
-            songs = self._soloTool.getSongs()
-            print("Songs:")
-            for i, s in enumerate(songs):
-                print(f"  {i} {s}")
-
-    def _midi(self, args):
-        if len(args) > 0 and args[0] == "connect":
-            print("Connecting to MIDI device...")
-            self._midiController.connect()
-        else:
-            print("Supported device: Novation Launchpad Mini MkII")
-
-    def _tick(self):
-        self._soloTool.tick()
-        threading.Timer(0.1, self._tick).start()
-
 def main():
     args = sys.argv[1:]
     if len(args) == 0:
         print("Please provide path to session file")
         sys.exit(1)
 
-    soloToolCli = SoloToolCLI(args[0])
-    while(True):
-        commandString = input("> ")
-        soloToolCli.input(commandString)
-
+    soloTool = SoloTool()
+    soloTool.loadSession(args[0])
+
+    def tick():
+        soloTool.tick()
+        threading.Timer(0.1, tick).start()
+
+    midiController = MidiController(soloTool)
+    midiController.connect()
+
+    try:
+        while(True):
+            time.sleep(0.1)
+            soloTool.tick()
+    except KeyboardInterrupt:
+        pass
+    finally:
+        midiController.disconnect()
+        
 if __name__ == '__main__':
     main()
diff --git a/cli-project/test/solo_tool_cli_integrationtest.py b/cli-project/test/solo_tool_cli_integrationtest.py
deleted file mode 100644
index c29a223..0000000
--- a/cli-project/test/solo_tool_cli_integrationtest.py
+++ /dev/null
@@ -1,41 +0,0 @@
-import pytest
-import io
-from contextlib import redirect_stdout
-
-from solo_tool_cli import SoloToolCLI
-from solo_tool import SoloTool
-
-class MockMidiController:
-    def __init__(self, soloTool):
-        self.connected = False
-
-    def connect(self):
-        self.connected = True
-    
-@pytest.fixture
-def soloTool():
-    return SoloTool()
-
-@pytest.fixture
-def mockMidi(soloTool):
-    return MockMidiController(soloTool)
-
-@pytest.fixture
-def uut(soloTool, mockMidi):
-    return SoloToolCLI("test_session.json", soloToolOverride=soloTool, midiOverride=mockMidi, tickEnable=False)
-
-def test_connectMidi(uut, mockMidi):
-    expectedOutput = """\
-Supported device: Novation Launchpad Mini MkII
-Connecting to MIDI device...
-"""
-
-    with io.StringIO() as buf, redirect_stdout(buf):
-        uut.input("midi")
-        assert not mockMidi.connected
-
-        uut.input("midi connect")
-        assert mockMidi.connected
-
-        assert buf.getvalue() == expectedOutput
-
diff --git a/cli-project/test/test.flac b/cli-project/test/test.flac
deleted file mode 100644
index 9164735..0000000
--- a/cli-project/test/test.flac
+++ /dev/null
diff --git a/cli-project/test/test.mp3 b/cli-project/test/test.mp3
deleted file mode 100644
index 3c353b7..0000000
--- a/cli-project/test/test.mp3
+++ /dev/null
diff --git a/cli-project/test/test_session.json b/cli-project/test/test_session.json
deleted file mode 100644
index f48b792..0000000
--- a/cli-project/test/test_session.json
+++ /dev/null
@@ -1,13 +0,0 @@
-[
-    {
-        "path" : "test.flac",
-        "ab_limits" : null
-    },
-    {
-        "path" : "test.mp3",
-        "ab_limits" : [
-            [0.1, 0.2],
-            [0.3, 0.4]
-        ]
-    }
-]
diff --git a/doc/known-issues.md b/doc/known-issues.md
index ae248f5..7604811 100644
--- a/doc/known-issues.md
+++ b/doc/known-issues.md
@@ -12,6 +12,14 @@
     * selection is not cleared properly when changing songs
     * sometimes crashes when selecting limits with MIDI controller
 
+* MIDI controller feature requests:
+    * skip ahead and behind by steps of a few seconds
+    * set A and B points at current play head position (during playback)
+
+* GUI feature requests:
+    * wipe LED state when application closes
+
+
 # Closed Issues
 
 * Moving AB sliders does not set AB limit. Instead need to save AB limit and then select it to apply
@@ -29,6 +37,10 @@
         * Not so easy to do actually, used Super L instead
 * AB repeat toggle in MIDI controller
 
+* CLI feature requests:
+    * close application without crashing
+    * wipe LED state when application closes
+
 # Use Cases
 
 ## Song/solo practice
diff --git a/solo-tool-project/src/solo_tool/midi_controller_launchpad_mini.py b/solo-tool-project/src/solo_tool/midi_controller_launchpad_mini.py
index 961127c..8f32650 100644
--- a/solo-tool-project/src/solo_tool/midi_controller_launchpad_mini.py
+++ b/solo-tool-project/src/solo_tool/midi_controller_launchpad_mini.py
@@ -51,10 +51,13 @@ class MidiController:
             self._handlers[button] = self._createSetPlaybackRateCallback(rate)
 
     def connect(self):
-        self._midiWrapper.setCallback(self._callback)
-        self._midiWrapper.connect(MidiController.DEVICE_NAME)
+        self._midiWrapper.connect(MidiController.DEVICE_NAME, self._callback)
         self._initialiseButtonLEDs()
 
+    def disconnect(self):
+        self._allLEDsOff()
+        self._midiWrapper.disconnect()
+
     def _callback(self, msg):
         if msg.type != "note_on" or msg.velocity < 127:
             return
diff --git a/solo-tool-project/src/solo_tool/midi_wrapper_mido.py b/solo-tool-project/src/solo_tool/midi_wrapper_mido.py
index bf3aa85..4cfc9c3 100644
--- a/solo-tool-project/src/solo_tool/midi_wrapper_mido.py
+++ b/solo-tool-project/src/solo_tool/midi_wrapper_mido.py
@@ -4,15 +4,20 @@ class MidiWrapper:
     def __init__(self):
         self._inPort = None
         self._outPort = None
-        self._callback = None
 
-    def setCallback(self, callback):
-        self._callback = callback
+    def connect(self, deviceName, callback):
+        if self._inPort is None and self._outPort is None:
+            self._inPort = mido.open_input(deviceName)
+            self._inPort.callback = callback
+            self._outPort = mido.open_output(deviceName)
 
-    def connect(self, deviceName):
-        self._inPort = mido.open_input(deviceName)
-        self._inPort.callback = self._callback
-        self._outPort = mido.open_output(deviceName)
+    def disconnect(self):
+        self._inPort.close()
+        self._inPort = None
+
+        self._outPort.reset()
+        self._outPort.close()
+        self._outPort = None
 
     def sendMessage(self, note, velocity, channel):
         if self._outPort is not None:
diff --git a/solo-tool-project/test/midi_launchpad_mini_integrationtest.py b/solo-tool-project/test/midi_launchpad_mini_integrationtest.py
index 8542aae..17649d4 100644
--- a/solo-tool-project/test/midi_launchpad_mini_integrationtest.py
+++ b/solo-tool-project/test/midi_launchpad_mini_integrationtest.py
@@ -24,11 +24,12 @@ class MidiWrapperMock:
         self.connectedDevice = None
         self.sentMessages = list()
 
-    def setCallback(self, callback):
+    def connect(self, deviceName, callback):
+        self.connectedDevice = deviceName
         self.callback = callback
 
-    def connect(self, deviceName):
-        self.connectedDevice = deviceName
+    def disconnect(self):
+        self.connectedDevice = None
     
     def sendMessage(self, note, velocity, channel):
         self.sentMessages.append((note, velocity, channel))
@@ -58,12 +59,6 @@ def midiWrapperMock():
 def uut(soloTool, midiWrapperMock):
     return MidiController(soloTool, midiWrapperMock)
 
-def test_connect(uut, midiWrapperMock):
-    expectedDevice = "Launchpad Mini MIDI 1"
-    uut.connect()
-
-    assert midiWrapperMock.connectedDevice == expectedDevice
-
 def test_startStopAndPauseButtons(uut, midiWrapperMock, playerMock):
     uut.connect()
 
@@ -345,8 +340,8 @@ def test_unassignedButton(uut, midiWrapperMock):
     midiWrapperMock.simulateInput(unassignedButton)
     # XXX would be better to assert that nothing changed in the solo tool
 
-def test_initializationMessages(uut, midiWrapperMock):
-    expectedMessages = set(
+def test_connectDisconnect(uut, midiWrapperMock):
+    startupMessages = list(
         [(int(i / 8) * 16 + (i % 8), LED_OFF, 0) for i in range(0, 64)] + # clear all
         [(i, LED_GREEN, 0) for i in range(0, 6)] +                      # volume row
         [(i, LED_YELLOW, 0) for i in range(16, 22)] +                   # playback rate row
@@ -359,13 +354,21 @@ def test_initializationMessages(uut, midiWrapperMock):
             (nextLimitButton,     LED_GREEN, 0),
             (previousSongButton,  LED_RED, 0),
             (nextSongButton,      LED_GREEN, 0)
-        ]
-    )
+        ])
+
+    teardownMessages = [(int(i / 8) * 16 + (i % 8), LED_OFF, 0) for i in range(0, 64)] # clear all
 
+    expectedDevice = "Launchpad Mini MIDI 1"
     uut.connect()
 
-    sentMessagesSet = set(midiWrapperMock.sentMessages)
-    assert sentMessagesSet == expectedMessages
+    assert midiWrapperMock.connectedDevice == expectedDevice
+    assert set(midiWrapperMock.sentMessages) == set(startupMessages)
+
+    midiWrapperMock.sentMessages.clear()
+
+    uut.disconnect()
+
+    assert set(midiWrapperMock.sentMessages) == set(teardownMessages)
 
 def test_playingFeedbackWhenChangingSong(uut, midiWrapperMock, soloTool, playerMock):
     songs = [