Fix up tests

app/classes.py

@@ -244,10 +244,6 @@ class MusicMusterSignals(QObject):
     # escape there to abandon an edit.
     enable_escape_signal = pyqtSignal(bool)
 
-    # Signals that the next-cued track has changed. Used to update
-    # playlist headers.
-    next_track_changed_signal = pyqtSignal()
-
     # Signals that the playlist_id passed should resize all rows.
     resize_rows_signal = pyqtSignal(int)
 
@@ -277,11 +273,14 @@ class MusicMusterSignals(QObject):
 
     # signal_set_next_track takes a PlaylistRow as an argument. We can't
     # specify that here as it requires us to import PlaylistRow from
-    # playlistrow.py, which itself imports MusicMusterSignals
-
-    # TBD
+    # playlistrow.py, which itself imports MusicMusterSignals. It tells
+    # musicmuster to set the passed track as the next one.
     signal_set_next_track = pyqtSignal(object)
 
+    # Signals that the next-cued track has changed. Used to update
+    # playlist headers and track timings.
+    signal_next_track_changed = pyqtSignal()
+
     # Emited when a track starts playing
     signal_track_started = pyqtSignal(TrackAndPlaylist)
 

app/helpers.py

@@ -270,15 +270,39 @@ def leading_silence(
     return min(trim_ms, len(audio_segment))
 
 
-def ms_to_mmss(ms: int | None, none: str = "-") -> str:
+def ms_to_mmss(
+    ms: Optional[int],
+    decimals: int = 0,
+    negative: bool = False,
+    none: Optional[str] = None,
+) -> str:
     """Convert milliseconds to mm:ss"""
 
-    if ms is None:
-        return none
+    minutes: int
+    remainder: int
+    seconds: float
 
-    minutes, seconds = divmod(ms // 1000, 60)
+    if not ms:
+        if none:
+            return none
+        else:
+            return "-"
+    sign = ""
+    if ms < 0:
+        if negative:
+            sign = "-"
+        else:
+            ms = 0
 
-    return f"{minutes}:{seconds:02d}"
+    minutes, remainder = divmod(ms, 60 * 1000)
+    seconds = remainder / 1000
+
+    # if seconds >= 59.5, it will be represented as 60, which looks odd.
+    # So, fake it under those circumstances
+    if seconds >= 59.5:
+        seconds = 59.0
+
+    return f"{sign}{minutes:.0f}:{seconds:02.{decimals}f}"
 
 
 def normalise_track(path: str) -> None:

app/music_manager.py

@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 import datetime as dt
+import threading
 from time import sleep
 
 
@@ -21,6 +22,7 @@ from config import Config
 import helpers
 from log import log
 
+
 class _FadeTrack(QThread):
     finished = pyqtSignal()
 
@@ -51,6 +53,7 @@ class _FadeTrack(QThread):
                 )
                 sleep(1 / Config.FADEOUT_STEPS_PER_SECOND)
 
+        self.player.stop()
         self.finished.emit()
 
 
@@ -78,9 +81,11 @@ class Music:
         self.vlc_instance = vlc_manager.get_instance()
         self.vlc_instance.set_user_agent(name, name)
         self.player: vlc.MediaPlayer | None = None
+        self.vlc_event_manager: vlc.EventManager | None = None
         self.max_volume: int = Config.VLC_VOLUME_DEFAULT
         self.start_dt: dt.datetime | None = None
         self.signals = MusicMusterSignals()
+        self.end_of_track_signalled = False
 
     def fade(self, fade_seconds: int) -> None:
         """
@@ -96,6 +101,8 @@ class Music:
         if not self.player.get_position() > 0 and self.player.is_playing():
             return
 
+        self.signal_track_ended()
+
         self.fader_worker = _FadeTrack(self.player, fade_seconds=fade_seconds)
         self.fader_worker.finished.connect(self.player.release)
         self.fader_worker.start()
@@ -140,10 +147,12 @@ class Music:
             < dt.timedelta(microseconds=Config.PLAY_SETTLE)
         )
 
+# @log_call
     def play(
         self,
         path: str,
         start_time: dt.datetime,
+        playlist_id: int,
         position: float | None = None,
     ) -> None:
         """
@@ -155,7 +164,7 @@ class Music:
         the start time is the same
         """
 
-        log.debug(f"Music[{self.name}].play({path=}, {position=}")
+        self.playlist_id = playlist_id
 
         if helpers.file_is_unreadable(path):
             log.error(f"play({path}): path not readable")
@@ -169,6 +178,10 @@ class Music:
             )
             return
 
+        self.events = self.player.event_manager()
+        self.events.event_attach(vlc.EventType.MediaPlayerEndReached, self.track_end_event_handler)
+        self.events.event_attach(vlc.EventType.MediaPlayerStopped, self.track_end_event_handler)
+
         _ = self.player.play()
         self.set_volume(self.max_volume)
 
@@ -211,6 +224,21 @@ class Music:
                 log.debug(f"Reset from {volume=}")
             sleep(0.1)
 
+    def signal_track_ended(self) -> None:
+        """
+        Multiple parts of the Music class can signal that the track has
+        ended. Handle them all here to ensure that only one such signal
+        is raised. Make this thead safe.
+        """
+
+        lock = threading.Lock()
+
+        with lock:
+            if self.end_of_track_signalled:
+                return
+            self.signals.track_ended_signal.emit(self.playlist_id)
+            self.end_of_track_signalled = True
+
     def stop(self) -> None:
         """Immediately stop playing"""
 
@@ -225,3 +253,11 @@ class Music:
             self.player.stop()
         self.player.release()
         self.player = None
+
+    def track_end_event_handler(self, event: vlc.Event) -> None:
+        """
+        Handler for MediaPlayerEndReached
+        """
+
+        log.debug("track_end_event_handler() called")
+        self.signal_track_ended()

app/musicmuster.py

@@ -101,9 +101,6 @@ class SignalMonitor:
         self.signals.enable_escape_signal.connect(
             partial(self.show_signal, "enable_escape_signal ")
         )
-        self.signals.next_track_changed_signal.connect(
-            partial(self.show_signal, "next_track_changed_signal ")
-        )
         self.signals.resize_rows_signal.connect(
             partial(self.show_signal, "resize_rows_signal ")
         )
@@ -140,6 +137,7 @@ class SignalMonitor:
         self.signals.signal_track_started.connect(
             partial(self.show_signal, "signal_track_started ")
         )
+        # span_cells_signal is very noisy
         # self.signals.span_cells_signal.connect(
         #     partial(self.show_signal, "span_cells_signal ")
         # )
@@ -1659,7 +1657,7 @@ class Window(QMainWindow):
         """
 
         self.track_sequence.set_next(None)
-        self.signals.next_track_changed_signal.emit()
+        self.signals.signal_set_next_track.emit(None)
 
     def clear_selection(self, checked: bool = False) -> None:
         """Clear row selection"""
@@ -1741,9 +1739,12 @@ class Window(QMainWindow):
         self.txtSearch.textChanged.connect(self.search_playlist_text_changed)
 
         self.signals.enable_escape_signal.connect(self.enable_escape)
-        self.signals.next_track_changed_signal.connect(self.update_headers)
-        self.signals.status_message_signal.connect(self.show_status_message)
+        self.signals.search_songfacts_signal.connect(self.open_songfacts_browser)
+        self.signals.search_wikipedia_signal.connect(self.open_wikipedia_browser)
         self.signals.show_warning_signal.connect(self.show_warning)
+        self.signals.signal_next_track_changed.connect(self.signal_next_track_changed_handler)
+        self.signals.signal_set_next_track.connect(self.signal_set_next_track_handler)
+        self.signals.status_message_signal.connect(self.show_status_message)
         self.signals.track_ended_signal.connect(self.end_of_track_actions)
 
         self.timer10.timeout.connect(self.tick_10ms)
@@ -1751,9 +1752,6 @@ class Window(QMainWindow):
         self.timer100.timeout.connect(self.tick_100ms)
         self.timer1000.timeout.connect(self.tick_1000ms)
 
-        self.signals.search_songfacts_signal.connect(self.open_songfacts_browser)
-        self.signals.search_wikipedia_signal.connect(self.open_wikipedia_browser)
-
     # @log_call
     def current_row_or_end(self) -> int:
         """
@@ -2523,6 +2521,21 @@ class Window(QMainWindow):
 
         self._active_tab().scroll_to_top(playlist_track.row_number)
 
+    def signal_set_next_track_handler(self, plr: PlaylistRow) -> None:
+        """
+        Handle signal_set_next_track
+        """
+
+        self.track_sequence.set_next(plr)
+        self.signals.signal_next_track_changed.emit()
+
+    def signal_next_track_changed_handler(self) -> None:
+        """
+        Handle next track changed
+        """
+
+        self.update_headers()
+
     # @log_call
     def stop(self, checked: bool = False) -> None:
         """Stop playing immediately"""
@@ -2550,8 +2563,6 @@ class Window(QMainWindow):
 
         if self.track_sequence.current:
             try:
-                self.track_sequence.current.check_for_end_of_track()
-
                 # Update intro counter if applicable and, if updated,
                 # return because playing an intro uses the intro field to
                 # show timing and this takes precedence over timing a

app/playlistmodel.py

@@ -101,6 +101,7 @@ class PlaylistModel(QAbstractTableModel):
         self.signals.signal_set_next_row.connect(self.set_next_row)
         self.signals.signal_track_started.connect(self.track_started)
         self.signals.track_ended_signal.connect(self.previous_track_ended)
+        self.signals.signal_next_track_changed.connect(self.signal_next_track_changed_handler)
 
         # Populate self.playlist_rows
         for dto in ds.playlistrows_by_playlist(self.playlist_id):
@@ -1056,7 +1057,7 @@ class PlaylistModel(QAbstractTableModel):
         Remove track from row, retaining row as a header row
         """
 
-        self.playlist_rows[row_number].track_id = None
+        self.playlist_rows[row_number].track_id = 0
 
         # only invalidate required roles
         roles = [
@@ -1303,8 +1304,6 @@ class PlaylistModel(QAbstractTableModel):
         if self.track_sequence.next:
             old_next_row = self.track_sequence.next.row_number
 
-        self.track_sequence.set_next(plr)
-
         roles = [
             Qt.ItemDataRole.BackgroundRole,
         ]
@@ -1314,8 +1313,19 @@ class PlaylistModel(QAbstractTableModel):
         # only invalidate required roles
         self.invalidate_row(plr.row_number, roles)
 
-        self.signals.next_track_changed_signal.emit()
+        self.signals.signal_set_next_track.emit(plr)
+
+    def signal_next_track_changed_handler(self) -> None:
+        """
+        Handle next track changed
+        """
+
         self.update_track_times()
+        # Refresh display to show new next track
+        if self.track_sequence.next:
+            next_row_number = self.track_sequence.next.row_number
+            if next_row_number is not None:
+                self.invalidate_row(next_row_number, [Qt.ItemDataRole.BackgroundRole])
 
     # @log_call
     def setData(
@@ -1339,13 +1349,13 @@ class PlaylistModel(QAbstractTableModel):
         plr = self.playlist_rows[row_number]
 
         if column == Col.NOTE.value:
-            plr.note = value
+            plr.note = str(value)
 
         elif column == Col.TITLE.value:
-            plr.title = value
+            plr.title = str(value)
 
         elif column == Col.ARTIST.value:
-            plr.artist = value
+            plr.artist = str(value)
 
         elif column == Col.INTRO.value:
             intro = int(round(float(value), 1) * 1000)

app/playlistrow.py

@@ -103,7 +103,7 @@ class PlaylistRow:
     @property
     def intro(self) -> int:
         if self.dto.track:
-            return self.dto.track.intro
+            return self.dto.track.intro or 0
         else:
             return 0
 
@@ -172,7 +172,9 @@ class PlaylistRow:
         """
 
         if self.track_id > 0:
-            raise ApplicationError("Attempting to add track to row with existing track ({self=}")
+            raise ApplicationError(
+                "Attempting to add track to row with existing track ({self=}"
+            )
 
         ds.track_add_to_header(playlistrow_id=self.playlistrow_id, track_id=track_id)
 
@@ -221,28 +223,6 @@ class PlaylistRow:
         # the change to the database.
         self.dto.row_number = value
 
-    def check_for_end_of_track(self) -> None:
-        """
-        Check whether track has ended. If so, emit track_ended_signal
-        """
-
-        if self.start_time is None:
-            return
-
-        if self.end_of_track_signalled:
-            return
-
-        if self.music.is_playing():
-            return
-
-        self.start_time = None
-        if self.fade_graph:
-            self.fade_graph.clear()
-        # Ensure that player is released
-        self.music.fade(0)
-        self.signals.track_ended_signal.emit(self.playlist_id)
-        self.end_of_track_signalled = True
-
     def drop3db(self, enable: bool) -> None:
         """
         If enable is true, drop output by 3db else restore to full volume
@@ -278,7 +258,12 @@ class PlaylistRow:
         self.start_time = now
 
         # Initialise player
-        self.music.play(self.path, start_time=now, position=position)
+        self.music.play(
+            path=self.path,
+            start_time=now,
+            playlist_id=self.playlist_id,
+            position=position,
+        )
 
         self.end_time = now + dt.timedelta(milliseconds=self.duration)
 
@@ -546,12 +531,17 @@ class TrackSequence:
         """
 
         if self.current is None:
-            raise ApplicationError("Tried to move non-existent track from current to previous")
+            raise ApplicationError(
+                "Tried to move non-existent track from current to previous"
+            )
 
         # Dereference the fade curve so it can be garbage collected
-        self.current.fade_graph = None
+        if self.current.fade_graph:
+            self.current.fade_graph.clear()
+            self.current.fade_graph = None
         self.previous = self.current
         self.current = None
+        self.start_time = None
 
     def move_previous_to_next(self) -> None:
         """

app/playlists.py

@@ -1142,4 +1142,4 @@ class PlaylistTab(QTableView):
 
         self.track_sequence.set_next(None)
         self.clear_selection()
-        self.signals.next_track_changed_signal.emit()
+        self.signals.signal_set_next_track.emit(None)

tests/test_ds.py

@@ -8,7 +8,7 @@ import unittest
 # App imports
 from app import playlistmodel
 from app import ds
-from app.models import db
+from dbmanager import db
 from classes import PlaylistDTO
 from helpers import get_all_track_metadata
 from playlistmodel import PlaylistModel

tests/test_file_importer.py

@@ -21,7 +21,7 @@ from pytestqt.plugin import QtBot  # type: ignore
 
 # App imports
 from app import ds, musicmuster
-from app.models import db
+from dbmanager import db
 from config import Config
 from file_importer import FileImporter
 

tests/test_misc.py

@@ -7,7 +7,7 @@ import unittest
 import pytest
 
 # App imports
-from app.models import db
+from dbmanager import db
 import ds
 
 
@@ -26,7 +26,6 @@ class TestMMMisc(unittest.TestCase):
 
     def test_create_settings(self):
         SETTING_NAME = "wombat"
-        NO_SUCH_SETTING = "abc"
         VALUE = 3
 
         test_non_existant = ds.setting_get(SETTING_NAME)

tests/test_playlistmodel.py

@@ -9,7 +9,7 @@ from PyQt6.QtCore import Qt, QModelIndex
 # App imports
 from app.helpers import get_all_track_metadata
 from app import ds, playlistmodel
-from app.models import db
+from dbmanager import db
 from classes import (
     InsertTrack,
     TrackAndPlaylist,

tests/test_queries.py

@@ -8,10 +8,7 @@ import unittest
 # Third party imports
 
 # App imports
-from app.models import (
-    db,
-    Tracks,
-)
+from dbmanager import db
 from classes import (
     Filter,
 )

tests/test_ui.py

@@ -10,12 +10,9 @@ from pytestqt.plugin import QtBot  # type: ignore
 
 # App imports
 from app import playlistmodel, utilities
-from app.models import (
-    db,
-    Playlists,
-    Tracks,
-)
+from dbmanager import db
 from app import ds, musicmuster
+from classes import InsertTrack
 
 
 # Custom fixture to adapt qtbot for use with unittest.TestCase
@@ -99,7 +96,13 @@ class MyTestCase(unittest.TestCase):
         model = playlistmodel.PlaylistModel(playlist.playlist_id, is_template=False)
 
         # Add a track with a note
-        model.insert_row_signal_handler(track_id=self.track1.track_id, note=note_text)
+        model.insert_row_signal_handler(
+            InsertTrack(
+                playlist_id=playlist.playlist_id,
+                track_id=self.track1.track_id,
+                note=note_text
+            )
+        )
 
         # Retrieve playlist
         all_playlists = ds.playlists_all()