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

about leave event #21744

Closed
Guotao-Aqara opened this issue Aug 9, 2022 · 2 comments · Fixed by #21760
Closed

about leave event #21744

Guotao-Aqara opened this issue Aug 9, 2022 · 2 comments · Fixed by #21760

Comments

@Guotao-Aqara
Copy link

Guotao-Aqara commented Aug 9, 2022

No matter how many fabrics the dut joins,dut just report fabricIndex 1 when i call chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics() before factroy reset

  • expected behavior
    report all fabric index
  • actual behavior
    report fabric index1
  • steps to reproduce
    first : dut commission to TH1 and TH2
    second : TH1 and TH2 subscribe leave event: basic subscribe-event leave 1 60 1 0、basic subscribe-event leave 1 60 2 0 --commissioner-name beta
    third : call chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics() befofe factory reset

I try to understand the code and I can see that the event has been pushed to eventManagerment,but it doesn't work well actually, I don't understand what's going on here

@bzbarsky-apple
Copy link
Contributor

What I would expect to happen here is:

  1. Fabric index 1 gets removed. This queues a Leave event, which is reported to both subscriptions.
  2. Fabric index 2 gets removed. This queues a second Leave event, which is reported to only fabric 2 (because fabric 1 is gone by this point).

I am seeing item 1 happen, but not seeing the second Leave event for item 2. Looking into why not.

@bzbarsky-apple
Copy link
Contributor

Ah, I think the issue is that the ReadHandler for the fabric 2 subscription has sent out the Leave event for fabric 1 and has not gotten the Status Response yet, so it's not able to send any more data reports.

It seems like we should only be trying to sync-deliver Leave on the fabric being left. So Engine::ScheduleUrgentEventDeliverySync needs to grow a way to only do sync delivery on a specific fabric index, perhaps....

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 9, 2022
We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes project-chip#21744
woody-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 9, 2022
We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes project-chip#21744
woody-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 9, 2022
We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes project-chip#21744
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 9, 2022
We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes project-chip#21744
woody-apple pushed a commit that referenced this issue Aug 10, 2022
* Fabric removal should only sync-deliver events for that fabric.

We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes #21744

* Address review comment.
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this issue Sep 16, 2022
…ect-chip#21760)

* Fabric removal should only sync-deliver events for that fabric.

We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes project-chip#21744

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

Successfully merging a pull request may close this issue.

2 participants