From 29c4547d6e7bcc27e163792b928a21df3d5ef0cf Mon Sep 17 00:00:00 2001 From: Michael Droogleever Fortuyn Date: Sun, 29 May 2022 23:04:06 +0200 Subject: [PATCH] Add Support For cycleway=opposite Covers part of #162 --- data/tests.yml | 9 ++- osm-tags/src/access.rs | 2 +- osm2lanes/src/test.rs | 14 +++-- osm2lanes/src/transform/lanes_to_tags/mod.rs | 4 ++ .../transform/tags_to_lanes/modes/bicycle.rs | 59 +++++++++++++------ .../tags_to_lanes/modes/non_motorized.rs | 16 +++-- osm2lanes/src/transform/tags_to_lanes/road.rs | 30 ++++------ 7 files changed, 83 insertions(+), 51 deletions(-) diff --git a/data/tests.yml b/data/tests.yml index 957977da..9f7741bc 100644 --- a/data/tests.yml +++ b/data/tests.yml @@ -710,10 +710,8 @@ designated: motor_vehicle - description: cycleway=opposite oneway=yes oneway:bicycle=no - rust: false # https://github.com/a-b-street/osm2lanes/issues/162 tags: highway: "road" - lanes: "1" oneway: "yes" oneway:bicycle: "no" cycleway: "opposite" @@ -723,12 +721,13 @@ road: highway: road lanes: - - type: travel - direction: backward - designated: bicycle - type: travel direction: forward designated: motor_vehicle + access: + bicycle: + access: "yes" + direction: "both" - description: "cycleway:BACKWARD:lane=advisory oneway=yes oneway:bicycle=no" way_id: 25745877 diff --git a/osm-tags/src/access.rs b/osm-tags/src/access.rs index f3a226d2..be32f357 100644 --- a/osm-tags/src/access.rs +++ b/osm-tags/src/access.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; /// Access variants from #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +#[serde(rename_all = "snake_case")] pub enum Access { Yes, No, diff --git a/osm2lanes/src/test.rs b/osm2lanes/src/test.rs index 47109ee1..44afcd4a 100644 --- a/osm2lanes/src/test.rs +++ b/osm2lanes/src/test.rs @@ -28,11 +28,14 @@ pub struct TestCase { // Metadata /// The OSM way unique identifier pub way_id: Option, + /// Relevant link pub link: Option, + /// Comment on test case pub comment: Option, + /// Description of test case pub description: Option, - // List as a named example in the web app + /// List as a named example in the web app, with the given name example: Option, // Config and Locale @@ -196,20 +199,21 @@ mod tests { direction: actual_direction, width: actual_width, max_speed: actual_max_speed, - access: _actual_access, + access: actual_access, }, Lane::Travel { designated: expected_designated, direction: expected_direction, width: expected_width, max_speed: expected_max_speed, - access: _expected_access, + access: expected_access, }, ) => { actual_designated == expected_designated && actual_direction == expected_direction && approx_eq(actual_width, expected_width) && approx_eq(actual_max_speed, expected_max_speed) + && approx_eq(actual_access, expected_access) }, ( Lane::Parking { @@ -242,7 +246,6 @@ mod tests { impl Marking { /// Eq where None is treaty as always equal - #[allow(clippy::unnested_or_patterns)] fn approx_eq(&self, expected: &Self) -> bool { self.style == expected.style && approx_eq(&self.color, &expected.color) @@ -508,7 +511,7 @@ mod tests { }, ) .unwrap(); - let (output_road, _warnings) = output_lanes.into_filtered_road(test); + let (output_road, warnings) = output_lanes.into_filtered_road(test); if !output_road.approx_eq(&input_road) { test.print(); println!("From:"); @@ -521,6 +524,7 @@ mod tests { println!("Got:"); println!(" {}", stringify_lane_types(&output_road)); println!(" {}", stringify_directions(&output_road)); + println!("{}", warnings); if stringify_lane_types(&input_road) == stringify_lane_types(&output_road) || stringify_directions(&input_road) == stringify_directions(&output_road) { diff --git a/osm2lanes/src/transform/lanes_to_tags/mod.rs b/osm2lanes/src/transform/lanes_to_tags/mod.rs index f1aad11a..1a7475f8 100644 --- a/osm2lanes/src/transform/lanes_to_tags/mod.rs +++ b/osm2lanes/src/transform/lanes_to_tags/mod.rs @@ -356,6 +356,7 @@ fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), Lan { tags.checked_insert("oneway:bicycle", "no")?; } + // indicate cycling traffic direction relative to the direction the osm way is oriented // yes: same direction // -1: contraflow @@ -394,6 +395,9 @@ fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), Lan tags.checked_insert("cycleway:right:width", width.val().to_string())?; } + // Handle shared lanes + if lanes.forward_inside() // TODO: this needs to exist... + Ok(()) } diff --git a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs index 83189313..2d541a27 100644 --- a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs +++ b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs @@ -2,11 +2,11 @@ use std::borrow::Borrow; use std::fmt::Display; use std::hash::Hash; -use osm_tags::TagKey; +use osm_tags::{Access, TagKey}; use crate::locale::Locale; use crate::metric::Metre; -use crate::road::{Designated, Direction}; +use crate::road::{AccessAndDirection, Designated, Direction}; use crate::tag::Tags; use crate::transform::tags::CYCLEWAY; use crate::transform::tags_to_lanes::oneway::Oneway; @@ -150,7 +150,7 @@ impl Scheme { road_oneway: Oneway, warnings: &mut RoadWarnings, ) -> Result { - let scheme_cycleway = Self::from_tags_cycleway(tags, locale, road_oneway, warnings); + let scheme_cycleway = Self::from_tags_cycleway(tags, locale, road_oneway, warnings)?; let scheme_cycleway_both = Self::from_tags_cycleway_both(tags, locale, road_oneway, warnings); let scheme_cycleway_forward = @@ -187,16 +187,16 @@ impl Scheme { locale: &Locale, road_oneway: Oneway, warnings: &mut RoadWarnings, - ) -> Option { + ) -> Result, TagsToLanesMsg> { match cycleway_variant(tags, None) { Ok(OptionNo::Some((variant, opposite))) => { if road_oneway.into() { if opposite.is_none() { - Some(Self(Location::Forward(Way { + Ok(Some(Self(Location::Forward(Way { variant, direction: Direction::Forward, width: None, - }))) + })))) } else { if let Variant::Lane | Variant::Track = variant { warnings.push(TagsToLanesMsg::deprecated( @@ -214,19 +214,19 @@ impl Scheme { .unwrap(), )); } - Some(Self(Location::Backward(Way { + Ok(Some(Self(Location::Backward(Way { variant, direction: Direction::Backward, width: None, - }))) + })))) } } else { if opposite.is_some() { - warnings.push(TagsToLanesMsg::unsupported_tags( + return Err(TagsToLanesMsg::unsupported_tags( tags.subset(["oneway", "cycleway"]), )); } - Some(Self(Location::Both { + Ok(Some(Self(Location::Both { forward: Way { variant, direction: Direction::Forward, @@ -237,14 +237,14 @@ impl Scheme { direction: Direction::Backward, width: None, }, - })) + }))) } }, - Ok(OptionNo::No) => Some(Self(Location::None)), - Ok(OptionNo::None) => None, + Ok(OptionNo::No) => Ok(Some(Self(Location::None))), + Ok(OptionNo::None) => Ok(None), Err(e) => { warnings.push(e.into()); - None + Ok(None) }, } } @@ -447,10 +447,22 @@ pub(in crate::transform::tags_to_lanes) fn bicycle( match scheme.0 { Location::None => {}, Location::Forward(way) => { - road.push_forward_outside(lane(way)); + if let Variant::Lane | Variant::Track = way.variant { + road.push_forward_outside(lane(way)); + } + // TODO: Do nothing if forward sharing the lane? What if we are on a bus-only road? }, - Location::Backward(way) => { - road.push_backward_outside(lane(way)); + Location::Backward(way) => match way.variant { + Variant::Lane | Variant::Track => road.push_backward_outside(lane(way)), + Variant::SharedMotor => { + road.forward_outside_mut() + .ok_or_else(|| TagsToLanesMsg::unsupported_str("no forward lanes for busway"))? + .access + .bicycle = Infer::Direct(AccessAndDirection { + access: Access::Yes, + direction: Some(Direction::Both), + }) + }, }, Location::Both { forward, backward } => { road.push_forward_outside(lane(forward)); @@ -605,6 +617,7 @@ mod tests { ); } + // cycleway=opposite #[test] fn opposite() { let mut warnings = RoadWarnings::default(); @@ -626,6 +639,18 @@ mod tests { ); } + // cycleway=opposite only applies to oneway + #[test] + fn err_opposite_twoway() { + let scheme = Scheme::from_tags( + &Tags::from_pair("cycleway", "opposite"), + &Locale::builder().build(), + Oneway::No, + &mut RoadWarnings::default(), + ); + assert!(scheme.is_err()); + } + #[test] fn warn_shoulder() { let mut warnings = RoadWarnings::default(); diff --git a/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs b/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs index 6ba230ed..56e2ee97 100644 --- a/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs +++ b/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs @@ -1,6 +1,8 @@ +use osm_tags::Access; + use crate::locale::Locale; -use crate::road::{Designated, Direction}; -use crate::tag::{Access, Tags, HIGHWAY}; +use crate::road::{AccessAndDirection, Designated, Direction}; +use crate::tag::{Tags, HIGHWAY}; use crate::transform::tags_to_lanes::{RoadBuilder, TagsToLanesMsg}; use crate::transform::{Infer, RoadWarnings}; @@ -24,8 +26,14 @@ pub(in crate::transform::tags_to_lanes) fn non_motorized( let lane = road.forward_outside_mut().unwrap(); lane.designated.set(Infer::Direct(Designated::Foot))?; lane.direction.set(Infer::Direct(Direction::Both))?; - lane.access.foot.set(Infer::Direct(Access::Designated))?; - lane.access.motor.set(Infer::Direct(Access::No))?; + lane.access.foot.set(Infer::Direct(AccessAndDirection { + access: Access::Designated, + direction: None, + }))?; + lane.access.motor.set(Infer::Direct(AccessAndDirection { + access: Access::No, + direction: None, + }))?; if v == "steps" { warnings.push(TagsToLanesMsg::unimplemented( "steps becomes sidewalk", diff --git a/osm2lanes/src/transform/tags_to_lanes/road.rs b/osm2lanes/src/transform/tags_to_lanes/road.rs index 3df2d218..01041410 100644 --- a/osm2lanes/src/transform/tags_to_lanes/road.rs +++ b/osm2lanes/src/transform/tags_to_lanes/road.rs @@ -16,7 +16,7 @@ use crate::road::{ AccessAndDirection as LaneAccessAndDirection, AccessByType as LaneAccessByType, Designated, Direction, Lane, }; -use crate::tag::{Access as AccessValue, Highway, TagKey, Tags}; +use crate::tag::{Highway, TagKey, Tags}; use crate::transform::error::{RoadError, RoadWarnings}; use crate::transform::tags_to_lanes::counts::{CentreTurnLaneScheme, Counts}; use crate::transform::tags_to_lanes::modes::BusLaneCount; @@ -54,23 +54,15 @@ pub struct Width { #[derive(Clone, Default, Debug)] pub struct Access { - pub foot: Infer, - pub bicycle: Infer, - pub taxi: Infer, - pub bus: Infer, - pub motor: Infer, + pub foot: Infer, + pub bicycle: Infer, + pub taxi: Infer, + pub bus: Infer, + pub motor: Infer, } impl From for Option { fn from(inferred: Access) -> Self { - impl LaneAccessAndDirection { - fn from(v: Infer) -> Option { - v.some().map(|v| Self { - access: v, - direction: None, - }) - } - } if inferred.foot.is_none() && inferred.bicycle.is_none() && inferred.taxi.is_none() @@ -80,11 +72,11 @@ impl From for Option { return None; } Some(LaneAccessByType { - foot: LaneAccessAndDirection::from(inferred.foot), - bicycle: LaneAccessAndDirection::from(inferred.bicycle), - taxi: LaneAccessAndDirection::from(inferred.taxi), - bus: LaneAccessAndDirection::from(inferred.bus), - motor: LaneAccessAndDirection::from(inferred.motor), + foot: inferred.foot.some(), + bicycle: inferred.bicycle.some(), + taxi: inferred.taxi.some(), + bus: inferred.bus.some(), + motor: inferred.motor.some(), }) } }