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

[4pt, 0pt] Staging settings with a commit token #174

Closed
vimpostor opened this issue Sep 29, 2023 · 5 comments · Fixed by #456
Closed

[4pt, 0pt] Staging settings with a commit token #174

vimpostor opened this issue Sep 29, 2023 · 5 comments · Fixed by #456
Assignees

Comments

@vimpostor
Copy link
Contributor

Right now the multiplexed settings only have a single storage of all the active settings.
As detailed in #99 (review), a new "staging" API could allow staging settings with a commit token (e.g. a std::string) such that they are first stored in a background "staging" collection, instead of in the active settings collection.

Then later on, the staged settings can be transported to the actually active collection of the ctx_settings via multiple ways:

  • It could be activated explicitly through the settings API by passing the unique commit token
  • Implicit activation via a tag_t on one of the streaming ports
  • Implicit activation via a pmt_t message on one of the message ports

Some things to consider:

  • Before staging the settings, they should be validated, i.e. check if the keys actually map to existing struct member fields in the node and check for correct types and potential Range constraints (e.g. a field might want to only accept positive integers)
  • Instead of using just a simple string as a commit token, we potentially might want to generate a unique hash - in a git-like fashion
@dantti
Copy link
Contributor

dantti commented Oct 7, 2024

Hi @drslebedev , Ralph on fair-acc/opendigitizer#212 mentioned that I should coordinated with you about that task, is it something I should wait for you to finish? Or is there something I could start working on? I guess the gui part could be started

@drslebedev
Copy link
Contributor

Hi @dantti , most of the issues are already implemented and merged. The only missing part is adding stored settings via Message port. But you can start working on UI.

@dantti
Copy link
Contributor

dantti commented Oct 17, 2024

UI wise it's mostly done https://github.com/fair-acc/opendigitizer/tree/dantti/blocks_context it seems we need BlockControlsPanelContext changed to have a map<string, DigitizerUi::Block*> ?

@drslebedev
Copy link
Contributor

Thank you very much for your commit! Is there anything else you need from our side to complete the issue and submit a PR?

@drslebedev
Copy link
Contributor

We propose extending the Message API to support updated CtxSettings with the following functionalities:

  • Get settings for a specific context
  • Set settings for a specific context
  • Activate a context (introduce a new Block::property)
  • Get a list of available contexts (introduce a new Block::property)

Our goal is to minimize the API surface. For example, if the UI needs to retrieve all stored settings for each context, it can do so by first getting the list of available contexts and then fetching the settings for each context individually.

@drslebedev drslebedev linked a pull request Nov 7, 2024 that will close this issue
@drslebedev drslebedev changed the title [4pt] Staging settings with a commit token [0pt] Staging settings with a commit token Nov 12, 2024
@RalphSteinhagen RalphSteinhagen changed the title [0pt] Staging settings with a commit token [4pt, 0pt] Staging settings with a commit token Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA-Accepted/Merged (∞)
Development

Successfully merging a pull request may close this issue.

4 participants