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

Subscription id handling in ReadClient is weird #21594

Closed
bzbarsky-apple opened this issue Aug 3, 2022 · 1 comment
Closed

Subscription id handling in ReadClient is weird #21594

bzbarsky-apple opened this issue Aug 3, 2022 · 1 comment
Assignees
Labels
Interaction Model Work spec Mismatch between spec and implementation V1.0

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

In ReadClient, we:

  1. Read and try to verify SubscriptionId values even if we're not a subscription. This is pretty broken.
  2. Enforce that the SubscriptionId in a Subscribe Response matches the previous Report Data, and fail if it does not. The spec does not say to do this. This might need to be a spec issue instead. It's really not clear to me why the spec has a SubscriptionId in Subscribe Response at all, given that it's in the Report Data bits....
  3. Have an mIsPrimingReports that seems to be used only for mSubscriptionId management and does not have clearly defined semantics. It does have a name that suggests it should be true during the entire priming read, though, which it's not....

Proposed Solution

Align with spec, make code understandable, add tests.

@bzbarsky-apple bzbarsky-apple added V1.0 spec Mismatch between spec and implementation Interaction Model Work labels Aug 3, 2022
@yunhanw-google
Copy link
Contributor

@yunhanw-google yunhanw-google self-assigned this Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Interaction Model Work spec Mismatch between spec and implementation V1.0
Projects
None yet
Development

No branches or pull requests

2 participants