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

refactor: refactor station graph to isolate algorithm, data structure and topological #302

Merged
merged 16 commits into from
Apr 23, 2020

Conversation

CodeBear801
Copy link

@CodeBear801 CodeBear801 commented Apr 22, 2020

Issue

#282

Description

  • Remove neighbor in node, keep connectivity information inside graph

  • Abstract the layer of interface Graph, make algorithm such as Dijkstra depends on interface only

  • Abstract the layer of connectivitymap.Querier, which hides the logic for how connectivity is been built

    • To support search along route charge station query, user just pass in the connectivity graph of charge stations between low energy point
    • To support charge station based routing, user just pass in the pre-built connectivity graph between each charge stations
    • either of them could be implemented by OSRM cloud query or pre-built base google:s2+OSRM
  • Remove logic of building graph in stationgraph, put them inside of node_graph. All connectivity is build on the fly, not like previous version, stationgraph babysits the creation of graph. The responsibility of stationgraph now is calling internal functions to do the work and convert result to certain format.

  • More information could be found here:

  • To to:

    • later will consider change name of connectivitymap.Querier to connectivitymap.TopoQuerier, still considering
    • remove locationinfo, use nav.location
    • put logic into internal

Tasklist

  • make changes
  • add tests

@CodeBear801 CodeBear801 added the Refactor Rewrite existing code in order to improve its readability, reusability or structure label Apr 22, 2020
@CodeBear801 CodeBear801 self-assigned this Apr 22, 2020
@CodeBear801 CodeBear801 added Bug Something isn't working Prototype Proof of concept labels Apr 22, 2020
@wangyoucao577
Copy link

Will read it later. Please follow the PR template and always add entry in CHANGELOG-FORK.md. The rules in https://github.com/Telenav/osrm-backend/wiki/CHANGELOG.

Copy link

@wangyoucao577 wangyoucao577 left a comment

Choose a reason for hiding this comment

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

I'll approve the PR first. But please remember to add the entry in CHANGELOG-FORK.md before merge. Also, use Resolves #282 in the PR description can link and close the issue automatically, which reduces your manual work, have a try! :)

duration float64
}

type edgeIDAndData struct {

Choose a reason for hiding this comment

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

I'll prefer to name it edge. Name the the former edge to metric or something like this.

Copy link
Author

Choose a reason for hiding this comment

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

Good suggestion. Will consider to make the change in the next pull.

func (nc *nodeContainer) getNode(id nodeID) *node {
if n, ok := nc.id2NodePtr[id]; ok {
return n
} else {

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

agree.

@CodeBear801 CodeBear801 merged commit 0e05d7a into master Apr 23, 2020
@CodeBear801 CodeBear801 deleted the feature/station-graph-adjustment branch April 23, 2020 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Prototype Proof of concept Refactor Rewrite existing code in order to improve its readability, reusability or structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants