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

Fix setting of back flag when splitting streets #16

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

evansiroky
Copy link

This fix includes most of the recommended suggestions made by @jwoyame. It corrects the setting of the back flag for StreetEdges in order for the geometry to be deserialized properly.

This should fix ibi-group/trimet-mod-otp#243 and also fix ibi-group/trimet-mod-otp#244.

Copy link

@landonreed landonreed left a comment

Choose a reason for hiding this comment

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

I don't have a great understanding of what exactly is going on here. I think it would be helpful if you could quote/extract bits of the relevant conversation in this PR. The link provided in the current description doesn't point to a particular comment.

Is there some case that demonstrates that something has been fixed with this change? I'm not necessarily requesting a test here, just curious if there are repeatable steps I could follow to demonstrate that a bug has been fixed.

Also, are you able to identify here why this value was set to false in the first place? Would it be useful to amend the javadoc for the constructor to be more descriptive about why it uses its parent edge's value?

@landonreed landonreed assigned evansiroky and unassigned landonreed Oct 2, 2019
@evansiroky
Copy link
Author

Yeah, I suppose I meant to link to the comment specifically made by @jwoyame. Basically the thing going on is that the way StreetEdge geometry was serialized and deserialized from internal OTP data structures became broken and needed fixing. When the StreetEdge geometry is stored, it is dependent on the back flag which is meant to store information about whether a StreetEdge represents the data about the corresponding OSM way in the backward direction.

The code I made for creating PartialStreetEdges was incorrect. At first, it was incorrectly setting the back flag to false for every PartialStreetEdge. This was an artifact of the previous version of code which might have been intended mainly for TemporaryFreeEdges which don't represent OSM ways and therefore wouldn't ever be a back representation of an OSM way, but I'm not sure how it ended up in PartialStreetEdge. However, that alone didn't cause the issue because the geometry was serialized and deserialized correctly because the back flag didn't change. The problem occurred after in StreetEdge.java when the back flag was set again to the parent edge's back flag which did result in changing the back flag after the geometry was already serialized. I mistakenly added this along with every other StreetEdge attribute that I thought needed copying to split edges. Therefore whenever the geometry was deserialized and the back flag wasn't false for the parent edge, the geometry would look reversed.

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Oct 2, 2019
@landonreed
Copy link

Thanks for the clarification!

@landonreed landonreed merged commit 5678243 into trimet-dev Oct 3, 2019
@landonreed landonreed deleted the edge-split-geometry-fix branch October 3, 2019 13:01
landonreed pushed a commit that referenced this pull request Jul 7, 2020
…le (#16)

* Only allow to use GTFS transfers after alighting from a transit vehicle

* Adding the same check for simple (auto-generated) transfers
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.

Incorrect display of scooter legs Incorrect display of walking legs at end of trip
2 participants