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

[WIP] feature: add preferredSource to common fields #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sbender9
Copy link
Member

@sbender9 sbender9 commented Nov 2, 2017

This allows the user to specify a preferred source for any given path. In node server this would be done in defaults.json.

Example:

{
  "vessels": {
    "self": {
      "navigation": {
        "position": {
          "preferredSource": "actisense.3"
        }
  }
}

The server then will only fill .value with data from that source, but others will still go into .values and have deltas sent. It will be up to he clients getting deltas to get the preferredSource setting from the server and filter based on that.

@sbender9
Copy link
Member Author

sbender9 commented Nov 2, 2017

I torn if we need to have some kind of timeout so that .value will get filled in with data from other sources if there is no data coming from the the preferred source.

On one hand, maybe it's better to something there in the case.
But maybe a user would use this because the data from the other source(s) is not good. In that case we would not want to start showing the user the bad data.

Opinions?

@mxtommy
Copy link

mxtommy commented Nov 3, 2017

while I imagine it may break existing apps/clients, if you have a preferredSource you don't need a ".value". You can put all the sources/values in .values and you know which one to use :) (instead of having the same information in two places)

@sbender9
Copy link
Member Author

sbender9 commented Nov 3, 2017

Yes, it would break existing clients that use REST.

@timmathews
Copy link
Member

Seems to me this should be in meta, no?

@sbender9
Copy link
Member Author

sbender9 commented Nov 3, 2017

@tkurki suggested to put it at the same level as .value since it specifically relates to that. meta gets put under the .values also, but would not really apply there.

@sbender9
Copy link
Member Author

sbender9 commented Nov 3, 2017

Was thinking maybe of having two values here to deal with issue when a source is missing. preferredSource could mean "use it if you have it" and masterSource could mean only use that source. (Probably a better name for that, can't think of one)

@joabakk
Copy link
Contributor

joabakk commented Nov 3, 2017

How about an array for preferred order? And possibly notifications if source changes

@tkurki
Copy link
Member

tkurki commented Nov 4, 2017

I think that making it plural preferredSources and an array we should be ok on the spec level.

Anything beyond that gets to the too complicated domain, as there are so many possible policies and parameters to them that we can think about a solution to cover all of them and still miss something to come along the next day.

@timmathews are you ok with this being a sibling of value and values and not under meta? Both have homes have their merits.

But we do need some test for it, to guard against accidentally breaking it and server as an example in the spec.

@joabakk
Copy link
Contributor

joabakk commented May 15, 2018

There are more use cases for this. One example is the design dimensions, which are input in the admin gui, and can also be fetched from ais self, where the options for accuracy may be limited:

"design": {
	"draft": {
		"value": {
			"current": 1
		},
		"values": {
			"undefined": {
				"value": {
					"maximum": 1.5
				}
			},
			"AIS_txrx.AI": {
				"value": {
					"current": 1
				},
				"timestamp": "2018-05-12T22:11:17.389Z",
				"sentence": "VDO"
			}
		}
	}

The undefined here is defaults.json. As this is user input, I would trust this more than AIS input (granted, that is input from me as well, but with less accuracy options).

@joabakk
Copy link
Contributor

joabakk commented May 15, 2018

The other example is for say position (two GPS-es and an AIS). Here I would be looking for a more traditional failover like kplex uses. And timeout would be more appropriate. If the preferred one fails, look for the next, then the next. With this in place, a plugin could look for duplicates and ask for preferred order and if a timeout is to be set

@tkurki
Copy link
Member

tkurki commented May 15, 2018

Please format json snippet for legibility.

What’s that current doing there`?

@joabakk
Copy link
Contributor

joabakk commented May 15, 2018

This is directly from AIS NMEA0183. I beleive it is trying to tell me that the current draft is 1m. (minimum, maximum, current or canoe)

@joabakk
Copy link
Contributor

joabakk commented May 15, 2018

Both max and current should have been displayed, right?

@rob42
Copy link
Contributor

rob42 commented Jun 5, 2018

I think this idea of ranking possible sources is very tangled between specification and the client and server implementation.

What about cases where two clients disagree of the ideal order? If the client doesnt set the order, then who does? an admin app maybe? thats just a client really...

The specification should concern itself with data and predictable access to it. The client should assume that the value in the value key is the 'official' value according to the server, and can get the alternates from values if needed. The server implementation should rank multiple values as it sees fit and keep the value tag updated to suit.

(The server may provide a UI to set ranking algorithms if required, again implementation specific.)

@sbender9
Copy link
Member Author

sbender9 commented Jun 6, 2018

The problem with that @rob42 is the deltas. There's nothing in the a delta to let the client know witch is the 'official' value.

@rob42
Copy link
Contributor

rob42 commented Jun 6, 2018

I think thats a problem of the delta format, which needs fixing, not the spec. The preferred or 'official' value is already identified (as above) but that data is lost due to the delta formats flattening effect.
The delta format needs to identify the preferred value eg

  "context": "vessels.urn:mrn:signalk:uuid:c0d79334-4e25-4245-8892-54e8ccc8021d",
  "updates": [
    {
      "source": {
        "label": "GPS-1",
        "type": "NMEA0183",
        "talker": "GP",
        "sentence": "RMC"
      },
      "timestamp": "2017-04-03T06:14:04.451Z",
      "values": [
        {
          "path": "navigation.courseOverGroundTrue",
          "value": 3.615624078431440,
***      "preferredSource": true  ***
        }
      ]
    },
    {
      "source": {
        "label": "actisense",
        "type": "NMEA2000",
        "src": "115",
        "pgn": 128267
      },
      "timestamp": "2017-04-03T06:14:04.451Z",
      "values": [
        {
          "path": "navigation.courseOverGroundTrue",
          "value": 3.615624078431453
        }
      ]
    }
  ]
}

@tkurki
Copy link
Member

tkurki commented Jun 6, 2018

If the case we are solving is as a generic client I want to display only data from the preferred sources we already have a way for that to happen in rest (value is always from the preferred source) and for deltas we need a way to subscribe to only preferred data - no need to look at each and every delta in the client and check a field.

Originally I thought that this issue was about defining a way for an (admin) client to set the preferred source order and get it to display it.

@tkurki
Copy link
Member

tkurki commented Jun 6, 2018

I don’t think sending an extra field in every single delta is prudent. In simpler cases it would always be true for all deltas.

@sbender9
Copy link
Member Author

sbender9 commented Jun 6, 2018

I thinking you guys are making this too complicated. I'm fine with it being handled on client side, WilhelmSK does this today. If there's an array of preferedSources in the meta data, then it does the right thing with REST or streaming. We just need a standard way for clients to find out what the preferred sources are if they care.

@tkurki
Copy link
Member

tkurki commented Jun 6, 2018

Yeah, that would be the get part in what I wrote above.

We can also think about this in terms of writing a compliance test for this. (This is something I'd like to move towards with spec changes). If we want to write a test that verifies Supports preferred sources what functionality would the test exercise? Just that when configured correctly a server's rest response contains the preferredSources item? For a certain path or all the paths? Is the client able to set them? For any path or just the paths that the server has data for? Are the sourceRefs validated against the server's sources list?

And I go down the rabbit hole...must-pull-back...I think just adding the preferredSources string array as a sibling of value would make sense, solve the data model problem and at this stage ignore the more detailed functionality.

@bkp7
Copy link
Contributor

bkp7 commented Jun 6, 2018

If preferredSources is something set/held within a server the preferredSources will itself have multiple sources when more than one server is present. ie. what is the preferredSource for preferredSources? And how does this ripple up through Slave/Master servers?

As the spec currently stands there is no reasonable way to interpret the Master/Slave server situation in a way that works and is scalable. I think that needs to be sorted before looking at this issue.

@fabdrol
Copy link
Member

fabdrol commented Jun 5, 2019

@sbender9 this PR has been inactive for a year or so. Is this still something we want in spec? Is there for instance support in node server for it?

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

Successfully merging this pull request may close these issues.

8 participants