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

Parse lit tag #3893

Merged
merged 9 commits into from
Jan 10, 2023
Merged

Parse lit tag #3893

merged 9 commits into from
Jan 10, 2023

Conversation

chrstnbwnkl
Copy link
Contributor

@chrstnbwnkl chrstnbwnkl commented Jan 6, 2023

fixes #3848

This PR adds parsing of the lit tag:

  • parse the possible values and map them to a boolean value in graph.lua
  • add lit as a member variable to OSMWay
  • add lit as a member variable to the DirectedEdge class, thereby allocating one of the remaining bits
  • add the lit tag and the values we parse to taginfo.json

I also added a short parameterized test going through the different possible values of lit, including its absence.

["no"] = "false",
["24/7"] = "true",
["automatic"] = "true",
["limited"] = "false",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lit=limited is a special case that, according to the OSM wiki mainly occurs in Austria and Germany, where it signifies that a street light marked with a "Laternenring" is not turned on during the whole night, so I guess we cannot reliably set the marked edge as lit=true

@nilsnolde
Copy link
Member

Yay thanks! Got my approval, but someone else should review and approve:)

@@ -1847,6 +1865,9 @@ struct OSMWay {

// layer index(Z-level) of the way relatively to other levels
int8_t layer_;

// whether or not the street is lit
bool lit_;
Copy link
Member

Choose a reason for hiding this comment

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

we should use a single bit of the spare2_ above

Copy link
Member

@kevinkreiser kevinkreiser left a comment

Choose a reason for hiding this comment

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

so other than the change to osmway.h this looks good, we'll ship it after we get that fixed up. thanks!

@nilsnolde nilsnolde merged commit 97a4098 into valhalla:master Jan 10, 2023
@nilsnolde nilsnolde deleted the cb-parse-lit-tag branch January 10, 2023 15:19
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.

Support lit OSM key
3 participants