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

Handling invalid data values #93

Closed
sumps opened this issue Oct 11, 2015 · 20 comments
Closed

Handling invalid data values #93

sumps opened this issue Oct 11, 2015 · 20 comments
Milestone

Comments

@sumps
Copy link
Contributor

sumps commented Oct 11, 2015

How should we handle invalid data values in the SK spec? Two examples are;

  1. Depth value when the depth has been lost i.e. turbulence hàs caused the depth unit to lose the bottom reading, a depth sounder normally flashes the last reading.
  2. GPS unit temporarily loses position and as a result all readings; position, COG, SOG etc. are not valid.
@tkurki
Copy link
Member

tkurki commented Oct 11, 2015

So the information we need to convey is that a sensor says "I'm operational & connected & ok, but I am unable to function for some reason" so it is not just the absence of data / updates.

Several options:

  • send a delta without the value field or have a missing value field in the full tree. Both make normally required fields optional, making validation problematic
  • replace value property with a special marker property like dataMissing: true, making either mandatory. Clearly marks missing values instead of just omitting them.

@sumps
Copy link
Contributor Author

sumps commented Oct 12, 2015

I think having a special marker property like dataValid: false, would be a good solution and suitable for most sensor data. Depth and GPS are the obvious ones and we should start with these ones but we might need to add this for other sensors if we find that they have a valid flag or some other way of indicating data invalid.

@rob42
Copy link
Contributor

rob42 commented Nov 21, 2015

Hmm, this brings up other issues - how does a sensor communicate its condition via signalk. Looking at the simple cases:

  1. ok
  2. ok, but invalid data
  3. failed, but alive
  4. dead

Well 4) is easy - its just gone, so nothing, no comms etc.

  1. Since this is most common, we can take the position its all ok unless notified otherwise.

  2. For each data key we currently have value, timestamp, source. We could add an optional state key, that is sent to provide for case 2) .

  3. The source tree should have some way to register device failed for case 3) above. Maybe a state there too?

@faceless2
Copy link

For what it's worth I'm not sure I see any practical difference between a sensor that is dead/missing, and a sensor that is alive but not reporting any usable values.

If this were a requirement then you could do this with a "last seen" timestamp on the source - this is the last communication with the source, which is distinct from the timestamp associated with any data fields it updates. But given I'm not sure I understand the problem this is trying to solve, this might not help.

@joabakk
Copy link
Contributor

joabakk commented Feb 22, 2016

Just came to think about this. I'm not sure how a depth sensor would react when the depth is greater than it could measure. But there is value in a measurement saying "it's really deep here", rather than "the sounder is not measuring since 2 mins ago"

@sumps
Copy link
Contributor Author

sumps commented Feb 22, 2016

Traditionally depth sounders flash the last good value and they can lose depth for a number of reasons, not just going "out of their depth" in deep water. GPS position is another value that can become invalid.

@faceless2 it is not about last seen, as the values could still be streaming in from NMEA sources, it is about this is the last value but it is no longer valid. You could just stop sending SK data but then the display values on a consumer would time out and disappear and you do not know what the last value was and whether you have a system failure, data connection drop out, etc.

@sumps
Copy link
Contributor Author

sumps commented Apr 4, 2016

I am going to have to make a decision on this one, as it is holding up iKommunicate testing.

If there is an NMEA sentence or PGN that is showing the data as not available/invalid then we will send the relevant Signal K value as a Null. The same is true if the data was coming in and it then times out, we will also change the Signal K value to a Null.

@keesverruijt
Copy link
Member

I think that is the best solution. All alternatives are worse!

@tkurki
Copy link
Member

tkurki commented Apr 7, 2016

There are three practical ways for sending 'data invalid' message:

Send delta with just path, no value.

Send value as undefined.

Send value as null.

I think null, indicating the absence of value, is the proper choice.

https://www.wwco.com/~wls/blog/2011/05/30/json-and-undefined-properties/

@tkurki
Copy link
Member

tkurki commented Apr 9, 2016

There are several explicit cases where a sensor can indicate that it is otherwise operational but can not produce vali ddata. Example cases are a depth sounder that can not discern bottom from the echo data and a GNSS that has no satellite fix.

In these cases a gateway/server will typically receive a message indicating invalid data. It must send out a delta message where the value of the data item is null and serve the value as null in the REST api.

@tkurki
Copy link
Member

tkurki commented Apr 9, 2016

SignalK/n2k-signalk#21
SignalK/nmea0183-signalk#56

Should be included in the spec document.

@sumps
Copy link
Contributor Author

sumps commented Dec 21, 2016

It seems to me that everyone is in agreement on the above solution of using "Null" in Signal K to indicate that data is not available or currently invalid.

I am not sure what changes are needed to the schema files to implement this but if someone wants to do that part I am happy to make changes to the documentation.

@timmathews
Copy link
Member

No change should be necessary to the schema. It just needs to be explicit in the documentation somewhere.

In these cases a gateway/server will typically receive a message indicating invalid data. It must send out a delta message where the value of the data item is null and serve the value as null in the REST api.

However, I think that this is a very JavaScript-centric approach which makes implementing consumers more difficult in other languages. JSON parsers typically treat null values and missing values as the same thing (i.e. null), but we're explicitly stating here that they're not the same thing (because in JavaScript, they're not).

I've verified this to be the case in C#, Go and Ruby so far. It would be illustrative to know what common parsers for C, C++, Objective-C, Java, Python and Swift do. I suspect the results will be similar.

A way to explicitly indicate that the value is invalid would be better.

@rob42
Copy link
Contributor

rob42 commented Dec 22, 2016

Since we use json as the data format we need to send data that conforms to json spec, and assume that different parsers dont have bugs. If they do its the parsers problem.

JSON only allows true, false, null and empty objects or valid data. We could send INVALID for an invalid object but many parsers would then barf out "Invalid numeric format" for numbers, so thats not a good option IMHO. Also we would have to test values everywhere, very messy.

In the end we can either make a value null, or make an object null, or we break the parsers.

Only other option is in the delta we send a block of invalid keys, eg

{
  "context":"vessels.urn....",
   "updates":[
        .......
    ],
    "invalid":[
         "navigation.position",
         "navigation.courseOverGroundTrue"
     ]
}

@tkurki
Copy link
Member

tkurki commented Dec 22, 2016

We do have the option of sending just path, no value, as mentioned already above.

Other options exist, like

{
  "path": "some.path",
  "valid": false
}

which is a more verbose version of just omitting value.

PS. Python is ok with JSON nulls (None) and Java has Gson.isJsonNull() and JSONObject.isNull(key).

@sumps
Copy link
Contributor Author

sumps commented Dec 22, 2016

I think the best option is to send the exact same JSON as we normally do but with value = null and then it is one change and can be applied to all objects. If we have the valid field it will add complexity and unless it is mandatory people will ignore it where as value = null will just happen and the client software will need no extra modification.

@faceless2
Copy link

faceless2 commented Dec 22, 2016 via email

@jboynes
Copy link
Contributor

jboynes commented Dec 22, 2016

There's a interaction with the REST API as well. For a key that is known but whose value is currently unknown, the appropriate response would be 200 with a null value; for a key that is not known the server should return a 404 response.

When retrieving a non-leaf path (e.g. environment or navigation) the server must include keys whose value is unknown i.e. explicitly contain the property with a null value. Not all JSON frameworks do that automatically so we need to call this out as a requirement.

There's a still problem in distinguishing between a full and sparse response. If the some.arbitrary.key property is missing, does that mean the key is not known to the server, or that the key is known but this is a sparse response where the server just did not include that key?

This stops being a problem if we switch from a hierarchical to the flat model.

@rob42
Copy link
Contributor

rob42 commented Dec 22, 2016

See #311 we need some way to identify if its a sparse or full response. I still need to catch up on recent discussions but is it possible to differentiate between sparse and full by the request or subscription?
It seems to me that REST will always be a full response, at least for the branch requested, that a subscription will know because it was part of the sub request?
eg in what use cases would the server decide between sparse/full without direction from the client?

@tkurki tkurki closed this as completed Oct 30, 2017
@tkurki tkurki moved this from In Progress to Completed in Release Version 1.0.0 Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

8 participants