Merge branch 'master' into 'master'

clean up code and fix bug

See merge request mpv-ipc/mpvipc!3

src/ipc.rs

@@ -189,30 +189,34 @@ pub fn get_mpv_property<T: TypeHandler>(instance: &Mpv, property: &str) -> Resul
 
 pub fn get_mpv_property_string(instance: &Mpv, property: &str) -> Result<String, Error> {
     let ipc_string = format!("{{ \"command\": [\"get_property\",\"{}\"] }}\n", property);
-    match serde_json::from_str::<Value>(&send_command_sync(instance, &ipc_string)) {
+    let val = serde_json::from_str::<Value>(&send_command_sync(instance, &ipc_string))
-        Ok(val) => {
+        .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?;
-            if let Value::Object(map) = val {
+
-                if let Value::String(ref error) = map["error"] {
+    let map = if let Value::Object(map) = val {
-                    if error == "success" && map.contains_key("data") {
+        Ok(map)
-                        match map["data"] {
+    } else {
+        Err(Error(ErrorCode::UnexpectedValue))
+    }?;
+
+    let error = if let Value::String(ref error) = map["error"] {
+        Ok(error)
+    } else {
+        Err(Error(ErrorCode::UnexpectedValue))
+    }?;
+
+    let data = if error == "success" {
+        Ok(&map["data"])
+    } else {
+        Err(Error(ErrorCode::MpvError(error.to_string())))
+    }?;
+
+    match data {
         Value::Bool(b) => Ok(b.to_string()),
         Value::Number(ref n) => Ok(n.to_string()),
         Value::String(ref s) => Ok(s.to_string()),
         Value::Array(ref array) => Ok(format!("{:?}", array)),
         Value::Object(ref map) => Ok(format!("{:?}", map)),
-                            _ => Err(Error(ErrorCode::UnsupportedType)),
+        Value::Null => Err(Error(ErrorCode::MissingValue)),
-                        }
-                    } else {
-                        Err(Error(ErrorCode::MpvError(error.to_string())))
-                    }
-                } else {
-                    Err(Error(ErrorCode::UnexpectedValue))
-                }
-            } else {
-                Err(Error(ErrorCode::UnexpectedValue))
-            }
-        }
-        Err(why) => Err(Error(ErrorCode::JsonParseError(why.to_string()))),
     }
 }
 
@@ -233,14 +237,11 @@ pub fn set_mpv_property<T: TypeHandler>(
 }
 
 pub fn run_mpv_command(instance: &Mpv, command: &str, args: &[&str]) -> Result<(), Error> {
-    let mut ipc_string = format!("{{ \"command\": [\"{}\"", command);
+    let mut ipc_string = format!(r#"{{ "command": ["{}""#, command);
-    if args.len() > 0 {
     for arg in args {
-            ipc_string.push_str(&format!(", \"{}\"", arg));
+        ipc_string.push_str(&format!(r#", "{}""#, arg));
-        }
     }
     ipc_string.push_str("] }\n");
-    ipc_string = ipc_string;
     match serde_json::from_str::<Value>(&send_command_sync(instance, &ipc_string)) {
         Ok(feedback) => {
             if let Value::String(ref error) = feedback["error"] {
@@ -334,139 +335,94 @@ fn try_convert_property(name: &str, id: usize, data: MpvDataType) -> Event {
 }
 
 pub fn listen(instance: &mut Mpv) -> Result<Event, Error> {
+    let mut e;
+    // sometimes we get responses unrelated to events, so we read a new line until we receive one
+    // with an event field
+    let name = loop {
         let mut response = String::new();
         instance.reader.read_line(&mut response).unwrap();
         response = response.trim_end().to_string();
         debug!("Event: {}", response);
-    match serde_json::from_str::<Value>(&response) {
+
-        Ok(e) => {
+        e = serde_json::from_str::<Value>(&response)
-            if let Value::String(ref name) = e["event"] {
+            .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?;
-                let event: Event;
+
-                match name.as_str() {
+        match e["event"] {
-                    "shutdown" => {
+            Value::String(ref name) => break name,
-                        event = Event::Shutdown;
+            _ => {
+                // It was not an event - try again
+                debug!("Bad response: {:?}", response)
             }
-                    "start-file" => {
-                        event = Event::StartFile;
-                    }
-                    "file-loaded" => {
-                        event = Event::FileLoaded;
-                    }
-                    "seek" => {
-                        event = Event::Seek;
-                    }
-                    "playback-restart" => {
-                        event = Event::PlaybackRestart;
-                    }
-                    "idle" => {
-                        event = Event::Idle;
-                    }
-                    "tick" => {
-                        event = Event::Tick;
-                    }
-                    "video-reconfig" => {
-                        event = Event::VideoReconfig;
-                    }
-                    "audio-reconfig" => {
-                        event = Event::AudioReconfig;
-                    }
-                    "tracks-changed" => {
-                        event = Event::TracksChanged;
-                    }
-                    "track-switched" => {
-                        event = Event::TrackSwitched;
-                    }
-                    "pause" => {
-                        event = Event::Pause;
-                    }
-                    "unpause" => {
-                        event = Event::Unpause;
-                    }
-                    "metadata-update" => {
-                        event = Event::MetadataUpdate;
-                    }
-                    "chapter-change" => {
-                        event = Event::ChapterChange;
-                    }
-                    "end-file" => {
-                        event = Event::EndFile;
         }
+    };
+
+    let event = match name.as_str() {
+        "shutdown" => Event::Shutdown,
+        "start-file" => Event::StartFile,
+        "file-loaded" => Event::FileLoaded,
+        "seek" => Event::Seek,
+        "playback-restart" => Event::PlaybackRestart,
+        "idle" => Event::Idle,
+        "tick" => Event::Tick,
+        "video-reconfig" => Event::VideoReconfig,
+        "audio-reconfig" => Event::AudioReconfig,
+        "tracks-changed" => Event::TracksChanged,
+        "track-switched" => Event::TrackSwitched,
+        "pause" => Event::Pause,
+        "unpause" => Event::Unpause,
+        "metadata-update" => Event::MetadataUpdate,
+        "chapter-change" => Event::ChapterChange,
+        "end-file" => Event::EndFile,
         "property-change" => {
-                        let name: String;
+            let name = match e["name"] {
-                        let id: usize;
+                Value::String(ref n) => Ok(n.to_string()),
-                        let data: MpvDataType;
+                _ => Err(Error(ErrorCode::JsonContainsUnexptectedType)),
+            }?;
 
-                        if let Value::String(ref n) = e["name"] {
+            let id: usize = match e["id"] {
-                            name = n.to_string();
+                Value::Number(ref n) => n.as_u64().unwrap() as usize,
-                        } else {
+                _ => 0,
-                            return Err(Error(ErrorCode::JsonContainsUnexptectedType));
+            };
-                        }
 
-                        if let Value::Number(ref n) = e["id"] {
+            let data: MpvDataType = match e["data"] {
-                            id = n.as_i64().unwrap() as usize;
+                Value::String(ref n) => MpvDataType::String(n.to_string()),
-                        } else {
-                            id = 0;
-                        }
-
-                        match e["data"] {
-                            Value::String(ref n) => {
-                                data = MpvDataType::String(n.to_string());
-                            }
 
                 Value::Array(ref a) => {
                     if name == "playlist".to_string() {
-                                    data =
+                        MpvDataType::Playlist(Playlist(json_array_to_playlist(a)))
-                                        MpvDataType::Playlist(Playlist(json_array_to_playlist(a)));
                     } else {
-                                    data = MpvDataType::Array(json_array_to_vec(a));
+                        MpvDataType::Array(json_array_to_vec(a))
                     }
                 }
 
-                            Value::Bool(ref b) => {
+                Value::Bool(b) => MpvDataType::Bool(b),
-                                data = MpvDataType::Bool(*b);
-                            }
 
                 Value::Number(ref n) => {
                     if n.is_u64() {
-                                    data = MpvDataType::Usize(n.as_u64().unwrap() as usize);
+                        MpvDataType::Usize(n.as_u64().unwrap() as usize)
                     } else if n.is_f64() {
-                                    data = MpvDataType::Double(n.as_f64().unwrap());
+                        MpvDataType::Double(n.as_f64().unwrap())
                     } else {
                         return Err(Error(ErrorCode::JsonContainsUnexptectedType));
                     }
                 }
 
-                            Value::Object(ref m) => {
+                Value::Object(ref m) => MpvDataType::HashMap(json_map_to_hashmap(m)),
-                                data = MpvDataType::HashMap(json_map_to_hashmap(m));
-                            }
 
-                            Value::Null => {
+                Value::Null => MpvDataType::Null,
-                                data = MpvDataType::Null;
-                            }
-                        }
-
-                        event = try_convert_property(name.as_ref(), id, data);
-                    }
-                    _ => {
-                        event = Event::Unimplemented;
-                    }
             };
-                return Ok(event);
+
+            try_convert_property(name.as_ref(), id, data)
         }
-        }
+        _ => Event::Unimplemented,
-        Err(why) => return Err(Error(ErrorCode::JsonParseError(why.to_string()))),
+    };
-    }
+    Ok(event)
-    unreachable!();
 }
 
 pub fn listen_raw(instance: &mut Mpv) -> String {
     let mut response = String::new();
     instance.reader.read_line(&mut response).unwrap();
     response.trim_end().to_string()
-    // let mut stream = &instance.0;
-    // let mut buffer = [0; 32];
-    // stream.read(&mut buffer[..]).unwrap();
-    // String::from_utf8_lossy(&buffer).into_owned()
 }
 
 fn send_command_sync(instance: &Mpv, command: &str) -> String {

src/lib.rs

@@ -54,7 +54,7 @@ pub enum MpvCommand {
     },
     Observe {
         id: isize,
-        property: String
+        property: String,
     },
     PlaylistNext,
     PlaylistPrev,
@@ -118,6 +118,7 @@ pub enum ErrorCode {
     JsonContainsUnexptectedType,
     UnexpectedResult,
     UnexpectedValue,
+    MissingValue,
     UnsupportedType,
     ValueDoesNotContainBool,
     ValueDoesNotContainF64,
@@ -190,6 +191,7 @@ impl Display for ErrorCode {
             }
             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\'")
@@ -486,9 +488,7 @@ impl Mpv {
                     },
                 ],
             ),
-            MpvCommand::Observe { id, property } => {
+            MpvCommand::Observe { id, property } => observe_mpv_property(self, &id, &property),
-                observe_mpv_property(self, &id, &property)
-            }
             MpvCommand::PlaylistClear => run_mpv_command(self, "playlist-clear", &[]),
             MpvCommand::PlaylistMove { from, to } => {
                 run_mpv_command(self, "playlist-move", &[&from.to_string(), &to.to_string()])
@@ -514,9 +514,7 @@ impl Mpv {
                 ],
             ),
             MpvCommand::Stop => run_mpv_command(self, "stop", &[]),
-            MpvCommand::Unobserve(id) => {
+            MpvCommand::Unobserve(id) => unobserve_mpv_property(self, &id),
-                unobserve_mpv_property(self, &id)
-            }
         }
     }