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

Connection Hello response for streaming is not valid signalk json #473

Closed bkp7 closed 6 years ago

bkp7 commented 6 years ago

The specification requires a hello message to be sent from the server upon successful subscription to streaming. The example in the documentation being:

{
  "version": "1.1.2",
  "timestamp": "2015-04-13T01:13:50.524Z",
  "self": "123456789"
}

This is neither a valid full nor delta signalk json message.

Assuming it should remain and the production of a new schema is required for this:

rob42 commented 6 years ago

The original reason for a hello message was to provide a confirmation of connection. Some protocols require that a non blank message is sent after the initial connection is made ( eg UDP packets)

I think this evolved into the current format, which provides info on version, self identifier and a timestamp from the server which can be compared to the client to validate any timing offsets.

It should probably be a valid signalk message in its own right or merged into delta format?

There is another case that could be attached to this: sending auth id/pass, and resulting message tokens. We currently have no way to do that except via std http methods which dont work for clients without a browser (eg raw tcp or serial)

fabdrol commented 6 years ago

Given the use cases mentioned by @rob42, I'd say the best way forward is to write a spec for the hello message.. The hello message will probably become more useful once we have multiple major versions and master/slave servers communicating.

bkp7 commented 6 years ago

@rob42, @fabdrol, at present it's a simple message so the spec will be easy enough to do. Am I right in saying that all other messages being streamed via ws will be deltas? If so my thought was that it would be neat if the hello message was itself a valid (empty) delta.

It strikes me that version should be present in the delta message anyway as it is with full, perhaps optional now becoming mandatory in future? Adding optional self property to the delta definition seems to make sense and enables a client to know which vessel is self (if applicable). Is time needed at all? If it's being used to set synchronised ship wide time is this the right methodology?

Whether the hello is an empty delta or a separate definition, if you could let me know your thoughts on the above questions I will prepare a PR to implement.

tkurki commented 6 years ago

Is there a pressing reason for the proposed changes? These will break backwards compatibility.

Version in every single delta seems very superfluous. Once there are multiple versions we can add deltas for msgs whose version is other than the one in hello.

bkp7 commented 6 years ago

@tkurki, I'm not proposing this should break anything hence suggesting each one as optional.

The need to clarify the specification is pressing because at present there is no way to know from the spec which parts of this hello message are mandatory. In fact the wording doesn't make it clear if a hello message is mandatory at all (ie is sent should be changed to may/should or must). Also if time is mandatory it implies every server must have an RTC.

So the requirement is to clarify the spec, and at present I don't know what was actually intended....

tkurki commented 6 years ago

Clarification is definitely good, hello is underspecified.

But in addition you clearly suggested two breaking changes that can not be optional, because they would change the current hello and delta formats:

tkurki commented 6 years ago

A schema for hello would be good, as well as more accurate wording.

Hello is mandatory.

Version should refer to the Signal K spec & protocol version. Major SK version is implicit in http and ws in the path, but for other streaming protocols is valuable in band.

Node server currently responds with

{
    "name": "signalk-server",
    "version": "1.3.0",
    "timestamp": "2018-06-21T15:09:16.704Z",
    "self": "vessels.urn:mrn:signalk:uuid:c0d79334-4e25-4245-8892-54e8ccc8021d",
    "roles": [
        "master",
        "main"
    ]
}

where version needs fixing, it is server software version now.

Name is a human oriented informative name, currently not configurable but hardcoded in node server.

Roles refers to discussion in #99. I am not too happy about it: underspecified, muddy and lacks real world use cases and the implementations that I nowadays would really like to have.

rob42 commented 6 years ago

Does the server need a reliable timestamp?

No, but that has implications for clients and other servers.

My new artemis server uses a time-series db, which has interesting possibilities. but if I receive data and the timestamp is wrong that data may not be correctly retrieved. eg data with a timestamp of 1/1/1970 will go deep into history and be overridden by an entry from yesterday.

In signalk we have assumed that the data arrives in realtime (like all current nav systems). By including a timestamp we can sequence that data, even if its received out-of-order. Unexplored territory really, but I feel that sharing historic data will prove very powerful.

If the hello message tells me the other sources timestamp is 1/1/1970, I can adjust their data. (Definitely not a good way to sync time across the network, ntp is the tool for that)

bkp7 commented 6 years ago

@rob42 timestamp is optional so that a very simple server (eg. a fridge) without time source is allowed; it just sends out data in real time. You can assume in that case that the data is valid at the time of receipt. Where there is a time available it may be included. From a time-series db point of view you will in any case have to deal with clocks being out of sync at least to some degree. Servers which have GPS may have pretty accurate timestamp against position data but processing does take time so other data may have inaccurate timestamps. A RTC will not be perfect either eg 1ms per minute. I think you just need a bit of thought over when you trust the date stamp in preference to your own server's time.

As you say for synchronisation an ntp server driven from GNSS would be the way to go.

I think the rationale for passing the time at all is so that a low spec consumer or upstream server could use that to rebase a cold clock in the absence of an ntp server.

If there's an issue with times we should raise a separate issue. We could look at including info about a server's time source, hardware accuracy and last sync?