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

Add cycler method for cycling change the resolution and transform (New) #1576

Merged
merged 14 commits into from
Dec 3, 2024

Conversation

hanhsuan
Copy link
Contributor

@hanhsuan hanhsuan commented Nov 4, 2024

Description

According to the #855 , parsing the output from gnome_randr is not acceptable. Therefore, add cycler method for cycling change the resolution and transform to instead of modifying the old gnome_randr_cycle.py.

Resolved issues

The private bug

Documentation

Tests

@hanhsuan hanhsuan changed the title Add cycler method for cycling change the resolution and transform Add cycler method for cycling change the resolution and transform (New) Nov 4, 2024
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 48.00%. Comparing base (7b5dd02) to head (020c0cc).
Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
...box-support/checkbox_support/dbus/gnome_monitor.py 93.10% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1576   +/-   ##
=======================================
  Coverage   48.00%   48.00%           
=======================================
  Files         371      370    -1     
  Lines       39833    39832    -1     
  Branches     6730     6728    -2     
=======================================
  Hits        19121    19121           
  Misses      19994    19994           
+ Partials      718      717    -1     
Flag Coverage Δ
checkbox-support 60.73% <96.42%> (+0.01%) ⬆️

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.

@hanhsuan hanhsuan marked this pull request as ready for review November 4, 2024 02:54
GabrielChenCC added a commit to GabrielChenCC/checkbox that referenced this pull request Nov 7, 2024
Copy link
Collaborator

@p-gentili p-gentili left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! A few suggestions below, mostly for clearness.

Do you have a test job we can use for testing?

checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
checkbox-support/checkbox_support/dbus/gnome_monitor.py Outdated Show resolved Hide resolved
Co-authored-by: Paolo Gentili <paolo.gentili@canonical.com>
@hanhsuan
Copy link
Contributor Author

hanhsuan commented Nov 27, 2024

Thanks for working on this! A few suggestions below, mostly for clearness.

Do you have a test job we can use for testing?

The example code is

#!/usr/bin/env python3

from collections import namedtuple
from typing import List
from fractions import Fraction
from checkbox_support.monitor_config import MonitorConfig
from checkbox_support.helpers import display_info
import subprocess
import time

Mode = namedtuple("Mode", ["id", "resolution", "is_preferred", "is_current"])

def resolution_filter(modes: List[Mode]):
    new_modes = []
    tmp_resolution = []
    sort_modes = sorted(
        modes, key=lambda m: int(m.resolution.split("x")[0]), reverse=True
    )
    for m in sort_modes:
        width, height = [int(x) for x in m.resolution.split("x")]
        aspect = Fraction(width, height)
        if width < 675 or width / aspect < 530:
            continue
        if m.resolution in tmp_resolution:
            continue
        if len(new_modes) >= 5:
            break
        new_modes.append(m)
        tmp_resolution.append(m.resolution)
    return new_modes

def action(filename, **kwargs):
    print(filename)
    time.sleep(5)
    subprocess.check_output(["sudo", "keyboard_mouse"])
    subprocess.check_output(["gnome-screenshot", "-f", filename])

def cycle_test():
    try:
        monitor_config = display_info.get_monitor_config()
    except ValueError as exc:
        raise SystemExit("Current host is not support")

    monitor_config.cycle(res=False, resolution_filter=None, transform=True, action=action)

if __name__ == "__main__":
    cycle_test()

the source code of keyboard_mouse command is in #1582

@fernando79513 fernando79513 added the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Nov 28, 2024
@hanhsuan hanhsuan requested a review from p-gentili December 3, 2024 01:20
@hanhsuan hanhsuan removed the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Dec 3, 2024
Copy link
Collaborator

@p-gentili p-gentili left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for all the changes!

@p-gentili p-gentili merged commit 399280a into main Dec 3, 2024
41 checks passed
@p-gentili p-gentili deleted the add_cycle_change_to_gnome_monitor branch December 3, 2024 09:33
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