From 843742b708dfdb380e37f0e4ed978136610e70d6 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 10 Oct 2018 10:02:46 -0700 Subject: [PATCH] adding error messages for map model queries that can fail --- map_model/Cargo.toml | 1 + map_model/src/lib.rs | 14 +++++++++ map_model/src/make/bus_stops.rs | 2 +- map_model/src/map.rs | 22 ++++++++------ map_model/src/road.rs | 54 ++++++++++++++++++++++++++------- sim/src/helpers.rs | 7 +++-- sim/src/router.rs | 1 + 7 files changed, 76 insertions(+), 25 deletions(-) diff --git a/map_model/Cargo.toml b/map_model/Cargo.toml index 112098aef8..5bdf0a711a 100644 --- a/map_model/Cargo.toml +++ b/map_model/Cargo.toml @@ -6,6 +6,7 @@ authors = ["Dustin Carlino "] [dependencies] abstutil = { path = "../abstutil" } dimensioned = { git = "https://github.com/paholg/dimensioned", rev = "0e1076ebfa5128d1ee544bdc9754c948987b6fe3", features = ["serde"] } +failure = "0.1.2" flame = "0.2.2" geo = "0.9.1" geom = { path = "../geom" } diff --git a/map_model/src/lib.rs b/map_model/src/lib.rs index 8b8234cf1f..f30c38f804 100644 --- a/map_model/src/lib.rs +++ b/map_model/src/lib.rs @@ -1,5 +1,7 @@ extern crate abstutil; extern crate dimensioned; +#[macro_use] +extern crate failure; extern crate flame; extern crate geo; extern crate geom; @@ -46,3 +48,15 @@ pub use traversable::Traversable; pub use turn::{Turn, TurnID}; pub const LANE_THICKNESS: f64 = 2.5; + +#[derive(Debug, Fail)] +#[fail(display = "{}", reason)] +pub struct MapError { + reason: String, +} + +impl MapError { + pub fn new(reason: String) -> MapError { + MapError { reason } + } +} diff --git a/map_model/src/make/bus_stops.rs b/map_model/src/make/bus_stops.rs index 9df5d37850..f5eb758eb9 100644 --- a/map_model/src/make/bus_stops.rs +++ b/map_model/src/make/bus_stops.rs @@ -35,7 +35,7 @@ pub fn make_bus_stops( for (id, dists) in stops_per_sidewalk.iter_all_mut() { let road = &roads[lanes[id.0].parent.0]; - if let Some(driving_lane) = road.find_driving_lane_from_sidewalk(*id) { + if let Ok(driving_lane) = road.find_driving_lane_from_sidewalk(*id) { dists.sort_by_key(|(dist, _)| NotNaN::new(dist.value_unsafe).unwrap()); for (idx, (dist_along, orig_pt)) in dists.iter().enumerate() { let stop_id = BusStopID { sidewalk: *id, idx }; diff --git a/map_model/src/map.rs b/map_model/src/map.rs index 2f9e40a1d5..e24764470d 100644 --- a/map_model/src/map.rs +++ b/map_model/src/map.rs @@ -2,12 +2,12 @@ use abstutil; use edits::RoadEdits; +use failure::{Error, ResultExt}; use flame; use geom::{Bounds, HashablePt2D, PolyLine, Pt2D}; use make; use raw_data; use std::collections::{BTreeMap, BTreeSet, HashMap}; -use std::io::Error; use std::path; use { Area, AreaID, Building, BuildingID, BusRoute, BusStop, BusStopID, Intersection, IntersectionID, @@ -379,24 +379,26 @@ impl Map { self.bounds.clone() } - pub fn get_driving_lane_from_bldg(&self, bldg: BuildingID) -> Option { + pub fn get_driving_lane_from_bldg(&self, bldg: BuildingID) -> Result { let sidewalk = self.get_b(bldg).front_path.sidewalk; let road = self.get_parent(sidewalk); - let parking = road.find_parking_lane(sidewalk)?; - road.find_driving_lane(parking) + road.find_parking_lane(sidewalk) + .and_then(|parking| road.find_driving_lane(parking)) + .context(format!("get_driving_lane_from_bldg({})", bldg)) } - pub fn get_sidewalk_from_driving_lane(&self, driving: LaneID) -> Option { + pub fn get_sidewalk_from_driving_lane(&self, driving: LaneID) -> Result { let road = self.get_parent(driving); // No parking lane? - if let Some(l) = road.find_sidewalk(driving) { - return Some(l); + if let Ok(l) = road.find_sidewalk(driving) { + return Ok(l); } - let parking = road.find_parking_lane(driving)?; - road.find_sidewalk(parking) + road.find_parking_lane(driving) + .and_then(|parking| road.find_sidewalk(parking)) + .context(format!("get_sidewalk_from_driving_lane({})", driving)) } - pub fn get_driving_lane_from_parking(&self, parking: LaneID) -> Option { + pub fn get_driving_lane_from_parking(&self, parking: LaneID) -> Result { self.get_parent(parking).find_driving_lane(parking) } diff --git a/map_model/src/road.rs b/map_model/src/road.rs index c9e26e45e0..aa825097d5 100644 --- a/map_model/src/road.rs +++ b/map_model/src/road.rs @@ -1,8 +1,9 @@ use dimensioned::si; +use failure::Error; use geom::PolyLine; use std::collections::BTreeMap; use std::fmt; -use {LaneID, LaneType}; +use {LaneID, LaneType, MapError}; // TODO reconsider pub usize. maybe outside world shouldnt know. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, Serialize, Deserialize)] @@ -82,25 +83,37 @@ impl Road { lane == self.children_backwards[0].0 } - pub fn find_sidewalk(&self, parking_or_driving: LaneID) -> Option { + pub fn find_sidewalk(&self, parking_or_driving: LaneID) -> Result { self.get_siblings(parking_or_driving) .iter() .find(|pair| pair.1 == LaneType::Sidewalk) .map(|pair| pair.0) + .ok_or_else(|| { + Error::from(MapError::new(format!( + "{} doesn't have sidewalk sibling", + parking_or_driving + ))) + }) } - pub fn find_driving_lane(&self, parking: LaneID) -> Option { + pub fn find_driving_lane(&self, parking: LaneID) -> Result { //assert_eq!(l.lane_type, LaneType::Parking); self.get_siblings(parking) .iter() .find(|pair| pair.1 == LaneType::Driving) .map(|pair| pair.0) + .ok_or_else(|| { + Error::from(MapError::new(format!( + "{} doesn't have driving lane sibling", + parking + ))) + }) } // Handles intermediate parking and bus lanes and such // Additionally handles one-ways with a sidewalk on only one side. // TODO but in reality, there probably isn't a sidewalk on the other side of the one-way. :\ - pub fn find_driving_lane_from_sidewalk(&self, sidewalk: LaneID) -> Option { + pub fn find_driving_lane_from_sidewalk(&self, sidewalk: LaneID) -> Result { let (this_side, opposite, idx) = if let Some(idx) = self .children_forwards .iter() @@ -125,25 +138,34 @@ impl Road { .find(|(_, lt)| *lt == LaneType::Driving) .map(|(l, _)| *l) { - return Some(l); + return Ok(l); } // Is the sidewalk on a one-way with the other side having a driving lane? if this_side.len() == 1 && opposite[0].1 == LaneType::Driving { - return Some(opposite[0].0); + return Ok(opposite[0].0); } - None + bail!(MapError::new(format!( + "Sidewalk {} doesn't have driving lane", + sidewalk + ))); } - pub fn find_parking_lane(&self, driving: LaneID) -> Option { + pub fn find_parking_lane(&self, driving: LaneID) -> Result { //assert_eq!(l.lane_type, LaneType::Driving); self.get_siblings(driving) .iter() .find(|pair| pair.1 == LaneType::Parking) .map(|pair| pair.0) + .ok_or_else(|| { + Error::from(MapError::new(format!( + "{} doesn't have parking lane sibling", + driving + ))) + }) } - pub fn get_opposite_lane(&self, lane: LaneID, lane_type: LaneType) -> Option { + pub fn get_opposite_lane(&self, lane: LaneID, lane_type: LaneType) -> Result { let forwards: Vec = self .children_forwards .iter() @@ -158,10 +180,20 @@ impl Road { .collect(); if let Some(idx) = forwards.iter().position(|id| *id == lane) { - return backwards.get(idx).map(|id| *id); + return backwards.get(idx).map(|id| *id).ok_or_else(|| { + Error::from(MapError::new(format!( + "{} doesn't have opposite lane of type {:?}", + lane, lane_type + ))) + }); } if let Some(idx) = backwards.iter().position(|id| *id == lane) { - return forwards.get(idx).map(|id| *id); + return forwards.get(idx).map(|id| *id).ok_or_else(|| { + Error::from(MapError::new(format!( + "{} doesn't have opposite lane of type {:?}", + lane, lane_type + ))) + }); } panic!("{} doesn't contain {}", self.id, lane); } diff --git a/sim/src/helpers.rs b/sim/src/helpers.rs index 70d8e117ce..d562f6c8c7 100644 --- a/sim/src/helpers.rs +++ b/sim/src/helpers.rs @@ -202,12 +202,13 @@ impl Sim { let has_bldgs = map .get_parent(lane) .find_sidewalk(lane) - .and_then(|sidewalk| Some(!map.get_l(sidewalk).building_paths.is_empty())) + .and_then(|sidewalk| Ok(!map.get_l(sidewalk).building_paths.is_empty())) .unwrap_or(false); if has_bldgs { map.get_parent(lane) .find_driving_lane(lane) - .and_then(|_driving_lane| Some(parked_car.car)) + .and_then(|_driving_lane| Ok(parked_car.car)) + .ok() } else { None } @@ -325,7 +326,7 @@ fn pick_car_goal(rng: &mut R, map: &Map, start: LaneID) -> Lane .iter() .filter_map(|l| { if l.id != start && l.is_driving() { - if let Some(sidewalk) = map.get_sidewalk_from_driving_lane(l.id) { + if let Ok(sidewalk) = map.get_sidewalk_from_driving_lane(l.id) { if !map.get_l(sidewalk).building_paths.is_empty() { return Some(l.id); } diff --git a/sim/src/router.rs b/sim/src/router.rs index b9b7887480..f78df02a9a 100644 --- a/sim/src/router.rs +++ b/sim/src/router.rs @@ -213,6 +213,7 @@ fn find_parking_spot( ) -> Option { map.get_parent(driving_lane) .find_parking_lane(driving_lane) + .ok() .and_then(|l| parking_sim.get_first_free_spot(l, dist_along)) }