Set extend mode after plugging a new monitor (BugFix)

p-gentili commented 3 months ago

Description

The Zapper EDID cycling job is failing on laptops because Mirror mode is making the external monitor match the internal display resolution. Like this:

Testing EDID cycling on HDMI-1
switching EDID to 1920x1080
PASS
switching EDID to 1280x1024
FAIL, got 1920x1080 but 1280x1024 expected
switching EDID to 2560x1440
FAIL, got 1920x1080 but 2560x1440 expected

gnome-randr can't be used for applying monitor configurations, and in general having two utilities (xrandr and gnome-randr) for x11 and wayland doesn't help with maintainability of this test.

This PR relies on DBus and Mutter to retrieve the current monitor configuration and apply extended mode every time we switch EDID.

Resolved issues

Resolves ZAP-677

Documentation

Up to date

Tests

Tested on 202302-31240 ~~side-loading the provider~~ running from source. Previously failing like above (ref).

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 95.42857% with 8 lines in your changes missing coverage. Please review.

Project coverage is 44.27%. Comparing base (76939c4) to head (3ea1eae). Report is 30 commits behind head on main.

Files	Patch %	Lines
providers/base/bin/edid_cycle.py	80.76%	4 Missing and 1 partial :warning:
...heckbox-support/checkbox_support/parsers/xrandr.py	95.83%	0 Missing and 2 partials :warning:
...box-support/checkbox_support/dbus/gnome_monitor.py	96.55%	0 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1286 +/- ## ========================================== + Coverage 44.10% 44.27% +0.17% ========================================== Files 358 364 +6 Lines 38765 38897 +132 Branches 6571 6584 +13 ========================================== + Hits 17096 17222 +126 - Misses 21006 21007 +1 - Partials 663 668 +5 ``` | [Flag](https://app.codecov.io/gh/canonical/checkbox/pull/1286/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | Coverage Δ | | |---|---|---| | [checkbox-support](https://app.codecov.io/gh/canonical/checkbox/pull/1286/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `54.07% <97.98%> (+1.50%)` | :arrow_up: | | [provider-base](https://app.codecov.io/gh/canonical/checkbox/pull/1286/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `18.40% <80.76%> (-0.17%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

p-gentili commented 3 months ago

A few changes below, but I've tried both this and the "original" script on my machine and it doesn't seem to work with i3 (unsurprisingly, as it doesn't use Mutter). My question is, how limiting is this? We do use Checkbox on non-mutter platforms as well (like Mir for example). Could this be a problem? Is it possible to somehow calculate the OBJECT_PATH, NAME and INTERFACE to "fix" this?

With the current implementation, relying on xrandr and gnome-randr, we are covering everything X11 + Gnome Wayland. With the latest few commits I re-included support for X11 via xrandr so that the coverage is the same. I still prefer to use Mutter over xrandr on Gnome because I feel like it should handle corner cases better (let me know if you're against this choice). We'll need to add support for other Wayland compositors, such as Mir or wlroots-based.

Got part of the xrandr parser from https://github.com/canonical/checkbox/pull/1229 and added support for setting a new display configuration, in extended mode.

Both handlers inherit from the same abstract class so that the interface is the same and the EDID test itself doesn't care about the actual system.

Tested on:

http://certification.canonical.com/hardware/202203-30003 (20.04, X11, GNOME and not GNOME)
https://certification.canonical.com/hardware/202302-31240 (22.04, wayland, GNOME)

canonical / checkbox