Add named attribute access to gnome-monitor.py (New)

[tomli380576]

Description

This PR adds complete type annotations and named properties to the original gnome-monitors script to hopefully make it more ergonomic to use. It's not something that needs to be urgently merged so don't worry about this if you have a fire to fix xD

Basically turns this:

state = MonitorConfigGnome().get_current_state()
curr_refresh_rates = {} 

for monitor in state[1]:
    for mode in monitor[1]:
        if mode[6].get("is-current", False):
            curr_refresh_rates[monitor[0][0]] = mode[3]

print(curr_refresh_rates)

into this:

state = MonitorConfigGnome().get_current_state()
curr_refresh_rates = {}

for monitor in state.physical_monitors:
    for mode in monitor.modes:
        if mode.is_current:
            curr_refresh_rates[monitor.info.connector] = mode.refresh_rate

print(curr_refresh_rates)

with nice IDE autocomplete.

#1843 could take advantage of this to more directly access resolution numbers instead of strings

Resolved issues

In the original MonitorConfigGnome, accessing deeply nested properties from the GetCurrentState's return value was a bit awkward. If we need to get the 1st refresh rate of the 1st monitor, we need to write state[1][1][0][1][0][3] which looks like magic numbers without looking at the original dbus xml. The original MonitorConfigGnome provided nice ways to get the resolution strings but all the other properties were not included.

This PR circumvents this magic index situation by providing named attribute access for the return value of GetCurrentState, so now we can write state.physical_monitors[0].modes[0].refresh_rate and only use indices in places where there's an actual array.

Documentation

The main reference point is this xml and gnome-randr

None of the new classes (MutterDisplayConfig, PhysicalMonitor, LogicalMonitor, MonitorInfo) should be directly constructed because they assume that the input GLib.Variant has the correct type. Use MonitorConfigGnome.get_current_state() as the entry point since it asks GLib to do a deep type check first before returning.

• Is there a way to enforce this?

Tests

Original unit tests

[codecov]

Codecov Report

❌ Patch coverage is 91.15646% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.99%. Comparing base (c5c19c7) to head (fb7d373).
⚠️ Report is 180 commits behind head on main.

Files with missing lines	Patch %	Lines
...box-support/checkbox_support/dbus/gnome_monitor.py	87.96%	10 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1916      +/-   ##
==========================================
+ Coverage   50.46%   50.99%   +0.53%     
==========================================
  Files         382      384       +2     
  Lines       41039    41591     +552     
  Branches     6892     7106     +214     
==========================================
+ Hits        20709    21208     +499     
- Misses      19585    19618      +33     
- Partials      745      765      +20     

Flag	Coverage Δ
checkbox-support	65.06% <91.15%> (+2.55%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hanhsuan]

Please let me know, if there is some misunderstanding. Thanks.

[hanhsuan]

LGTM.
Modify the randr_cycle.py as following

from checkbox_support.dbus.gnome_monitor import MutterDisplayMode as Mode

and

if args.cycle == "resolution":
            monitor_config.cycle(
                cycle_resolution=True,
                resolution_filter=resolution_filter,
                cycle_transform=False,
                post_cycle_action=action,
                path=screenshot_path,
            )
        elif args.cycle == "transform":
            monitor_config.cycle(
                cycle_resolution=False,
                resolution_filter=resolution_filter,
                cycle_transform=True,
                post_cycle_action=action,
                path=screenshot_path,
            )
        else:
            monitor_config.cycle(
                cycle_resolution=True,
                resolution_filter=resolution_filter,
                cycle_transform=True,
                post_cycle_action=action,
                path=screenshot_path,
            )

are working as expected.

[fernando79513]

Thank you for this change, It looks like a really nice quality of life improvement.
I just left two small optional comments but overall it LGTM.

I think this can be merged after implementing the suggestion of hanhsuan.
Regarding #1843, should this PR be merged before?

[hanhsuan]

I think this can be merged after implementing the suggestion of hanhsuan. Regarding #1843, should this PR be merged before?

Yes, this one could be merged first since #1843 hasn't been reviewed by anyone and the script hasn't been used by any test case. I could rebase my PR to adapt it to this one, which will simplify the process.

[hanhsuan]

LGTM