From cb0921144d20d87d4e85752908d6d9fba4a75481 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:25 +0200 Subject: [PATCH 1/9] rework error messages --- Cargo.toml | 1 + examples/fetch_state.rs | 2 +- examples/media_player.rs | 4 +- src/core_api.rs | 94 +++++++++------------ src/error.rs | 102 +++++++++-------------- src/event_parser.rs | 148 +++++++++++++++------------------ src/event_property_parser.rs | 110 +++++++++++++++++------- src/highlevel_api_extension.rs | 98 +++++++++++----------- src/ipc.rs | 76 +++++++++++------ src/message_parser.rs | 99 ++++++++++++++-------- tests/get_property.rs | 49 ++++++----- tests/integration.rs | 4 +- tests/set_property.rs | 56 ++++++++----- 13 files changed, 460 insertions(+), 383 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5993462..74b22ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ tokio = { version = "1.37.0", features = ["sync", "macros", "rt", "net"] } tokio-util = { version = "0.7.10", features = ["codec"] } futures = "0.3.30" tokio-stream = { version = "0.1.15", features = ["sync"] } +thiserror = "1.0.59" [dev-dependencies] env_logger = "0.10.0" diff --git a/examples/fetch_state.rs b/examples/fetch_state.rs index 8c699fd..a4fbcb1 100644 --- a/examples/fetch_state.rs +++ b/examples/fetch_state.rs @@ -1,4 +1,4 @@ -use mpvipc::{Error as MpvError, Mpv, MpvExt}; +use mpvipc::{MpvError, Mpv, MpvExt}; #[tokio::main] async fn main() -> Result<(), MpvError> { diff --git a/examples/media_player.rs b/examples/media_player.rs index 0bd7b3b..9def21a 100644 --- a/examples/media_player.rs +++ b/examples/media_player.rs @@ -1,4 +1,4 @@ -use mpvipc::{Error, Mpv, MpvExt}; +use mpvipc::{MpvError, Mpv, MpvExt}; fn seconds_to_hms(total: f64) -> String { let total = total as u64; @@ -10,7 +10,7 @@ fn seconds_to_hms(total: f64) -> String { } #[tokio::main] -async fn main() -> Result<(), Error> { +async fn main() -> Result<(), MpvError> { env_logger::init(); let mpv = Mpv::connect("/tmp/mpv.sock").await?; diff --git a/src/core_api.rs b/src/core_api.rs index 797474d..e45dbd4 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -12,7 +12,7 @@ use tokio::{ use crate::{ ipc::{MpvIpc, MpvIpcCommand, MpvIpcEvent, MpvIpcResponse}, message_parser::TypeHandler, - Error, ErrorCode, Event, + Event, MpvError, }; /// All possible commands that can be sent to mpv. @@ -134,14 +134,14 @@ impl IntoRawCommandPart for SeekOptions { pub trait GetPropertyTypeHandler: Sized { // TODO: fix this #[allow(async_fn_in_trait)] - async fn get_property_generic(instance: &Mpv, property: &str) -> Result; + async fn get_property_generic(instance: &Mpv, property: &str) -> Result; } impl GetPropertyTypeHandler for T where T: TypeHandler, { - async fn get_property_generic(instance: &Mpv, property: &str) -> Result { + async fn get_property_generic(instance: &Mpv, property: &str) -> Result { instance .get_property_value(property) .await @@ -153,17 +153,22 @@ where pub trait SetPropertyTypeHandler { // TODO: fix this #[allow(async_fn_in_trait)] - async fn set_property_generic(instance: &Mpv, property: &str, value: T) -> Result<(), Error>; + async fn set_property_generic(instance: &Mpv, property: &str, value: T) + -> Result<(), MpvError>; } impl SetPropertyTypeHandler for T where T: Serialize, { - async fn set_property_generic(instance: &Mpv, property: &str, value: T) -> Result<(), Error> { + async fn set_property_generic( + instance: &Mpv, + property: &str, + value: T, + ) -> Result<(), MpvError> { let (res_tx, res_rx) = oneshot::channel(); - let value = serde_json::to_value(value) - .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; + let value = serde_json::to_value(value).map_err(|why| MpvError::JsonParseError(why))?; + instance .command_sender .send(( @@ -171,15 +176,11 @@ where res_tx, )) .await - .map_err(|_| { - Error(ErrorCode::ConnectError( - "Failed to send command".to_string(), - )) - })?; + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; match res_rx.await { Ok(MpvIpcResponse(response)) => response.map(|_| ()), - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } } @@ -205,18 +206,18 @@ impl fmt::Debug for Mpv { } impl Mpv { - pub async fn connect(socket_path: &str) -> Result { + pub async fn connect(socket_path: &str) -> Result { log::debug!("Connecting to mpv socket at {}", socket_path); let socket = match UnixStream::connect(socket_path).await { Ok(stream) => Ok(stream), - Err(internal_error) => Err(Error(ErrorCode::ConnectError(internal_error.to_string()))), + Err(err) => Err(MpvError::MpvSocketConnectionError(err.to_string())), }?; Self::connect_socket(socket).await } - pub async fn connect_socket(socket: UnixStream) -> Result { + pub async fn connect_socket(socket: UnixStream) -> Result { let (com_tx, com_rx) = mpsc::channel(100); let (ev_tx, _) = broadcast::channel(100); let ipc = MpvIpc::new(socket, com_rx, ev_tx.clone()); @@ -230,29 +231,24 @@ impl Mpv { }) } - pub async fn disconnect(&self) -> Result<(), Error> { + pub async fn disconnect(&self) -> Result<(), MpvError> { let (res_tx, res_rx) = oneshot::channel(); self.command_sender .send((MpvIpcCommand::Exit, res_tx)) .await - .map_err(|_| { - Error(ErrorCode::ConnectError( - "Failed to send command".to_string(), - )) - })?; + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; + match res_rx.await { Ok(MpvIpcResponse(response)) => response.map(|_| ()), - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } - pub async fn get_event_stream(&self) -> impl futures::Stream> { + pub async fn get_event_stream(&self) -> impl futures::Stream> { tokio_stream::wrappers::BroadcastStream::new(self.broadcast_channel.subscribe()).map( |event| match event { Ok(event) => crate::event_parser::parse_event(event), - Err(_) => Err(Error(ErrorCode::ConnectError( - "Failed to receive event".to_string(), - ))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), }, ) } @@ -264,7 +260,7 @@ impl Mpv { &self, command: &str, args: &[&str], - ) -> Result, Error> { + ) -> Result, MpvError> { let command = Vec::from( [command] .iter() @@ -277,15 +273,11 @@ impl Mpv { self.command_sender .send((MpvIpcCommand::Command(command), res_tx)) .await - .map_err(|_| { - Error(ErrorCode::ConnectError( - "Failed to send command".to_string(), - )) - })?; + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; match res_rx.await { Ok(MpvIpcResponse(response)) => response, - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } @@ -293,7 +285,7 @@ impl Mpv { &self, command: &str, args: &[&str], - ) -> Result<(), Error> { + ) -> Result<(), MpvError> { self.run_command_raw(command, args).await.map(|_| ()) } @@ -323,7 +315,7 @@ impl Mpv { /// Ok(()) /// } /// ``` - pub async fn run_command(&self, command: MpvCommand) -> Result<(), Error> { + pub async fn run_command(&self, command: MpvCommand) -> Result<(), MpvError> { log::trace!("Running command: {:?}", command); let result = match command { MpvCommand::LoadFile { file, option } => { @@ -345,15 +337,11 @@ impl Mpv { self.command_sender .send((MpvIpcCommand::ObserveProperty(id, property), res_tx)) .await - .map_err(|_| { - Error(ErrorCode::ConnectError( - "Failed to send command".to_string(), - )) - })?; + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; match res_rx.await { Ok(MpvIpcResponse(response)) => response.map(|_| ()), - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } MpvCommand::PlaylistClear => { @@ -412,10 +400,11 @@ impl Mpv { self.command_sender .send((MpvIpcCommand::UnobserveProperty(id), res_tx)) .await - .unwrap(); + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; + match res_rx.await { Ok(MpvIpcResponse(response)) => response.map(|_| ()), - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } }; @@ -452,7 +441,7 @@ impl Mpv { pub async fn get_property( &self, property: &str, - ) -> Result { + ) -> Result { T::get_property_generic(self, property).await } @@ -475,21 +464,18 @@ impl Mpv { /// Ok(()) /// } /// ``` - pub async fn get_property_value(&self, property: &str) -> Result { + pub async fn get_property_value(&self, property: &str) -> Result { let (res_tx, res_rx) = oneshot::channel(); self.command_sender .send((MpvIpcCommand::GetProperty(property.to_owned()), res_tx)) .await - .map_err(|_| { - Error(ErrorCode::ConnectError( - "Failed to send command".to_string(), - )) - })?; + .map_err(|err| MpvError::InternalConnectionError(err.to_string()))?; + match res_rx.await { Ok(MpvIpcResponse(response)) => { - response.and_then(|value| value.ok_or(Error(ErrorCode::MissingValue))) + response.and_then(|value| value.ok_or(MpvError::MissingMpvData)) } - Err(err) => Err(Error(ErrorCode::ConnectError(err.to_string()))), + Err(err) => Err(MpvError::InternalConnectionError(err.to_string())), } } @@ -521,7 +507,7 @@ impl Mpv { &self, property: &str, value: T, - ) -> Result<(), Error> { + ) -> Result<(), MpvError> { T::set_property_generic(self, property, value).await } } diff --git a/src/error.rs b/src/error.rs index 4d11189..0a75ccb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,72 +1,46 @@ //! Library specific error messages. -use core::fmt; -use std::fmt::Display; +use thiserror::Error; +use serde_json::{Map, Value}; -use serde::{Deserialize, Serialize}; - -/// All possible errors that can occur when interacting with mpv. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum ErrorCode { - MpvError(String), - JsonParseError(String), - ConnectError(String), - JsonContainsUnexptectedType, - UnexpectedResult, - UnexpectedValue, - MissingValue, - UnsupportedType, - ValueDoesNotContainBool, - ValueDoesNotContainF64, - ValueDoesNotContainHashMap, - ValueDoesNotContainPlaylist, - ValueDoesNotContainString, - ValueDoesNotContainUsize, -} +use crate::MpvDataType; /// Any error that can occur when interacting with mpv. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct Error(pub ErrorCode); +#[derive(Error, Debug)] +pub enum MpvError { + #[error("MpvError: {0}")] + MpvError(String), -impl Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(&self.0, f) - } -} + #[error("Error communicating over mpv socket: {0}")] + MpvSocketConnectionError(String), -impl std::error::Error for Error {} + #[error("Internal connection error: {0}")] + InternalConnectionError(String), -impl Display for ErrorCode { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - ErrorCode::ConnectError(ref msg) => f.write_str(&format!("ConnectError: {}", msg)), - ErrorCode::JsonParseError(ref msg) => f.write_str(&format!("JsonParseError: {}", msg)), - ErrorCode::MpvError(ref msg) => f.write_str(&format!("MpvError: {}", msg)), - ErrorCode::JsonContainsUnexptectedType => { - f.write_str("Mpv sent a value with an unexpected type") - } - ErrorCode::UnexpectedResult => f.write_str("Unexpected result received"), - ErrorCode::UnexpectedValue => f.write_str("Unexpected value received"), - ErrorCode::MissingValue => f.write_str("Missing value"), - ErrorCode::UnsupportedType => f.write_str("Unsupported type received"), - ErrorCode::ValueDoesNotContainBool => { - f.write_str("The received value is not of type \'std::bool\'") - } - ErrorCode::ValueDoesNotContainF64 => { - f.write_str("The received value is not of type \'std::f64\'") - } - ErrorCode::ValueDoesNotContainHashMap => { - f.write_str("The received value is not of type \'std::collections::HashMap\'") - } - ErrorCode::ValueDoesNotContainPlaylist => { - f.write_str("The received value is not of type \'mpvipc::Playlist\'") - } - ErrorCode::ValueDoesNotContainString => { - f.write_str("The received value is not of type \'std::string::String\'") - } - ErrorCode::ValueDoesNotContainUsize => { - f.write_str("The received value is not of type \'std::usize\'") - } - } - } -} + #[error("JsonParseError: {0}")] + JsonParseError(#[from] serde_json::Error), + + #[error("Mpv sent a value with an unexpected type:\nExpected {expected_type}, received {received:#?}")] + ValueContainsUnexpectedType { + expected_type: String, + received: Value, + }, + + #[error("Mpv sent data with an unexpected type:\nExpected {expected_type}, received {received:#?}")] + DataContainsUnexpectedType { + expected_type: String, + received: MpvDataType, + }, + + #[error("Missing expected 'data' field in mpv message")] + MissingMpvData, + + #[error("Missing key in object:\nExpected {key} in {map:#?}")] + MissingKeyInObject { + key: String, + map: Map, + }, + + #[error("Unknown error: {0}")] + Other(String), +} \ No newline at end of file diff --git a/src/event_parser.rs b/src/event_parser.rs index ebcd736..5df061f 100644 --- a/src/event_parser.rs +++ b/src/event_parser.rs @@ -5,7 +5,7 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; -use crate::{ipc::MpvIpcEvent, message_parser::json_to_value, Error, ErrorCode, MpvDataType}; +use crate::{ipc::MpvIpcEvent, message_parser::json_to_value, MpvDataType, MpvError}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] @@ -147,6 +147,37 @@ pub enum Event { Unimplemented(Map), } +macro_rules! get_key_as { + ($as_type:ident, $key:expr, $event:ident) => {{ + let tmp = $event.get($key).ok_or(MpvError::MissingKeyInObject { + key: $key.to_owned(), + map: $event.clone(), + })?; + + tmp.$as_type() + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: stringify!($as_type).strip_prefix("as_").unwrap().to_owned(), + received: tmp.clone(), + })? + }}; +} + +macro_rules! get_optional_key_as { + ($as_type:ident, $key:expr, $event:ident) => {{ + if let Some(tmp) = $event.get($key) { + Some( + tmp.$as_type() + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: stringify!($as_type).strip_prefix("as_").unwrap().to_owned(), + received: tmp.clone(), + })?, + ) + } else { + None + } + }}; +} + // NOTE: I have not been able to test all of these events, // so some of the parsing logic might be incorrect. // In particular, I have not been able to make mpv @@ -157,19 +188,17 @@ pub enum Event { // If you need this, please open an issue or a PR. /// Parse a highlevel [`Event`] objects from json. -#[allow(deprecated)] -pub(crate) fn parse_event(raw_event: MpvIpcEvent) -> Result { +pub(crate) fn parse_event(raw_event: MpvIpcEvent) -> Result { let MpvIpcEvent(event) = raw_event; event .as_object() - .ok_or(Error(ErrorCode::JsonContainsUnexptectedType)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "object".to_owned(), + received: event.clone(), + }) .and_then(|event| { - let event_name = event - .get("event") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))?; + let event_name = get_key_as!(as_str, "event", event); match event_name { "start-file" => parse_start_file(event), @@ -200,35 +229,20 @@ pub(crate) fn parse_event(raw_event: MpvIpcEvent) -> Result { }) } -fn parse_start_file(event: &Map) -> Result { - let playlist_entry_id = event - .get("playlist_entry_id") - .ok_or(Error(ErrorCode::MissingValue))? - .as_u64() - .ok_or(Error(ErrorCode::ValueDoesNotContainUsize))? as usize; +fn parse_start_file(event: &Map) -> Result { + let playlist_entry_id = get_key_as!(as_u64, "playlist_entry_id", event) as usize; + Ok(Event::StartFile { playlist_entry_id }) } -fn parse_end_file(event: &Map) -> Result { - let reason = event - .get("reason") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))?; - let playlist_entry_id = event - .get("playlist_entry_id") - .ok_or(Error(ErrorCode::MissingValue))? - .as_u64() - .ok_or(Error(ErrorCode::ValueDoesNotContainUsize))? as usize; - let file_error = event - .get("file_error") - .and_then(|v| v.as_str().map(|s| s.to_string())); - let playlist_insert_id = event - .get("playlist_insert_id") - .and_then(|v| v.as_u64().map(|u| u as usize)); - let playlist_insert_num_entries = event - .get("playlist_insert_num_entries") - .and_then(|v| v.as_u64().map(|u| u as usize)); +fn parse_end_file(event: &Map) -> Result { + let reason = get_key_as!(as_str, "reason", event); + let playlist_entry_id = get_key_as!(as_u64, "playlist_entry_id", event) as usize; + let file_error = get_optional_key_as!(as_str, "file_error", event).map(|s| s.to_string()); + let playlist_insert_id = + get_optional_key_as!(as_u64, "playlist_insert_id", event).map(|i| i as usize); + let playlist_insert_num_entries = + get_optional_key_as!(as_u64, "playlist_insert_num_entries", event).map(|i| i as usize); Ok(Event::EndFile { reason: reason @@ -241,24 +255,10 @@ fn parse_end_file(event: &Map) -> Result { }) } -fn parse_log_message(event: &Map) -> Result { - let prefix = event - .get("prefix") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))? - .to_string(); - let level = event - .get("level") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))?; - let text = event - .get("text") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))? - .to_string(); +fn parse_log_message(event: &Map) -> Result { + let prefix = get_key_as!(as_str, "prefix", event).to_owned(); + let level = get_key_as!(as_str, "level", event); + let text = get_key_as!(as_str, "text", event).to_owned(); Ok(Event::LogMessage { prefix, @@ -269,45 +269,35 @@ fn parse_log_message(event: &Map) -> Result { }) } -fn parse_hook(event: &Map) -> Result { - let hook_id = event - .get("hook_id") - .ok_or(Error(ErrorCode::MissingValue))? - .as_u64() - .ok_or(Error(ErrorCode::ValueDoesNotContainUsize))? as usize; +fn parse_hook(event: &Map) -> Result { + let hook_id = get_key_as!(as_u64, "hook_id", event) as usize; Ok(Event::Hook { hook_id }) } -fn parse_client_message(event: &Map) -> Result { - let args = event - .get("args") - .ok_or(Error(ErrorCode::MissingValue))? - .as_array() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))? +fn parse_client_message(event: &Map) -> Result { + let args = get_key_as!(as_array, "args", event) .iter() .map(|arg| { arg.as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "string".to_owned(), + received: arg.clone(), + }) .map(|s| s.to_string()) }) - .collect::, Error>>()?; + .collect::, MpvError>>()?; Ok(Event::ClientMessage { args }) } -fn parse_property_change(event: &Map) -> Result { - let id = event - .get("id") - .ok_or(Error(ErrorCode::MissingValue))? - .as_u64() - .ok_or(Error(ErrorCode::ValueDoesNotContainUsize))? as usize; - let property_name = event - .get("name") - .ok_or(Error(ErrorCode::MissingValue))? - .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString))?; +fn parse_property_change(event: &Map) -> Result { + let id = get_key_as!(as_u64, "id", event) as usize; + let property_name = get_key_as!(as_str, "name", event); let data = event .get("data") - .ok_or(Error(ErrorCode::MissingValue))? + .ok_or(MpvError::MissingKeyInObject { + key: "data".to_owned(), + map: event.clone(), + })? .clone(); Ok(Event::PropertyChange { diff --git a/src/event_property_parser.rs b/src/event_property_parser.rs index 4ace4d2..3da8361 100644 --- a/src/event_property_parser.rs +++ b/src/event_property_parser.rs @@ -8,7 +8,7 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use crate::{Error, ErrorCode, Event, MpvDataType, PlaylistEntry}; +use crate::{Event, MpvDataType, MpvError, PlaylistEntry}; /// All possible properties that can be observed through the event system. /// @@ -46,7 +46,7 @@ pub enum LoopProperty { } /// Parse a highlevel [`Property`] object from json, used for [`Event::PropertyChange`]. -pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { +pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> { let (id, name, data) = match event { Event::PropertyChange { id, name, data } => (id, name, data), // TODO: return proper error @@ -60,14 +60,24 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { let path = match data { MpvDataType::String(s) => Some(s), MpvDataType::Null => None, - _ => return Err(Error(ErrorCode::ValueDoesNotContainString)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "String".to_owned(), + received: data, + }) + } }; Ok((id, Property::Path(path))) } "pause" => { let pause = match data { MpvDataType::Bool(b) => b, - _ => return Err(Error(ErrorCode::ValueDoesNotContainBool)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "bool".to_owned(), + received: data, + }) + } }; Ok((id, Property::Pause(pause))) } @@ -75,7 +85,12 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { let playback_time = match data { MpvDataType::Double(d) => Some(d), MpvDataType::Null => None, - _ => return Err(Error(ErrorCode::ValueDoesNotContainF64)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "f64".to_owned(), + received: data, + }) + } }; Ok((id, Property::PlaybackTime(playback_time))) } @@ -83,7 +98,12 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { let duration = match data { MpvDataType::Double(d) => Some(d), MpvDataType::Null => None, - _ => return Err(Error(ErrorCode::ValueDoesNotContainF64)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "f64".to_owned(), + received: data, + }) + } }; Ok((id, Property::Duration(duration))) } @@ -91,7 +111,12 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { let metadata = match data { MpvDataType::HashMap(m) => Some(m), MpvDataType::Null => None, - _ => return Err(Error(ErrorCode::ValueDoesNotContainHashMap)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "HashMap".to_owned(), + received: data, + }) + } }; Ok((id, Property::Metadata(metadata))) } @@ -108,60 +133,87 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), Error> { MpvDataType::Usize(u) => Some(u), MpvDataType::MinusOne => None, MpvDataType::Null => None, - _ => return Err(Error(ErrorCode::ValueDoesNotContainUsize)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "usize or -1".to_owned(), + received: data, + }) + } }; Ok((id, Property::PlaylistPos(playlist_pos))) } "loop-file" => { - let loop_file = match data { - MpvDataType::Usize(n) => LoopProperty::N(n), + let loop_file = match data.to_owned() { + MpvDataType::Usize(n) => Some(LoopProperty::N(n)), MpvDataType::Bool(b) => match b { - true => LoopProperty::Inf, - false => LoopProperty::No, + true => Some(LoopProperty::Inf), + false => Some(LoopProperty::No), }, MpvDataType::String(s) => match s.as_str() { - "inf" => LoopProperty::Inf, - "no" => LoopProperty::No, - _ => return Err(Error(ErrorCode::ValueDoesNotContainString)), + "inf" => Some(LoopProperty::Inf), + _ => None, }, - _ => return Err(Error(ErrorCode::ValueDoesNotContainString)), - }; + _ => None, + } + .ok_or(MpvError::DataContainsUnexpectedType { + expected_type: "'inf', bool, or usize".to_owned(), + received: data, + })?; Ok((id, Property::LoopFile(loop_file))) } "loop-playlist" => { - let loop_playlist = match data { - MpvDataType::Usize(n) => LoopProperty::N(n), + let loop_playlist = match data.to_owned() { + MpvDataType::Usize(n) => Some(LoopProperty::N(n)), MpvDataType::Bool(b) => match b { - true => LoopProperty::Inf, - false => LoopProperty::No, + true => Some(LoopProperty::Inf), + false => Some(LoopProperty::No), }, MpvDataType::String(s) => match s.as_str() { - "inf" => LoopProperty::Inf, - "no" => LoopProperty::No, - _ => return Err(Error(ErrorCode::ValueDoesNotContainString)), + "inf" => Some(LoopProperty::Inf), + _ => None, }, - _ => return Err(Error(ErrorCode::ValueDoesNotContainString)), - }; + _ => None, + } + .ok_or(MpvError::DataContainsUnexpectedType { + expected_type: "'inf', bool, or usize".to_owned(), + received: data, + })?; + Ok((id, Property::LoopPlaylist(loop_playlist))) } "speed" => { let speed = match data { MpvDataType::Double(d) => d, - _ => return Err(Error(ErrorCode::ValueDoesNotContainF64)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "f64".to_owned(), + received: data, + }) + } }; Ok((id, Property::Speed(speed))) } "volume" => { let volume = match data { MpvDataType::Double(d) => d, - _ => return Err(Error(ErrorCode::ValueDoesNotContainF64)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "f64".to_owned(), + received: data, + }) + } }; Ok((id, Property::Volume(volume))) } "mute" => { let mute = match data { MpvDataType::Bool(b) => b, - _ => return Err(Error(ErrorCode::ValueDoesNotContainBool)), + _ => { + return Err(MpvError::DataContainsUnexpectedType { + expected_type: "bool".to_owned(), + received: data, + }) + } }; Ok((id, Property::Mute(mute))) } diff --git a/src/highlevel_api_extension.rs b/src/highlevel_api_extension.rs index 5cf7404..d6df375 100644 --- a/src/highlevel_api_extension.rs +++ b/src/highlevel_api_extension.rs @@ -1,7 +1,7 @@ //! High-level API extension for [`Mpv`]. use crate::{ - Error, IntoRawCommandPart, Mpv, MpvCommand, MpvDataType, Playlist, PlaylistAddOptions, + MpvError, IntoRawCommandPart, Mpv, MpvCommand, MpvDataType, Playlist, PlaylistAddOptions, PlaylistEntry, SeekOptions, }; use serde::{Deserialize, Serialize}; @@ -44,58 +44,58 @@ pub enum PlaylistAddTypeOptions { // TODO: fix this #[allow(async_fn_in_trait)] pub trait MpvExt { - async fn toggle(&self) -> Result<(), Error>; - async fn stop(&self) -> Result<(), Error>; + async fn toggle(&self) -> Result<(), MpvError>; + async fn stop(&self) -> Result<(), MpvError>; async fn set_volume(&self, input_volume: f64, option: NumberChangeOptions) - -> Result<(), Error>; - async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), Error>; - async fn set_mute(&self, option: Switch) -> Result<(), Error>; - async fn set_loop_playlist(&self, option: Switch) -> Result<(), Error>; - async fn set_loop_file(&self, option: Switch) -> Result<(), Error>; - async fn seek(&self, seconds: f64, option: SeekOptions) -> Result<(), Error>; - async fn playlist_shuffle(&self) -> Result<(), Error>; - async fn playlist_remove_id(&self, id: usize) -> Result<(), Error>; - async fn playlist_play_next(&self, id: usize) -> Result<(), Error>; - async fn playlist_play_id(&self, id: usize) -> Result<(), Error>; - async fn playlist_move_id(&self, from: usize, to: usize) -> Result<(), Error>; - async fn playlist_clear(&self) -> Result<(), Error>; + -> Result<(), MpvError>; + async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), MpvError>; + async fn set_mute(&self, option: Switch) -> Result<(), MpvError>; + async fn set_loop_playlist(&self, option: Switch) -> Result<(), MpvError>; + async fn set_loop_file(&self, option: Switch) -> Result<(), MpvError>; + async fn seek(&self, seconds: f64, option: SeekOptions) -> Result<(), MpvError>; + async fn playlist_shuffle(&self) -> Result<(), MpvError>; + async fn playlist_remove_id(&self, id: usize) -> Result<(), MpvError>; + async fn playlist_play_next(&self, id: usize) -> Result<(), MpvError>; + async fn playlist_play_id(&self, id: usize) -> Result<(), MpvError>; + async fn playlist_move_id(&self, from: usize, to: usize) -> Result<(), MpvError>; + async fn playlist_clear(&self) -> Result<(), MpvError>; async fn playlist_add( &self, file: &str, file_type: PlaylistAddTypeOptions, option: PlaylistAddOptions, - ) -> Result<(), Error>; - async fn restart(&self) -> Result<(), Error>; - async fn prev(&self) -> Result<(), Error>; - async fn pause(&self) -> Result<(), Error>; - async fn unobserve_property(&self, id: isize) -> Result<(), Error>; - async fn observe_property(&self, id: isize, property: &str) -> Result<(), Error>; - async fn next(&self) -> Result<(), Error>; - async fn kill(&self) -> Result<(), Error>; - async fn get_playlist(&self) -> Result; - async fn get_metadata(&self) -> Result, Error>; + ) -> Result<(), MpvError>; + async fn restart(&self) -> Result<(), MpvError>; + async fn prev(&self) -> Result<(), MpvError>; + async fn pause(&self) -> Result<(), MpvError>; + async fn unobserve_property(&self, id: isize) -> Result<(), MpvError>; + async fn observe_property(&self, id: isize, property: &str) -> Result<(), MpvError>; + async fn next(&self) -> Result<(), MpvError>; + async fn kill(&self) -> Result<(), MpvError>; + async fn get_playlist(&self) -> Result; + async fn get_metadata(&self) -> Result, MpvError>; } impl MpvExt for Mpv { - async fn get_metadata(&self) -> Result, Error> { + async fn get_metadata(&self) -> Result, MpvError> { self.get_property("metadata").await } - async fn get_playlist(&self) -> Result { + async fn get_playlist(&self) -> Result { self.get_property::>("playlist") .await .map(Playlist) } - async fn kill(&self) -> Result<(), Error> { + async fn kill(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::Quit).await } - async fn next(&self) -> Result<(), Error> { + async fn next(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistNext).await } - async fn observe_property(&self, id: isize, property: &str) -> Result<(), Error> { + async fn observe_property(&self, id: isize, property: &str) -> Result<(), MpvError> { self.run_command(MpvCommand::Observe { id, property: property.to_string(), @@ -103,19 +103,19 @@ impl MpvExt for Mpv { .await } - async fn unobserve_property(&self, id: isize) -> Result<(), Error> { + async fn unobserve_property(&self, id: isize) -> Result<(), MpvError> { self.run_command(MpvCommand::Unobserve(id)).await } - async fn pause(&self) -> Result<(), Error> { + async fn pause(&self) -> Result<(), MpvError> { self.set_property("pause", true).await } - async fn prev(&self) -> Result<(), Error> { + async fn prev(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistPrev).await } - async fn restart(&self) -> Result<(), Error> { + async fn restart(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::Seek { seconds: 0f64, option: SeekOptions::Absolute, @@ -128,7 +128,7 @@ impl MpvExt for Mpv { file: &str, file_type: PlaylistAddTypeOptions, option: PlaylistAddOptions, - ) -> Result<(), Error> { + ) -> Result<(), MpvError> { match file_type { PlaylistAddTypeOptions::File => { self.run_command(MpvCommand::LoadFile { @@ -148,20 +148,20 @@ impl MpvExt for Mpv { } } - async fn playlist_clear(&self) -> Result<(), Error> { + async fn playlist_clear(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistClear).await } - async fn playlist_move_id(&self, from: usize, to: usize) -> Result<(), Error> { + async fn playlist_move_id(&self, from: usize, to: usize) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistMove { from, to }) .await } - async fn playlist_play_id(&self, id: usize) -> Result<(), Error> { + async fn playlist_play_id(&self, id: usize) -> Result<(), MpvError> { self.set_property("playlist-pos", id).await } - async fn playlist_play_next(&self, id: usize) -> Result<(), Error> { + async fn playlist_play_next(&self, id: usize) -> Result<(), MpvError> { match self.get_property::("playlist-pos").await { Ok(current_id) => { self.run_command(MpvCommand::PlaylistMove { @@ -174,19 +174,19 @@ impl MpvExt for Mpv { } } - async fn playlist_remove_id(&self, id: usize) -> Result<(), Error> { + async fn playlist_remove_id(&self, id: usize) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistRemove(id)).await } - async fn playlist_shuffle(&self) -> Result<(), Error> { + async fn playlist_shuffle(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::PlaylistShuffle).await } - async fn seek(&self, seconds: f64, option: SeekOptions) -> Result<(), Error> { + async fn seek(&self, seconds: f64, option: SeekOptions) -> Result<(), MpvError> { self.run_command(MpvCommand::Seek { seconds, option }).await } - async fn set_loop_file(&self, option: Switch) -> Result<(), Error> { + async fn set_loop_file(&self, option: Switch) -> Result<(), MpvError> { let enabled = match option { Switch::On => "inf", Switch::Off => "no", @@ -203,7 +203,7 @@ impl MpvExt for Mpv { self.set_property("loop-file", enabled).await } - async fn set_loop_playlist(&self, option: Switch) -> Result<(), Error> { + async fn set_loop_playlist(&self, option: Switch) -> Result<(), MpvError> { let enabled = match option { Switch::On => "inf", Switch::Off => "no", @@ -220,7 +220,7 @@ impl MpvExt for Mpv { self.set_property("loo-playlist", enabled).await } - async fn set_mute(&self, option: Switch) -> Result<(), Error> { + async fn set_mute(&self, option: Switch) -> Result<(), MpvError> { let enabled = match option { Switch::On => "yes", Switch::Off => "no", @@ -237,7 +237,7 @@ impl MpvExt for Mpv { self.set_property("mute", enabled).await } - async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), Error> { + async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), MpvError> { match self.get_property::("speed").await { Ok(speed) => match option { NumberChangeOptions::Increase => { @@ -258,7 +258,7 @@ impl MpvExt for Mpv { &self, input_volume: f64, option: NumberChangeOptions, - ) -> Result<(), Error> { + ) -> Result<(), MpvError> { match self.get_property::("volume").await { Ok(volume) => match option { NumberChangeOptions::Increase => { @@ -275,11 +275,11 @@ impl MpvExt for Mpv { } } - async fn stop(&self) -> Result<(), Error> { + async fn stop(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::Stop).await } - async fn toggle(&self) -> Result<(), Error> { + async fn toggle(&self) -> Result<(), MpvError> { self.run_command_raw("cycle", &["pause"]).await.map(|_| ()) } } diff --git a/src/ipc.rs b/src/ipc.rs index 341c81b..e269c7f 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -8,7 +8,7 @@ use tokio::{ }; use tokio_util::codec::{Framed, LinesCodec}; -use crate::{Error, ErrorCode}; +use crate::MpvError; /// Container for all state that regards communication with the mpv IPC socket /// and message passing with [`Mpv`](crate::Mpv) controllers. @@ -30,8 +30,8 @@ pub(crate) enum MpvIpcCommand { } /// [`MpvIpc`]'s response to a [`MpvIpcCommand`]. -#[derive(Debug, Clone)] -pub(crate) struct MpvIpcResponse(pub(crate) Result, Error>); +#[derive(Debug)] +pub(crate) struct MpvIpcResponse(pub(crate) Result, MpvError>); /// A deserialized and partially parsed event from mpv. #[derive(Debug, Clone)] @@ -50,28 +50,33 @@ impl MpvIpc { } } - pub(crate) async fn send_command(&mut self, command: &[Value]) -> Result, Error> { + pub(crate) async fn send_command( + &mut self, + command: &[Value], + ) -> Result, MpvError> { let ipc_command = json!({ "command": command }); - let ipc_command_str = serde_json::to_string(&ipc_command) - .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; + let ipc_command_str = + serde_json::to_string(&ipc_command).map_err(|why| MpvError::JsonParseError(why))?; log::trace!("Sending command: {}", ipc_command_str); self.socket .send(ipc_command_str) .await - .map_err(|why| Error(ErrorCode::ConnectError(why.to_string())))?; + .map_err(|why| MpvError::MpvSocketConnectionError(why.to_string()))?; let response = loop { let response = self .socket .next() .await - .ok_or(Error(ErrorCode::MissingValue))? - .map_err(|why| Error(ErrorCode::ConnectError(why.to_string())))?; + .ok_or(MpvError::MpvSocketConnectionError( + "Could not receive response from mpv".to_owned(), + ))? + .map_err(|why| MpvError::MpvSocketConnectionError(why.to_string()))?; let parsed_response = serde_json::from_str::(&response) - .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string()))); + .map_err(|why| MpvError::JsonParseError(why)); if parsed_response .as_ref() @@ -93,7 +98,7 @@ impl MpvIpc { pub(crate) async fn get_mpv_property( &mut self, property: &str, - ) -> Result, Error> { + ) -> Result, MpvError> { self.send_command(&[json!("get_property"), json!(property)]) .await } @@ -102,7 +107,7 @@ impl MpvIpc { &mut self, property: &str, value: Value, - ) -> Result, Error> { + ) -> Result, MpvError> { self.send_command(&[json!("set_property"), json!(property), value]) .await } @@ -111,17 +116,20 @@ impl MpvIpc { &mut self, id: isize, property: &str, - ) -> Result, Error> { + ) -> Result, MpvError> { self.send_command(&[json!("observe_property"), json!(id), json!(property)]) .await } - pub(crate) async fn unobserve_property(&mut self, id: isize) -> Result, Error> { + pub(crate) async fn unobserve_property( + &mut self, + id: isize, + ) -> Result, MpvError> { self.send_command(&[json!("unobserve_property"), json!(id)]) .await } - async fn handle_event(&mut self, event: Result) { + async fn handle_event(&mut self, event: Result) { match &event { Ok(event) => { log::trace!("Parsed event: {:?}", event); @@ -137,17 +145,17 @@ impl MpvIpc { } } - pub(crate) async fn run(mut self) -> Result<(), Error> { + pub(crate) async fn run(mut self) -> Result<(), MpvError> { loop { tokio::select! { Some(event) = self.socket.next() => { log::trace!("Got event: {:?}", event); let parsed_event = event - .map_err(|why| Error(ErrorCode::ConnectError(why.to_string()))) + .map_err(|why| MpvError::MpvSocketConnectionError(why.to_string())) .and_then(|event| serde_json::from_str::(&event) - .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))); + .map_err(|why| MpvError::JsonParseError(why))); self.handle_event(parsed_event).await; } @@ -189,20 +197,40 @@ impl MpvIpc { /// This function does the most basic JSON parsing and error handling /// for status codes and errors that all responses from mpv are /// expected to contain. -fn parse_mpv_response_data(value: Value) -> Result, Error> { +fn parse_mpv_response_data(value: Value) -> Result, MpvError> { log::trace!("Parsing mpv response data: {:?}", value); let result = value .as_object() - .map(|o| (o.get("error").and_then(|e| e.as_str()), o.get("data"))) - .ok_or(Error(ErrorCode::UnexpectedValue)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "object".to_string(), + received: value.clone(), + }) + .and_then(|o| { + let error = o + .get("error") + .ok_or(MpvError::MissingKeyInObject { + key: "error".to_string(), + map: o.clone(), + })? + .as_str() + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "string".to_string(), + received: o.get("error").unwrap().clone(), + })?; + + let data = o.get("data"); + + Ok((error, data)) + }) .and_then(|(error, data)| match error { - Some("success") => Ok(data), - Some(e) => Err(Error(ErrorCode::MpvError(e.to_string()))), - None => Err(Error(ErrorCode::UnexpectedValue)), + "success" => Ok(data), + err => Err(MpvError::MpvError(err.to_string())), }); + match &result { Ok(v) => log::trace!("Successfully parsed mpv response data: {:?}", v), Err(e) => log::trace!("Error parsing mpv response data: {:?}", e), } + result.map(|opt| opt.cloned()) } diff --git a/src/message_parser.rs b/src/message_parser.rs index cef858c..ba335c0 100644 --- a/src/message_parser.rs +++ b/src/message_parser.rs @@ -4,18 +4,21 @@ use std::collections::HashMap; use serde_json::Value; -use crate::{Error, ErrorCode, MpvDataType, PlaylistEntry}; +use crate::{MpvDataType, MpvError, PlaylistEntry}; pub trait TypeHandler: Sized { - fn get_value(value: Value) -> Result; + fn get_value(value: Value) -> Result; fn as_string(&self) -> String; } impl TypeHandler for String { - fn get_value(value: Value) -> Result { + fn get_value(value: Value) -> Result { value .as_str() - .ok_or(Error(ErrorCode::ValueDoesNotContainString)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "String".to_string(), + received: value.clone(), + }) .map(|s| s.to_string()) } @@ -25,10 +28,13 @@ impl TypeHandler for String { } impl TypeHandler for bool { - fn get_value(value: Value) -> Result { + fn get_value(value: Value) -> Result { value .as_bool() - .ok_or(Error(ErrorCode::ValueDoesNotContainBool)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "bool".to_string(), + received: value.clone(), + }) } fn as_string(&self) -> String { @@ -41,10 +47,11 @@ impl TypeHandler for bool { } impl TypeHandler for f64 { - fn get_value(value: Value) -> Result { - value - .as_f64() - .ok_or(Error(ErrorCode::ValueDoesNotContainF64)) + fn get_value(value: Value) -> Result { + value.as_f64().ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "f64".to_string(), + received: value.clone(), + }) } fn as_string(&self) -> String { @@ -53,11 +60,14 @@ impl TypeHandler for f64 { } impl TypeHandler for usize { - fn get_value(value: Value) -> Result { + fn get_value(value: Value) -> Result { value .as_u64() .map(|u| u as usize) - .ok_or(Error(ErrorCode::ValueDoesNotContainUsize)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "usize".to_string(), + received: value.clone(), + }) } fn as_string(&self) -> String { @@ -65,12 +75,25 @@ impl TypeHandler for usize { } } +impl TypeHandler for MpvDataType { + fn get_value(value: Value) -> Result { + json_to_value(&value) + } + + fn as_string(&self) -> String { + format!("{:?}", self) + } +} + impl TypeHandler for HashMap { - fn get_value(value: Value) -> Result, Error> { + fn get_value(value: Value) -> Result, MpvError> { value .as_object() - .ok_or(Error(ErrorCode::ValueDoesNotContainHashMap)) - .map(json_map_to_hashmap) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "Map".to_string(), + received: value.clone(), + }) + .and_then(json_map_to_hashmap) } fn as_string(&self) -> String { @@ -79,10 +102,13 @@ impl TypeHandler for HashMap { } impl TypeHandler for Vec { - fn get_value(value: Value) -> Result, Error> { + fn get_value(value: Value) -> Result, MpvError> { value .as_array() - .ok_or(Error(ErrorCode::ValueDoesNotContainPlaylist)) + .ok_or(MpvError::ValueContainsUnexpectedType { + expected_type: "Array".to_string(), + received: value.clone(), + }) .map(|array| json_array_to_playlist(array)) } @@ -91,9 +117,9 @@ impl TypeHandler for Vec { } } -pub(crate) fn json_to_value(value: &Value) -> Result { +pub(crate) fn json_to_value(value: &Value) -> Result { match value { - Value::Array(array) => Ok(MpvDataType::Array(json_array_to_vec(array))), + Value::Array(array) => Ok(MpvDataType::Array(json_array_to_vec(array)?)), Value::Bool(b) => Ok(MpvDataType::Bool(*b)), Value::Number(n) => { if n.is_i64() && n.as_i64().unwrap() == -1 { @@ -103,11 +129,13 @@ pub(crate) fn json_to_value(value: &Value) -> Result { } else if n.is_f64() { Ok(MpvDataType::Double(n.as_f64().unwrap())) } else { - // TODO: proper error handling - panic!("Unexpected number type"); + Err(MpvError::ValueContainsUnexpectedType { + expected_type: "i64, u64, or f64".to_string(), + received: value.clone(), + }) } } - Value::Object(map) => Ok(MpvDataType::HashMap(json_map_to_hashmap(map))), + Value::Object(map) => Ok(MpvDataType::HashMap(json_map_to_hashmap(map)?)), Value::String(s) => Ok(MpvDataType::String(s.to_string())), Value::Null => Ok(MpvDataType::Null), } @@ -115,23 +143,16 @@ pub(crate) fn json_to_value(value: &Value) -> Result { pub(crate) fn json_map_to_hashmap( map: &serde_json::map::Map, -) -> HashMap { +) -> Result, MpvError> { let mut output_map: HashMap = HashMap::new(); for (ref key, value) in map.iter() { - // TODO: proper error handling - if let Ok(value) = json_to_value(value) { - output_map.insert(key.to_string(), value); - } + output_map.insert(key.to_string(), json_to_value(value)?); } - output_map + Ok(output_map) } -pub(crate) fn json_array_to_vec(array: &[Value]) -> Vec { - array - .iter() - // TODO: proper error handling - .filter_map(|entry| json_to_value(entry).ok()) - .collect() +pub(crate) fn json_array_to_vec(array: &[Value]) -> Result, MpvError> { + array.iter().map(|entry| json_to_value(entry)).collect() } pub(crate) fn json_array_to_playlist(array: &[Value]) -> Vec { @@ -207,7 +228,10 @@ mod test { )])), ); - assert_eq!(json_map_to_hashmap(json.as_object().unwrap()), expected); + match json_map_to_hashmap(json.as_object().unwrap()) { + Ok(m) => assert_eq!(m, expected), + Err(e) => panic!("{:?}", e), + } } #[test] @@ -246,7 +270,10 @@ mod test { )])), ]; - assert_eq!(json_array_to_vec(json.as_array().unwrap()), expected); + match json_array_to_vec(json.as_array().unwrap()) { + Ok(v) => assert_eq!(v, expected), + Err(e) => panic!("{:?}", e), + } } #[test] diff --git a/tests/get_property.rs b/tests/get_property.rs index 9baac20..3a3b0b0 100644 --- a/tests/get_property.rs +++ b/tests/get_property.rs @@ -1,7 +1,7 @@ use std::{panic, time::Duration}; use futures::{stream::FuturesUnordered, SinkExt, StreamExt}; -use mpvipc::{Error, ErrorCode, Mpv, MpvExt, Playlist, PlaylistEntry}; +use mpvipc::{MpvError, Mpv, MpvExt, Playlist, PlaylistEntry}; use serde_json::{json, Value}; use test_log::test; use tokio::{net::UnixStream, task::JoinHandle}; @@ -41,12 +41,12 @@ async fn test_get_property_broken_pipe() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_volume = mpv.get_property::("volume").await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::ConnectError( - "Broken pipe (os error 32)".to_owned() - ))) - ); + match maybe_volume { + Err(MpvError::MpvSocketConnectionError(err)) => { + assert_eq!(err.to_string(), "Broken pipe (os error 32)"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); } @@ -59,7 +59,16 @@ async fn test_get_property_wrong_type() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_volume = mpv.get_property::("volume").await; - assert_eq!(maybe_volume, Err(Error(ErrorCode::ValueDoesNotContainBool))); + match maybe_volume { + Err(MpvError::ValueContainsUnexpectedType { + expected_type, + received, + }) => { + assert_eq!(expected_type, "bool"); + assert_eq!(received, json!(100.0)); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); } @@ -72,12 +81,12 @@ async fn test_get_property_error() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_volume = mpv.get_property::("volume").await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::MpvError( - "property unavailable".to_owned() - ))) - ); + match maybe_volume { + Err(MpvError::MpvError(err)) => { + assert_eq!(err, "property unavailable"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); } @@ -140,12 +149,12 @@ async fn test_get_property_simultaneous_requests() { loop { tokio::time::sleep(Duration::from_millis(2)).await; let maybe_volume = mpv_clone_3.get_property::("nonexistent").await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::MpvError( - "property unavailable".to_owned() - ))) - ); + match maybe_volume { + Err(MpvError::MpvError(err)) => { + assert_eq!(err, "property unavailable"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } } }); diff --git a/tests/integration.rs b/tests/integration.rs index 87546a6..a75b323 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1,4 +1,4 @@ -use mpvipc::{Error, Mpv, MpvExt}; +use mpvipc::{MpvError, Mpv, MpvExt}; use std::path::Path; use tokio::{ process::{Child, Command}, @@ -6,7 +6,7 @@ use tokio::{ }; #[cfg(target_family = "unix")] -async fn spawn_headless_mpv() -> Result<(Child, Mpv), Error> { +async fn spawn_headless_mpv() -> Result<(Child, Mpv), MpvError> { let socket_path_str = format!("/tmp/mpv-ipc-{}", uuid::Uuid::new_v4()); let socket_path = Path::new(&socket_path_str); diff --git a/tests/set_property.rs b/tests/set_property.rs index edafaea..22b249f 100644 --- a/tests/set_property.rs +++ b/tests/set_property.rs @@ -1,7 +1,7 @@ use std::{panic, time::Duration}; use futures::{stream::FuturesUnordered, SinkExt, StreamExt}; -use mpvipc::{Error, ErrorCode, Mpv, MpvExt, Playlist, PlaylistEntry}; +use mpvipc::{MpvError, Mpv, MpvExt, Playlist, PlaylistEntry}; use serde_json::{json, Value}; use test_log::test; use tokio::{net::UnixStream, task::JoinHandle}; @@ -41,12 +41,12 @@ async fn test_set_property_broken_pipe() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_set_volume = mpv.set_property("volume", 64.0).await; - assert_eq!( - maybe_set_volume, - Err(Error(ErrorCode::ConnectError( - "Broken pipe (os error 32)".to_owned() - ))) - ); + match maybe_set_volume { + Err(MpvError::MpvSocketConnectionError(err)) => { + assert_eq!(err.to_string(), "Broken pipe (os error 32)"); + } + _ => panic!("Unexpected result: {:?}", maybe_set_volume), + } join_handle.await.unwrap().unwrap(); } @@ -59,12 +59,12 @@ async fn test_set_property_wrong_type() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_volume = mpv.set_property::("volume", true).await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::MpvError( - "unsupported format for accessing property".to_owned() - ))) - ); + match maybe_volume { + Err(MpvError::MpvError(err)) => { + assert_eq!(err, "unsupported format for accessing property"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); } @@ -77,10 +77,12 @@ async fn test_get_property_error() { let mpv = Mpv::connect_socket(server).await.unwrap(); let maybe_volume = mpv.set_property("nonexistent", true).await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::MpvError("property not found".to_owned()))) - ); + match maybe_volume { + Err(MpvError::MpvError(err)) => { + assert_eq!(err, "property not found"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); } @@ -127,7 +129,10 @@ async fn test_set_property_simultaneous_requests() { let mpv_poller_1 = tokio::spawn(async move { loop { let status = mpv_clone_1.set_property("volume", 100).await; - assert_eq!(status, Ok(())); + match status { + Ok(()) => {}, + _ => panic!("Unexpected result: {:?}", status), + } } }); @@ -136,7 +141,10 @@ async fn test_set_property_simultaneous_requests() { loop { tokio::time::sleep(Duration::from_millis(1)).await; let status = mpv_clone_2.set_property("pause", false).await; - assert_eq!(status, Ok(())); + match status { + Ok(()) => {}, + _ => panic!("Unexpected result: {:?}", status), + } } }); @@ -145,10 +153,12 @@ async fn test_set_property_simultaneous_requests() { loop { tokio::time::sleep(Duration::from_millis(2)).await; let maybe_volume = mpv_clone_3.set_property("nonexistent", "a").await; - assert_eq!( - maybe_volume, - Err(Error(ErrorCode::MpvError("property not found".to_owned()))) - ); + match maybe_volume { + Err(MpvError::MpvError(err)) => { + assert_eq!(err, "property not found"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } } }); -- 2.47.1 From 878cebbc9f72906f776a7946e6b9f7ea451e2cb3 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:27 +0200 Subject: [PATCH 2/9] use usize for request ids --- src/core_api.rs | 4 ++-- src/highlevel_api_extension.rs | 8 ++++---- src/ipc.rs | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core_api.rs b/src/core_api.rs index e45dbd4..69e8686 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -41,7 +41,7 @@ pub enum MpvCommand { to: usize, }, Observe { - id: isize, + id: usize, property: String, }, PlaylistNext, @@ -59,7 +59,7 @@ pub enum MpvCommand { option: SeekOptions, }, Stop, - Unobserve(isize), + Unobserve(usize), } /// Helper trait to keep track of the string literals that mpv expects. diff --git a/src/highlevel_api_extension.rs b/src/highlevel_api_extension.rs index d6df375..61056b7 100644 --- a/src/highlevel_api_extension.rs +++ b/src/highlevel_api_extension.rs @@ -68,8 +68,8 @@ pub trait MpvExt { async fn restart(&self) -> Result<(), MpvError>; async fn prev(&self) -> Result<(), MpvError>; async fn pause(&self) -> Result<(), MpvError>; - async fn unobserve_property(&self, id: isize) -> Result<(), MpvError>; - async fn observe_property(&self, id: isize, property: &str) -> Result<(), MpvError>; + async fn unobserve_property(&self, id: usize) -> Result<(), MpvError>; + async fn observe_property(&self, id: usize, property: &str) -> Result<(), MpvError>; async fn next(&self) -> Result<(), MpvError>; async fn kill(&self) -> Result<(), MpvError>; async fn get_playlist(&self) -> Result; @@ -95,7 +95,7 @@ impl MpvExt for Mpv { self.run_command(MpvCommand::PlaylistNext).await } - async fn observe_property(&self, id: isize, property: &str) -> Result<(), MpvError> { + async fn observe_property(&self, id: usize, property: &str) -> Result<(), MpvError> { self.run_command(MpvCommand::Observe { id, property: property.to_string(), @@ -103,7 +103,7 @@ impl MpvExt for Mpv { .await } - async fn unobserve_property(&self, id: isize) -> Result<(), MpvError> { + async fn unobserve_property(&self, id: usize) -> Result<(), MpvError> { self.run_command(MpvCommand::Unobserve(id)).await } diff --git a/src/ipc.rs b/src/ipc.rs index e269c7f..8531639 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -24,8 +24,8 @@ pub(crate) enum MpvIpcCommand { Command(Vec), GetProperty(String), SetProperty(String, Value), - ObserveProperty(isize, String), - UnobserveProperty(isize), + ObserveProperty(usize, String), + UnobserveProperty(usize), Exit, } @@ -114,7 +114,7 @@ impl MpvIpc { pub(crate) async fn observe_property( &mut self, - id: isize, + id: usize, property: &str, ) -> Result, MpvError> { self.send_command(&[json!("observe_property"), json!(id), json!(property)]) @@ -123,7 +123,7 @@ impl MpvIpc { pub(crate) async fn unobserve_property( &mut self, - id: isize, + id: usize, ) -> Result, MpvError> { self.send_command(&[json!("unobserve_property"), json!(id)]) .await -- 2.47.1 From 2736d1d7ada38934b5d0549d1780df11e9e00bd9 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:28 +0200 Subject: [PATCH 3/9] wrap event data in `Option` --- src/event_parser.rs | 12 ++--- src/event_property_parser.rs | 96 ++++++++++++++++++++++-------------- tests/events.rs | 5 +- 3 files changed, 65 insertions(+), 48 deletions(-) diff --git a/src/event_parser.rs b/src/event_parser.rs index 5df061f..8518e67 100644 --- a/src/event_parser.rs +++ b/src/event_parser.rs @@ -111,7 +111,7 @@ pub enum Event { PropertyChange { id: usize, name: String, - data: MpvDataType, + data: Option, }, EventQueueOverflow, None, @@ -292,17 +292,11 @@ fn parse_client_message(event: &Map) -> Result { fn parse_property_change(event: &Map) -> Result { let id = get_key_as!(as_u64, "id", event) as usize; let property_name = get_key_as!(as_str, "name", event); - let data = event - .get("data") - .ok_or(MpvError::MissingKeyInObject { - key: "data".to_owned(), - map: event.clone(), - })? - .clone(); + let data = event.get("data").map(|d| json_to_value(d)).transpose()?; Ok(Event::PropertyChange { id, name: property_name.to_string(), - data: json_to_value(&data)?, + data: data, }) } diff --git a/src/event_property_parser.rs b/src/event_property_parser.rs index 3da8361..c3b7d2d 100644 --- a/src/event_property_parser.rs +++ b/src/event_property_parser.rs @@ -34,7 +34,7 @@ pub enum Property { Speed(f64), Volume(f64), Mute(bool), - Unknown { name: String, data: MpvDataType }, + Unknown { name: String, data: Option }, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -58,34 +58,40 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> match name.as_str() { "path" => { let path = match data { - MpvDataType::String(s) => Some(s), - MpvDataType::Null => None, - _ => { + Some(MpvDataType::String(s)) => Some(s), + Some(MpvDataType::Null) => None, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "String".to_owned(), received: data, }) } + None => { + return Err(MpvError::MissingMpvData); + } }; Ok((id, Property::Path(path))) } "pause" => { let pause = match data { - MpvDataType::Bool(b) => b, - _ => { + Some(MpvDataType::Bool(b)) => b, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "bool".to_owned(), received: data, }) } + None => { + return Err(MpvError::MissingMpvData); + } }; Ok((id, Property::Pause(pause))) } "playback-time" => { let playback_time = match data { - MpvDataType::Double(d) => Some(d), - MpvDataType::Null => None, - _ => { + Some(MpvDataType::Double(d)) => Some(d), + None | Some(MpvDataType::Null) => None, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "f64".to_owned(), received: data, @@ -96,9 +102,9 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> } "duration" => { let duration = match data { - MpvDataType::Double(d) => Some(d), - MpvDataType::Null => None, - _ => { + Some(MpvDataType::Double(d)) => Some(d), + None | Some(MpvDataType::Null) => None, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "f64".to_owned(), received: data, @@ -109,9 +115,9 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> } "metadata" => { let metadata = match data { - MpvDataType::HashMap(m) => Some(m), - MpvDataType::Null => None, - _ => { + Some(MpvDataType::HashMap(m)) => Some(m), + None | Some(MpvDataType::Null) => None, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "HashMap".to_owned(), received: data, @@ -130,10 +136,11 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> // } "playlist-pos" => { let playlist_pos = match data { - MpvDataType::Usize(u) => Some(u), - MpvDataType::MinusOne => None, - MpvDataType::Null => None, - _ => { + Some(MpvDataType::Usize(u)) => Some(u), + Some(MpvDataType::MinusOne) => None, + Some(MpvDataType::Null) => None, + None => None, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "usize or -1".to_owned(), received: data, @@ -144,76 +151,91 @@ pub fn parse_event_property(event: Event) -> Result<(usize, Property), MpvError> } "loop-file" => { let loop_file = match data.to_owned() { - MpvDataType::Usize(n) => Some(LoopProperty::N(n)), - MpvDataType::Bool(b) => match b { + Some(MpvDataType::Usize(n)) => Some(LoopProperty::N(n)), + Some(MpvDataType::Bool(b)) => match b { true => Some(LoopProperty::Inf), false => Some(LoopProperty::No), }, - MpvDataType::String(s) => match s.as_str() { + Some(MpvDataType::String(s)) => match s.as_str() { "inf" => Some(LoopProperty::Inf), _ => None, }, _ => None, } - .ok_or(MpvError::DataContainsUnexpectedType { - expected_type: "'inf', bool, or usize".to_owned(), - received: data, + .ok_or(match data { + Some(data) => MpvError::DataContainsUnexpectedType { + expected_type: "'inf', bool, or usize".to_owned(), + received: data, + }, + None => MpvError::MissingMpvData, })?; Ok((id, Property::LoopFile(loop_file))) } "loop-playlist" => { let loop_playlist = match data.to_owned() { - MpvDataType::Usize(n) => Some(LoopProperty::N(n)), - MpvDataType::Bool(b) => match b { + Some(MpvDataType::Usize(n)) => Some(LoopProperty::N(n)), + Some(MpvDataType::Bool(b)) => match b { true => Some(LoopProperty::Inf), false => Some(LoopProperty::No), }, - MpvDataType::String(s) => match s.as_str() { + Some(MpvDataType::String(s)) => match s.as_str() { "inf" => Some(LoopProperty::Inf), _ => None, }, _ => None, } - .ok_or(MpvError::DataContainsUnexpectedType { - expected_type: "'inf', bool, or usize".to_owned(), - received: data, + .ok_or(match data { + Some(data) => MpvError::DataContainsUnexpectedType { + expected_type: "'inf', bool, or usize".to_owned(), + received: data, + }, + None => MpvError::MissingMpvData, })?; Ok((id, Property::LoopPlaylist(loop_playlist))) } "speed" => { let speed = match data { - MpvDataType::Double(d) => d, - _ => { + Some(MpvDataType::Double(d)) => d, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "f64".to_owned(), received: data, }) } + None => { + return Err(MpvError::MissingMpvData); + } }; Ok((id, Property::Speed(speed))) } "volume" => { let volume = match data { - MpvDataType::Double(d) => d, - _ => { + Some(MpvDataType::Double(d)) => d, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "f64".to_owned(), received: data, }) } + None => { + return Err(MpvError::MissingMpvData); + } }; Ok((id, Property::Volume(volume))) } "mute" => { let mute = match data { - MpvDataType::Bool(b) => b, - _ => { + Some(MpvDataType::Bool(b)) => b, + Some(data) => { return Err(MpvError::DataContainsUnexpectedType { expected_type: "bool".to_owned(), received: data, }) } + None => { + return Err(MpvError::MissingMpvData); + } }; Ok((id, Property::Mute(mute))) } diff --git a/tests/events.rs b/tests/events.rs index 0d59bef..1e482af 100644 --- a/tests/events.rs +++ b/tests/events.rs @@ -60,8 +60,9 @@ async fn test_observe_event_successful() { Err(err) => panic!("{:?}", err), }; match data { - MpvDataType::Double(data) => assert_eq!(data, 64.0), - err => panic!("{:?}", err), + Some(MpvDataType::Double(data)) => assert_eq!(data, 64.0), + Some(data) => panic!("Unexpected value: {:?}", data), + None => panic!("No data"), } }); -- 2.47.1 From 48cbb51b775ddd3d21549dd74680e4527957365f Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:28 +0200 Subject: [PATCH 4/9] use nextest for running tests --- .gitea/workflows/build-and-test.yml | 24 ++++-------------------- scripts/coverage.sh | 2 +- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/.gitea/workflows/build-and-test.yml b/.gitea/workflows/build-and-test.yml index afe1b1d..475c25f 100644 --- a/.gitea/workflows/build-and-test.yml +++ b/.gitea/workflows/build-and-test.yml @@ -63,33 +63,17 @@ jobs: - name: Cache dependencies uses: Swatinem/rust-cache@v2 - - name: Create necessary directories - run: mkdir -p target/test-report + - name: Install nextest + run: cargo binstall -y cargo-nextest --secure - name: Run tests run: | - cargo test --all-features --release --no-fail-fast -- -Zunstable-options --format json --report-time \ - | tee target/test-report/test-report.json + cargo nextest run --all-features --release --no-fail-fast env: + RUST_LOG: "trace" RUSTFLAGS: "-Cinstrument-coverage" LLVM_PROFILE_FILE: "target/coverage/%p-%m.profraw" - - name: Install markdown-test-report - run: cargo binstall -y markdown-test-report - - - name: Generate test report - run: markdown-test-report target/test-report/test-report.json --output target/test-report/test-report.md - - - name: Upload test report - uses: https://git.pvv.ntnu.no/oysteikt/rsync-action@main - with: - source: target/test-report/test-report.md - target: mpvipc/${{ gitea.ref_name }}/ - username: oysteikt - ssh-key: ${{ secrets.OYSTEIKT_GITEA_WEBDOCS_SSH_KEY }} - host: microbel.pvv.ntnu.no - known-hosts: "microbel.pvv.ntnu.no ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEq0yasKP0mH6PI6ypmuzPzMnbHELo9k+YB5yW534aKudKZS65YsHJKQ9vapOtmegrn5MQbCCgrshf+/XwZcjbM=" - - name: Install grcov run: cargo binstall -y grcov diff --git a/scripts/coverage.sh b/scripts/coverage.sh index a712de2..cdb5897 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -3,7 +3,7 @@ rm -rf target/coverage || true mkdir -p target/coverage echo "Running tests" -RUST_LOG=mpvipc=trace RUSTFLAGS="-Cinstrument-coverage" LLVM_PROFILE_FILE="target/coverage/%p-%m.profraw" cargo test --all-features --release --no-fail-fast +RUST_LOG=mpvipc=trace RUSTFLAGS="-Cinstrument-coverage" LLVM_PROFILE_FILE="target/coverage/%p-%m.profraw" cargo nextest run --all-features --release --no-fail-fast echo "Generating coverage report" grcov \ -- 2.47.1 From f50b4defc181f6d7ddf19110529b23bc7e662d54 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:29 +0200 Subject: [PATCH 5/9] add some tests for event property parser --- tests/integration.rs | 99 ---------- tests/integration/event_property_parser.rs | 216 +++++++++++++++++++++ tests/integration/misc.rs | 26 +++ tests/integration/mod.rs | 5 + tests/integration/util.rs | 43 ++++ tests/mod.rs | 1 + 6 files changed, 291 insertions(+), 99 deletions(-) delete mode 100644 tests/integration.rs create mode 100644 tests/integration/event_property_parser.rs create mode 100644 tests/integration/misc.rs create mode 100644 tests/integration/mod.rs create mode 100644 tests/integration/util.rs create mode 100644 tests/mod.rs diff --git a/tests/integration.rs b/tests/integration.rs deleted file mode 100644 index a75b323..0000000 --- a/tests/integration.rs +++ /dev/null @@ -1,99 +0,0 @@ -use mpvipc::{MpvError, Mpv, MpvExt}; -use std::path::Path; -use tokio::{ - process::{Child, Command}, - time::{sleep, timeout, Duration}, -}; - -#[cfg(target_family = "unix")] -async fn spawn_headless_mpv() -> Result<(Child, Mpv), MpvError> { - let socket_path_str = format!("/tmp/mpv-ipc-{}", uuid::Uuid::new_v4()); - let socket_path = Path::new(&socket_path_str); - - let process_handle = Command::new("mpv") - .arg("--no-config") - .arg("--idle") - .arg("--no-video") - .arg("--no-audio") - .arg(format!( - "--input-ipc-server={}", - &socket_path.to_str().unwrap() - )) - .spawn() - .expect("Failed to start mpv"); - - if timeout(Duration::from_millis(500), async { - while !&socket_path.exists() { - sleep(Duration::from_millis(10)).await; - } - }) - .await - .is_err() - { - panic!("Failed to create mpv socket at {:?}", &socket_path); - } - - let mpv = Mpv::connect(socket_path.to_str().unwrap()).await.unwrap(); - Ok((process_handle, mpv)) -} - -#[tokio::test] -#[cfg(target_family = "unix")] -async fn test_get_mpv_version() { - let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); - let version: String = mpv.get_property("mpv-version").await.unwrap(); - assert!(version.starts_with("mpv")); - - mpv.kill().await.unwrap(); - proc.kill().await.unwrap(); -} - -#[tokio::test] -#[cfg(target_family = "unix")] -async fn test_set_property() { - let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); - mpv.set_property("pause", true).await.unwrap(); - let paused: bool = mpv.get_property("pause").await.unwrap(); - assert!(paused); - - mpv.kill().await.unwrap(); - proc.kill().await.unwrap(); -} - -#[tokio::test] -#[cfg(target_family = "unix")] -async fn test_events() { - use futures::stream::StreamExt; - - let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); - - mpv.observe_property(1337, "pause").await.unwrap(); - - let mut events = mpv.get_event_stream().await; - let event_checking_thread = tokio::spawn(async move { - loop { - let event = events.next().await.unwrap().unwrap(); - if let (1337, property) = mpvipc::parse_event_property(event).unwrap() { - assert_eq!(property, mpvipc::Property::Pause(true)); - break; - } - } - }); - - tokio::time::sleep(Duration::from_millis(10)).await; - - mpv.set_property("pause", true).await.unwrap(); - - if tokio::time::timeout( - tokio::time::Duration::from_millis(500), - event_checking_thread, - ) - .await - .is_err() - { - panic!("Event checking thread timed out"); - } - - mpv.kill().await.unwrap(); - proc.kill().await.unwrap(); -} diff --git a/tests/integration/event_property_parser.rs b/tests/integration/event_property_parser.rs new file mode 100644 index 0000000..6981e97 --- /dev/null +++ b/tests/integration/event_property_parser.rs @@ -0,0 +1,216 @@ +use futures::{stream::StreamExt, Stream}; +use mpvipc::{parse_event_property, Event, Mpv, MpvError, MpvExt}; +use thiserror::Error; +use tokio::time::sleep; +use tokio::time::{timeout, Duration}; + +use test_log::test; + +use super::*; + +const MPV_CHANNEL_ID: usize = 1337; + +#[derive(Error, Debug)] +enum PropertyCheckingThreadError { + #[error("Unexpected property: {0:?}")] + UnexpectedPropertyError(mpvipc::Property), + + #[error(transparent)] + MpvError(#[from] MpvError), +} + +fn create_interruptable_event_property_checking_thread( + mut events: impl Stream> + Unpin + Send + 'static, + on_property: T, +) -> ( + tokio::task::JoinHandle>, + tokio_util::sync::CancellationToken, +) +where + T: Fn(mpvipc::Property) -> bool + Send + 'static, +{ + let cancellation_token = tokio_util::sync::CancellationToken::new(); + let cancellation_token_clone = cancellation_token.clone(); + let handle = tokio::spawn(async move { + loop { + tokio::select! { + event = events.next() => { + match event { + Some(Ok(event)) => { + match event { + Event::PropertyChange { id: MPV_CHANNEL_ID, .. } => { + let property = parse_event_property(event).unwrap().1; + if !on_property(property.clone()) { + return Err(PropertyCheckingThreadError::UnexpectedPropertyError(property)) + } + } + _ => { + log::trace!("Received unrelated event, ignoring: {:?}", event); + } + } + } + Some(Err(err)) => return Err(err.into()), + None => return Ok(()), + } + } + _ = cancellation_token_clone.cancelled() => return Ok(()), + } + } + }); + + (handle, cancellation_token) +} + +async fn graceful_shutdown( + cancellation_token: tokio_util::sync::CancellationToken, + handle: tokio::task::JoinHandle>, + mpv: Mpv, + mut proc: tokio::process::Child, +) -> Result<(), MpvError> { + cancellation_token.cancel(); + + match timeout(Duration::from_millis(500), handle).await { + Ok(Ok(Ok(()))) => {} + Ok(Ok(Err(err))) => match err { + PropertyCheckingThreadError::UnexpectedPropertyError(property) => { + return Err(MpvError::Other(format!( + "Unexpected property: {:?}", + property + ))); + } + PropertyCheckingThreadError::MpvError(err) => return Err(err), + }, + Ok(Err(_)) => { + return Err(MpvError::InternalConnectionError( + "Event checking thread timed out".to_owned(), + )); + } + Err(_) => { + return Err(MpvError::InternalConnectionError( + "Event checking thread panicked".to_owned(), + )); + } + } + + mpv.kill().await?; + proc.wait().await.map_err(|err| { + MpvError::InternalConnectionError(format!( + "Failed to wait for mpv process to exit: {}", + err + )) + })?; + + Ok(()) +} + +#[test(tokio::test)] +#[cfg(target_family = "unix")] +async fn test_highlevel_event_pause() -> Result<(), MpvError> { + let (proc, mpv) = spawn_headless_mpv().await?; + + mpv.observe_property(MPV_CHANNEL_ID, "pause").await?; + + let events = mpv.get_event_stream().await; + let (handle, cancellation_token) = + create_interruptable_event_property_checking_thread(events, |property| match property { + mpvipc::Property::Pause(_) => { + log::debug!("{:?}", property); + true + } + _ => false, + }); + + sleep(Duration::from_millis(5)).await; + mpv.set_property("pause", false).await?; + sleep(Duration::from_millis(5)).await; + mpv.set_property("pause", true).await?; + sleep(Duration::from_millis(5)).await; + + graceful_shutdown(cancellation_token, handle, mpv, proc).await?; + + Ok(()) +} + +#[test(tokio::test)] +#[cfg(target_family = "unix")] +async fn test_highlevel_event_volume() -> Result<(), MpvError> { + let (proc, mpv) = spawn_headless_mpv().await?; + + mpv.observe_property(1337, "volume").await?; + let events = mpv.get_event_stream().await; + let (handle, cancellation_token) = + create_interruptable_event_property_checking_thread(events, |property| match property { + mpvipc::Property::Volume(_) => { + log::trace!("{:?}", property); + true + } + _ => false, + }); + + sleep(Duration::from_millis(5)).await; + mpv.set_property("volume", 100.0).await?; + sleep(Duration::from_millis(5)).await; + mpv.set_property("volume", 40).await?; + sleep(Duration::from_millis(5)).await; + mpv.set_property("volume", 0.0).await?; + sleep(Duration::from_millis(5)).await; + + graceful_shutdown(cancellation_token, handle, mpv, proc).await?; + + Ok(()) +} + +#[test(tokio::test)] +#[cfg(target_family = "unix")] +async fn test_highlevel_event_mute() -> Result<(), MpvError> { + let (proc, mpv) = spawn_headless_mpv().await?; + + mpv.observe_property(1337, "mute").await?; + let events = mpv.get_event_stream().await; + let (handle, cancellation_token) = + create_interruptable_event_property_checking_thread(events, |property| match property { + mpvipc::Property::Mute(_) => { + log::trace!("{:?}", property); + true + } + _ => false, + }); + + sleep(Duration::from_millis(5)).await; + mpv.set_property("mute", true).await?; + sleep(Duration::from_millis(5)).await; + mpv.set_property("mute", false).await?; + sleep(Duration::from_millis(5)).await; + + graceful_shutdown(cancellation_token, handle, mpv, proc).await?; + + Ok(()) +} + +#[test(tokio::test)] +#[cfg(target_family = "unix")] +async fn test_highlevel_event_duration() -> Result<(), MpvError> { + let (proc, mpv) = spawn_headless_mpv().await?; + + mpv.observe_property(1337, "duration").await?; + + let events = mpv.get_event_stream().await; + let (handle, cancellation_token) = + create_interruptable_event_property_checking_thread(events, |property| match property { + mpvipc::Property::Duration(_) => { + log::trace!("{:?}", property); + true + } + _ => false, + }); + + sleep(Duration::from_millis(5)).await; + mpv.set_property("pause", true).await?; + sleep(Duration::from_millis(5)).await; + mpv.set_property("pause", false).await?; + sleep(Duration::from_millis(5)).await; + + graceful_shutdown(cancellation_token, handle, mpv, proc).await?; + + Ok(()) +} diff --git a/tests/integration/misc.rs b/tests/integration/misc.rs new file mode 100644 index 0000000..8049f78 --- /dev/null +++ b/tests/integration/misc.rs @@ -0,0 +1,26 @@ +use mpvipc::MpvExt; + +use super::*; + +#[tokio::test] +#[cfg(target_family = "unix")] +async fn test_get_mpv_version() { + let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); + let version: String = mpv.get_property("mpv-version").await.unwrap(); + assert!(version.starts_with("mpv")); + + mpv.kill().await.unwrap(); + proc.kill().await.unwrap(); +} + +#[tokio::test] +#[cfg(target_family = "unix")] +async fn test_set_property() { + let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); + mpv.set_property("pause", true).await.unwrap(); + let paused: bool = mpv.get_property("pause").await.unwrap(); + assert!(paused); + + mpv.kill().await.unwrap(); + proc.kill().await.unwrap(); +} \ No newline at end of file diff --git a/tests/integration/mod.rs b/tests/integration/mod.rs new file mode 100644 index 0000000..212fb84 --- /dev/null +++ b/tests/integration/mod.rs @@ -0,0 +1,5 @@ +mod event_property_parser; +mod util; +mod misc; + +use util::*; \ No newline at end of file diff --git a/tests/integration/util.rs b/tests/integration/util.rs new file mode 100644 index 0000000..1de4bad --- /dev/null +++ b/tests/integration/util.rs @@ -0,0 +1,43 @@ +use std::{path::Path, time::Duration}; + +use mpvipc::{Mpv, MpvError}; +use tokio::{ + process::{Child, Command}, + time::{sleep, timeout}, +}; + +#[cfg(target_family = "unix")] +pub async fn spawn_headless_mpv() -> Result<(Child, Mpv), MpvError> { + let socket_path_str = format!("/tmp/mpv-ipc-{}", uuid::Uuid::new_v4()); + let socket_path = Path::new(&socket_path_str); + + // TODO: Verify that `mpv` exists in `PATH`` + let process_handle = Command::new("mpv") + .arg("--no-config") + .arg("--idle") + .arg("--no-video") + .arg("--no-audio") + .arg(format!( + "--input-ipc-server={}", + &socket_path.to_str().unwrap() + )) + .kill_on_drop(true) + .spawn() + .expect("Failed to start mpv"); + + timeout(Duration::from_millis(500), async { + while !&socket_path.exists() { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .map_err(|_| { + MpvError::MpvSocketConnectionError(format!( + "Failed to create mpv socket at {:?}, timed out waiting for socket file to be created", + &socket_path + )) + })?; + + let mpv = Mpv::connect(socket_path.to_str().unwrap()).await?; + Ok((process_handle, mpv)) +} diff --git a/tests/mod.rs b/tests/mod.rs new file mode 100644 index 0000000..605134b --- /dev/null +++ b/tests/mod.rs @@ -0,0 +1 @@ +mod integration; \ No newline at end of file -- 2.47.1 From 3a04cd14f156a4ce4233f295e2cb1ce3e3f4fcdb Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 22:29:30 +0200 Subject: [PATCH 6/9] restructure test directory --- tests/integration.rs | 8 ++++++++ .../event_property_parser.rs | 0 tests/{integration => integration_tests}/misc.rs | 0 tests/{integration => integration_tests}/mod.rs | 0 tests/{integration => integration_tests}/util.rs | 0 tests/mock_socket.rs | 1 + tests/{ => mock_socket_tests}/events.rs | 0 tests/{ => mock_socket_tests}/get_property.rs | 0 tests/mock_socket_tests/mod.rs | 3 +++ tests/{ => mock_socket_tests}/set_property.rs | 0 tests/mod.rs | 1 - 11 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 tests/integration.rs rename tests/{integration => integration_tests}/event_property_parser.rs (100%) rename tests/{integration => integration_tests}/misc.rs (100%) rename tests/{integration => integration_tests}/mod.rs (100%) rename tests/{integration => integration_tests}/util.rs (100%) create mode 100644 tests/mock_socket.rs rename tests/{ => mock_socket_tests}/events.rs (100%) rename tests/{ => mock_socket_tests}/get_property.rs (100%) create mode 100644 tests/mock_socket_tests/mod.rs rename tests/{ => mock_socket_tests}/set_property.rs (100%) delete mode 100644 tests/mod.rs diff --git a/tests/integration.rs b/tests/integration.rs new file mode 100644 index 0000000..9df9e65 --- /dev/null +++ b/tests/integration.rs @@ -0,0 +1,8 @@ +// mod event_property_parser { +// include!("integration/event_property_parser.rs") +// } +// mod util; +// mod misc; +mod integration_tests; + +// use util::*; \ No newline at end of file diff --git a/tests/integration/event_property_parser.rs b/tests/integration_tests/event_property_parser.rs similarity index 100% rename from tests/integration/event_property_parser.rs rename to tests/integration_tests/event_property_parser.rs diff --git a/tests/integration/misc.rs b/tests/integration_tests/misc.rs similarity index 100% rename from tests/integration/misc.rs rename to tests/integration_tests/misc.rs diff --git a/tests/integration/mod.rs b/tests/integration_tests/mod.rs similarity index 100% rename from tests/integration/mod.rs rename to tests/integration_tests/mod.rs diff --git a/tests/integration/util.rs b/tests/integration_tests/util.rs similarity index 100% rename from tests/integration/util.rs rename to tests/integration_tests/util.rs diff --git a/tests/mock_socket.rs b/tests/mock_socket.rs new file mode 100644 index 0000000..2fac2b4 --- /dev/null +++ b/tests/mock_socket.rs @@ -0,0 +1 @@ +mod mock_socket_tests; \ No newline at end of file diff --git a/tests/events.rs b/tests/mock_socket_tests/events.rs similarity index 100% rename from tests/events.rs rename to tests/mock_socket_tests/events.rs diff --git a/tests/get_property.rs b/tests/mock_socket_tests/get_property.rs similarity index 100% rename from tests/get_property.rs rename to tests/mock_socket_tests/get_property.rs diff --git a/tests/mock_socket_tests/mod.rs b/tests/mock_socket_tests/mod.rs new file mode 100644 index 0000000..c50d4ee --- /dev/null +++ b/tests/mock_socket_tests/mod.rs @@ -0,0 +1,3 @@ +mod events; +mod get_property; +mod set_property; \ No newline at end of file diff --git a/tests/set_property.rs b/tests/mock_socket_tests/set_property.rs similarity index 100% rename from tests/set_property.rs rename to tests/mock_socket_tests/set_property.rs diff --git a/tests/mod.rs b/tests/mod.rs deleted file mode 100644 index 605134b..0000000 --- a/tests/mod.rs +++ /dev/null @@ -1 +0,0 @@ -mod integration; \ No newline at end of file -- 2.47.1 From f1687fe07b1d9971e3ae92f12ebe25cc5899d8da Mon Sep 17 00:00:00 2001 From: h7x4 Date: Fri, 3 May 2024 23:02:57 +0200 Subject: [PATCH 7/9] add/fix more docstrings --- src/core_api.rs | 55 +++++++++++++++------- src/highlevel_api_extension.rs | 83 ++++++++++++++++++++++++++++++---- 2 files changed, 113 insertions(+), 25 deletions(-) diff --git a/src/core_api.rs b/src/core_api.rs index 69e8686..ff2612a 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -199,6 +199,7 @@ pub struct Mpv { broadcast_channel: broadcast::Sender, } +// TODO: Can we somehow provide a more useful Debug implementation? impl fmt::Debug for Mpv { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("Mpv").finish() @@ -206,6 +207,8 @@ impl fmt::Debug for Mpv { } impl Mpv { + /// Connect to a unix socket, hosted by mpv, at the given path. + /// This is the inteded way of creating a new [`Mpv`] instance. pub async fn connect(socket_path: &str) -> Result { log::debug!("Connecting to mpv socket at {}", socket_path); @@ -217,6 +220,10 @@ impl Mpv { Self::connect_socket(socket).await } + /// Connect to an existing [`UnixStream`]. + /// This is an alternative to [`Mpv::connect`], if you already have a [`UnixStream`] available. + /// + /// Internally, this is used for testing purposes. pub async fn connect_socket(socket: UnixStream) -> Result { let (com_tx, com_rx) = mpsc::channel(100); let (ev_tx, _) = broadcast::channel(100); @@ -231,6 +238,11 @@ impl Mpv { }) } + /// Disconnect from the mpv socket. + /// + /// Note that this will also kill communication for all other clones of this instance. + /// It will not kill the mpv process itself - for that you should use [`MpvCommand::Quit`] + /// or run [`MpvExt::kill`](crate::MpvExt::kill). pub async fn disconnect(&self) -> Result<(), MpvError> { let (res_tx, res_rx) = oneshot::channel(); self.command_sender @@ -244,6 +256,10 @@ impl Mpv { } } + /// Create a new stream, providing [`Event`]s from mpv. + /// + /// This is intended to be used with [`MpvCommand::Observe`] and [`MpvCommand::Unobserve`] + /// (or [`MpvExt::observe_property`] and [`MpvExt::unobserve_property`] respectively). pub async fn get_event_stream(&self) -> impl futures::Stream> { tokio_stream::wrappers::BroadcastStream::new(self.broadcast_channel.subscribe()).map( |event| match event { @@ -255,7 +271,7 @@ impl Mpv { /// Run a custom command. /// This should only be used if the desired command is not implemented - /// with [MpvCommand]. + /// with [`MpvCommand`]. pub async fn run_command_raw( &self, command: &str, @@ -281,6 +297,7 @@ impl Mpv { } } + /// Helper function to ignore the return value of a command, and only check for errors. async fn run_command_raw_ignore_value( &self, command: &str, @@ -300,18 +317,20 @@ impl Mpv { /// /// # Example /// ``` - /// use mpvipc::{Mpv, Error}; - /// fn main() -> Result<(), Error> { - /// let mpv = Mpv::connect("/tmp/mpvsocket")?; + /// use mpvipc::{Mpv, MpvError}; + /// + /// #[tokio::main] + /// async fn main() -> Result<(), MpvError> { + /// let mpv = Mpv::connect("/tmp/mpvsocket").await?; /// /// //Run command 'playlist-shuffle' which takes no arguments - /// mpv.run_command(MpvCommand::PlaylistShuffle)?; + /// mpv.run_command(MpvCommand::PlaylistShuffle).await?; /// /// //Run command 'seek' which in this case takes two arguments /// mpv.run_command(MpvCommand::Seek { /// seconds: 0f64, /// option: SeekOptions::Absolute, - /// })?; + /// }).await?; /// Ok(()) /// } /// ``` @@ -430,9 +449,11 @@ impl Mpv { /// /// # Example /// ``` - /// use mpvipc::{Mpv, Error}; - /// async fn main() -> Result<(), Error> { - /// let mpv = Mpv::connect("/tmp/mpvsocket")?; + /// use mpvipc::{Mpv, MpvError}; + /// + /// #[tokio::main] + /// async fn main() -> Result<(), MpvError> { + /// let mpv = Mpv::connect("/tmp/mpvsocket").await?; /// let paused: bool = mpv.get_property("pause").await?; /// let title: String = mpv.get_property("media-title").await?; /// Ok(()) @@ -457,10 +478,12 @@ impl Mpv { /// # Example /// /// ``` - /// use mpvipc::{Mpv, Error}; - /// fn main() -> Result<(), Error> { - /// let mpv = Mpv::connect("/tmp/mpvsocket")?; - /// let title = mpv.get_property_string("media-title")?; + /// use mpvipc::{Mpv, MpvError}; + /// + /// #[tokio::main] + /// async fn main() -> Result<(), MpvError> { + /// let mpv = Mpv::connect("/tmp/mpvsocket").await?; + /// let title = mpv.get_property_string("media-title").await?; /// Ok(()) /// } /// ``` @@ -496,9 +519,9 @@ impl Mpv { /// /// # Example /// ``` - /// use mpvipc::{Mpv, Error}; - /// fn async main() -> Result<(), Error> { - /// let mpv = Mpv::connect("/tmp/mpvsocket")?; + /// use mpvipc::{Mpv, MpvError}; + /// async fn main() -> Result<(), MpvError> { + /// let mpv = Mpv::connect("/tmp/mpvsocket").await?; /// mpv.set_property("pause", true).await?; /// Ok(()) /// } diff --git a/src/highlevel_api_extension.rs b/src/highlevel_api_extension.rs index 61056b7..8f4b75e 100644 --- a/src/highlevel_api_extension.rs +++ b/src/highlevel_api_extension.rs @@ -44,35 +44,91 @@ pub enum PlaylistAddTypeOptions { // TODO: fix this #[allow(async_fn_in_trait)] pub trait MpvExt { - async fn toggle(&self) -> Result<(), MpvError>; + /// Stop the player completely (as opposed to pausing), + /// removing the pointer to the current video. async fn stop(&self) -> Result<(), MpvError>; + + /// Set the volume of the player. async fn set_volume(&self, input_volume: f64, option: NumberChangeOptions) -> Result<(), MpvError>; + + /// Set the playback speed of the player. async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), MpvError>; + + /// Toggle/set the pause state of the player. + async fn set_playback(&self, option: Switch) -> Result<(), MpvError>; + + /// Toggle/set the mute state of the player. async fn set_mute(&self, option: Switch) -> Result<(), MpvError>; + + /// Toggle/set whether the player should loop the current playlist. async fn set_loop_playlist(&self, option: Switch) -> Result<(), MpvError>; + + /// Toggle/set whether the player should loop the current video. async fn set_loop_file(&self, option: Switch) -> Result<(), MpvError>; + + /// Seek to a specific position in the current video. async fn seek(&self, seconds: f64, option: SeekOptions) -> Result<(), MpvError>; + + /// Shuffle the current playlist. async fn playlist_shuffle(&self) -> Result<(), MpvError>; + + /// Remove an entry from the playlist. async fn playlist_remove_id(&self, id: usize) -> Result<(), MpvError>; + + /// Play the next entry in the playlist. async fn playlist_play_next(&self, id: usize) -> Result<(), MpvError>; + + /// Play a specific entry in the playlist. async fn playlist_play_id(&self, id: usize) -> Result<(), MpvError>; + + /// Move an entry in the playlist. + /// + /// The `from` parameter is the current position of the entry, and the `to` parameter is the new position. + /// Mpv will then move the entry from the `from` position to the `to` position, + /// shifting after `to` one number up. Paradoxically, that means that moving an entry further down the list + /// will result in a final position that is one less than the `to` parameter. async fn playlist_move_id(&self, from: usize, to: usize) -> Result<(), MpvError>; + + /// Remove all entries from the playlist. async fn playlist_clear(&self) -> Result<(), MpvError>; + + /// Add a file or playlist to the playlist. async fn playlist_add( &self, file: &str, file_type: PlaylistAddTypeOptions, option: PlaylistAddOptions, ) -> Result<(), MpvError>; + + /// Start the current video from the beginning. async fn restart(&self) -> Result<(), MpvError>; + + /// Play the previous entry in the playlist. async fn prev(&self) -> Result<(), MpvError>; - async fn pause(&self) -> Result<(), MpvError>; - async fn unobserve_property(&self, id: usize) -> Result<(), MpvError>; + + /// Notify mpv to send events whenever a property changes. + /// See [`Mpv::get_event_stream`] and [`Property`](crate::Property) for more information. async fn observe_property(&self, id: usize, property: &str) -> Result<(), MpvError>; + + /// Stop observing a property. + /// See [`Mpv::get_event_stream`] and [`Property`](crate::Property) for more information. + async fn unobserve_property(&self, id: usize) -> Result<(), MpvError>; + + /// Skip to the next entry in the playlist. async fn next(&self) -> Result<(), MpvError>; + + /// Stop mpv completely, and kill the process. + /// + /// Note that this is different than forcefully killing the process using + /// as handle to a subprocess, it will only send a command to mpv to ask + /// it to exit itself. If mpv is stuck, it may not respond to this command. async fn kill(&self) -> Result<(), MpvError>; + + /// Get a list of all entries in the playlist. async fn get_playlist(&self) -> Result; + + /// Get metadata about the current video. async fn get_metadata(&self) -> Result, MpvError>; } @@ -107,8 +163,21 @@ impl MpvExt for Mpv { self.run_command(MpvCommand::Unobserve(id)).await } - async fn pause(&self) -> Result<(), MpvError> { - self.set_property("pause", true).await + async fn set_playback(&self, option: Switch) -> Result<(), MpvError> { + let enabled = match option { + Switch::On => "yes", + Switch::Off => "no", + Switch::Toggle => { + self.get_property::("pause") + .await + .map(|s| match s.as_str() { + "yes" => "no", + "no" => "yes", + _ => "no", + })? + } + }; + self.set_property("pause", enabled).await } async fn prev(&self) -> Result<(), MpvError> { @@ -278,8 +347,4 @@ impl MpvExt for Mpv { async fn stop(&self) -> Result<(), MpvError> { self.run_command(MpvCommand::Stop).await } - - async fn toggle(&self) -> Result<(), MpvError> { - self.run_command_raw("cycle", &["pause"]).await.map(|_| ()) - } } -- 2.47.1 From e044246cba658945fa4e4a04564ea271cd42f214 Mon Sep 17 00:00:00 2001 From: h7x4 Date: Sat, 4 May 2024 00:06:22 +0200 Subject: [PATCH 8/9] fixup: fmt + clippy --- src/core_api.rs | 2 +- src/error.rs | 20 +++++++++++--------- src/event_parser.rs | 4 ++-- src/event_property_parser.rs | 5 ++++- src/highlevel_api_extension.rs | 21 ++++++++++++++++----- src/ipc.rs | 8 ++++---- src/message_parser.rs | 2 +- tests/integration.rs | 2 +- tests/integration_tests/misc.rs | 2 +- tests/integration_tests/mod.rs | 4 ++-- tests/mock_socket.rs | 2 +- tests/mock_socket_tests/get_property.rs | 2 +- tests/mock_socket_tests/mod.rs | 2 +- tests/mock_socket_tests/set_property.rs | 6 +++--- 14 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/core_api.rs b/src/core_api.rs index ff2612a..92e7fee 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -167,7 +167,7 @@ where value: T, ) -> Result<(), MpvError> { let (res_tx, res_rx) = oneshot::channel(); - let value = serde_json::to_value(value).map_err(|why| MpvError::JsonParseError(why))?; + let value = serde_json::to_value(value).map_err(MpvError::JsonParseError)?; instance .command_sender diff --git a/src/error.rs b/src/error.rs index 0a75ccb..7adb07e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,7 @@ //! Library specific error messages. -use thiserror::Error; use serde_json::{Map, Value}; +use thiserror::Error; use crate::MpvDataType; @@ -22,14 +22,16 @@ pub enum MpvError { #[error("Mpv sent a value with an unexpected type:\nExpected {expected_type}, received {received:#?}")] ValueContainsUnexpectedType { - expected_type: String, - received: Value, + expected_type: String, + received: Value, }, - #[error("Mpv sent data with an unexpected type:\nExpected {expected_type}, received {received:#?}")] + #[error( + "Mpv sent data with an unexpected type:\nExpected {expected_type}, received {received:#?}" + )] DataContainsUnexpectedType { - expected_type: String, - received: MpvDataType, + expected_type: String, + received: MpvDataType, }, #[error("Missing expected 'data' field in mpv message")] @@ -37,10 +39,10 @@ pub enum MpvError { #[error("Missing key in object:\nExpected {key} in {map:#?}")] MissingKeyInObject { - key: String, - map: Map, + key: String, + map: Map, }, #[error("Unknown error: {0}")] Other(String), -} \ No newline at end of file +} diff --git a/src/event_parser.rs b/src/event_parser.rs index 8518e67..241c01f 100644 --- a/src/event_parser.rs +++ b/src/event_parser.rs @@ -292,11 +292,11 @@ fn parse_client_message(event: &Map) -> Result { fn parse_property_change(event: &Map) -> Result { let id = get_key_as!(as_u64, "id", event) as usize; let property_name = get_key_as!(as_str, "name", event); - let data = event.get("data").map(|d| json_to_value(d)).transpose()?; + let data = event.get("data").map(json_to_value).transpose()?; Ok(Event::PropertyChange { id, name: property_name.to_string(), - data: data, + data, }) } diff --git a/src/event_property_parser.rs b/src/event_property_parser.rs index c3b7d2d..bfab40a 100644 --- a/src/event_property_parser.rs +++ b/src/event_property_parser.rs @@ -34,7 +34,10 @@ pub enum Property { Speed(f64), Volume(f64), Mute(bool), - Unknown { name: String, data: Option }, + Unknown { + name: String, + data: Option, + }, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/src/highlevel_api_extension.rs b/src/highlevel_api_extension.rs index 8f4b75e..4d7a4c0 100644 --- a/src/highlevel_api_extension.rs +++ b/src/highlevel_api_extension.rs @@ -1,7 +1,7 @@ //! High-level API extension for [`Mpv`]. use crate::{ - MpvError, IntoRawCommandPart, Mpv, MpvCommand, MpvDataType, Playlist, PlaylistAddOptions, + IntoRawCommandPart, Mpv, MpvCommand, MpvDataType, MpvError, Playlist, PlaylistAddOptions, PlaylistEntry, SeekOptions, }; use serde::{Deserialize, Serialize}; @@ -49,11 +49,18 @@ pub trait MpvExt { async fn stop(&self) -> Result<(), MpvError>; /// Set the volume of the player. - async fn set_volume(&self, input_volume: f64, option: NumberChangeOptions) - -> Result<(), MpvError>; + async fn set_volume( + &self, + input_volume: f64, + option: NumberChangeOptions, + ) -> Result<(), MpvError>; /// Set the playback speed of the player. - async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), MpvError>; + async fn set_speed( + &self, + input_speed: f64, + option: NumberChangeOptions, + ) -> Result<(), MpvError>; /// Toggle/set the pause state of the player. async fn set_playback(&self, option: Switch) -> Result<(), MpvError>; @@ -306,7 +313,11 @@ impl MpvExt for Mpv { self.set_property("mute", enabled).await } - async fn set_speed(&self, input_speed: f64, option: NumberChangeOptions) -> Result<(), MpvError> { + async fn set_speed( + &self, + input_speed: f64, + option: NumberChangeOptions, + ) -> Result<(), MpvError> { match self.get_property::("speed").await { Ok(speed) => match option { NumberChangeOptions::Increase => { diff --git a/src/ipc.rs b/src/ipc.rs index 8531639..0310c82 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -56,7 +56,7 @@ impl MpvIpc { ) -> Result, MpvError> { let ipc_command = json!({ "command": command }); let ipc_command_str = - serde_json::to_string(&ipc_command).map_err(|why| MpvError::JsonParseError(why))?; + serde_json::to_string(&ipc_command).map_err(MpvError::JsonParseError)?; log::trace!("Sending command: {}", ipc_command_str); @@ -75,8 +75,8 @@ impl MpvIpc { ))? .map_err(|why| MpvError::MpvSocketConnectionError(why.to_string()))?; - let parsed_response = serde_json::from_str::(&response) - .map_err(|why| MpvError::JsonParseError(why)); + let parsed_response = + serde_json::from_str::(&response).map_err(MpvError::JsonParseError); if parsed_response .as_ref() @@ -155,7 +155,7 @@ impl MpvIpc { .map_err(|why| MpvError::MpvSocketConnectionError(why.to_string())) .and_then(|event| serde_json::from_str::(&event) - .map_err(|why| MpvError::JsonParseError(why))); + .map_err(MpvError::JsonParseError)); self.handle_event(parsed_event).await; } diff --git a/src/message_parser.rs b/src/message_parser.rs index ba335c0..3a652ba 100644 --- a/src/message_parser.rs +++ b/src/message_parser.rs @@ -152,7 +152,7 @@ pub(crate) fn json_map_to_hashmap( } pub(crate) fn json_array_to_vec(array: &[Value]) -> Result, MpvError> { - array.iter().map(|entry| json_to_value(entry)).collect() + array.iter().map(json_to_value).collect() } pub(crate) fn json_array_to_playlist(array: &[Value]) -> Vec { diff --git a/tests/integration.rs b/tests/integration.rs index 9df9e65..7822251 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -5,4 +5,4 @@ // mod misc; mod integration_tests; -// use util::*; \ No newline at end of file +// use util::*; diff --git a/tests/integration_tests/misc.rs b/tests/integration_tests/misc.rs index 8049f78..13d110b 100644 --- a/tests/integration_tests/misc.rs +++ b/tests/integration_tests/misc.rs @@ -23,4 +23,4 @@ async fn test_set_property() { mpv.kill().await.unwrap(); proc.kill().await.unwrap(); -} \ No newline at end of file +} diff --git a/tests/integration_tests/mod.rs b/tests/integration_tests/mod.rs index 212fb84..2cfc348 100644 --- a/tests/integration_tests/mod.rs +++ b/tests/integration_tests/mod.rs @@ -1,5 +1,5 @@ mod event_property_parser; -mod util; mod misc; +mod util; -use util::*; \ No newline at end of file +use util::*; diff --git a/tests/mock_socket.rs b/tests/mock_socket.rs index 2fac2b4..f3f8950 100644 --- a/tests/mock_socket.rs +++ b/tests/mock_socket.rs @@ -1 +1 @@ -mod mock_socket_tests; \ No newline at end of file +mod mock_socket_tests; diff --git a/tests/mock_socket_tests/get_property.rs b/tests/mock_socket_tests/get_property.rs index 3a3b0b0..815e89a 100644 --- a/tests/mock_socket_tests/get_property.rs +++ b/tests/mock_socket_tests/get_property.rs @@ -1,7 +1,7 @@ use std::{panic, time::Duration}; use futures::{stream::FuturesUnordered, SinkExt, StreamExt}; -use mpvipc::{MpvError, Mpv, MpvExt, Playlist, PlaylistEntry}; +use mpvipc::{Mpv, MpvError, MpvExt, Playlist, PlaylistEntry}; use serde_json::{json, Value}; use test_log::test; use tokio::{net::UnixStream, task::JoinHandle}; diff --git a/tests/mock_socket_tests/mod.rs b/tests/mock_socket_tests/mod.rs index c50d4ee..f9b88d0 100644 --- a/tests/mock_socket_tests/mod.rs +++ b/tests/mock_socket_tests/mod.rs @@ -1,3 +1,3 @@ mod events; mod get_property; -mod set_property; \ No newline at end of file +mod set_property; diff --git a/tests/mock_socket_tests/set_property.rs b/tests/mock_socket_tests/set_property.rs index 22b249f..79b1a51 100644 --- a/tests/mock_socket_tests/set_property.rs +++ b/tests/mock_socket_tests/set_property.rs @@ -1,7 +1,7 @@ use std::{panic, time::Duration}; use futures::{stream::FuturesUnordered, SinkExt, StreamExt}; -use mpvipc::{MpvError, Mpv, MpvExt, Playlist, PlaylistEntry}; +use mpvipc::{Mpv, MpvError, MpvExt, Playlist, PlaylistEntry}; use serde_json::{json, Value}; use test_log::test; use tokio::{net::UnixStream, task::JoinHandle}; @@ -130,7 +130,7 @@ async fn test_set_property_simultaneous_requests() { loop { let status = mpv_clone_1.set_property("volume", 100).await; match status { - Ok(()) => {}, + Ok(()) => {} _ => panic!("Unexpected result: {:?}", status), } } @@ -142,7 +142,7 @@ async fn test_set_property_simultaneous_requests() { tokio::time::sleep(Duration::from_millis(1)).await; let status = mpv_clone_2.set_property("pause", false).await; match status { - Ok(()) => {}, + Ok(()) => {} _ => panic!("Unexpected result: {:?}", status), } } -- 2.47.1 From 2ed802504613e93e199b84f11e505f0e0377d9fb Mon Sep 17 00:00:00 2001 From: h7x4 Date: Sat, 4 May 2024 00:06:43 +0200 Subject: [PATCH 9/9] fix examples and documentation --- Cargo.toml | 11 +++-- README.md | 46 +++++++-------------- examples/fetch_state.rs | 6 ++- examples/media_player.rs | 87 +++++++++++++++++++--------------------- 4 files changed, 68 insertions(+), 82 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 74b22ad..99e9fae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,12 +1,15 @@ [package] name = "mpvipc" version = "1.3.0" -authors = ["Jonas Frei "] +authors = [ + "Jonas Frei ", + "h7x4 " +] description = "A small library which provides bindings to control existing mpv instances through sockets." license = "GPL-3.0" -homepage = "https://gitlab.com/mpv-ipc/mpvipc" -repository = "https://gitlab.com/mpv-ipc/mpvipc" -documentation = "https://docs.rs/mpvipc/" +homepage = "https://git.pvv.ntnu.no/oysteikt/mpvipc" +repository = "https://git.pvv.ntnu.no/oysteikt/mpvipc" +documentation = "https://pvv.ntnu.no/~oysteikt/gitea/mpvipc/master/docs/mpvipc/" edition = "2021" rust-version = "1.75" diff --git a/README.md b/README.md index 97c98cb..89d797f 100644 --- a/README.md +++ b/README.md @@ -5,46 +5,30 @@ A small library which provides bindings to control existing mpv instances through sockets. -To make use of this library, please make sure mpv is started with the following option: -` -$ mpv --input-ipc-server=/tmp/mpv.sock --idle ... -` - ## Dependencies - `mpv` -- `cargo` (makedep) - -## Install - -- [Cargo](https://crates.io/crates/mpvipc) - -You can use this package with cargo. +- `cargo` (make dependency) +- `cargo-nextest` (test depencency) +- `grcov` (test depencency) ## Example Make sure mpv is started with the following option: -` + +```bash $ mpv --input-ipc-server=/tmp/mpv.sock --idle -` - -Here is a small code example which connects to the socket /tmp/mpv.sock and toggles playback. - -```rust -extern crate mpvipc; - -use mpvipc::*; -use std::sync::mpsc::channel; - -fn main() { - let mpv = Mpv::connect("/tmp/mpv.sock").unwrap(); - let paused: bool = mpv.get_property("pause").unwrap(); - mpv.set_property("pause", !paused).expect("Error pausing"); -} ``` -For a more extensive example and proof of concept, see project [mpvc](https://gitlab.com/mpv-ipc/mpvc). +Here is a small code example which connects to the socket `/tmp/mpv.sock` and toggles playback. -## Bugs / Ideas +```rust +use mpvipc::*; -Check out the [Issue Tracker](https://gitlab.com/mpv-ipc/mpvipc/issues) +#[tokio::main] +async fn main() -> Result<(), MpvError> { + let mpv = Mpv::connect("/tmp/mpv.sock").await?; + let paused: bool = mpv.get_property("pause").await?; + mpv.set_property("pause", !paused).expect("Error pausing"); +} +``` \ No newline at end of file diff --git a/examples/fetch_state.rs b/examples/fetch_state.rs index a4fbcb1..7a47029 100644 --- a/examples/fetch_state.rs +++ b/examples/fetch_state.rs @@ -1,15 +1,19 @@ -use mpvipc::{MpvError, Mpv, MpvExt}; +use mpvipc::{Mpv, MpvError, MpvExt}; #[tokio::main] async fn main() -> Result<(), MpvError> { env_logger::init(); let mpv = Mpv::connect("/tmp/mpv.sock").await?; + let meta = mpv.get_metadata().await?; println!("metadata: {:?}", meta); + let playlist = mpv.get_playlist().await?; println!("playlist: {:?}", playlist); + let playback_time: f64 = mpv.get_property("playback-time").await?; println!("playback-time: {}", playback_time); + Ok(()) } diff --git a/examples/media_player.rs b/examples/media_player.rs index 9def21a..2f394de 100644 --- a/examples/media_player.rs +++ b/examples/media_player.rs @@ -1,4 +1,5 @@ -use mpvipc::{MpvError, Mpv, MpvExt}; +use futures::StreamExt; +use mpvipc::{parse_event_property, Event, Mpv, MpvDataType, MpvError, MpvExt, Property}; fn seconds_to_hms(total: f64) -> String { let total = total as u64; @@ -14,55 +15,49 @@ async fn main() -> Result<(), MpvError> { env_logger::init(); let mpv = Mpv::connect("/tmp/mpv.sock").await?; - let pause = false; - let playback_time = std::f64::NAN; - let duration = std::f64::NAN; + mpv.observe_property(1, "path").await?; mpv.observe_property(2, "pause").await?; mpv.observe_property(3, "playback-time").await?; mpv.observe_property(4, "duration").await?; mpv.observe_property(5, "metadata").await?; - loop { - // TODO: - // let event = mpv.event_listen()?; - // match event { - // Event::PropertyChange { id: _, property } => match property { - // Property::Path(Some(value)) => println!("\nPlaying: {}", value), - // Property::Path(None) => (), - // Property::Pause(value) => pause = value, - // Property::PlaybackTime(Some(value)) => playback_time = value, - // Property::PlaybackTime(None) => playback_time = std::f64::NAN, - // Property::Duration(Some(value)) => duration = value, - // Property::Duration(None) => duration = std::f64::NAN, - // Property::Metadata(Some(value)) => { - // println!("File tags:"); - // if let Some(MpvDataType::String(value)) = value.get("ARTIST") { - // println!(" Artist: {}", value); - // } - // if let Some(MpvDataType::String(value)) = value.get("ALBUM") { - // println!(" Album: {}", value); - // } - // if let Some(MpvDataType::String(value)) = value.get("TITLE") { - // println!(" Title: {}", value); - // } - // if let Some(MpvDataType::String(value)) = value.get("TRACK") { - // println!(" Track: {}", value); - // } - // } - // Property::Metadata(None) => (), - // Property::Unknown { name: _, data: _ } => (), - // }, - // Event::Shutdown => return Ok(()), - // Event::Unimplemented => panic!("Unimplemented event"), - // _ => (), - // } - // print!( - // "{}{} / {} ({:.0}%)\r", - // if pause { "(Paused) " } else { "" }, - // seconds_to_hms(playback_time), - // seconds_to_hms(duration), - // 100. * playback_time / duration - // ); - // io::stdout().flush().unwrap(); + + let mut events = mpv.get_event_stream().await; + while let Some(Ok(event)) = events.next().await { + match event { + mpvipc::Event::PropertyChange { .. } => match parse_event_property(event)? { + (1, Property::Path(Some(value))) => println!("\nPlaying: {}", value), + (2, Property::Pause(value)) => { + println!("Pause: {}", value); + } + (3, Property::PlaybackTime(Some(value))) => { + println!("Playback time: {}", seconds_to_hms(value)); + } + (4, Property::Duration(Some(value))) => { + println!("Duration: {}", seconds_to_hms(value)); + } + (5, Property::Metadata(Some(value))) => { + println!("File tags:"); + if let Some(MpvDataType::String(value)) = value.get("ARTIST") { + println!(" Artist: {}", value); + } + if let Some(MpvDataType::String(value)) = value.get("ALBUM") { + println!(" Album: {}", value); + } + if let Some(MpvDataType::String(value)) = value.get("TITLE") { + println!(" Title: {}", value); + } + if let Some(MpvDataType::String(value)) = value.get("TRACK") { + println!(" Track: {}", value); + } + } + _ => (), + }, + Event::Shutdown => return Ok(()), + Event::Unimplemented(_) => panic!("Unimplemented event"), + _ => (), + } } + + Ok(()) } -- 2.47.1