Skip to content

Conversation

@fabdrol
Copy link
Member

@fabdrol fabdrol commented Apr 16, 2016

This PR contains an alternative to #168. The course object is simpler, containing just the information required to navigate to the next Point in the currently active course. Each point can (optionally) reference an object under resources, such as a waypoint.

Some notes (related to a discussion on Slack):

  • I went with previousPoint instead of lastPoint, as I felt that lastPoint could be interpreted wrongly as "the last point of the route".
  • I went with href instead of id for referencing other objects, as it's much less ambiguous

@tkurki
Copy link
Member

tkurki commented Apr 20, 2016

Could you please add the nice example document, please? I can add the actual test then.

@tkurki
Copy link
Member

tkurki commented Apr 24, 2016

The sample course data file uses resources section, that is nowhere in the schema yet and the test naturally fails. However I think it should be added really quickly. @fabdrol did you have this in the works?

The other natural choice is to strip the resources section out of the test data file, making the test pass. Then this is mergable from my point of view and Great Circle equivalents can be added separately.

@sumps
Copy link
Contributor

sumps commented Apr 24, 2016

This looks really good now and I am gradually getting use to the idea of having navigation.course and a parallel set of keys in navigation.courseGC

I noticed that we are missing a timeToGo key in the navigation.course sub-group. This is different to ETA which is the estimated time that we arrive at the end of the route. Time to Go is the time in hours, mins, secs till we reach the next waypoint.

@sumps
Copy link
Contributor

sumps commented Apr 24, 2016

I think we should remove the resources section as the navigaction.course group could be driven without any of the data from resources i.e. Just a go to position entered on a chart plotter app and the position of the boat when the go to was started.

@sumps
Copy link
Contributor

sumps commented Apr 24, 2016

The resources data in the test file looked pretty good what needs to be done to get this in to the schema?

@tkurki
Copy link
Member

tkurki commented Apr 26, 2016

I removed the resources section as it can be handled separately and we can get this merged.

The notification part needs a bit more work, the test is failing because of notifications/arrivalCircleEntered/$source:Unknown property.

@tkurki tkurki merged commit 1596d40 into master May 1, 2016
@tkurki tkurki deleted the course-alt-proposal branch May 1, 2016 06:28
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.

4 participants