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

callsignVhf not according to the spec #221

Open
wellenvogel opened this issue Apr 3, 2022 · 8 comments
Open

callsignVhf not according to the spec #221

wellenvogel opened this issue Apr 3, 2022 · 8 comments

Comments

@wellenvogel
Copy link

When decoding AIS data (AIVDM) the decoder creates at vessels//communication an object with the following layout:
{"callsignVhf":"DQGG"}.
According to the spec the decoder should instead generate a simple string value at:
vessels/
/communication/callsignVhf.
https://signalk.org/specification/1.5.0/doc/vesselsBranch.html#vesselsregexpcommunicationcallsignvhf
Seems a bug to me.

@Hakansv
Copy link
Contributor

Hakansv commented Apr 3, 2022

What's the OCPN issue here?
OpenCPN does check signalk path (Vessel) ["communication"]["callsignVhf"]
Works fine when the source is NMEA0183->Signalk.
N2k->SignalK I haven't tested but would use the same SK path I suppose.

When parsing AIVDM O checks message 5 and 24
No problem here what I can see

@wellenvogel
Copy link
Author

N2k-Signalk does not include callsignVhf at all currently. Just created a PR there - see link above.
And I was not talking about OpenCPN...
I see in the OpenCPN code that OpenCPN is just parsing what it currently gets from SK - so assuming the nested structure that is currently generated.
But finally it is not according to the spec (at least to my understanding and according to the spec files that signalK brings with it)- so either the spec should be changed at this point, it should be documented that the SK server is not following the spec or whatever....
But it should be clear to the user what to expect at this path.
Finally users (including OpenCPN) can simply have a look at the current implementation and use what is there - but this is maybe not the way it should work.

@Hakansv
Copy link
Contributor

Hakansv commented Apr 3, 2022

From the spec you refer to the path would be the one we use in O?

And I was not talking about OpenCPN...

I'm sorry but do not understand why an issue here, or is it still anything to change in O?

@tkurki
Copy link
Member

tkurki commented Apr 3, 2022

@Hakansv no issue in O.

@wellenvogel please check again https://github.com/SignalK/nmea0183-signalk/blob/master/hooks/VDM.js#L189-L190

Usually you send a path to a leaf value and the value in the delta. That is handled by the conversion to Full model by adding timestamp and source information to the leaf value and if there are multiple sources for that particular path the multiple values handling kicsk in and creates the values substructure.

But what if we want a plain value in the leaf? There's special provision in the delta to full conversion that merges the data from pathValues with an empty (non null) path's value with the full data.

So to set callsignVhf to a string value you send a delta that has a pathvalue with an empty path and a nested object structure communication => callsignVhf => value.

@wellenvogel
Copy link
Author

wellenvogel commented Apr 3, 2022

Not sure what you mean.
But finally I see in the data browser an object at communication. But I would expect a string at communication/callsignVhf -like for all the other values.
Objects are there according to the spec e.g. at navigation/position.
So what would happen if someone would e.g. send any other value below communication?
Finally data would look different if i send a path communication/callsignVhf with a simple value.
You can just compare this to all the other values.
Simply e.g. the lines exactly above the ones you linked.

@wellenvogel
Copy link
Author

wellenvogel commented Apr 4, 2022

@tkurki
I just made some small test on what is reported at http://xxxxx:3000/signalk/v1/api/vessels/.
(1) original as it is now:

"communication" : {
         "callsignVhf" : "DBBU"
      },

(2) When I changed lines 187++ to

if (data.callsign) {
     values.push({
       path: 'communication.callsignVhf',
       value:  data.callsign ,
     })
  }

I receive:

"communication" : {
         "callsignVhf" : {
            "meta" : {
               "description" : "Callsign for VHF communication"
            },
            "$source" : "testsrc.AI",
            "value" : "DBBU",
            "sentence" : "VDM",
            "timestamp" : "2022-04-03T21:30:22.648Z"
         }
      },

(My) conclusion:
With the current state there is no valid meta information and the value is just reported as any value that is not in the spec.
With the change the value now follows the spec and correct meta data is reported for it.

@tkurki
Copy link
Member

tkurki commented Apr 4, 2022

The JSON schema files in specification are the real master data for the schema and Appendix A Keys reference is derived from that. JSON schema is much more expressive, but hard to understand and we created keys reference for humans, but it is not complete.

In JSON schema callsignVHF is a plain string value, not an object with metadata: https://github.com/SignalK/specification/blob/38147bc0ff7d16edb78d3bc55f131940799f528b/schemas/groups/communication.json#L8-L12

To validate nmea0183-signalk output we can convert the delta to full data and validate that with @signalk/signalk-schema module's chaiUtilities: https://github.com/SignalK/nmea0183-signalk/blob/master/test/VDM.js#L67

This is the approach in used in many tests in nmea0183 and n2k converters.

The difference between the values with metadata and values without it is confusing. We went a little overboard with "one full model and associated schema to rule them all" idea and deltas, that are actually often much more relevant and convenient, came later. Something to improve on in a subsequent SK version.

@wellenvogel
Copy link
Author

Ok, played a bit with the tests.
And confusingly they really reject setting a plain value as communication.callsignVhf.
My question:
How shoudl I know (as a user of the API) what to expect?
When looking at the Schema doc communication/callsignVhf is looking very similar to e.g. design/beam.
So where to get the info of what to expect if I issue a query to http://xxx:3000/signalk/va/api/vessels/urn:mrn:imo:mmsi:nnnnn?
beam will come with

"design": {
"beam" : {
         "sentence" : "VDM",
         "value" : 8,
         "timestamp" : "2022-04-04T09:00:01.712Z",
         "$source" : "testsrc.AI",
         "meta" : {
            "description" : "Beam length",
            "units" : "m"
         }
      }
}

whereas callsignVhf will come as:

"communication" : {
      "callsignVhf" : "DJSO"
   }

no $source, no timestamp, no meta - and even no "value" property.
This is at least not in line with the full spec at https://signalk.org/specification/1.5.0/doc/data_model.html - excerpt:
Each data object has a value property which holds the actual value for that specific key. The value property may contain a number, a string or another object.

I understand that it works right now the way you described - but for me (as a user) it's nearly impossible to get an idea about what to expect at the API by looking at the documentation.
As I already wrote - finally a user can have a look on what the server currently delivers and just use this as it is - but I feel a bit unhappy with such an approach as I would like to follow some "official" (i.e. documented) API - otherwise I might use something that relies on some internal details that could easily change in the future.

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

3 participants