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

Master waypoints ref #142

Merged
merged 14 commits into from Jan 31, 2016
Merged

Master waypoints ref #142

merged 14 commits into from Jan 31, 2016

Conversation

rob42
Copy link
Contributor

@rob42 rob42 commented Dec 21, 2015

Moved position{} object to definitions, and reworked regions, routes, waypoints and misc position related structures to suit. This change is the same as #124, but that pull request was done against v1-beta1, and now difficult to merge to master. So this one is the specific changes cherry-picked against master.

@rob42
Copy link
Contributor Author

rob42 commented Dec 21, 2015

Finally...

}
},
"waypoints": {
"waypoint-1": {
Copy link
Member

Choose a reason for hiding this comment

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

Spec says uuid, this is not. I don't think we can specify the id scheme here. I guess one minimum functionality is to support waypoing ids from NMEA 0183 WPL sentences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably used UUID badly - i really meant unique ID within the scope of the waypoints list - but thinking beyond a single boat UUID is better, then we can merge two waypoint lists reliably. In which case they may need an id field for NMEA names.

@rob42
Copy link
Contributor Author

rob42 commented Dec 30, 2015

Looking into GeoJSON and TopoJSON further there is a fundamental difference with signal k. The root point in GeoJSON/TopoJSON is a Point, represented by an array [lon,lat,altitude]. All other objects are built from that. Since we use an object "position" : {"latitude": lat, "longitude":lon, "altitude": alt, "timestamp":..,"source":...} we cannot simply write GeoJSON with signal k without point/position translation.

Other than that problem we can reuse GeoJSON objects for the regions, waypoints, notes, routes etc. But there are complexities - a route becomes a linestring, but a linestring is composed of multiple points, so it difficult to create a route between a set of existing waypoints and maintain reference to the waypoints (eg if you change a waypoint location, the linestring wont change because they are just an array of points now.)
Ideas welcome...

@rob42
Copy link
Contributor Author

rob42 commented Dec 30, 2015

Hmm, one way would be to use the GeoJSON geometry objects to define paths or boundaries etc between positions or regions defined in signalk.

So a route might go from signalk (waypoint)w1 to w2 to w3, with each leg defined by a linestring, not necessarily by straight line. So we would have w1->linestring1->w2->linestring2->w3. eg the route references the 3 waypoints of interest but the details are just a linestring between.

Likewise we could store signalk positions or waypoints as bounding box for a region (eg shallow area), but have a GeoJSON polyline defining the more precise outline. This makes for fairly easy auto-nav alerts and simple drawing of the region on the chart.

@rob42
Copy link
Contributor Author

rob42 commented Dec 30, 2015

Just found this https://en.wikipedia.org/wiki/Geohash - basically a specialized hash that is unique by region, getting more precise with more precise locations. If we use this for the UID of a waypoint, then everyone will get the same hash for the same point, making merging easy and grouping nearby waypoints easy.

Also its two way, lat/lon to hash, back to lat/lon.

@tkurki
Copy link
Member

tkurki commented Dec 30, 2015

@rob42 my point re:GeoJSON is that instead of re-inventing stuff that is like Polygon in GeoJSON (region outline) we should just reuse constructs from other specs or refer to them explicitly.

Other than that a route doesn't need to be linestring, since it often in practice is a collection of waypoints.

You bring up an interesting point about coordinates: a quick look into toolkits like

shows that the way Signal K represents coordinates requires some kind of conversion to all of these formats (and that the order of 2 dimensional coordinates is not standard...).

@fabdrol
Copy link
Member

fabdrol commented Jan 15, 2016

It seems weird to me to use different types of notations for positions. I agree that our representation is different from libraries out there (the examples were all javascript though, which is not necessarily representative. Google Maps on iOS uses a constructor which takes two arguments (lon, lat), apple maps take a dict with keys, etc).

One (massively breaking) solution would be to change the position object, remove latitude, longitude and altitude and substitute it with an array on the value property (value: [ x, y, z ]). That's much less clear and we all know that arrays are evil. So, why not stick to our conventions of being verbose/self-documenting?

If we want to support existing formats like KML, and include them in the spec, some parser would have to translate them anyway. We could perhaps do something similar to notes: add a mimeType and an url field.

In other words: I don't see the upside of using GeoJSON or TopoJSON over Signal K, when it goes against the conventions of the spec so far.

@rob42
Copy link
Contributor Author

rob42 commented Jan 18, 2016

I dont much like the array solutions used in other formats. The definition of 2,3 or more fields is as loose as, and the problems of lat/lon, lon/lat and x/y make it very easy to get it wrong in calcs etc. But @tkurki is right we should re-use the geojson as a way to describe geo objects. Its ideal for describing lines, areas, polygons etc. Plus most geo tools and browser mapping can render them directly. So think of it as a geo-drawing format within signalk.

The main change I want to get merged here is moving the position object to the definitions, and re-using it throughout.

@fabdrol
Copy link
Member

fabdrol commented Jan 18, 2016

but, in that case, shouldn't we stay true to convention and define them as URLs; eg track "foo" is a geojson file located here: file://...foo.json.
That way, SK remains SK format (and we don't mix in another format). Mixed in the client would have to "extract" the geojson bits anyway.

Re position in definitions: +1

@tkurki
Copy link
Member

tkurki commented Jan 24, 2016

@fabdrol Json schema is built on top of the idea of being able to include / reference different schemas so technically it is possible to specify that a certain part of the document uses a separate schema. In fact the SK schema itself already uses this technique.

There is no reason to keep things so that "SK remains SK format". It may pose inconsistencies in notation, but I don't there is intrinsic value in keeping SK separate.

That said it may become unwieldy to keep everything in one "document". The idea that you can retrieve all the data the server has knowledge from the root will become impractical at the latest with the advent of the first medium scale (think large regattas size) SK compliant sharing service.

@fabdrol
Copy link
Member

fabdrol commented Jan 24, 2016

@tkurki good point. I suppose my objection (that this creates confusion as we now have two "types" of position - and that I would prefer a solution where the GeoJSON is referenced but not included - similar to how "notes" work) is more of a personal than an objective argument - so I'll stay out of this one. Maybe someone else can comment (@canboat, @timmathews, @sumps)?

Regarding the size of the tree: I think that's a different problem, but in such a case people will need a good interface of browsing the data anyway. We should try and get a foot in the door with the next Volvo Ocean Race to install a SK server in their HQ to test this.

rob42 added a commit that referenced this pull request Jan 31, 2016
Master waypoints ref

Merging, as there are no major objections and we need to proceeed with the position object in the definitions so it can be used elsewhere by $ref.
@fabdrol I agree that long geojson should be stored separately and the url provided instead. I currently do this internally in the java server for tracks i think. Basically the incoming is examined for large objects in certain keys, and these are saved out to files. the file uri's are placed in the objects instead. The reverse happens on export. 

I will look into sending the url instead.
@rob42 rob42 merged commit ff96905 into master Jan 31, 2016
@rob42
Copy link
Contributor Author

rob42 commented Jan 31, 2016

Merging, as there are no major objections and we need to proceeed with the position object in the definitions so it can be used elsewhere by $ref.
@fabdrol I agree that long geojson should be stored separately and the url provided instead. I currently do this internally in the java server for tracks i think. Basically the incoming is examined for large objects in certain keys, and these are saved out to files. the file uri's are placed in the objects instead. The reverse happens on export.

I will look into sending the url instead.

@timmathews timmathews deleted the master-waypoints-ref branch May 18, 2016 14:58
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.

None yet

3 participants