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

Implement better buffer abstractions in Silicon Labs code #2168

Closed
bzbarsky-apple opened this issue Aug 14, 2020 · 1 comment · Fixed by #3575
Closed

Implement better buffer abstractions in Silicon Labs code #2168

bzbarsky-apple opened this issue Aug 14, 2020 · 1 comment · Fixed by #3575
Assignees
Milestone

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

Silicon Labs code has abstractions for reading/writing buffers (things like emberAfPutInt8uInResp and emberAfGetInt16u) but these are not perfect in terms of abstracting away the storage. The emberAfPut*InResp ones are probably OK as long as the caller does not desugar larger things into bytes by hand, since they already update the written length themselves. The emberAfGet* APIs have an obvious problem: they don't indicate how many bytes of buffer they consumed, so the consumer has to make assumptions about how much to advance the read pointer. This is OK if the storage is fixed-width, but if we move to TLV or CBOR that won't really work.

Proposed Solution

I think we should do several things, probably after we have landed all the clusters we want:

  1. Introduce a struct of some sort that encapsulates "the buffer we are reading right now" and maintains at minimum the current read position and how many more bytes can be read.
  2. Switch all the places to-be-read buffers are passed through (command dispatch, command implementations as needed, etc) to this new struct. This would include the emberAfGet* APIs, which would be fixed to advance the read pointer themselves so consumers don't have to.
  3. Introduce Get and Put APIs for higher-level constructs like "attribute id", not just integer sizes. Under the hood this can call the appropriate integer thing, but make it clearer what we're really reading/writing.

We may also want to rename the APIs to nix "ember", and possibly move them into code that's built as part of libCHIP, so encoder/decoder code (which is built into libCHIP) can use them.

@woody-apple @CamWms @bhaskar-apple @rwalker-apple

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.94. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

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.

3 participants