-
Notifications
You must be signed in to change notification settings - Fork 69
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
Introduction of raw TCP rules #286
Conversation
Added service type for raw TCP socket
added end point detail for tcp
added service type for a raw TCP socket
added instructions on streaming signal-delta of a raw TCP
added description of hello message added source to hello message added use of the timestamp by the client to create a delta to be applied to other messages
1. The first character of each signalk-delta message MUST be the openning curly bracke (char(123), '{'). | ||
2. Each signalk-delta message MUST be terminated by a carriage return and new line (char(10) + char(13), '\r\n') sent after the final closing curly bracket (char(125), '}'). | ||
3. The server SHOULD send signalk-delta in a compact manner with any unnessesary white space removed. | ||
4. The server MAY also send sentences for other protocals (e.g. NMEA0183) interleaved with the signalk-delta messages provided that sentences of the other protocal: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixing messages from multiple protocols adds a lot of ambiguity to the format and I'm not clear on the benefit.
Can we make this just a stream of JSON objects separated by /r/n where each object is a signalk-delta.
|
||
When signalk-delta is streamed over a raw TCP socket: | ||
|
||
1. The first character of each signalk-delta message MUST be the openning curly bracke (char(123), '{'). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: opening curly bracket
The wording here is a bit ambiguous on whether the JSON object is surrounded by {} or not. Suggest:
The stream MUST comprise a set of compact JSON objects separated by a carriage return, new line sequence (char(13) followed by char(10), "\r\n"). Each JSON object SHOULD conform to the JSON schema for a signalk-delta message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo corrected
|
||
"version" - The 'hello' message MUST contain a "version" element which MUST have a string value identifying the lowest version of signalk-delta that defines the format of the messages that will be sent by the server. | ||
|
||
"source" - The 'hello' message MUST contain a "source" element which MUST have a string value that identifies the server as follows: manufacturer/publisher.device/application.uuid, where: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this description of the string format confusing - is the "/" part of the syntax or not? Suggest we just make this property conform to the schema for /defintions/source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the same source as in definitions.
While I think that adding something like a source
property may be a good idea I don't think it should be added as part of addition of TCP streaming, as it is not coupled to this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The source of the data here is the SK server itself. The type in definitions can represent that, I don't think we need/should add anything new.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be a blt clearer, the 'hello' message comes from whatever is initiating the SK stream. This could be a client sending to a server, or a server responding to a client. In either case, the sender wants to identify themselves as the source of the information which is what the type in /definitions is for.
For example, a battery monitoring device discovers a SK server on its network, connects, sends a 'hello' identifying itself and then starts sending readings. It doesn't have a RTC so it sends a base timestamp of '0' and then time deltas in ms with the 'update' messages. The SK server includes information from the source value in its model and in delta messages it sends out.
A client discovers the SK server, connects to it and subscribes to a stream. The SK server identifies itself in the 'hello' and then forwards updates as it receives messages over SK, N2K or '183. The client uses the source information from the battery monitor to distinguish updates for the house bank from those from the engine bank.
A intermediate SK server can act as a forwarding proxy for SK delta without needing be configured with what every device it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source removed form this update
Will raise a separate issue as I view this as vital.
|
||
Upon connection a 'hello' message is sent as follows: | ||
Upon accepting a connection a server MUST send a 'hello' message before any other communicaiton is sent. The format of the 'hello' message is as follows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works when there is specific connection setup (e.g. after opening a TCP connection) but won't allow a listener to tap into a stream in progress (e.g. if they start listening to a RS422 bus or a multicast session).
Instead, suppose we add a sync message to the delta stream where the server sends all or part of the signalk tree. A minimal set would be very similar to this but by sending the tree the server can provide the context to which subsequent deltas would apply and re-synchronize clients that are new listeners or whose version of the model may have drifted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Support of serial connections needs separate consideration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most nodes dealing with signal-delta wont have the full tree
Terminology: is the word |
While it is good to consider similar transports we might want to concentrate on getting TCP done. We can add stuff like periodic sync messages separately. |
I first found the rule about beginning the line with { a bit odd but on second thought it sounded like a good idea. With very little effort we can add support for coexistence on the same port with NMEA0183 data. The fact that the payload is Signal K delta in JSON format already states that SK data is in JSON format. However many JSON parsers allow whitespace before the first opening curly brace (just tried JS and Python) and in my opinion stating that the first character must be the opning curly brace and one can tell if it is NMEA0183 or SK delta from that is a good idea. |
Formating: I always write Signal K as either
Unless there are good reasons for not doing so I think we should use the Signal K format in the documentation. |
@@ -55,6 +57,7 @@ Using the information above a web client or http capable device can discover and | |||
"version": "1.1.2", | |||
"signalk-http": "http://192.168.1.2/signalk/v1/api/", | |||
"signalk-ws": "ws://192.168.1.2:34567/signalk/v1/stream" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing trailing comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected
"timestamp": "2015-04-13T01:13:50.524Z", | ||
"self": "123456789" | ||
} | ||
``` | ||
|
||
"version" - The 'hello' message MUST contain a "version" element which MUST have a string value identifying the lowest version of signalk-delta that defines the format of the messages that will be sent by the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As JSON is JavaScript Object Notation and JavaScript objects have properties it might be prudent to refer to properties
instaed of elements
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected
"version" - The 'hello' message MUST contain a "version" element which MUST have a string value identifying the lowest version of signalk-delta that defines the format of the messages that will be sent by the server. | ||
|
||
"source" - The 'hello' message MUST contain a "source" element which MUST have a string value that identifies the server as follows: manufacturer/publisher.device/application.uuid, where: | ||
* maufacturer is the name of a manufacturer or publisher in the list maintained in this repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to start maintaining such a list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be no overhead to maintaining the list. Manufacturers/publishers would just add to it them selves under version control here. Documented as a JSON object it would provide a useful resource for validating messages.
* uuid is a uneque identifier attributed to the physical device or installation of the application by the manufacturer/publisher | ||
When a source is not provided in a signalk-delta message the client SHOULD take the source from the 'hello' as a default. | ||
|
||
"timestamp" - The 'hello' message SHOULD contain a "timestamp" element which MUST have a string value representing the date and time at the server that the 'hello' mesage was sent at. The client SHOULD compare the timestamp to the local time at the client at which the 'hello' is received and use the delta to correct the timestamp of subsequent signalk-delta messages recieved from the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format and UTC need to be specified.
Maybe the wording should be that The client MAY compare..., as this greatly depends on the client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which ISO standard is used else where and I'll add.
The timestamp cannot be restricted to UTC. For devices that do not have an RTC it needs to be left open as the local time maintained by the node. I think it's stronger than MAY. The chances of all the nodes in a network being time synchronized and there being not latency in the communication in all instances is slim. Nearly suggested is a MUST in fact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What ISo standard is used and I'll update.
UTC can't be mandated as some nodes -- in particular low function node from which values originate -- do not have an RTC. They should all be able to provide a relative time stamp/"local time at the node" for each sk-delta though. Nearly suggested the client making a timestamp adjustment as a MUST, so SHOULD is a reasonable compromise I think.
Mixing SK messages with others seems confusing. An existing '183 client may be confused by the unexpected SK data, especially where the JSON contains '183 sentences (e.g in source properties). I don't see the advantage for a new implementation of a client as the SK feed can represent more data anyway. If a client wants both SK data and a '183 stream it can simply open two connections and not worry about demultiplexing (IP does that for you). It's like mixing the raw binary output from an NGT-1 with '183 data: do-able but weird. |
On whitespace, it would be simpler to either allow it everywhere or ban it entirely. Allowing it makes things more readable and fits naturally with JSON. The CR-LF delimiter is not needed to parse a stream of objects as the token sequence "}" "{" can't legitimately appear inside an object. |
Anything but a simple delimiter that can not appear in valid JSON strings and we exlude from the message leads to unnecessary complexity. Like do you allow whitespace between "}" and "{"? What constitutes whitespace? Multiple whitespace chars? All this requires a considerably more complex state machine than a delimiter. Complex state machines lead to errors and differences in implementations. |
JSON parsers already have to handle whitespace, which is why I specifically said "token sequence" and not characters. Parsers for "concatenated JSON" already exist and handle this automatically. They tend to be event based, which would be needed on a small-footprint device anyway as it would not have the resources to buffer a potentially long line of text between CR/LF delimiters. In terms of avoiding unnecessary complexity, the simplest definition would be to define the byte stream to be a simple sequence of JSON objects encoded using UTF-8 and avoid all the new-line delimited, multi-protocol stuff. |
That's beyond what is currently specified for the streaming interface and a This ticket is concerned with the basics of using a raw TCP socket as a e.g. How does a node know another node actually wants to be sent the info ... The existence of the timestamp in the hello addresses a separate concern I When an SK aggregator connects to a node to get sk-deltas it can compare the On 5 November 2016 at 20:37, Jeremy Boynes [email protected] wrote:
|
One mans unnecessary is another mans necessary. Supporting multiple On 5 November 2016 at 21:33, Jeremy Boynes [email protected] wrote:
|
Happy to change the delimiter to }<>{ if agreed on |
While I really appreciate the discussion I really wish that people would have voiced their thoughts earlier in #147 which was raised already early this year. @jboynes I think your point about mutual / two way hello is valid, thanks for raising it. However that is not the most crucial point in TCP streaming and applies to WebSockets as well, so I think the source handling should be handled separately. |
2. The first character of each message SHOULD be the openning curly bracket (char(123), '{'). | ||
3. The message SHOULD be sent with any unnessesary white space removed. | ||
|
||
Note on hardware implementation: The serial APIs for ethernet and wifi modules often close a TCP or UDP packet on receipt of a new line ( char(13) ). Please check the data sheet and ensure the new line character is still sent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an example of this? That's not how TCP and UDP work normally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
When a hello or signalk-delta message is sent over a TCP socket: | ||
|
||
1. Each message MUST be terminated by a carriage return and new line (char(10) + char(13), '\r\n') sent immediatly after the final closing curly bracket (char(125), '}'). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/char(10) + char(13)/char(13) + char(10)/
When a hello or signalk-delta message is sent over a TCP socket: | ||
|
||
1. Each message MUST be terminated by a carriage return and new line (char(10) + char(13), '\r\n') sent immediatly after the final closing curly bracket (char(125), '}'). | ||
2. The first character of each message SHOULD be the openning curly bracket (char(123), '{'). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/openning/opening/
```json | ||
{ | ||
"endpoints": { | ||
"v1": { | ||
"endpoints": { "v1": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not part of this change so can't comment
|
||
"version" - The 'hello' message MUST contain a "version" property which MUST have a string value identifying the lowest version of signalk-delta that defines the format of the messages that will follow. | ||
|
||
"timestamp" - The 'hello' message SHOULD contain a "timestamp" property which MUST have a string value representing the date and time at the node that sent the 'hello' mesage. The timestamp SHOULD be synchronised with UTC. If the timestamp is not synchronised with UTC then the receiving node SHOULD compare the timestamp with the time at the receiving node at which the 'hello' is received and use the delta to correct the timestamp of subsequent signalk-delta messages recieved from the sending node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/recieved/received/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected (thanks for the review)
|
||
"timestamp" - The 'hello' message SHOULD contain a "timestamp" property which MUST have a string value representing the date and time at the node that sent the 'hello' mesage. The timestamp SHOULD be synchronised with UTC. If the timestamp is not synchronised with UTC then the receiving node SHOULD compare the timestamp with the time at the receiving node at which the 'hello' is received and use the delta to correct the timestamp of subsequent signalk-delta messages recieved from the sending node. | ||
|
||
"self" - The 'hello' massege MAY contain a "self" property which MUST have a string value that identifies the vessle the message pertains to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/massege/message/
s/vessle/vessel/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected
With a WS connection, the client can specify what messages it wishes to be sent in the WS URL using the
I would recommend "no changes until subscription" as that prevents the risk of flooding the TCP connection with unneeded data, especially when bandwidth or client resources may be limited. Note this is different to the WS protocol which defaults to "everything for the vessel" but which avoids the problem by allowing the client to specify the initial subscription. |
@jboynes For this change "none of the above". No mechanism is suggested for deciding what should be sent as part of this change. As a result the receiver must handle anything the sender sends. That may be values across the whole domain, it may not. Certainly worthy of a separate discussion. I'm not sure a straight TCP connection is the best option if throttling is required or if its best done at the messaging level if it is. |
@jboynes no confusion on my side, I am talking about a specific but very common use case where a serial data stream is converted to TCP or UDP packets and the line feed character is used to tell the adaptor to close and transmit the network packet. If you use the other two mechanisms; number of characters or time period you run the risk of sentences or massages being spread across multiple packets and in the case of UDP packets can (and I have send this first hand) be received out of order with bad results. |
I would also request that a Hello message is not mandated but recommended as it is only possible with TCP and not UDP. |
Could get into a discussion on "sends" vs "receives", but for now I've added clarification that the hello is only mandated for WS and TCP. UDP isn't covered in this update. It probably needs separate discussion to make sure everything is captured before it's included. |
@sumps That was what I meant by "serial port emulators" that are trying to mimic the behaviour of a TTY (such as waiting for a CR to denote end-of-user-input or to scroll a display up) on top of a data stream. At the TCP or UDP level none of that applies. When send(buffer) is called the data will be sent[1], irrespective of any newline characters in the buffer. TCP guarantees the bytes in the stream will be delivered to the other end, reliably, and in order, no matter how the stream gets fragmented into packets for transmission. UDP is a datagram protocol and not relevant to this PR, see #289. UDP datagrams do get fragmented at the IP or Ethernet layer but UDP guarantees that its datagram gets reassembled correctly. It does not guarantee that it will be delivered, or the order in which different datagrams get delivered. I think crux of the matter is whether we are talking about a TTY-style protocol here or raw TCP. [1] Subject to TCP_NODELAY and other socket options but that's not particularly relevant to how TCP does not care about newlines in data |
I hacked out a prototype using the low-level socket interfaces which shows that the newline stuff is not needed. Tracing with Wireshark & netcat shows the TCP segments being sent when the application calls write() even though there are no newlines. IOW, the concerns about needing newlines to make TCP work as expected are not valid. I think everyone here thinks the sender should send out compact messages separated by newlines to make the raw stream easier for humans to read e.g. with netcat. I think the difference is I that I think receivers should not be able to rely on that (with the implication that senders must add newlines and not emit whitespace). I think there's more flexibility if receivers must only rely on receiving a stream of JSON objects with no special handling for whitespace (per the JSON spec). |
@jboynes we are still not on the same page and perhaps I have not made myself clear. Use case - a simple and cheap SK sensor outputs SK over serial with a checksum and \r\n end of message characters. Then someone wants to connect the serial data to a serial to ethernet adaptor (wired or wireless) and what I want to make sure of is that we do not make this impossible by insisting on a mandatory "Hello" message and not allowing a TCP or UDP SK message to have a checksum or \r\n characters. |
The hello is currently mandatory so the question @tkurki is why? The only mandatory content is the version. Checksum. Makes it easier for the dumb bridge but more complicated for what ever receives the delta from TCP/WS as it's not going to get valid JSON anymore. The \r\n is harmless as it's just seen as white space. If we have to allow for a checksum everywhere to cater for this then I'm definitely on the side of a decimal. |
@sumps not sure that's actually a valid use case. A device that simple could well end up putting rubbish onto TCP/WS. It needs as a minium to check the checksum, why bother with it otherwise. |
This is a very common use case, with most of the current generation of wireless nmea devices using this approach. The DIY user would be even more likely to do this as you do not have to write any code, just buy a module that does the serial to ethernet conversion. |
Well "you might be getting crap" is a bit of a default position with NMEA0183 and the checksum was designed in with it being a serial protocol in essence. Not the same with here. Serial support is the edge case. Doing the checksum even remotely properly requires the messages to be buffered, so stripping the checksum is no effort compaired to the other work that should be done. Once it's on to TCP mandatory buffering and checksum validation at every step, just in case crap was sent out from a serial connection? That's a serious imposition. |
I don't want to get in to an arguement over the benefits of one solution over another, I am just highlighting a popular use case that allows for cost effective data transmission, that we should not make impossible by making certain things mandatory. |
Hello is mandatory in ws because we thought it would be a good idea to define a message that conveys some information about the server that the client is connecting to and that can be expected and is distinguishable from deltas. |
@sumps We have #287 to cover sending SK over serial which deals with the additional packetization and framing a low level data link like that needs. The serial-ethernet-converter application is about transporting that over an IP network and the applications should treat it as such; for example, it can't rely on TCP's guarantees as data can be lost on the serial segment. The case I think we are reviewing here is where the device talks TCP natively and hence is able to rely on the properties of a TCP connection in terms of reliability and ordering. That makes using the connection simpler as TCP is doing all the work of framing, checksumming and recovering from lost packets. As a DIY user it's easy and cheap to pick up a controller able to talk Ethernet or Wifi directly, with TCP support built in. For example, the Arduino EthernetClient makes it about as easy as using the Serial library. |
@tkurki is it the node that is due to send or receive the deltas that's supposed to send the hello?? |
I don't really get why you are trying to avoid the words server and client: the tcp connection is initiated by the client by connecting to the socket that the server is already listening to. There is clearly a server, that waits for incoming connections. The parties are not symmetrical in terms of the TCP connection, even if the tcp and ws connections are duplex. The server sends the hello and informs what SK version it uses. The client then has the option to disconnect, if it can not support the version that the server speaks. So far the usual arrangement has been that there is a server, that provides one or more vessel's data when connected to. I believe java server accepts deltas from a ws client, but I know node server doesn't. If needed that capability can be added easily. With the duplex capability we can devise different setups, like
Both of these case assume a "master" server on the boat. |
Everything gets flipped on its head when you talk about sensor providers and SK Server consumers, rather than SK Server providers and browser consumers. The following use cases have been suggested:
So some times clients are providers; some times servers are providers. I also wanted to avoid using the word "server" as the reference aggregator/broker has been named Signal K Server. I'd assumed (apologies) that the provider would send a "hello" and then the consumer would decide whether it wanted to read what followed or disconnect and refuse future connection if not. If the consumer found the provider through discovery it now knows not to bother connecting again. This only requires a mono directional connection and the "hello" can carry more interesting stuff like local timestamp and default context, device and source information. If the consumer says hello that assumes bi directional communication and requires the provider to handle receiving the "hello". ... There's also the issue whether data should be sent before a subscribe. For me the provider should declare that it requires a subscribe and the keys it can send in the hello. |
So... It looks like we have 2 different services here. _signalk_stream_serial._tcp which must satisfy the conditions for a serial connection with limitations on data integrity that consumers must cater for. And _signalk_stream_tcp._tcp which must satisfy the conditions here. In the absence of discovery a consumer will be able to differentiate between streets based on receipt of a hello as the first message. I'm also going to expand the hello to include a list of paths where a subscription request is required to start receiving a stream. |
I think it would also be wise to retract this PR and document a proposal separate from the current WS standard. |
We're definitely deep into RFC territory here. I agree that it makes sense to close this PR, write up the current state of the proposed TCP layer and submit that as an RFC. That said, for clarity of communication going forward, here are some definitions I think we can use:
This is all TCP/IP 101, and @tkurki said as much above, but I think it bears repeating. I do not see the value in having two different TCP/IP services, it will lead to confusion and compatibility issues down the road. As if having a WebSockets service, a TCP/IP service, a UDP/IP service, and a message broker (MQTT?) service isn't already fragmented enough. We need to pick one and move forward with it. |
No problem Tim. Agree and will put forward an RTC for a consistent approach I'm going to leave a number of options open on the basis that adoption may I'll also commit to produce an MIT licenced reference sensor implementation On Nov 22, 2016 12:44, "Tim Mathews" [email protected] wrote:
|
RFC 0005 created |
Added rules for supporting mixed message TCP sockets.
Each message must begin {
Each message must end \r\n
Added source to hello message
Added description of how to use hello timestamp to increase the accuracy of subsequent delta messages