Skip to content

bicycle: respect cycleway:*=separate tags#7431

Merged
DennisOSRM merged 3 commits intomasterfrom
feature/bicycle-cycleway-separate
Mar 28, 2026
Merged

bicycle: respect cycleway:*=separate tags#7431
DennisOSRM merged 3 commits intomasterfrom
feature/bicycle-cycleway-separate

Conversation

@DennisOSRM
Copy link
Copy Markdown
Collaborator

Problem

Fixes #1171.

The bicycle profile already blacklists bicycle=use_sidepath (PR #5622), but ignores the equivalent infrastructure tags cycleway=separate, cycleway:both=separate, cycleway:left=separate, and cycleway:right=separate. When a road carries one of these tags, a parallel cycleway is mapped as a separate OSM way and cyclists should be routed via that cycleway instead of the carriageway.

Solution

Add a handle_cycleway_separate handler to the bicycle profile that blocks carriageway access when any cycleway:* tag is set to separate. Explicit bicycle access tags (bicycle=yes, bicycle=permissive, bicycle=designated, bicycle=destination) override the inference.

This mirrors the pattern established in the foot profile for sidewalk:*=separate (PR #7426).

Tests

  • features/bicycle/use_sidepath.feature (new): two route-preference scenarios verifying that routing goes via the parallel cycleway rather than the blocked carriageway, for both bicycle=use_sidepath and cycleway:right=separate.
  • features/bicycle/access.feature: added use_sidepath + explicit access combination rows; added a new scenario verifying that explicit bicycle=destination/bicycle=yes still routes when cycleway:right=separate is present.

All 106 @bicycle scenarios pass.

Roads tagged with cycleway=separate, cycleway:both=separate, or
cycleway:left/right=separate have a separately mapped parallel cycleway.
Cyclists should be routed via that cycleway instead of the carriageway.

Add a handle_cycleway_separate handler to the bicycle profile that blocks
carriageway access when any cycleway:* tag is set to 'separate'. Explicit
bicycle access tags (bicycle=yes/permissive/designated/destination) override
the inference, matching the pattern established in the foot profile for
sidewalk:*=separate (PR #7426).

Fixes #1171.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 26, 2026 11:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the bicycle routing profile to treat cycleway:*=separate as an indicator that cyclists should be routed onto a separately-mapped parallel cycleway, mirroring the existing use_sidepath behavior and aligning with similar logic in the foot profile.

Changes:

  • Added a handle_cycleway_separate handler to block carriageway access when cycleway=separate / cycleway:both=separate / cycleway:left/right=separate is present, unless bicycle access is explicitly allowed.
  • Added/extended cucumber tests covering bicycle=use_sidepath, cycleway:right=separate, and explicit bicycle access overrides.
  • Added a changelog entry documenting the behavioral fix.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
profiles/bicycle.lua Adds a new handler to block ways tagged with cycleway:*=separate (with an override for explicit bicycle access).
features/bicycle/use_sidepath.feature New scenarios asserting routing prefers the parallel cycleway over the tagged carriageway.
features/bicycle/access.feature Extends routability tables and adds a scenario for explicit access overriding the cycleway:*=separate inference.
CHANGELOG.md Documents the bicycle profile fix for cycleway:*=separate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread profiles/bicycle.lua Outdated
Comment on lines +371 to +375
-- An explicit non-blacklisted access tag (resolved via the bicycle > vehicle > access
-- hierarchy) overrides the cycleway inference.
if data.access and not profile.access_tag_blacklist[data.access] then
return
end
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The override check here is broader than the PR description: data.access is resolved via the bicycle→vehicle→access hierarchy and the condition allows any non-blacklisted value (e.g. bicycle=dismount, vehicle=yes, or an arbitrary some_tag) to bypass the cycleway:*=separate block. That makes the carriageway routable even when bicycle access is not explicitly whitelisted.

Consider restricting the override to explicit bicycle access values only (e.g. bicycle=yes|permissive|designated|destination, as described), rather than any non-blacklisted access tag from the hierarchy.

Copilot uses AI. Check for mistakes.
Comment thread profiles/bicycle.lua Outdated
Comment on lines +361 to +364
local cycleway = way:get_value_by_key('cycleway')
local cycleway_both = way:get_value_by_key('cycleway:both')
local cycleway_left = way:get_value_by_key('cycleway:left')
local cycleway_right = way:get_value_by_key('cycleway:right')
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This handler re-fetches cycleway* tags directly from way even though related values are already prefetched/derived in handle_bicycle_tags (data.cycleway, data.cycleway_left, data.cycleway_right). Reusing data.* would reduce repeated tag lookups and keep all cycleway-related logic operating on the same cached values.

Suggested change
local cycleway = way:get_value_by_key('cycleway')
local cycleway_both = way:get_value_by_key('cycleway:both')
local cycleway_left = way:get_value_by_key('cycleway:left')
local cycleway_right = way:get_value_by_key('cycleway:right')
local cycleway = data.cycleway or way:get_value_by_key('cycleway')
local cycleway_both = data.cycleway_both or way:get_value_by_key('cycleway:both')
local cycleway_left = data.cycleway_left or way:get_value_by_key('cycleway:left')
local cycleway_right = data.cycleway_right or way:get_value_by_key('cycleway:right')

Copilot uses AI. Check for mistakes.
DennisOSRM and others added 2 commits March 26, 2026 15:45
…ta fields

- Use data.cycleway/data.cycleway_left/data.cycleway_right (prefetched in
  handle_bicycle_tags) instead of re-fetching from way, falling back to
  way:get_value_by_key only when the cached field is nil.
- Replace the broad 'any non-blacklisted access value from the bicycle >
  vehicle > access hierarchy' override with an explicit check on the bicycle
  tag itself (yes/permissive/designated/destination). This prevents tags like
  vehicle=yes or access=yes from unintentionally bypassing the separate-
  cycleway block.

Addresses reviewer feedback on PR #7431.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@DennisOSRM DennisOSRM merged commit f9b562b into master Mar 28, 2026
23 checks passed
@DennisOSRM DennisOSRM deleted the feature/bicycle-cycleway-separate branch March 28, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bicycle=use_sidepath

2 participants