Skip to content

EDID test fails at maximum resolution (BugFix)#1393

Merged
Hook25 merged 6 commits intomainfrom
extend-to-maximum-resolution
Aug 7, 2024
Merged

EDID test fails at maximum resolution (BugFix)#1393
Hook25 merged 6 commits intomainfrom
extend-to-maximum-resolution

Conversation

@p-gentili
Copy link
Copy Markdown
Collaborator

@p-gentili p-gentili commented Aug 6, 2024

Description

Another PR dedicated to the EDID job. It turns out that the new implementation (#1286) shed some light on the other related problem, where older machines could read the maximum allowed resolution.

In case where the HW doesn't support an available mode from the monitor EDID, no preferred mode is available (which makes sense). This PR takes care of such scenario, attempting to configure the monitor with the maximum available resolution.

  1. in case the maximum allowed resolution still matches what the test requested, the assertion over the actual configuration is performed as usual. Common example is with unsupported refresh rate.
  2. in case the maximum allowed resolution is lower than requested, the testcase can be skipped since we can assume the HW does not support it. Common example is 2K resolution with HDMI < 1.3

Resolved issues

Resolves ZAP-678

Documentation

N/A

Tests

Tested on some machines.

Previous behaviour:

Running from source:

Testing EDID cycling on HDMI-4
switching EDID to 1280x1024
PASS
switching EDID to 2560x1440
SKIP, max available was 2048x1152
switching EDID to 1920x1080
PASS

It's common to have a mismatch between the available modes
at EDID level and what the device actually supports. This commit
selects the maximum allowed resolution and returns the applied
configuration.
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 5 lines in your changes missing coverage. Please review.

Project coverage is 45.18%. Comparing base (7ab7c0f) to head (8c08ee4).
Report is 125 commits behind head on main.

Files with missing lines Patch % Lines
...box-support/checkbox_support/dbus/gnome_monitor.py 76.92% 2 Missing and 1 partial ⚠️
...heckbox-support/checkbox_support/monitor_config.py 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1393      +/-   ##
==========================================
+ Coverage   45.12%   45.18%   +0.05%     
==========================================
  Files         366      366              
  Lines       39058    39102      +44     
  Branches     6607     6609       +2     
==========================================
+ Hits        17626    17669      +43     
- Misses      20758    20760       +2     
+ Partials      674      673       -1     
Flag Coverage Δ
checkbox-support 59.69% <89.36%> (+0.25%) ⬆️
provider-base 18.65% <100.00%> (+0.03%) ⬆️

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.

@p-gentili p-gentili changed the title Extend to maximum resolution (BugFix) EDID test fails at maximum resolution (BugFix) Aug 6, 2024
@Hook25 Hook25 self-assigned this Aug 7, 2024
Hook25
Hook25 previously approved these changes Aug 7, 2024
Copy link
Copy Markdown
Collaborator

@Hook25 Hook25 left a comment

Choose a reason for hiding this comment

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

See if you like the change proposed below but it makes sense

Co-authored-by: Massimiliano <massimiliano.girardi@canonical.com>
@p-gentili p-gentili force-pushed the extend-to-maximum-resolution branch from d067982 to 8c08ee4 Compare August 7, 2024 08:22
@Hook25 Hook25 merged commit 904692d into main Aug 7, 2024
@Hook25 Hook25 deleted the extend-to-maximum-resolution branch August 7, 2024 08:32
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.

2 participants