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

live-input-configuation #3531

Merged
merged 7 commits into from
Sep 3, 2024
Merged

live-input-configuation #3531

merged 7 commits into from
Sep 3, 2024

Conversation

AlanGriffiths
Copy link
Collaborator

@AlanGriffiths AlanGriffiths commented Aug 7, 2024

Allow servers to dynamically update input configuration.

There's a very simple "mir_demo_server.input" addition to mir_demo_server to exercise part of this API, but that is not intended prototype.

@AlanGriffiths
Copy link
Collaborator Author

AlanGriffiths commented Aug 7, 2024

This is still not complete:

  • There's no code/tests to exercise this API
  • There's no facility to recover the default or configured configuration

@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from c8a1430 to fd8d0bc Compare August 7, 2024 16:38
@AlanGriffiths AlanGriffiths mentioned this pull request Aug 9, 2024
@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from 6a9c9f5 to 4fd4248 Compare August 9, 2024 16:18
@AlanGriffiths AlanGriffiths changed the base branch from main to ReloadingConfigFile August 9, 2024 16:20
@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from 4fd4248 to 155b388 Compare August 16, 2024 15:37
@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

Recap of discussion:

  • input configuration should remain in the main .config file
  • we're leaning towards SIGHUP-reloading of the main config file
    • incrementally we'll enable live reinterpretation of the individual options
    • initial target are the input configuration options
    • some might not ever become re-interpretable
  • as today, environment variables and command line takes precedence, basically making those static

@AlanGriffiths
Copy link
Collaborator Author

  • input configuration should remain in the main .config file

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

Agree, that will have to come from the shell implementation, e.g. integrating gsettings.

@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from 155b388 to 9d7fbdb Compare August 20, 2024 13:34
@AlanGriffiths
Copy link
Collaborator Author

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

Agree, that will have to come from the shell implementation, e.g. integrating gsettings.

Right, so the input configuration needs to be exposed (like this WIP), not "remain in the main .config file"

@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from 9d7fbdb to 65c61fe Compare August 20, 2024 13:45
@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from 65c61fe to edfbc68 Compare August 20, 2024 16:58
@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

Right, so the input configuration needs to be exposed (like this WIP), not "remain in the main .config file"

Sorry, different trains of thought. …as opposed to a dedicated file.

Base automatically changed from ReloadingConfigFile to main August 21, 2024 13:39
@AlanGriffiths AlanGriffiths marked this pull request as ready for review August 21, 2024 16:21
@AlanGriffiths AlanGriffiths requested a review from a team as a code owner August 21, 2024 16:21
@Saviq Saviq requested a review from mattkae August 22, 2024 08:05
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor things, but seems like a good change overall! Now we just need to get the keyboard stuff in there too :)

examples/mir_demo_server/server_example.cpp Show resolved Hide resolved
examples/mir_demo_server/server_example.cpp Show resolved Hide resolved
src/miral/input_config.h Outdated Show resolved Hide resolved
src/miral/input_configuration.cpp Show resolved Hide resolved
@AlanGriffiths AlanGriffiths force-pushed the live-input-configuation branch from e859f60 to 551eb06 Compare August 23, 2024 11:43
@AlanGriffiths AlanGriffiths requested a review from mattkae August 23, 2024 15:16
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a sensible change form the options that I tested!

@mattkae mattkae enabled auto-merge August 23, 2024 17:27
@mattkae mattkae added this pull request to the merge queue Sep 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 3, 2024
@AlanGriffiths
Copy link
Collaborator Author

Seems I'm plagued with #3581 today

@AlanGriffiths AlanGriffiths added this pull request to the merge queue Sep 3, 2024
Merged via the queue into main with commit ef592f8 Sep 3, 2024
37 checks passed
@AlanGriffiths AlanGriffiths deleted the live-input-configuation branch September 3, 2024 13:26
AlanGriffiths added a commit that referenced this pull request Sep 9, 2024
Allow servers to dynamically update input configuration.

There's a very simple "mir_demo_server.input" addition to
`mir_demo_server` to exercise part of this API, but that is not intended
prototype.
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

Successfully merging this pull request may close these issues.

3 participants