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

Expose Subscription liveness callbacks to the application layer #11641

Closed
sagar-apple opened this issue Nov 10, 2021 · 6 comments · Fixed by #12758
Closed

Expose Subscription liveness callbacks to the application layer #11641

sagar-apple opened this issue Nov 10, 2021 · 6 comments · Fixed by #12758
Assignees
Labels

Comments

@sagar-apple
Copy link
Contributor

Problem

There doesn't seem to be a way for an application to sign up for SubscriptionTerminated notifications from the SDK.

This means that if a subscription expires, no notification is sent to the application to attempt a retry, for example.

Proposed Solution

Expose the IMDelegate APIs to the application layer.

@bzbarsky-apple
Copy link
Contributor

@mrjerryjohns
Copy link
Contributor

I presume you're referring to the client side here.

There are a couple of different callback APIs in question here - starting at the base with ReadClient::Callback, terminal errors are delivered through ReadClient::Callback::OnError, while non-terminal, attribute-path specific errors are delivered through ReadClient::Callabck::OnAttributeData.

The next level up is chip::Controller::TypedReadCallback (which is passed into chip::Controller::ReadAttribute). The OnErrorCallbackType signature will convey terminal errors if aPath == nullptr, including those that are fatal to the subscription, with the actual error embedded in a combination of the IM status code + CHIP_ERROR provided.

What we don't do well today is to convey liveness timeout failures correctly with a precise error code - it's conveyed as a CHIP_ERROR_TIMEOUT when we should in fact, be sending something more descriptive.

Levels above that are still WIP I believe..

@mrjerryjohns
Copy link
Contributor

@sagar-apple is this good enough for you?

@mrjerryjohns
Copy link
Contributor

@yunhanw-google to fix the emission of a more precise error code for liveness failure.

@bzbarsky-apple
Copy link
Contributor

@sagar-apple needs those "levels above" fixed is the point.

@yunhanw-google
Copy link
Contributor

Per chat with @bzbarsky-apple , Boris's incoming PR would take care of this, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants