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

Introduction of raw TCP rules #286

Closed
wants to merge 20 commits into from
Closed

Introduction of raw TCP rules #286

wants to merge 20 commits into from

Conversation

pod909
Copy link

@pod909 pod909 commented Nov 5, 2016

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

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:
Copy link
Contributor

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), '{').
Copy link
Contributor

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.

Copy link
Author

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:
Copy link
Contributor

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

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Author

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:
Copy link
Contributor

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.

Copy link
Author

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

Copy link
Author

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

@tkurki
Copy link
Member

tkurki commented Nov 5, 2016

Terminology: is the word raw correct in this context? Raw socket is not a TCP socket: https://en.wikipedia.org/wiki/Raw_socket. Is raw TCP socket an oxymoron?

@tkurki
Copy link
Member

tkurki commented Nov 5, 2016

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.

@tkurki
Copy link
Member

tkurki commented Nov 5, 2016

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.

@tkurki
Copy link
Member

tkurki commented Nov 5, 2016

Formating:

I always write Signal K as either

  • capitalised and separate: Signal K
  • abbreviated: SK

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"
Copy link
Member

Choose a reason for hiding this comment

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

Missing trailing comma

Copy link
Author

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.
Copy link
Member

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.

Copy link
Author

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
Copy link
Member

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?

Copy link
Author

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.
Copy link
Member

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.

Copy link
Author

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.

Copy link
Author

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.

@jboynes
Copy link
Contributor

jboynes commented Nov 5, 2016

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.

@jboynes
Copy link
Contributor

jboynes commented Nov 5, 2016

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.

@tkurki
Copy link
Member

tkurki commented Nov 5, 2016

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.

@jboynes
Copy link
Contributor

jboynes commented Nov 5, 2016

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.

@pod909
Copy link
Author

pod909 commented Nov 5, 2016

That's beyond what is currently specified for the streaming interface and a
more complex issue than the scope of this ticket. Currently the stream is
defined as a pull interface with the source device is defined as a server;
with the node that wants the information connecting to it and receiving
deltas from it.

This ticket is concerned with the basics of using a raw TCP socket as a
transport instead of a websocket. Pushing deltas from one node to another
is a bigger subject and a separate issue should be raised. A proper
discussion would be needed on managing subscriptions etc. and there are
existing protocols to deal with that sort of thing.

e.g. How does a node know another node actually wants to be sent the info
it can produce?

...

The existence of the timestamp in the hello addresses a separate concern I
had with identifying the age of deltas sent by a node that doesn't have an
RTC.

When an SK aggregator connects to a node to get sk-deltas it can compare the
hello timestamp to it's RTC time at the point the hello is received. That
delta can then be added to the timestamp in subsequent sk-deltas to rebase
line the sk-deltas to the local time on the aggregator. The aggregator should
not blindly apply a timestamp based on the time of receipt. That would be
highly inaccurate for trivial examples like boat speed from a paddle wheel
and apparent wind from a mast head unit.

On 5 November 2016 at 20:37, Jeremy Boynes [email protected] wrote:

@jboynes commented on this pull request.

In gitbook-docs/urls_etc.md
#286:

"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.
+
+"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:

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.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#286, or mute the thread
https://github.com/notifications/unsubscribe-auth/AF0KF_SKqL519LuypdSAi3wtfXRS92H7ks5q7OjzgaJpZM4KqOUb
.

@pod909
Copy link
Author

pod909 commented Nov 5, 2016

One mans unnecessary is another mans necessary. Supporting multiple
protocols on a single socket vs writing multi threaded firmware to provide
separate ports for different protocols....

On 5 November 2016 at 21:33, Jeremy Boynes [email protected] wrote:

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.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#286 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF0KF1KW653xlFPwwO0nJvDj_7CNqFn3ks5q7PYegaJpZM4KqOUb
.

@pod909
Copy link
Author

pod909 commented Nov 5, 2016

Happy to change the delimiter to }<>{ if agreed on

@tkurki
Copy link
Member

tkurki commented Nov 6, 2016

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.
Copy link
Contributor

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.

Copy link
Author

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), '}').
Copy link
Contributor

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), '{').
Copy link
Contributor

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": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo?

Copy link
Author

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/recieved/received/

Copy link
Author

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.
Copy link
Contributor

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/

Copy link
Author

Choose a reason for hiding this comment

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

corrected

@jboynes
Copy link
Contributor

jboynes commented Nov 20, 2016

With a WS connection, the client can specify what messages it wishes to be sent in the WS URL using the subscribe query parameter. No similar mechanism exists when opening a simple TCP connection. What is the default behaviour? For example,

  • all changes
  • everything applying to vessels.{vesselId}
  • no changes until a subscription message is sent upstream
  • something else?

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.

@pod909
Copy link
Author

pod909 commented Nov 20, 2016

@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.

@sumps
Copy link
Contributor

sumps commented Nov 20, 2016

@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.

@sumps
Copy link
Contributor

sumps commented Nov 20, 2016

I would also request that a Hello message is not mandated but recommended as it is only possible with TCP and not UDP.

@pod909
Copy link
Author

pod909 commented Nov 20, 2016

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.

@jboynes
Copy link
Contributor

jboynes commented Nov 20, 2016

@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

@jboynes
Copy link
Contributor

jboynes commented Nov 21, 2016

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).

@sumps
Copy link
Contributor

sumps commented Nov 21, 2016

@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.

@pod909
Copy link
Author

pod909 commented Nov 21, 2016

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.

@pod909
Copy link
Author

pod909 commented Nov 21, 2016

@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.

@sumps
Copy link
Contributor

sumps commented Nov 21, 2016

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.

@pod909
Copy link
Author

pod909 commented Nov 21, 2016

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.

@sumps
Copy link
Contributor

sumps commented Nov 21, 2016

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.

@tkurki
Copy link
Member

tkurki commented Nov 21, 2016

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.

@jboynes
Copy link
Contributor

jboynes commented Nov 21, 2016

@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.

@pod909
Copy link
Author

pod909 commented Nov 21, 2016

@tkurki is it the node that is due to send or receive the deltas that's supposed to send the hello??

@tkurki
Copy link
Member

tkurki commented Nov 21, 2016

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

  • a display connects to the boat's SK server and starts streaming updates that the server receives from different inputs
  • a sensor connects to the boat's SK server and starts streaming updates to the server, doesn't care about the server's data (so maybe would like to tell the server that)

Both of these case assume a "master" server on the boat.

@pod909
Copy link
Author

pod909 commented Nov 22, 2016

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:

  1. The sensor is a TCP server and the Signal K Server will connect to it as a TCP client to pull messages from it (this is the model described by the WS standard at the moment)
  2. The sensor is a TCP client and connects to a TCP server at the Signal K Server in order to push messages to it.
  3. Bi directional traffic

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.

@pod909
Copy link
Author

pod909 commented Nov 22, 2016

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.

@pod909
Copy link
Author

pod909 commented Nov 22, 2016

I think it would also be wise to retract this PR and document a proposal separate from the current WS standard.

@timmathews
Copy link
Member

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:

  • client: a client is a device, program, etc. which initiates a connection to a server. Clients do not expose a signal k service on a well defined port, but rather open an ephemeral port for the duration of the connection
  • server: a server is a device, program, etc. which responds to a connection request, and more importantly has a signal k process listening on a well defined port

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.

@pod909
Copy link
Author

pod909 commented Nov 22, 2016

No problem Tim. Agree and will put forward an RTC for a consistent approach
to streamed updates based on the discussion here.

I'm going to leave a number of options open on the basis that adoption may
be the best arbiter.

I'll also commit to produce an MIT licenced reference sensor implementation
with an Arduino HAL. Assistance most welcome.

On Nov 22, 2016 12:44, "Tim Mathews" [email protected] wrote:

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:

  • client: a client is a device, program, etc. which initiates a
    connection to a server. Clients do not expose a signal k service on a well
    defined port, but rather open an ephemeral port for the duration of the
    connection
  • server: a server is a device, program, etc. which responds to a
    connection request, and more importantly has a signal k process listening
    on a well defined port

This is all TCP/IP 101, and @tkurki https://github.com/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.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#286 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF0KF8A6oiSJ7nKy6b9d0WkfQ8BUgC2Gks5rAuOqgaJpZM4KqOUb
.

@pod909
Copy link
Author

pod909 commented Nov 25, 2016

RFC 0005 created

@pod909 pod909 closed this Nov 25, 2016
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.

5 participants