Skip to content
This repository has been archived by the owner on Jun 13, 2019. It is now read-only.

OicClient: Remove handle when PRESENCE_TRIGGER_DELETE event is received #35

Closed
wants to merge 1 commit into from

Conversation

nagineni
Copy link

@nagineni nagineni commented Jan 7, 2016

Delete the handle from the list when the presence is disabled and device has been deleted.

@nagineni
Copy link
Author

nagineni commented Jan 7, 2016

@gabrielschulhof

Signed-off-by: Sudarsana Nagineni <sudarsana.nagineni@intel.com>
@gabrielschulhof
Copy link

I think a better way would be to OCCancel() not during OC_PRESENCE_TRIGGER_DELETE, but during device addition, because there's a case where you device.disablePresence(), but you don't terminate the process right thereafter. That's the scenario being explored by the failing test. I'll make a test where the server is being restarted, to make sure you get the delete event the second time around as well.

@gabrielschulhof
Copy link

Actually, it's much simpler than that. All you have to do is unconditionally re-issue the open-ended presence request upon rediscovery of a given server, because _openEndedRequest() does the OCCancel() for you. IOW, you just remove the if-statement from around the _openEndedRequest() for presence tracking.

@nagineni
Copy link
Author

Initially I tested by removing that if condition to solve the issue, but I was not fully sure whether it's okay to remove it or not. Please merge the commit, if you are sure that it will not introduce any regression by removing it.

@gabrielschulhof
Copy link

OK. I'm relying on the tests to spot regressions, and they're all passing, so I'm assuming that we have no regressions.

@nagineni
Copy link
Author

Great! :)

@nagineni nagineni deleted the delete_event branch January 11, 2016 12:52
@sanbeam sanbeam mentioned this pull request Dec 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants