Improve detection of "historical" events in Matter.framework. #36506
+135
−15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review changes with |
|
PR #36506: Size comparison from 73ff58f to 8a9833a Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
force-pushed
the
priming-more-restrictive
branch
from
8a9833a
to
c03b76d
Compare
We were marking events as "historical" if there were any attributes in the same report that have the C quality. This had a few problems: * Some devices randomly report C-quality attributes, for various reasons. * Some attributes stop having the C quality in favor of Q, in some cases, so this is not very forward-compatible. The fix is to remove this C-quality check altogether. At that point we have the following situations: 1. We're setting up a new subscription and getting our initial priming report. This case will still have _receivingPrimingReport set, and will cause any events reported as part of priming to be marked "historical". 2. We are getting a "priming" report from a server-side subscription resumption after the server timed out on a subscription. If this happens before the max-interval elapses, we will see this as a normal report on our subscription, but with all attributes and events the server knows about included. In this case, we mark ourselves as being in a "priming report" if we get an event that has an event number we have already seen, so that we mark any not-seen-before events in that report as "historical", since we don't know how long ago they are from. 3. We are getting a normal incremental report, and will not mark any such events as "historical". Since we are now keeping track of last-received event numbers, we can also use those for our subscriptions and can filter out any events that have an event number no greater than our last-observed one.
bzbarsky-apple
force-pushed
the
priming-more-restrictive
branch
from
c03b76d
to
31c5839
Compare
|
PR #36506: Size comparison from 1bdcb74 to 31c5839 Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were marking events as "historical" if there were any attributes in the same report that have the C quality. This had a few problems:
The fix is to remove this C-quality check altogether. At that point we have the following situations:
Since we are now keeping track of last-received event numbers, we can also use those for our subscriptions and can filter out any events that have an event number no greater than our last-observed one.