Clean up codebase a little bit, remove all unwraps and throw proper errors
Diff
Cargo.toml | 1 -
src/device.rs | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------
src/discovery.rs | 14 ++++++--------
src/error.rs | 5 +++++
src/lib.rs | 2 +-
tests/integration_test.rs | 29 +++++++++++++++++++++++------
6 files changed, 71 insertions(+), 53 deletions(-)
@@ -9,7 +9,6 @@
readme = "README.md"
[dependencies]
clippy = {version = "0.0", optional = true}
reqwest = "0.8"
log = "0.3"
ssdp = "0.6"
@@ -6,6 +6,7 @@
use std::io::Read;
use std::time::Duration;
use error::*;
pub(crate) use self::xmltree::ParseError;
use self::xmltree::Element;
use self::reqwest::header::{ContentType, Headers};
use self::regex::Regex;
@@ -43,7 +44,6 @@
Transitioning,
}
lazy_static! {
static ref COORDINATOR_REGEX: Regex = Regex::new(r"^https?://(.+?):1400/xml")
.expect("Failed to create regex");
@@ -51,7 +51,7 @@
fn element_to_string(el: &Element) -> String {
el.text.to_owned().unwrap()
el.text.to_owned().unwrap_or_default()
}
impl Speaker {
@@ -64,7 +64,7 @@
return Err(ErrorKind::BadResponse.into());
}
let elements = Element::parse(resp).unwrap();
let elements = Element::parse(resp)?;
let device_description = elements
.get_child("device")
.chain_err(|| ErrorKind::ParseError)?;
@@ -123,19 +123,17 @@
.chain_err(|| ErrorKind::ParseError)?;
let group = zone_players
let group = &zone_players
.children
.iter()
.find(|ref child| child.attributes.get("uuid").unwrap() == &self.uuid)
.find(|child| child.attributes["uuid"] == self.uuid)
.chain_err(|| ErrorKind::DeviceNotFound(self.uuid.to_string()))?
.attributes
.get("group")
.unwrap();
.attributes["group"];
Ok(COORDINATOR_REGEX
.captures(zone_players.children.iter()
.find(|ref child|
.find(|child|
child.attributes.get("coordinator").unwrap_or(&"false".to_string()) == "true" &&
child.attributes.get("group").unwrap_or(&"".to_string()) == group)
.chain_err(|| ErrorKind::DeviceNotFound(self.uuid.to_string()))?
@@ -169,7 +167,11 @@
headers.set_raw("SOAPAction", format!("\"{}#{}\"", service, action));
let client = reqwest::Client::new();
let coordinator = if coordinator { self.coordinator()? } else { self.ip };
let coordinator = if coordinator {
self.coordinator()?
} else {
self.ip
};
debug!("Running {}#{} on {}", service, action, coordinator);
@@ -193,14 +195,15 @@
.send()
.chain_err(|| ErrorKind::DeviceUnreachable)?;
let element =
Element::parse(request).chain_err(|| ErrorKind::ParseError)?;
let element = Element::parse(request).chain_err(|| ErrorKind::ParseError)?;
let body = element.get_child("Body")
let body = element
.get_child("Body")
.chain_err(|| ErrorKind::ParseError)?;
if let Some(fault) = body.get_child("Fault") {
let error_code = element_to_string(fault.get_child("detail")
let error_code = element_to_string(fault
.get_child("detail")
.chain_err(|| ErrorKind::ParseError)?
.get_child("UPnPError")
.chain_err(|| ErrorKind::ParseError)?
@@ -213,8 +216,7 @@
error!("Got state {:?} from {}#{} call.", state, service, action);
Err(ErrorKind::from(state).into())
} else {
Ok(body
.get_child(format!("{}Response", action))
Ok(body.get_child(format!("{}Response", action))
.chain_err(|| ErrorKind::ParseError)?
.clone())
}
@@ -301,9 +303,7 @@
"Seek",
&format!(
"<InstanceID>0</InstanceID><Unit>REL_TIME</Unit><Target>{:02}:{:02}:{:02}</Target>",
hours,
minutes,
seconds
hours, minutes, seconds
),
true,
)?;
@@ -432,7 +432,7 @@
.to_owned()
.chain_err(|| ErrorKind::ParseError)?
.parse::<u8>()
.unwrap();
.chain_err(|| ErrorKind::ParseError)?;
Ok(volume)
}
@@ -465,7 +465,7 @@
"MediaRenderer/RenderingControl/Control",
"urn:schemas-upnp-org:service:RenderingControl:1",
"GetMute",
&format!("<InstanceID>0</InstanceID><Channel>Master</Channel>"),
"<InstanceID>0</InstanceID><Channel>Master</Channel>",
false,
)?;
@@ -474,8 +474,7 @@
.as_str()
{
"1" => true,
"0" => false,
_ => false,
"0" | _ => false,
})
}
@@ -520,13 +519,12 @@
.chain_err(|| ErrorKind::ParseError)?)
.as_str()
{
"STOPPED" => TransportState::Stopped,
"PLAYING" => TransportState::Playing,
"PAUSED_PLAYBACK" => TransportState::PausedPlayback,
"PAUSED_RECORDING" => TransportState::PausedRecording,
"RECORDING" => TransportState::Recording,
"TRANSITIONING" => TransportState::Transitioning,
_ => TransportState::Stopped,
"STOPPED" | _ => TransportState::Stopped,
},
)
}
@@ -554,17 +552,27 @@
let duration: Vec<u64> = element_to_string(resp.get_child("TrackDuration")
.chain_err(|| ErrorKind::ParseError)?)
.splitn(3, ":")
.map(|s| s.parse::<u64>().unwrap())
.splitn(3, ':')
.map(|s| {
s.parse::<u64>()
.chain_err(|| ErrorKind::ParseError)
.unwrap()
})
.collect();
let duration = Duration::from_secs((duration[0] * 3600) + (duration[1] * 60) + duration[2]);
let running_time: Vec<u64> = element_to_string(resp.get_child("RelTime")
.chain_err(|| ErrorKind::ParseError)?)
.splitn(3, ":")
.map(|s| s.parse::<u64>().unwrap())
.splitn(3, ':')
.map(|s| {
s.parse::<u64>()
.chain_err(|| ErrorKind::ParseError)
.unwrap()
})
.collect();
let running_time = Duration::from_secs((running_time[0] * 3600) + (running_time[1] * 60) + running_time[2]);
let running_time = Duration::from_secs(
(running_time[0] * 3600) + (running_time[1] * 60) + running_time[2],
);
Ok(Track {
title: element_to_string(metadata
@@ -576,10 +584,9 @@
album: element_to_string(metadata
.get_child("album")
.chain_err(|| ErrorKind::ParseError)?),
queue_position: element_to_string(resp.get_child("Track")
.chain_err(|| ErrorKind::ParseError)?)
queue_position: element_to_string(resp.get_child("Track").chain_err(|| ErrorKind::ParseError)?)
.parse::<u64>()
.unwrap(),
.chain_err(|| ErrorKind::ParseError)?,
uri: element_to_string(resp.get_child("TrackURI")
.chain_err(|| ErrorKind::ParseError)?),
duration,
@@ -1,8 +1,9 @@
extern crate ssdp;
use self::ssdp::FieldMap;
use self::ssdp::header::{HeaderMut, HeaderRef, Man, MX, ST};
use self::ssdp::message::{Multicast, SearchRequest, SearchResponse};
pub(crate) use self::ssdp::SSDPError;
use device::Speaker;
use error::*;
@@ -10,11 +11,9 @@
fn get_header(msg: &SearchResponse, header: &str) -> Result<String> {
let bytes = msg.get_raw(header)
.chain_err(|| ErrorKind::ParseError)?;
String::from_utf8(bytes.get(0).unwrap().to_vec())
.chain_err(|| ErrorKind::ParseError)
let bytes = msg.get_raw(header).chain_err(|| ErrorKind::ParseError)?;
String::from_utf8(bytes[0].clone()).chain_err(|| ErrorKind::ParseError)
}
@@ -29,7 +28,7 @@
let mut speakers = Vec::new();
for (msg, src) in request.multicast().unwrap() {
for (msg, src) in request.multicast()? {
let usn = get_header(&msg, "USN")?;
if !usn.contains(SONOS_URN) {
@@ -37,8 +36,7 @@
continue;
}
speakers.push(Speaker::from_ip(src.ip())
.chain_err(|| ErrorKind::ParseError)?);
speakers.push(Speaker::from_ip(src.ip()).chain_err(|| ErrorKind::ParseError)?);
}
Ok(speakers)
@@ -25,6 +25,11 @@
display("Couldn't find a device by the given identifier ({})", identifier)
}
}
foreign_links {
Discovery(::discovery::SSDPError) #[doc = "An error occurred while attempting to discover devices"];
XMLParse(::device::ParseError) #[doc = "An error occurred while parsing device response"];
}
}
impl From<AVTransportError> for ErrorKind {
@@ -19,4 +19,4 @@
pub use device::TransportState;
pub use error::*;
pub use discovery::discover;
pub use discovery::discover;
@@ -1,9 +1,12 @@
extern crate sonos;
use sonos::TransportState;
fn get_speaker() -> sonos::Speaker {
let devices = sonos::discover().unwrap();
devices.into_iter()
devices
.into_iter()
.find(|d| d.name == "Bedroom")
.ok_or("Couldn't find bedroom")
.unwrap()
@@ -12,7 +15,7 @@
#[test]
fn can_discover_devices() {
let devices = sonos::discover().unwrap();
assert!(devices.len() > 0, "No devices discovered");
assert!(!devices.is_empty(), "No devices discovered");
}
#[test]
@@ -47,22 +50,19 @@
device.play().expect("Couldn't play track");
assert!(match device.transport_state().unwrap() {
sonos::TransportState::Playing => true,
sonos::TransportState::Transitioning => true,
TransportState::Playing | TransportState::Transitioning => true,
_ => false,
});
device.pause().expect("Couldn't pause track");
assert!(match device.transport_state().unwrap() {
sonos::TransportState::PausedPlayback => true,
sonos::TransportState::Transitioning => true,
TransportState::PausedPlayback | TransportState::Transitioning => true,
_ => false,
});
device.stop().expect("Couldn't stop track");
assert!(match device.transport_state().unwrap() {
sonos::TransportState::Stopped => true,
sonos::TransportState::Transitioning => true,
TransportState::Stopped | TransportState::Transitioning => true,
_ => false,
});
}
@@ -76,8 +76,17 @@
#[test]
fn seek() {
let device = get_speaker();
device.seek(&std::time::Duration::from_secs(30)).expect("Failed to seek to 30 seconds");
assert_eq!(device.track().expect("Failed to get track info").running_time.as_secs(), 30);
device
.seek(&std::time::Duration::from_secs(30))
.expect("Failed to seek to 30 seconds");
assert_eq!(
device
.track()
.expect("Failed to get track info")
.running_time
.as_secs(),
30
);
}
#[test]