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 69 forks source link

Add Documentation for Sources #432

Closed timmathews closed 6 years ago

timmathews commented 6 years ago

Work in progress:

timmathews commented 6 years ago

There are some things I'm unclear on w.r.t. sources.

  1. The src property is specified as a string. At this point, it's an historical artifact and shouldn't change, but it would be useful to include a sentence explaining why it's a string. I don't have a good example or reason.
  2. The properties for source included directly within a data object are different than the properties for a source when it exists under sources. Is this intentional? If so, why?
  3. Lines 82-88 of sources.json. Why is this block here and what does it do? It serves no purpose I can see.
  4. Why are the hardware informational props, e.g. softwareVersion, hardwareVersion, manufacturerInfo direct children of the "bus" identifier in sources? Shouldn't they be one level down? Specifically, the path to manufacturerInfo in the schema as defined is sources.ttyUSB0.manufacturerInfo when it seems like it really should be sources.ttyUSB0.35.manufacturerInfo.
bkp7 commented 6 years ago

@timmathews I did a PR #440 to the sources branch to make your pointer example schema compliant. You can test it here.

Your 'Direct Inclusion Method' cannot comply with the schema because $source is a required property and source is not an allowed property. I don't know whether it's the schema or your example which is wrong but on checking back the history of the schema it's been like this for over 2 years.

If the src property is not required for N2K sources, presumably by the same logic talker is no longer required for NMEA0183?

As regards Lines 82-88 the current effect is actually to render Lines 12-81 completely useless such that they can be removed from the schema without changing it. This is clearly not the intent! I assume lines 82-88 are there to accommodate sources other than NMEA0183/2000, eg. I2C, 1-wire, etc. If that is the case the Schema needs to be modified to reflect this.

It would be useful to explain/understand how sources covers multiple vessels, or whether sources can only ever relate to 'self'. Also, how are multiple SK servers on one vessel dealt with? eg. if there is more than one master plus a slave, how does the sources section identify each. Your explanation has an interface (ttyUSB0) at the top level without identifying what it is an interface of.

timmathews commented 6 years ago

Your 'Direct Inclusion Method' cannot comply with the schema because $source is a required property and source is not an allowed property.

It looks like the direct inclusion method first appears in commit 29fdc4872 from @rob42 last June. Rob will need to weigh in here, but I think the reason was to align with the delta format. Delta is defined to allow either source or $source.

It would be useful to explain/understand how sources covers multiple vessels, or whether sources can only ever relate to 'self'.

Sources relate to the server you're connected to. If you had two servers connected together via Ethernet, instead of "ttyUSB0" you'd probably have the peer server's hostname or IP address. I think the same would apply if you were connected to another boat via WiFi or the Internet and receiving data from it. I'll add an example to the docs.

tkurki commented 6 years ago

The "empty subject" on lines 82-88 is there to accommodate the situation where the source object in delta has the instance field and results in an extra level in the $source value.

The delta in test https://github.com/SignalK/specification/blob/master/test/sources.js#L89 results in full like this:

{
  "vessels": {
    "urn:mrn:imo:mmsi:200000000": {
      "mmsi": "200000000",
      "environment": {
        "inside": {
          "engineRoom": {
            "temperature": {
              "meta": {
                "units": "K",
                "description": "Temperature"
              },
              "value": 70,
              "$source": "aLabel.41.5",   <--------- label.src.instance
              "timestamp": "2015-01-15T16:15:18.136Z",
              "pgn": 130312
            }
          }
        }
      }
    }
  },
  "self": "vessels.urn:mrn:imo:mmsi:200000000",
  "version": "0.1.0",
  "sources": {
    "aLabel": {
      "41": {
        "5": {},  <----------- corresponding placeholder object, could hold a human oriented label
        "n2k": {
          "src": "41",
          "pgns": {
            "130312": "2015-01-15T16:15:18.136Z"
          }
        }
      },
      "label": "aLabel",
      "type": "NMEA2000"
    }
  }
}

Looked odd to me as well, but then I removed it and found out the failing test.

tkurki commented 6 years ago

The properties for source included directly within a data object are different than the properties for a source when it exists under sources. Is this intentional? If so, why?

The structures under /sources are meant for holding metadata for sources. The source structure in the data object is there to identify the source of the data.

tkurki commented 6 years ago

I think the hardware informational props were mistakenly lifted in ed7b75ce8559ff63aa90c5be4fc6399d4681bc88 and should be fixed.

tkurki commented 6 years ago

The src property is specified as a string. At this point, it's an historical artifact and shouldn't change, but it would be useful to include a sentence explaining why it's a string. I don't have a good example or reason.

Me neither.

bkp7 commented 6 years ago

@timmathews, on the assumption that the delta version is defined correctly I've put together a delta message which uses a comprehensive selection of sources including NMEA2000, NMEA0183, OneNet, NMEA0183 sample(file) and another SignalK server. I have used realistic values throughout including (I hope) matching PGNs and Signal K paths. Putting aside the SignalK server on eth0, it all seems to tie in with the documentation and would result in $source consisting of 4 or 5 levels depending on the type and whether or not there is an instance per @tkurki 's comment.

{ "context": "vessels.urn:mrn:imo:mmsi:299999999",
  "updates": [
    { "source": {
        "label": "CANBUS-1",
        "type": "NMEA2000",
        "src": "36",
        "pgn": 130316,
        "instance": "1"
      },
      "values": [{
        "path": "environment.inside.engineRoom.temperature",
        "value": 325.0
      }]
    },
    { "source": {
        "label": "RS422-1",
        "type": "NMEA0183",
        "talker": "GP",
        "sentence": "RMC"
      },
      "values": [{
        "path": "navigation.speedOverGround",
        "value": 4.23
      }]
    },
    { "source": {
        "label": "eth1",
        "type": "OneNet",
        "src": "2001:0db8:85a3:0000:0000:8a2e:0370:7334",
        "pgn": 130316,
        "instance": "7"
      },
      "values": [{
        "path": "environment.inside.aftCabin.temperature",
        "value": 301.25
      }]
    },
    { "source": {
        "label": "nmea0183-sample-data",
        "type": "NMEA0183",
        "talker": "II",
        "sentence": "VPW"
      },
      "values": [{
        "path": "performance.velocityMadeGood",
        "value": 3.0660896656316043
      }]
    },
    { "source": {
        "label": "eth0",
        "type": "signalk",
        "src": "<192.168.1.5>.vessels.urn:mrn:imo:mmsi:299999999.RS422-1.NMEA0183.HC.HDM"
      },
      "values": [{
        "path": "navigation.headingMagnetic",
        "value": 0.34
      }]
    },
    { "source": {
        "label": "eth0",
        "type": "signalk",
        "src": "<192.168.1.5>.vessels.urn:mrn:imo:mmsi:299999999.CANBUS-1.NMEA2000.21.127250"
      },
      "values": [{
        "path": "navigation.headingMagnetic",
        "value": 0.335
      }]
}]}

As regards the eth0 interface which is receiving data from another Signal K (slave) server I could not find any examples. I have put something in there but clearly using an IP address with dot notation will be a problem. Maybe the port should be in there too? "src" is described in the Schema as "NMEA2000 src value or any similar value for encapsulating the original source of the data". There is also the issue of what the source would look like for this data when served by an upstream server amalgamating multiple vessel data... Perhaps it's envisaged that the Signal K slave is meant to give the upstream server an instance number and keep the lower level source info to itself?

The delta being sent from the slave server in my example would look like:

{ "context": "vessels.urn:mrn:imo:mmsi:299999999",
  "updates": [
    { "source": {
        "label": "RS422-1",
        "type": "NMEA0183",
        "talker": "HC",
        "sentence": "HDM"
      },
      "values": [{
        "path": "navigation.headingMagnetic",
        "value": 0.34
      }]
    },
    { "source": {
        "label": "CANBUS-1",
        "type": "NMEA2000",
        "src": "21",
        "pgn": 127250
      },
      "values": [{
        "path": "navigation.headingMagnetic",
        "value": 0.335
      }]
}]}

If we can get consensus on how the data above should look, I propose a creating matching message in full format using both $source and source if that's correct. Then we can get the schema and documentation updated accordingly.

timmathews commented 6 years ago

@bkp7, your (very comprehensive) example seems to cover all the bases. I'm not sure if we should include the instance property at this point or not. @tkurki, @sbender9 does the node server populate that prop? The example deltas I saw from demo.wilhelmsk.com don't have it and demo.signalk.org is configured with an NMEA 0183 log.

I've updated the docs to make it clear that the source prop only exists in deltas. The only thing left is documenting the structure of the sources group. Almost there ...

sbender9 commented 6 years ago

I think instance would be difficult, I don’t think it should be included. It’s pgn specfic.

timmathews commented 6 years ago

I fell down a bit of a rabbit hole this evening trying to remember the instance stuff. I’m not sure it’s still necessary given that we incorporate instance into the path most (all?) of the time now.

Originally, it was thought that we would have a single propulsion.engine.* and multiple values. In that case, you would need to look at the source and it’s instance to know if the reported value was for the port or starboard engine. Likewise for batteries and things. I’m not sure that that approach is used anywhere in the schema anymore, and if it’s not then we don’t need this instance prop any more.

In the current model whenever a path has multiple values, that means multiple sensors measuring the same data (multiple GPS receivers or multiple depth sounders for instance).

Is that mostly correct?

tkurki commented 6 years ago

Instance is still used on its own in handling temperature instances: pgn 130312 handled by n2kMapper, in the schema and schema test, handled when mapping deltas to full with test.

If I recall correctly: Even if the values are handled per multiple sources logic the instance is needed to track down the exact source of the data.

bkp7 commented 6 years ago

As I understand it the instance is required, that is why I picked PGN 130316 for my example above.

There is not a one to one relationship between instance (per NMEA) and the Signal K Branch ie. it is possible to have a device with 2 senors (for redundancy) measuring the same thing, eg. instance 7 and 8 on the same N2K device both measuring environment.inside.aftCabin.temperature. They would appear as 2 values within signal K and you'd need the instance to differentiate their source.

timmathews commented 6 years ago

My concern is that we don't handle instance consistently.

There are at least 28 PGNs with an instance field. n2k-signalk supports eight of these.

127245 - Rudder: instance is ignored 127488 - Engine params, rapid update: propulsion.port or propulsion.starboard; 0 = port, 1 = starboard, other instances ignored 127489 - Engine params, dynamic: propulsion.port or propulsion.starboard; 0 = port, any other instance = starboard 127505 - Fluid level: instance in path: tanks.<<type>>.<<instance>> 127506 - DC detailed status: instance in path: electrical.batteries.<<instance>> 127508 - Battery status: electrical.batteries.<<instance>> 130312 - Temperature: it's complicated, but we split sources into inside and outside groups, and instance doesn't end up in the path 130314 - Pressure: instance is ignored

As far as I can tell, the only time we populate instance under sources is for 130312 (and the Lowrance proprietary version). In every other case where the source PGN has an instance field, we put the instance in the path or ignore it altogether.

Why have this complicated special case for one type of data?

timmathews commented 6 years ago

That said, I am neither suggesting or implying that we rework temperatures at this point. However, I would like to consider officially deprecating this method of handling instances and call it out in the documentation.

That way, as we continue to add support for instanced data, the instance is included in the path.

timmathews commented 6 years ago

I've added a paragraph with a rough description of the instance field. I'd like to merge this and get it published ahead of the 1.0 announcement. Thoughts?

timmathews commented 6 years ago

I've also opened #458 to fix the source props that got misplaced.

tkurki commented 6 years ago

Nice addition for the pesky instance field!

+1 for merging. Some conflicts though and the title is still [WIP].

bkp7 commented 6 years ago

The source is required in the PGN examples given:

The n2k-signalk is just one implementation of the Signal K standard.

As an example the standard requires that the engine identifier be a "Human readable label for the propulsion unit". On vessels with more than 2 engines this would have to include something other than just "port" or "starboard". The source is still required at the lower level to identify the N2K source. Also there could be a different source# for the same engine, and ideally these would be grouped in under the same path.

My understanding is that the n2k-signalk project uses the instance in the path rather than a human readable description as a simplified implementation which will only work for implementations with a maximum of 2 engines.

bkp7 commented 6 years ago

There is still no example for a slave SignalK server as a source. Without this there is no way to know how to implement native SignalK devices, nor is there any way to know how higher level consolidating SignalK servers should be implemented.