Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Multiple ways and restrictions missing for FOOT and BIKE #50

Closed
NopMap opened this issue May 25, 2013 · 11 comments
Closed

Multiple ways and restrictions missing for FOOT and BIKE #50

NopMap opened this issue May 25, 2013 · 11 comments
Assignees
Milestone

Comments

@NopMap
Copy link
Contributor

NopMap commented May 25, 2013

The OSM Reader is ignoring multiple taggings allowing ways like bicycle=designated, bicycle=official etc. It is also ignoring motorroads which are prohibited to pedestrians and cyclists.

@karussell
Copy link
Member

Thanks, would you mind to add those tags (probably in an example osm file)?

@NopMap
Copy link
Contributor Author

NopMap commented May 25, 2013

Already working on it.

@NopMap
Copy link
Contributor Author

NopMap commented May 25, 2013

Theoretically, an access vehicle=no also excludes bicycles. In practice, cylcists ignore those signs. I'd implement the practice rather than an unrealistic, legally correct interpretation.

For ferries: If they are not marked for any particular vehicle, they are currently accepted for cars. I did some research and I believe this is wrong. The unmarked ferries are usually only for pedestrians, the large ferries that carry cars are ususally marked. So I would change this to assume that unmarked ferries are foot only.

@NopMap
Copy link
Contributor Author

NopMap commented May 25, 2013

I just noted that there are 2 CarFlagEncoder Tests and no BikeFlagEncoderTest. Is there a reason?

@NopMap
Copy link
Contributor Author

NopMap commented May 26, 2013

There is a more accurate set of allowed/disallowed ways now. Currently 4 of the routing tests are broken, probably because new edges were introduced and some were removed from the graph. The other tests are running fine.

It would be helpful to get the routing tests fixed before doing any refactoring that is not supposed to change their results, but I need help for that as I can't judge whether they are ok and the graph changed or whether there actually is something wrong.

@karussell
Copy link
Member

Thanks! Some days ago I noticed the bike encoder is broken. today I'll fix it and also merge you changes!

@NopMap
Copy link
Contributor Author

NopMap commented May 27, 2013

I have used the rainy weather yesterday and done a lot more of refactoring on the encoders, maybe you should wait fort that before applying fixes. I can push it this evening.

The bike encoder does not produce valid speed values, it looks for highway values like the car encoder, but provides another set of values. Except for that, everything has been fixed and there are some improvements.

But it is lacking a test. Could you add a Bike test? I can extend existing test cases but I don't know how to add a new test class.

@karussell
Copy link
Member

I merged your pull request and added a bike test.

Now bike produces valid speed values. But I think we should add 'surface' parsing for bike speed instead of 'highway' which can also improve car speeds as well e.g. for bouldering or foot speed for trail vs. road

@karussell karussell reopened this May 27, 2013
@karussell
Copy link
Member

and done a lot more of refactoring on the encoders

I would really like to take a look at it now as I would improve up on this with the surface stuff!

I can extend existing test cases but I don't know how to add a new test class.

Add a class with the same name and a 'Test' end somewhere into the src/test/java directory structure. 'somewhere' -> same package of course

@NopMap
Copy link
Contributor Author

NopMap commented May 27, 2013

Ok. How can I get your changes into my fork and try to resolve the conflicts in Git?

As I refactored the flag encoders, there's likely to be conflicts.

As for speed:

  • If you parse surface you should also parse tracktype
  • You will need different settings for racing bikes, trekking bikes and mountain bikes.

@karussell
Copy link
Member

How can I get your changes into my fork and try to resolve the conflicts in Git?

try

git checkout master
git fetch # this will give you the latest stuff into the master branch of your fork
git merge # this will merge my changes to yours into master (if you didn't do anything in master this will just fast forward and do no merge commit)
git checkout myfork # this will switch to your fork
git merge master # now merge the changes to your fork

BTW: git pull == git fetch + git merge: http://stackoverflow.com/questions/292357/whats-the-difference-between-git-pull-and-git-fetch

If the last step produces conflicts just edit with your favorite editor/IDE. E.g. in netbeans you can easily view and revert local changes and/or git commits.

You will need different settings for racing bikes, trekking bikes and mountain bikes.

I feared that someone will suggest this ;)

Probably just an extended encoder class for every type? For now I would like to concentrate on trekking bike (just selfish reasons ;)).

If you parse surface you should also parse tracktype

Thanks! It looks like there is yet another one: http://wiki.openstreetmap.org/wiki/Key:smoothness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants