Open timmathews opened 8 years ago
I am very sorry but iKommunicate V1 firmware is being signed off tonight, we have units built and tested all over our production floor and they are shipping tomorrow. Here is a document that outlines how Sources are implemented in iKommunicate.
I think we need to firm up and document what we have now in Sources and look at any changes for a V1.x.
The document you linked to is more in line with what I've proposed above than what is currently in the spec. Existing schema allows for another level of hierarchy in sources to account for instance
. But that doesn't really make sense, so that's why I'm proposing adding instance
to source
under values and in deltas.
I don't think you can ship without addressing instance.
Thinking this through some more, Sources is device specific so that you can trace a particular Signal K message back to a specific device on the boat, while Instances are data specific i.e. this is data from Engine Instance 0 or this is data from Temperature Instance 2.
We are already creating Data objects based on the Instance i.e. propulsion.engine0.oilPressure and propulsion.engine1.oilPressure so we do not need to include this data in the source data of the Delta.
I have been looking over the current sources.json definition file, and have an observation. The 'properties' section is largely generic, strings for source ID, type, versions, etc. However there are three very protocol specific items - productFunction, productClass, and productCode.
Each of these three define their content as " NEMA2000 Product ...", and reflect specific J1939/NMEA2000 CAN message fields both in their definitions as well as the values they are allowed to contain.
I think these three very n2K source specific items should be either replaced with more generic definitions, and/or moved to the 'N2K' portion of the specification.
I am not keen on generalisation for the sake of it. If you can show some real benefits and added value in introducing a more generalised approach to Signal K then I am more than happy to not only listen but assist in moving SK in this direction. Signal K is a marine standard and should be optimised for efficient and effective integration with the equipment that the majority of boats have, which rightly or wrongly is >95% NMEA0183, NMEA2000 or J1939 based.
My understanding is that JSON schemas are extensible, so we can have some protocol specific items for the popular marine protocols/standards and then we can have extra items for other more "niche" protocols. Is this not correct ?
Moving a few N2K specific fields to the n2k
subgroup is hardly a generalisation, more just making the overall structure logical.
More specifically: moving these https://github.com/SignalK/specification/blob/51a898e6b3e4aaa2f5356c797ee2d2e29c19bfca/schemas/groups/sources.json#L119-L130 fields.
Hmm, there is some duplication there as well. Somebody didn't clean it up last time reorganising that group....
@thomasonw care to create a more specific issue about those? Following Tim's rfc protocol?
@tkurki - sure, will take a stab at it. @timmathews, do have a couple of questions on the details of your RFQ process - will send separate Email and perhaps we can work things out.
Re generalizations etc. I agree with @tkurki and @timmathews, while we need to live nicely with existing stds (basically NMEA) we dont want to simply inherit their flaws. We can design our own flaws :-)
In terms of the single hierarchy @tkurki makes a good point, but I think we have a good solution already. The issue under discussion is the alternator, is it engine or electrical? So lets say its electrical by nature, but its engine by location.
The simple solution for signalk is the same we use for $source, and will eventually use for deprecated keys and branches. We need a reference (aka a linux file link) which allows the same branch to exist in more than one place. So in the case of the alternator place it in electrical
, and add it as a reference in the engine
object, eg
{
"uuid": "urn:mrn:signalk:uuid:c0d79334-4e25-4245-8892-54e8ccc8021d",
"propulsion": {
"instance0": {
"label": "Main Engine - The Olde Faithful",
"alternators": {
"balmar0":{
"$ref": "electrical.alternator.balmar0"
}
}
}
}
}
@rob42, thoughts on my associations
idea from #262? I think I'll try out my RFC idea with it at least.
Oh, didnt see that..been too busy to read up.. Yes similar idea, the $ref can also signify an assocication, and more explicitly too. Maybe a better name is $link? BTW I use the graphviz .dot language a lot to document stuff (see http://www.planttext.com/planttext and play). By running a parser through a signalk dump you can create a suitable text file quite quickly and easily, and graphviz can create the diagram from that.
@startuml
rectangle vessel {
rectangle propulsion {
(Engine1) --> (Gearbox1)
(Engine2) --> (Gearbox2)
}
rectangle electrical {
(HouseBattery) --> (Balmar1)
(HouseBattery) --> (Balmar2)
(Balmar1) --> (StartBattery1)
(Balmar2) --> (StartBattery2)
}
}
(Engine1) --> (StartBattery1)
(Engine2) --> (StartBattery2)
(Engine1) --> (Balmar1)
(Engine2) --> (Balmar2)
@enduml
I like the graphs, took me a bit to decipher that you're preprocessing them with plantuml. I think RFC 0001 (#264) could be easily parsed into dot files.
@sumps @canboat : Reviewing sources.json, there are two N2K CAN ententes now defined as strings
Would it be more appropriate to declare these two as numbers? Or is there some other json dependency that requires these to be cast as strings.
If these two SK entities are always going to be N2K specific then yes numbers would make sense - they are certainly never going to change in the N2K world, so no chance of any future breakage.
OK, will change the current STRING type to NUMBER in the submission. Once I figure out how to run the 'test' data against the schema will work out an RFQ. Thanks,
@sumps : Looks like need to leave the .src as a string (even though N2K only has 8-bit ints). Changing it to number caused the test script to fail all over the place, looks like this entry is reused for other purposes throughout. If someone else has some thoughts here as to why using a number causes issues... Else will leave .scr as a string but change the .productID to a number.
@thomasonw, it's probably not reused per se, but many of the snippets used for testing include a src
value as a string. I don't think that there is anything wrong with converting it to a number, but you'll probably need to do a pretty big search-and-replace on the test examples to get them all to validate again.
@timmathews I was thinking it might be something like that. Perhaps after the current RFC 0002 closes could open a new one addressing specifically that one change across the many files. Would be nice I think to get the .src into a number, given that is what it really is..
This is ground we've been over before, but unfortunately it needs to be addressed again. Documentation on sources, the actual schema files and implementer understanding have all diverged.
The docs on the website have things like
with a string value of "self" for "source", and this:
where "source" is an object with a property of "device". Tests on the other hand use an object that looks like:
with "label" and "type" fields. The wiki has this source object:
with "bus" taking the place of "device" and a human friendly "label" string.
Clearly, we've had a lot of ideas and we haven't been very good about documenting them. There are really just two use cases for source information:
The first is outlined in sources.json and we've pretty much settled on a tree which organizes devices by bus (NMEA0183-n, NMEA2000-n). An example source tree could look like:
Then for storing data in a full Signal K tree, we have to handle multiple data instances. The example below is 101% of what we have today, I added the
instance
field at the same level as$source
andpgn
as all three of those values are required to uniquely identify the origin of a value.And here are the deltas that would generate that full object:
In the above delta example, I'd like someone (@sumps) with insider n2k knowledge to confirm that there aren't any PGNs with multiple instance fields (e.g. something that could have a temperature instance 0 and a pressure instance 1). If such PGNs exist, then
instance
needs to be moved to thevalues
object.In summary, I'd like to get everyone's buy-in on the following proposed changes:
instance
fieldsrc
tobusAddress
(or justaddress
)sources
hierarchypgn
,busAddress
,uniqueId
,productId
sources is the working branch for these changes. I haven't created a pull request yet.