diff --git a/src/enrich.rs b/src/enrich.rs index e533f8d..51de8f2 100644 --- a/src/enrich.rs +++ b/src/enrich.rs @@ -815,17 +815,8 @@ fn urlencoded(s: &str) -> String { // Main enrichment function // --------------------------------------------------------------------------- -/// Run the enrichment process: find all uncached albums and fetch their -/// metadata and cover art from MusicBrainz / Cover Art Archive. -/// -/// Progress is printed to stderr so the user can see what's happening. -/// -/// # Arguments -/// - `conn` — SQLite database connection -/// - `force` — if true, re-fetch all albums even if already cached -/// - `quiet` — if true, suppress the "nothing to do" hint (used when called -/// from `report --html` where the user didn't explicitly ask for enrichment) /// Enrich only the albums in `needed` that aren't already cached. +/// /// Used by `report --html` so we fetch covers only for what the report /// will actually display, rather than the entire scrobble library. pub fn run_enrich_targeted( @@ -848,6 +839,17 @@ pub fn run_enrich_targeted( run_enrich_albums(conn, albums, quiet); } +/// Run the enrichment process: find all uncached albums and fetch their +/// metadata and cover art from MusicBrainz / Cover Art Archive. +/// +/// Progress is printed to stderr so the user can see what's happening. +/// +/// # Arguments +/// +/// - `conn` — SQLite database connection +/// - `force` — if true, re-fetch all albums even if already cached +/// - `quiet` — if true, suppress the "nothing to do" hint (used when called +/// from `report --html` where the user didn't explicitly ask for enrichment) pub fn run_enrich(conn: &Connection, force: bool, quiet: bool) { if force { conn.execute("DELETE FROM album_cache", []) diff --git a/src/main.rs b/src/main.rs index 58d9e8e..71bb871 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,9 +32,9 @@ //! [playerctl status] ──reader thread──→ ┘ //! ``` //! -//! ### MPD watcher (optional, enabled with `--mpd`) +//! ### MPD watcher (on by default, disabled with `--no-mpd`) //! -//! When `--mpd` is passed, a separate thread is spawned that connects to MPD +//! Unless `--no-mpd` is passed, a separate thread is spawned that connects to MPD //! using the idle protocol. It maintains its own `ScrobbleTracker` and writes //! to the same database. The two watchers are fully independent — neither //! knows about the other, and no synchronisation is needed between them. @@ -404,6 +404,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option, // When MPRIS is disabled, this loop exits immediately on shutdown. let mut tracker = watcher::create_db_tracker(conn); let mut eof_count = 0; + let mut flushed = false; while running.load(Ordering::SeqCst) { match rx.recv() { @@ -415,6 +416,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option, // sends one Eof, so we may receive more than `expected_eofs`. if eof_count >= expected_eofs { tracker.handle_event(watcher::Event::Eof); + flushed = true; break; } continue; @@ -426,9 +428,11 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option, } } - // Final flush: safe even if Eof was already handled — the tracker - // is a no-op when there is no current track. - tracker.handle_event(watcher::Event::Eof); + // Final flush in case the loop exited via channel disconnect rather than + // the normal Eof path above (e.g., all senders dropped simultaneously). + if !flushed { + tracker.handle_event(watcher::Event::Eof); + } // Wait for the MPD watcher thread to finish its graceful shutdown. // It will notice `running == false` on the next idle timeout (≤ 500 ms). diff --git a/src/mpd.rs b/src/mpd.rs index e27d7a0..595767c 100644 --- a/src/mpd.rs +++ b/src/mpd.rs @@ -100,13 +100,13 @@ use crate::watcher::{self, Event, PlayerStatus}; /// /// ``` /// // Standard localhost installation -/// let cfg = MpdConfig::default_tcp(); +/// let cfg = MpdConfig { host: "localhost".into(), port: 6600 }; /// /// // Custom port /// let cfg = MpdConfig { host: "localhost".into(), port: 6601 }; /// /// // Unix socket (when MPD runs under a different user account) -/// let cfg = MpdConfig { host: "/run/user/1000/mpd/socket".into(), port: 0 }; +/// let cfg = MpdConfig { host: "/run/mpd/socket".into(), port: 0 }; /// ``` #[derive(Debug, Clone)] pub struct MpdConfig { @@ -578,17 +578,17 @@ impl MpdPlayerState { } } -/// Run the MPD watcher loop. This function blocks until `running` is set to -/// `false` (e.g. by a Ctrl+C handler) or MPD becomes persistently unreachable. +/// Run the MPD watcher reconnect-and-event loop. +/// +/// Connects to MPD, then loops waiting for player events. If the connection +/// drops (MPD restarted, network hiccup), waits 5 seconds and reconnects +/// automatically. Blocks until `running` is set to `false` (e.g. by a +/// Ctrl+C handler), at which point it flushes any in-progress scrobble +/// and returns. /// /// It maintains its own [`watcher::ScrobbleTracker`] backed by the supplied /// database connection. Scrobbles are written to the same SQLite database as /// the MPRIS watcher, so the two sources appear together in reports. -/// -/// ## Reconnection -/// -/// If the MPD connection drops (MPD restarted, network hiccup), this function -/// waits 5 seconds and retries. It gives up after the shutdown flag is set. pub fn run_mpd_watch(config: &MpdConfig, conn: Arc>, running: Arc) { eprintln!( "[mpd] Connecting to MPD at {}{}", @@ -692,7 +692,7 @@ fn run_event_loop( // either get the player event or the shutdown flag is set. let changed = wait_for_idle_response(mpd_conn, running); match changed { - IdleResult::PlayerChanged => {} + IdleResult::Wakeup => {} IdleResult::Shutdown => return true, IdleResult::ConnectionLost => return false, } @@ -711,7 +711,11 @@ fn run_event_loop( /// The three possible outcomes of waiting for an MPD idle response. enum IdleResult { - PlayerChanged, + /// The idle wait ended and state should be re-queried. This covers both + /// the normal case (`changed: player` received) and the degenerate case + /// where MPD returns `OK` without a `changed` line (e.g., a different + /// subsystem fired despite `idle player` — unusual but handled safely). + Wakeup, Shutdown, ConnectionLost, } @@ -722,27 +726,26 @@ enum IdleResult { /// Because the socket has a 500 ms read timeout, `read_line` returns a /// `TimedOut`/`WouldBlock` error periodically. We use those timeouts to check /// the shutdown flag without blocking for the full duration of a track. +/// +/// Returns [`IdleResult::Wakeup`] when the response terminates — regardless +/// of whether a `changed: player` line appeared. In both cases the caller +/// re-queries MPD state, so the distinction doesn't matter. fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc) -> IdleResult { - let mut saw_changed_player = false; - loop { let mut line = String::new(); match conn.reader.read_line(&mut line) { Ok(0) => return IdleResult::ConnectionLost, // EOF Ok(_) => { let trimmed = line.trim(); - if trimmed == "changed: player" { - saw_changed_player = true; - } else if trimmed == "OK" { - // End of idle response — if we saw "changed: player", report it. - if saw_changed_player { - return IdleResult::PlayerChanged; - } - // "OK" with no "changed: player" means the idle was cancelled - // without a relevant event (e.g., a different subsystem changed). - // Re-enter idle by returning and letting the outer loop retry. - return IdleResult::PlayerChanged; // Re-query state regardless. + if trimmed == "OK" { + // End of idle response — re-query state regardless of whether + // "changed: player" appeared. With `idle player` the line + // should always appear, but if it's absent (degenerate case) + // a spurious re-query is harmless. + return IdleResult::Wakeup; } + // "changed: player" and any other key/value lines are consumed + // and discarded here — we query currentsong/status ourselves. } Err(ref e) if e.kind() == io::ErrorKind::WouldBlock || e.kind() == io::ErrorKind::TimedOut => diff --git a/src/report.rs b/src/report.rs index 4ecaa5d..0698170 100644 --- a/src/report.rs +++ b/src/report.rs @@ -453,15 +453,6 @@ fn truncate_cell(s: &str, width: usize) -> String { // Report generation // --------------------------------------------------------------------------- -/// Query the database and assemble all sections of the report. -/// -/// This is the main entry point for report generation. It runs all the -/// necessary SQL queries for the given period and limit, and returns -/// a `ReportData` struct ready for rendering. -/// -/// # Arguments -/// - `conn` — SQLite database connection -/// - `period` — time period filter ("today", "week", "month", "year", "all") /// Return the set of `(artist, album)` pairs that will appear anywhere in the /// HTML report at the given limits. Used to focus automatic enrichment on /// exactly what the report needs rather than the entire scrobble library. @@ -506,6 +497,16 @@ pub fn albums_needed_for_report( pairs } +/// Query the database and assemble all sections of the report. +/// +/// This is the main entry point for report generation. It runs all the +/// necessary SQL queries for the given period and limit, and returns +/// a `ReportData` struct ready for rendering. +/// +/// # Arguments +/// +/// - `conn` — SQLite database connection +/// - `period` — time period filter ("today", "week", "month", "year", "all") /// - `limit` — maximum number of entries in each top-N list pub fn gather_report( conn: &Connection,