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

Removal of source from full model schema (standardize on $source) #379

Open
mxtommy opened this issue Sep 6, 2017 · 8 comments
Open

Removal of source from full model schema (standardize on $source) #379

mxtommy opened this issue Sep 6, 2017 · 8 comments
Milestone

Comments

@mxtommy
Copy link

mxtommy commented Sep 6, 2017

I have not seen any discussion regarding this or how this decision came to be. If there is some feel free to point to it and close this issue :)

Currently a "value" can have both $source, a reference to the source under /sources, as well as a source object directly. In theory this is duplicated data? I do not see an advantage to having the source object on the value, when you have a well defined place to have them in /sources. Not to mention if same source is used for many data values, having the same source data on each object is very "redundant".

I've observed from OpenPlotter both "source" and "$source" being set on the same "value" (when fetching the full tree) , and them being different. It was not a case of multiple values as well.(no "values" property).

I propose we ensure every value has a $source, and remove all references to "source". That way there is no ambiguity on which to use and when.

@sailoog
Copy link
Contributor

sailoog commented Sep 7, 2017

I've observed from OpenPlotter both "source" and "$source" being set on the same "value" (when fetching the full tree) , and them being different. It was not a case of multiple values as well.(no "values" property).

It is really strange that you see "source" and "$source" on OpenPlotter for the same key if it is not a multisource data . Could you be more specific?

It could happen that you have navigation/headingMagnetic from NMEA ($source) and you have navigation/headingMagnetic from an IMU sensor set in OP (source), but that makes sense for me.

@mxtommy
Copy link
Author

mxtommy commented Sep 7, 2017

Got this from the REST api the other day. Didn't save the full result though.

"gnss": { "source":{ "type":"NMEA0183", "sentence":"GGA", "label":"signalk-parser-nmea0183", "talker":"GP" }, "timestamp":"2017-09-04T17:47:09.061Z", "quality":1, "satellites":4, "antennaAltitude":-43, "horizontalDilution":12, "geoidalSeparation":-32, "differentialAge":0, "differentialReference":0, "$source":"OPkplex.XX" }

Only one GPS (a usb dongle) connected to the openplotter. (no NMEA or anything, though did not dig into kplex's config.)

If there was a second source, I would have assumed there would have been a .values attribute with the values of each source no?

Regardless if this is an Openplotter bug or not, is there a benefit of having two places to have the source?

@joabakk
Copy link
Contributor

joabakk commented Sep 8, 2017

#358 ?

@mxtommy
Copy link
Author

mxtommy commented Sep 8, 2017

#358 discusses source's definitions, but not really removing "source" properties from the schema?

I just realized, there needs to be a way to populate /source. I think it is currently done by parsing the source object from delta messages, so that needs to stay I think.

So basically, leave the delta message format alone, and just remove the "source" property everywhere else in the schema? (leaving sourceRef)

With only sourceRef left, you A) avoid duplicate data everywhere, B) ensure you have only one source of truth for the source of data (vs having it possible to have both defined, in that case which is right if they're different?)

@sailoog
Copy link
Contributor

sailoog commented Sep 8, 2017

Ok, I see now. It is not an OpenPlotter bug, it is sk server node behavior when the key it is not a leaf but an NMEA 0183 object:

      "position": {
        "longitude": x.xxxxxxx,
        "latitude": x.xxxxxxx,
        "$source": "OPkplex.GP",
        "timestamp": "2017-09-08T15:31:39.000Z",
        "sentence": "GLL",
        "source": {
          "type": "NMEA0183",
          "sentence": "GLL",
          "label": "signalk-parser-nmea0183",
          "talker": "GP"
        }
      "gnss": {
        "source": {
          "type": "NMEA0183",
          "sentence": "GGA",
          "label": "signalk-parser-nmea0183",
          "talker": "GP"
        },
        "timestamp": "2017-09-08T15:31:39.060Z",
        "quality": 2,
        "satellites": 6,
        "antennaAltitude": -3,
        "horizontalDilution": 1,
        "geoidalSeparation": 49,
        "differentialAge": 0,
        "differentialReference": 0,
        "$source": "OPkplex.XX"
      }

In "$source": "OPkplex.GP" the only redundant data would be the talker because OPkplex is the provider id. I suspect that changing that will affect the leaf keys:

      "speedOverGround": {
        "value": 0.054016680350892354,
        "$source": "OPkplex.GP",
        "timestamp": "2017-09-08T15:31:39.000Z",
        "sentence": "VTG"
      }

Regarding my examples I see a strange thing, why gnss object has "$source": "OPkplex.XX" if sentence GGA is supported by signalk-parser-nmea0183?

This is the settings file that generate examples:

{
  "vessel": {
    "uuid": "urn:mrn:imo:mmsi:98765432"
  },
  "pipedProviders": [
    {
      "pipeElements": [
        {
          "type": "providers/udp",
          "options": {
            "port": "55556"
          }
        },
        {
          "type": "providers/nmea0183-signalk",
          "optionMappings": [
            {
              "toOption": "selfId",
              "fromAppProperty": "selfId"
            },
            {
              "toOption": "selfType",
              "fromAppProperty": "selfType"
            }
          ]
        }
      ],
      "id": "OPkplex"
    }
  ],
  "interfaces": {
    
  }
}

@tkurki
Copy link
Member

tkurki commented Sep 10, 2017

The talker .XX is coming from https://github.com/SignalK/specification/blob/master/src/index.js#L243-L245, which triggers when the delta has a source object but no talker.

Which in this case is caused by an ancient 0183 parser bug, that I discovered thanks to you: SignalK/nmea0183-signalk#91. Feel liking fixing that? Plenty of example code in the parser, for codec and test code.

@tkurki
Copy link
Member

tkurki commented Sep 10, 2017

I just realized, there needs to be a way to populate /source. I think it is currently done by parsing the source object from delta messages, so that needs to stay I think.

Yes, that is how Node server currently works.

So basically, leave the delta message format alone, and just remove the "source" property everywhere else in the schema? (leaving sourceRef)

Just to be clear here: there are two schemas, one for full and one for delta JSON documents. Here we are talking about removing source usage from the full schema.

For version 1 think source needs to stay, but for post 1 era this is definitely something we should do.

@tkurki tkurki added this to the After v1 milestone Sep 10, 2017
@tkurki tkurki changed the title Removal of source from data values (standardize on $source) Removal of source from full model schema (standardize on $source) Sep 10, 2017
@tkurki
Copy link
Member

tkurki commented Sep 10, 2017

Changed the title to better reflect the change.

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

No branches or pull requests

4 participants