vanadium / issues

Vanadium issue tracker
1 stars 1 forks source link

Discovery: Scan stream includes services advertised by the same discovery client. #1050

Open aghassemi opened 8 years ago

aghassemi commented 8 years ago

Maybe we should excludes services that are advertised by the same discovery client? I expect the common case to be the advertiser ignoring their own advertisements since they would be handling those differently than services from others.

@bjornick @asadovsky @jhahn21

jhahn21 commented 8 years ago

This is on our todo: https://docs.google.com/document/d/1RkN5ndOfVh728OHXEb4fYaXL1hWaHBEQPrhnBMdT5og/edit#heading=h.ide9lpycot1z

But it's not clear how to identify the same discovery client, since there may be multiple clients for a single discovery instance in a device.

@asimshankar

asadovsky commented 8 years ago

A couple ideas:

(Of course, each of these has drawbacks.)

jhahn21 commented 8 years ago

I was thinking a similar way like putting a session id to context.

myCtx = context.WithNewSession(ctx)
(or myCtx = context.WithSession(ctx, sessionId) ?)
...

ctxForAd = context.WithCancel(myCtx)
discovery.Advertise(ctxForAd, ...)
...

ctxForScan = context.WithCancel(myCtx)
discovery.Scan(ctxForScan, ...)  // Scan will filter out all the advertisements from the same session
...

But I'm not sure whether we want to put this WithSession() into context (@mattr-google) :), although I think it's clearer than providing as a discovery util function.

monopole commented 8 years ago

My 'moments' app ignores its own ads by storing the ad's ID in a table when advertising is on, and consulting that table on the scan side - dropping self adverts on the floor.

Ken's 'syncslides' app sends a "device UUID" as an advertising attribute, and uses that to drop the ad on the floor on the scan side.

First option is more bookkeeping, the second option adds the string encoding of a 128 bit ID to each ad.

Would like this to be opaque, e.g. my app makes an advertiser, and from it creates a scanner 'child' that knows to ignore ads made by its parent. An app that made a scanner with no parent wouldn't have this built in filter thingie.

jhahn21 commented 8 years ago

I'm not sure yet whether it's a good idea to add WithSession to context until we have more general use cases for it. So I'll add WithSession() to discovery.T for now like

ctx, session = discovery.WithNewSession(ctx)

Please let me know if you have any better idea.

jhahn21 commented 8 years ago

And we also have Service and Mojo interface for discovery, which have no context. So, we need to pass a SessionId or something like that to Advertise() and Scan()