Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

Add Support For cycleway=opposite #215

Merged
merged 2 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions data/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion osm-tags/src/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize};

/// Access variants from <https://wiki.openstreetmap.org/wiki/Key:access#List_of_possible_values>
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
#[serde(rename_all = "snake_case")]
pub enum Access {
Yes,
No,
Expand Down
14 changes: 9 additions & 5 deletions osm2lanes/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ pub struct TestCase {
// Metadata
/// The OSM way unique identifier
pub way_id: Option<i64>,
/// Relevant link
pub link: Option<String>,
/// Comment on test case
pub comment: Option<String>,
/// Description of test case
pub description: Option<String>,

// List as a named example in the web app
/// List as a named example in the web app, with the given name
example: Option<String>,

// Config and Locale
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:");
Expand All @@ -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)
{
Expand Down
4 changes: 4 additions & 0 deletions osm2lanes/src/transform/lanes_to_tags/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...
droogmic marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
}

Expand Down
59 changes: 42 additions & 17 deletions osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -150,7 +150,7 @@ impl Scheme {
road_oneway: Oneway,
warnings: &mut RoadWarnings,
) -> Result<Self, TagsToLanesMsg> {
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 =
Expand Down Expand Up @@ -187,16 +187,16 @@ impl Scheme {
locale: &Locale,
road_oneway: Oneway,
warnings: &mut RoadWarnings,
) -> Option<Self> {
) -> Result<Option<Self>, 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(
Expand All @@ -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,
Expand All @@ -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)
},
}
}
Expand Down Expand Up @@ -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"))?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in error string.

.access
.bicycle = Infer::Direct(AccessAndDirection {
access: Access::Yes,
direction: Some(Direction::Both),
})
},
},
Location::Both { forward, backward } => {
road.push_forward_outside(lane(forward));
Expand Down Expand Up @@ -605,6 +617,7 @@ mod tests {
);
}

// cycleway=opposite
#[test]
fn opposite() {
let mut warnings = RoadWarnings::default();
Expand All @@ -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();
Expand Down
16 changes: 12 additions & 4 deletions osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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",
Expand Down
30 changes: 11 additions & 19 deletions osm2lanes/src/transform/tags_to_lanes/road.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,23 +54,15 @@ pub struct Width {

#[derive(Clone, Default, Debug)]
pub struct Access {
pub foot: Infer<AccessValue>,
pub bicycle: Infer<AccessValue>,
pub taxi: Infer<AccessValue>,
pub bus: Infer<AccessValue>,
pub motor: Infer<AccessValue>,
pub foot: Infer<LaneAccessAndDirection>,
pub bicycle: Infer<LaneAccessAndDirection>,
pub taxi: Infer<LaneAccessAndDirection>,
pub bus: Infer<LaneAccessAndDirection>,
pub motor: Infer<LaneAccessAndDirection>,
}

impl From<Access> for Option<LaneAccessByType> {
fn from(inferred: Access) -> Self {
impl LaneAccessAndDirection {
fn from(v: Infer<AccessValue>) -> Option<Self> {
v.some().map(|v| Self {
access: v,
direction: None,
})
}
}
if inferred.foot.is_none()
&& inferred.bicycle.is_none()
&& inferred.taxi.is_none()
Expand All @@ -80,11 +72,11 @@ impl From<Access> for Option<LaneAccessByType> {
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(),
})
}
}
Expand Down