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

V1.0.0 branch 25023 fix #110

Conversation

domichae-amazon
Copy link
Collaborator

Fixes project-chip#25023 on the v1.0.0 branch.


Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.
@domichae-amazon domichae-amazon changed the base branch from master to v1.0.0-branch February 27, 2023 23:19
@@ -2083,7 +2137,7 @@ - (void)applicationBasic_readVendorID:(ContentApp * _Nonnull)contentApp
[_readSuccessCallbacks setObject:successCallback forKey:@"applicationBasic_readVendorID"];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be cleaner if we just entirely skipped storing these callbacks in the _readSuccess/failureCallbacks property?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, but unfortunately it gets a little gnarly because the Read-style APIs take a ReadResponseSuccessCallback and ReadResponseFailureCallback which are void (*)(void * context, T responseData) and void (*)(void * context, CHIP_ERROR err), respectively.

When I updated the APIs with Response callbacks to avoid the Dictionary lookup, I was relying on the fact that those APIs take a std::function for the response callback and Objective C lambdas with captured variables can be implicitly converted to std::functions (this is necessary because we need to capture the clientQueue and the response callback). Unfortunately, Objective C lambdas with captured variables can't be implicitly converted to std::functions so you can't easily keep around a reference to the callback function or client queue.

If we find a solution for the Read-style methods, though, then we should be able to extend that to the Subscribe methods pretty easily and cut down on a lot of the boilerplate in the CastingServerBridge.

* Refactored the Certification Tests to more fully exercise threading behavior
@domichae-amazon domichae-amazon requested review from sharadb-amazon and cliffamzn and removed request for sharadb-amazon and cliffamzn March 1, 2023 18:18
@cliffamzn cliffamzn merged commit 8c2d260 into sharadb-amazon:v1.0.0-branch Mar 1, 2023
@domichae-amazon domichae-amazon deleted the v1.0.0-branch-25023-fix branch March 1, 2023 18:47
sharadb-amazon pushed a commit that referenced this pull request Mar 9, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon pushed a commit that referenced this pull request Mar 9, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon pushed a commit that referenced this pull request Mar 17, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon pushed a commit that referenced this pull request Mar 17, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon pushed a commit that referenced this pull request May 24, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon pushed a commit that referenced this pull request May 24, 2023
* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs
sharadb-amazon added a commit that referenced this pull request Jun 13, 2023
…chip#26817)

* V1.0.0 branch 25023 fix (#110)

* Lock the SDK before interacting with it.

[Problem]

The Matter SDK assumes that the client will take a lock on the SDK prior to
interacting with any APIs. There is 1 Android function and several iOS functions
that do not take the lock prior to interacting with the SDK APIs.

There are also some functions that invoke the Matter SDK in a way that may
result in out-of-order callback invocation (e.g. the conceptual "started"
callback may be invoked after the conceptual "ended" callback).

[Solution]

Create utility functions that help ensure that the Matter SDK is invoked safely
and that callbacks are delivered in the correct order.

[Testing]

Performed local, manual testing with the 'tv-casting-app' example on iOS against
a Raspberry Pi running the 'tv-app'. Ran the certification test in the
'tv-casting-app' on iOS against the same Raspberry Pi.

* * Updating naming and parameter order for new CastingServerBridge APIs
* Refactored the Certification Tests to more fully exercise threading behavior

* * Updating the documentation for modified APIs

* Restyling

---------

Co-authored-by: Michael Douglas <[email protected]>
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 this pull request may close these issues.

[BUG] tv-casting-app does not take the SDK lock prior to SDK interactions
3 participants