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 DBus plugin and its child plugins #290

Open
wants to merge 14 commits into
base: beta
Choose a base branch
from

Conversation

heddxh
Copy link
Contributor

@heddxh heddxh commented Apr 17, 2024

Working on the following:

  • refactor parent class of DBus plugin, avoid using cli but using pyside6 dbus package, since some distros like Arch using qdbus6 as binary name but some distros are not.
  • Port wallpaper(_kde) plugin to use DBus
  • Port gtk-kde-plugin to the new DBus plugin, using dconf and xsettingsd as fallback
  • konsole-plugin to the new DBus plugin

@heddxh
Copy link
Contributor Author

heddxh commented Apr 17, 2024

By the way, is it possible to push the linter/formater rules to the repo?

Copy link
Contributor Author

@heddxh heddxh left a comment

Choose a reason for hiding this comment

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

This function does nothing if the root logger already has handlers configured, unless the keyword argument force is set to True.

Refer to logging doc, the basicConfig() is called after addHandler so it doesn't work. Can we only add the notification handler if __debug__ is false?

@l0drex
Copy link
Collaborator

l0drex commented Apr 17, 2024

By the way, is it possible to push the linter/formater rules to the repo?

We are using flake8 with default configurations. Do you want to change something?

@l0drex
Copy link
Collaborator

l0drex commented Apr 17, 2024

Refer to logging doc, the basicConfig() is called after addHandler so it doesn't work.

Can't we just add the handlers after the config?

@heddxh
Copy link
Contributor Author

heddxh commented Apr 17, 2024

yes that's ok, but when debugging we maybe don't need the notification?

@heddxh
Copy link
Contributor Author

heddxh commented Apr 17, 2024

We are using flake8 with default configurations. Do you want to change something?

My flake8 complains a lot like line length and sort order. And I should manually turn off my formatter(black) in case it makes a lot of changes across code files. It's not a big problem but if we use the same formatter life will be easier😃

@l0drex
Copy link
Collaborator

l0drex commented Apr 18, 2024

yes that's ok, but when debugging we maybe don't need the notification?

I'd like to keep it in, just to keep the variances between developer version and normal user version low. Also, it might help if something goes wrong at an unexpected time.

@l0drex
Copy link
Collaborator

l0drex commented Apr 18, 2024

My flake8 complains a lot like line length and sort order. And I should manually turn off my formatter(black) in case it makes a lot of changes across code files. It's not a big problem but if we use the same formatter life will be easier😃

I see. I don't use any automatic formatter, but I also have many warnings from flake8 as I don't regularly run it (which I probably should). So yeah, please push it! Maybe in a different PR though, with all files formatted.

@l0drex
Copy link
Collaborator

l0drex commented Apr 18, 2024

@heddxh
Copy link
Contributor Author

heddxh commented Apr 19, 2024

Might be interesting: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Wallpaper.html

Surprised to know even wallpaper has its portal. I will change my code to use it.

@heddxh
Copy link
Contributor Author

heddxh commented Apr 20, 2024

Might be interesting: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Wallpaper.html

I only found xdg-portal-gtk impl this interface: https://github.com/flatpak/xdg-desktop-portal-gtk/blob/main/src/wallpaper.c , but can't find it on DBus session bus.

图片

Also I don't see any similar/related code for xdg-portal-kde. But they have plasma-apply-wallpaperimage and can apply wallpaper to all virtual desktop, maybe I will use that.

@l0drex
Copy link
Collaborator

l0drex commented Apr 22, 2024

Have you tried to use setWallpaper in org.kde.kuiserver: PlasmaShell/org.kde.PlasmaShell in the KDE plugin? It is also available in org.kde.plasmashell.

I prefer to use the dbus over the command line, as I hope that we can get rid of the command line completely and can remove the flatpak permission at some point.

@heddxh
Copy link
Contributor Author

heddxh commented Apr 22, 2024

It seems like plasma-apply-wallpaperimage just using dbus under the hood, so it's work fine!
But for Gnome, dbus-monitor suggests that they use Gvariant as serialization data 😢 , so I don't know how to get rid of gsettings...

@l0drex
Copy link
Collaborator

l0drex commented Apr 22, 2024

There is a variant class in qts dbus lib

@heddxh
Copy link
Contributor Author

heddxh commented Apr 23, 2024

emmm, I don't know how to get this(I change /org/gnome/desktop/background/picture-options in dconf-editor and below is from dbus-monitor):

method call time=1713841847.551184 sender=:1.562 -> destination=ca.desrt.dconf serial=48 path=/ca/desrt/dconf/Writer/user; interface=ca.desrt.dconf.Writer; member=Change
   array of bytes [
      2f 6f 72 67 2f 67 6e 6f 6d 65 2f 64 65 73 6b 74 6f 70 2f 62 61 63 6b 67
      72 6f 75 6e 64 2f 70 69 63 74 75 72 65 2d 6f 70 74 69 6f 6e 73 00 00 00
      2e 31
   ]
signal time=1713841847.581724 sender=:1.53 -> destination=(null destination) serial=5 path=/ca/desrt/dconf/Writer/user; interface=ca.desrt.dconf.Writer; member=Notify
   string "/org/gnome/desktop/background/picture-options"
   array [
      string ""
   ]
   string ":1.53:user:1"          

And unluckily: https://forum.qt.io/topic/140253/uint-argument-in-dbus-call-with-python/ . It seems if there is a method need uint as arguments , then we can't call it unless involve in another dependency like https://pypi.org/project/uint

@heddxh
Copy link
Contributor Author

heddxh commented Apr 23, 2024

Such as KDE's setWallpaper:
图片

screenNum take a uint32 😢

@l0drex
Copy link
Collaborator

l0drex commented Apr 23, 2024

It seems if there is a method need uint as arguments , then we can't call it unless involve in another dependency like https://pypi.org/project/uint

Seems like I already had that problem somewhere, as I have created that post 😆
I probably should start documenting when I tried something, and it didn't work out.

Either way, in that case, I would prefer to stick to the old implementation using the dbus of the KDE wallpaper plugin.

@heddxh
Copy link
Contributor Author

heddxh commented Apr 23, 2024

Seems like I already had that problem somewhere, as I have created that post 😆

I didn't find out until you said it lol😂
Don't know why QT guys let such a regression remain till now... and reading their documentation is like a nightmare.

@heddxh
Copy link
Contributor Author

heddxh commented Apr 25, 2024

Still working on adding some tests and cleaning my code

# send signal to read new config
subprocess.run(['killall', '-HUP', 'xsettingsd'])

# change dconf db. since dconf sending data as GVariant, use gsettings instead
subprocess.run(['gsettings', 'set', 'org.gnome.desktop.interface', 'gtk-theme', f'{theme}'])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't get rid of external command here. But since they are fallback method, can we avoid running them when under flatpak build? (Not read #185 yet, is there an easy way to detect that?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I think I got it. Waiting for that PR to be merged

@heddxh heddxh marked this pull request as ready for review April 28, 2024 11:07
yin_yang/plugins/_plugin.py Outdated Show resolved Hide resolved
@l0drex l0drex mentioned this pull request May 9, 2024
yin_yang/plugins/_plugin.py Outdated Show resolved Hide resolved
@heddxh
Copy link
Contributor Author

heddxh commented Jun 9, 2024

Today I discover that I should edit katerc manully to make konsole profile within has correct color scheme...
Maybe other KDE applications need similar terrible hacking 😢

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