Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Tag smoothness=impassable breaks pedestrian routing #5023

Merged
merged 11 commits into from
Dec 18, 2024
Merged

Conversation

gknisely
Copy link
Member

@gknisely gknisely commented Dec 16, 2024

Issue

Smoothness tag should not delete the edge in general. The smoothness tag is for the physical usability of a way for wheeled vehicles as defined in the wiki and pedestrian access should not be removed.

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@chrstnbwnkl chrstnbwnkl self-requested a review December 17, 2024 07:57
chrstnbwnkl
chrstnbwnkl previously approved these changes Dec 17, 2024
@chrstnbwnkl
Copy link
Member

Ah wait, it shouldn't have wheelchair access though right? 😄

@chrstnbwnkl chrstnbwnkl dismissed their stale review December 17, 2024 08:04

wheelchair access on impassable surface

@gknisely
Copy link
Member Author

Ah wait, it shouldn't have wheelchair access though right? 😄

Yes. We use pedestrian access to assign wheelchair when we can do so. https://github.com/valhalla/valhalla/blob/master/src/mjolnir/directededgebuilder.cc#L164-L165

@gknisely
Copy link
Member Author

gknisely commented Dec 17, 2024

@chrstnbwnkl Let me update the directed edge builder and test

@gknisely gknisely closed this Dec 17, 2024
@gknisely gknisely reopened this Dec 17, 2024
@chrstnbwnkl
Copy link
Member

I guess it doesn't matter because wheelchair will still be disallowed due to the surface value?

if (type == "wheelchair") {
// TODO(nils): this needs to control much more in costing, e.g.
// we could add more AccessRestrictions for curb height or so
type_ = PedestrianType::kWheelchair;
access_mask_ = kWheelchairAccess;
minimal_allowed_surface_ = Surface::kCompacted;

@gknisely
Copy link
Member Author

I guess it doesn't matter because wheelchair will still be disallowed due to the surface value?

if (type == "wheelchair") {
// TODO(nils): this needs to control much more in costing, e.g.
// we could add more AccessRestrictions for curb height or so
type_ = PedestrianType::kWheelchair;
access_mask_ = kWheelchairAccess;
minimal_allowed_surface_ = Surface::kCompacted;

Nah. I think it is better to not assign. Give me a few

@gknisely
Copy link
Member Author

@chrstnbwnkl should be all set

@chrstnbwnkl chrstnbwnkl self-requested a review December 18, 2024 09:11
@gknisely gknisely merged commit cda4478 into master Dec 18, 2024
9 checks passed
@gknisely gknisely deleted the smoothness_fix branch December 18, 2024 13:45
ianthetechie pushed a commit to ianthetechie/valhalla that referenced this pull request Feb 7, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag smoothness=impassable breaks pedestrian routing
2 participants