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

Buffered command check for feature flag? #264

Closed
stevesims opened this issue Dec 4, 2024 · 4 comments
Closed

Buffered command check for feature flag? #264

stevesims opened this issue Dec 4, 2024 · 4 comments

Comments

@stevesims
Copy link
Contributor

It may be useful to add in support to the buffered commands API for checking whether a particular feature flag has been set, or possibly if a feature flag is set to a particular value

This would be a new condition check type(s), and so work with existing conditional commands

@stevesims
Copy link
Contributor Author

An alternative to this approach of adding new conditionals would be to have an "expose flags as a buffer" flag.

This would set a flag to define a bufferId to be used to represent the flags. Once that is set, a buffer command attempting to reference that bufferId would instead get its data from the corresponding flag. This would likely be read-only, so could not be used as a target. The offset within the buffer would be used to reference a specific flag. Flags that are not defined would return zero as their values.

As flags are stored as 16-bit values, and a buffer offset points to an individual byte, the offset would probably need to be halved to match the flag ID, or conversely the flag ID doubled to find the corresponding offset. This would prevent flags with their top-bit set from being accessed, however as noted against #263 we may wish to use top-bit-set on the "set flag" command to represent a "read" anyway, so this would be OK.

Conceptually this provides a more straightforward way for buffers to interact with flags without changing the buffered commands API. The challenge comes in how to expose the flags - it would not be simple to expose the flags as a BufferStream type of object, which is essentially what the API implementation relies on. Specifically the getBuffer method which provides direct access to the byte array can't really work, and that call is the foundation of many/most API calls

@stevesims
Copy link
Contributor Author

checking for feature flags, or particular values within feature flags, could be done by adding a new upper bit to "conditional operations" to indicate that it's a flag being checked against, not a buffer.

this would remove the problems with attempting to expose flags as if they were a buffer - problematic given their 16-bit values.

it may also be useful to add flag(s) to indicate that 16-bit values should be used for comparisons, and possibly also support for 32-bit - two bits could cover the use cases of the current 8-bit, and then add 16, 32, and 16-bit flags.

@stevesims
Copy link
Contributor Author

"expose flags as a buffer" is hard. flags are potentially sparsely populated, and as noted there are issues with flag IDs being 16-bit values, and therefore offsets in a buffer would need doubling

additionally without physically assigning a chunk of 128kb of PSRAM to store the whole 16-bit flags space the pseudo-buffer to expose flags would need a fairly complex implementation to comply with the general buffer interface

it is therefore simpler to expose flags to the buffered commands API via two mechanism

  1. add support to conditional operations for checking against flags
  2. add a command to read a flag value into a buffer

arguably one could implement only this second mechanism, and extend the conditional operations to support 16-bit values

@stevesims
Copy link
Contributor Author

buffer conditionals have been extended to allow for checking against 16-bit values, and also checking against values stored in a feature flag.

a command has also been added to copy the value of a feature flag into a buffer at a given offset

these features were added in #285

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

No branches or pull requests

1 participant