search

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

fix: allow merge meta user in signalk schemas #547

Closed cmotelet closed 5 years ago

cmotelet commented 5 years ago

This PR allows you to merge meta-users into an existing path in the signalk schema.

rob42 commented 5 years ago

Can you please add some more detail about this? I dont understand what meta-users is exactly.

cmotelet commented 5 years ago

When you use the function FullSignalK(id, type, defaults), the last parameter defaults and may contain part of the schema but if this object contained meta, they were not displayed correctly (especially for the paths existing in the schema). Look at my issue: https://github.com/SignalK/signalk-server-node/issues/754

rob42 commented 5 years ago

Ah, so this just adjusts the test code, not the spec definition itself?

cmotelet commented 5 years ago

Yes, only the ability to inject metadatas and retrieve them correctly. No change in the spec definition. And I have discovered an additional case that is not being handled properly, the correction is underway for tomorrow.

tkurki commented 5 years ago

If I understand this correctly this will add some entries to metadataByRegex every time a FullSignalK object is created. metadataByRegex is a module level singleton, initialised from data that is derived from the static schema files. I think that is a bad idea: the current Node server creates a new FullSignalK object for every http request that queries the full data model. Furthermore what if I create two different FullSignalK objects with different user provided metadata - the paths will end up in the shared metadataByRegex.

cmotelet commented 5 years ago

Yes, that's right, I forgot the multiple call to FullSignalK ! To fix, if I export a new function into index.js like addMetaForPath(fullPathWithoutMetaStr, metaObj) and that this function builds the regex and adds it first to metadataByRegex.

In signalk-server-node at startup, we will iter in defaults.json to find the meta values if there are any. If there is a meta value we complete them, they must be all there otherwise addValue in fullsignalk.js does not add any.

Then these metas are added only once in metadataByRegex with the new function addMetaForPath(fullPathWithoutMeta, metaObj). fullPathWithoutMeta must be of the full path type with the uuid of the vessel so as not to interfere with other regexes.

Do you agree with this new mode?

And another thing, add values to the schema, from defaults.json such as design.[draft|length|beam|airHeight|aisShipType], sensors.gps.[fromBow|fromCenter] doesn't seem right to me without going through a delta. This direct addition is not valid if you check with validSignalK.

fabdrol commented 5 years ago

@cmotelet please implement your proposed changes so we can have a look if it resolves @tkurki 's review.

cmotelet commented 5 years ago

closed due to bad implementation.