SignalK / specification

Signal K is a JSON-based format for storing and sharing marine data from different sources (e.g. nmea 0183, 2000, seatalk, etc)
Other
91 stars 68 forks source link

docs: fix 'context' in Subscription Protocol examples #487

Closed bkp7 closed 5 years ago

bkp7 commented 6 years ago

Examples in the documentation showed vessels.self in the context section, whereas self includes the vessels (or aton, etc) part. (vessels.self would expand to vessels.vessels.urn:mrn...)

Just to clarify, http://<<IP>>/signalk/v1/api/self returns something like "vessels.urn:mrn:signalk:uuid:490be140-16f7-48bc-9c17-05ca7efbb745",

the websockets hello has:

{
  ...
  "self": "vessels.urn:mrn:signalk:uuid:490be140-16f7-48bc-9c17-05ca7efbb745",
}

The above examples are taken from node-server.

Also checked node-server to ensure that the subscription messages as modified here, work correctly.

fabdrol commented 6 years ago

This change to docs makes sense judging by the behaviour, provided the behaviour is right. @tkurki is this how it should be? Was there a issue/reason/discussion behind that change to server? Because to me it feels like we're doing this the wrong way round (server change -> docs changes).

Btw, I don't see any reason not to merge this, if node-server is behaving as intended.

tkurki commented 6 years ago

This change is not correct and confuses two things: the value of the self property (as reported in the hello message) and using vessels.self as a shorthand for referring to the self vessel, as in subscribe and rest path /signalk/v1/api/vessels/self .

fabdrol commented 6 years ago

@tkurki okay, so the current server implementation is wrong then?

tkurki commented 6 years ago

No errors, just misunderstanding.

http://node-master.signalk.org/signalk/v1/api/self returns self property value.

http://node-master.signalk.org/signalk/v1/api/vessels/self Returns own vessel’s full data, same as http://node-master.signalk.org/signalk/v1/api/vessels/urn:mrn:signalk:uuid:c0d79334-4e25-4245-8892-54e8ccc8021d

bkp7 commented 6 years ago

Sorry, I assumed that http://node-master.signalk.org/signalk/v1/api/vessels/self was a hangover in node-server from when self had to be a vessel.

So, in order to clarify the expected behaviour: For streaming

{
  "context": "vessels.self",
  "subscribe": [{"path": "*"}]
}

and

{
  "context": "self",
  "subscribe": [{"path": "*"}]
}

are equivalent as long as self is a vessel (node-server currently does this). Presumably if self is not a vessel the former should produce no results.

Similarly on the API http://node-master.signalk.org/signalk/v1/api/vessels/self should produce nothing if self is not a vessel.

I can't see the use case for vessels.self over self as I would have thought a client device using self would want results whatever type of 'thing' self is. Perhaps backward compatibility? Either way, if we can confirm that this is the intended behaviour I shall document accordingly.

tkurki commented 5 years ago

I think you have it right, including the fact that vessels.self is a leftover, but to remain backwards compatible we should keep it.

fabdrol commented 5 years ago

@bkp7 @tkurki should there be more work on this, is this irrelevant or should we go ahead and merge?

fabdrol commented 5 years ago

I'm closing this as this was a misunderstanding (just read through the thread again)