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

Refactor GUI related code using MVP pattern #2395

Open
1 of 5 tasks
MattHag opened this issue Mar 16, 2024 · 2 comments
Open
1 of 5 tasks

Refactor GUI related code using MVP pattern #2395

MattHag opened this issue Mar 16, 2024 · 2 comments

Comments

@MattHag
Copy link
Collaborator

MattHag commented Mar 16, 2024

The GUI implementations seem tightly coupled to the business logic and model, which does the hard work behind the scenes. That hard dependency on GTK is not testable and hard to maintain. As is, it is hard to add new features.

Proposed solution

A conversion to the Model-View-Presenter architecture (video) seems the best approach to make the code future-proof, as it enforces cleaner code than Model-View-Controller.

Goals

  • Separation of concerns: Extract business logic from GUI code.
  • Write testable code
  • Ease upgrade to GTK 4 (or other GUI frameworks) by decoupling. No tight coupling with business logic.

Tasks

The refactoring can be split up as follows:

Solaar version: 1.1.11

MattHag added a commit to MattHag/Solaar that referenced this issue Mar 16, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 16, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 16, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 16, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 16, 2024
@pfps
Copy link
Collaborator

pfps commented Mar 16, 2024

Consider config_panel.

Basically, config_panel shows a sequence of boxes of various kinds for a device.

Each setting box shows the current value of a setting, created by _create_sbox. The most important part is a Control, which shows the value and has callbacks to change the value.

The basic part of changing a value is handled by the update method, which calls _write_async where the real work is done inside _do_write. The actual change is done by the write or write_key_value of the setting. There is housekeeping done by _write_async before the write and housekeeping done after the write by _update_setting_item, which also displays the actual new value.

The initial value shown in a setting box is set up by _read_async which instead of writing a new value via the setting instead reads the current value and then uses _update_setting_item to display it.

There are some external interfaces as well - change_setting and record_setting.

But the interface to the device is already quite simple - it's the read and write methods (and their variations) of settings. So setting changes from the GUI are done through this interface and can be tested without involving the GUI at all. There are a couple of things that should be done to further separate the GIU from the low-level code. There should be better separation for LEDEffectSettings and it would be useful to have a settings_constants module so that there is no need to import from settings.

So how does this fit into a Model-View-Presenter paradigm? I don't know, but if Solaar is to change to this paradigm this is one of the places that need to be addressed. If the paradigm doesn't work for this panel then it's not going to be very useful to use the paradigm elsewhere so it seems to me that this is a good place to start. The other alternative would be to start with the rules stuff. In either case, going after the parts of the GUI that are much simpler isn't going to provide evidence that the paradigm is going to work for Solaar.

@MattHag
Copy link
Collaborator Author

MattHag commented Mar 16, 2024

MVP is one out of 3 basic patterns for GUI design. So it will certainly work ;) There's nothing of which I fear here. The view can hold all references to the sub views and the pattern can handle asynchronous things, as already used for pairing. Going from easy to hard is good to get used to it and get it successfully done.

I added a nice article, which also shows the difference to the Model-View-Controller, which would be the fallback if code complexity would get unbearable.

MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
Mocks view and thus does not require a GUI framework for tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
Mocks view and thus does not require a GUI framework for tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
Mocks view and thus does not require a GUI framework for tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
Mocks view and thus does not require a GUI framework for tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 6, 2024
Mocks view and thus does not require a GUI framework for tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 10, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 16, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Apr 16, 2024
Mocks view and thus does not require a GUI framework to run tests.

Related pwr-Solaar#2395
@MattHag MattHag changed the title Refactor GUI code to use Model-View-Presenter pattern Refactor GUI related code using MVP pattern Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants