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

[im/test] Fix interaction model subscription test #9677

Merged

Conversation

erjiaqing
Copy link
Contributor

Problem

What is being fixed? Examples:

Change overview

  • Resolve the comment, including removing unreviewed code.

Testing

*This PR updated the test.

@vivien-apple
Copy link
Contributor

Instead of doing review comment I spent time doing a branch at: https://github.com/vivien-apple/connectedhomeip-1/pull/new/YAML_TestSubscribe

The branch does not support Darwin at the moment, it still needs to be done.

In my understanding the following block could also be remove if a method to remove the reporting callback is added to the tree ?

        if (runner->mReceivedReport_{{index}}) {                                                                                                                                  
            // Receiving report more than once is not an error since the subscription may be alive for a long time.                                                               
            ChipLogProgress(chipTool, "Note: on report called more than once.");                                                                                                  
            return;
        }

Copy link
Contributor

@vivien-apple vivien-apple left a comment

Choose a reason for hiding this comment

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

Per my previous comment.

@erjiaqing erjiaqing force-pushed the im/subscription-test-followup branch from fff232f to 148c2f9 Compare September 15, 2021 09:08
@erjiaqing
Copy link
Contributor Author

/rebase

@woody-apple woody-apple force-pushed the im/subscription-test-followup branch from 148c2f9 to dc79acf Compare September 15, 2021 09:52
@erjiaqing
Copy link
Contributor Author

erjiaqing commented Sep 16, 2021

@bzbarsky-apple @saurabhst @LuDuda ptal

@woody-apple woody-apple merged commit b4d7eb3 into project-chip:master Sep 16, 2021
woody-apple added a commit that referenced this pull request Sep 16, 2021
erjiaqing added a commit to erjiaqing/connectedhomeip that referenced this pull request Sep 16, 2021
erjiaqing added a commit to erjiaqing/connectedhomeip that referenced this pull request Sep 16, 2021
woody-apple pushed a commit that referenced this pull request Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants