search

daitj / gnome-display-brightness-ddcutil

Display brightness slider for gnome shell using ddcutil backend
GNU General Public License v3.0
294 stars 36 forks source link

GNOME 45 support #111

Closed yochananmarqos closed 11 months ago

yochananmarqos commented 1 year ago

There are breaking changes which will require a separate release for 45 and <=44:

See https://gjs.guide/extensions/upgrading/gnome-shell-45.html

maniacx commented 1 year ago

There are breaking changes which will require a separate release for 45 and <=44:

See https://gjs.guide/extensions/upgrading/gnome-shell-45.html

@yochananmarqos Hi. you are here too!! :) Test the pull request. And see if it works.

https://github.com/daitj/gnome-display-brightness-ddcutil/pull/112

yochananmarqos commented 1 year ago

@maniacx Yes, it works fine on GNOME 45.rc. :+1:

Tamas-Toth-ebola commented 1 year ago

I can confirm also that @maniacx 's version works on the latest Ubuntu 23.10 beta, with almost GNOME 45, so please accept the PR and release it :smile:

daitj commented 1 year ago

@maniacx and others

I got some feedback from GSE and I have fixed those issues, could you guys check it again if it work?

maniacx commented 1 year ago

@daitj Just tested it now. It doesnt work with the latest commit "GSE review changes"

Verified changing brightness manually using command line ddcutil setvcp 20 100

On Reverting "GSE review changes" the extension works again. I have noticed that you have removed functions clearInterval, clearTimeout, setInterval, setTimeout, from convenienceExt

but still calling them in extension.js

daitj commented 1 year ago

Yeah I was told by the reviewer, that those functions are available by default. GNOME 45 is supposed to support all basic JS functions. Do you see any error logs ?

You need to enable Verbose debugging from this extension's settings and then reload this extension first.

journalctl --no-pager -b /usr/bin/gnome-shell

Run above command and paste relevant the log here

maniacx commented 1 year ago

No error when I enable the extension. But when I change the slider, I have error logged below. (verbose logging is still off though)

JS ERROR: Error: GSettings key [object GjsGlobal] not found in schema org.gnome.shell.extensions.display-brightness-ddcutil _checkKey@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:779:23 createCheckedMethod/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:729:30 setInterval@resource:///org/gnome/gjs/modules/esm/_timers.js:93:36 ddcWriteInQueue@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:149:52 ddcWriteCollector@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:184:18 setBrightness@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:211:14 onSliderChange@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:259:18 sliderValueChangeCommon@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/indicator.js:57:14 _SliderChange@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/indicator.js:264:32 set value@resource:///org/gnome/shell/ui/barLevel.js:61:14 _moveHandle@resource:///org/gnome/shell/ui/slider.js:212:9 startDragging@resource:///org/gnome/shell/ui/slider.js:92:14 vfunc_button_press_event@resource:///org/gnome/shell/ui/slider.js:69:21 @resource:///org/gnome/shell/ui/init.js:21:20

maniacx commented 1 year ago

With verbose debugging on.

Below are the logs when extension is enabled


display-brightness-ddcutil extension:
Adding to panel
display-brightness-ddcutil extension:
Add keyboard shortcuts
display-brightness-ddcutil extension:
ddcutil brief info:
Display 1
   I2C bus:          /dev/i2c-19
   DRM connector:    card2-HDMI-A-1
   Monitor:          GSM:LG ULTRAGEAR:308MXZJ11293

display-brightness-ddcutil extension:
ddcutil brief info found bus line:
    I2C bus:          /dev/i2c-19
display-brightness-ddcutil extension:
ddcutil reading display state for bus: 19
display-brightness-ddcutil extension:
ddcutil reading display status for bus: 19 is: VCP D6 SNC x01

display-brightness-ddcutil extension:
Reloading widgets

Below are the log when I try to change brightness

display-brightness-ddcutil extension:
display LG ULTRAGEAR, current: 0.93 => 0.49, new brightness: 49, new value: 49
display-brightness-ddcutil extension:
Write collector defining new display 19 and adding it to queue
JS ERROR: Error: GSettings key [object GjsGlobal] not found in schema org.gnome.shell.extensions.display-brightness-ddcutil
_checkKey@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:779:23
createCheckedMethod/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:729:30
setInterval@resource:///org/gnome/gjs/modules/esm/_timers.js:93:36
ddcWriteInQueue@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:149:52
ddcWriteCollector@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:184:18
setBrightness@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:211:14
onSliderChange@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/extension.js:259:18
sliderValueChangeCommon@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/indicator.js:57:14
_SliderChange@file:///home/maniacx/.local/share/gnome-shell/extensions/display-brightness-ddcutil@themightydeity.github.com/indicator.js:264:32
set value@resource:///org/gnome/shell/ui/barLevel.js:61:14
_moveHandle@resource:///org/gnome/shell/ui/slider.js:212:9
startDragging@resource:///org/gnome/shell/ui/slider.js:92:14
vfunc_button_press_event@resource:///org/gnome/shell/ui/slider.js:69:21
@resource:///org/gnome/shell/ui/init.js:21:20
yochananmarqos commented 1 year ago

@daitj Same with me.

maniacx commented 1 year ago

Found the fix. Need to remove this.settings from setinterval on the line below https://github.com/daitj/gnome-display-brightness-ddcutil/blob/1ec328568382f170d0cc74e20e9c6b4f389f2ff0/display-brightness-ddcutil%40themightydeity.github.com/extension.js#L149

I refered to this https://gitlab.gnome.org/GNOME/gjs/-/blob/master/modules/esm/_timers.js?ref_type=heads

Then I realized I was the one added this.settings to get brightnessLog working here. https://github.com/daitj/gnome-display-brightness-ddcutil/blob/37da556f81411bc488cfb1043f11decb7efe43d2/display-brightness-ddcutil%40themightydeity.github.com/convenienceExt.js#L65 My bad.

Removing after removing the mentioned this.settings extension works.

maniacx commented 1 year ago

Ok. there is another issue is showing up now seems like quicksettings menu is not getting destroyed. I have only one LG monitor and everytime i lock screen and unlocking screen I have multiple sliders.

First login Screenshot from 2023-10-01 08-21-10

Locked and unlocked twice Screenshot from 2023-10-01 08-20-39

display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Destroy all quick settings items
display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Adding to system menu
display-brightness-ddcutil extension:
Add keyboard shortcuts
display-brightness-ddcutil extension:
ddcutil brief info:
Display 1
   I2C bus:          /dev/i2c-19
   DRM connector:    card2-HDMI-A-1
   Monitor:          GSM:LG ULTRAGEAR:308MXZJ11293

display-brightness-ddcutil extension:
ddcutil brief info found bus line:
    I2C bus:          /dev/i2c-19
display-brightness-ddcutil extension:
ddcutil reading display state for bus: 19
display-brightness-ddcutil extension:
ddcutil reading display status for bus: 19 is: VCP D6 SNC x01

display-brightness-ddcutil extension:
Reloading widgets
display-brightness-ddcutil extension:
Remove keyboard shortcuts
display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Destroy all quick settings items
display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Adding to system menu
display-brightness-ddcutil extension:
Add keyboard shortcuts
display-brightness-ddcutil extension:
ddcutil brief info:
Display 1
   I2C bus:          /dev/i2c-19
   DRM connector:    card2-HDMI-A-1
   Monitor:          GSM:LG ULTRAGEAR:308MXZJ11293

display-brightness-ddcutil extension:
ddcutil brief info found bus line:
    I2C bus:          /dev/i2c-19
display-brightness-ddcutil extension:
ddcutil reading display state for bus: 19
display-brightness-ddcutil extension:
ddcutil reading display status for bus: 19 is: VCP D6 SNC x01

display-brightness-ddcutil extension:
Reloading widgets
display-brightness-ddcutil extension:
Remove keyboard shortcuts
display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Destroy all quick settings items
display-brightness-ddcutil extension:
Destroy quick settings single slider item
display-brightness-ddcutil extension:
Adding to system menu
display-brightness-ddcutil extension:
Add keyboard shortcuts
display-brightness-ddcutil extension:
ddcutil brief info:
Display 1
   I2C bus:          /dev/i2c-19
   DRM connector:    card2-HDMI-A-1
   Monitor:          GSM:LG ULTRAGEAR:308MXZJ11293

display-brightness-ddcutil extension:
ddcutil brief info found bus line:
    I2C bus:          /dev/i2c-19
display-brightness-ddcutil extension:
ddcutil reading display state for bus: 19
display-brightness-ddcutil extension:
ddcutil reading display status for bus: 19 is: VCP D6 SNC x01

display-brightness-ddcutil extension:
Reloading widgets

The Topbar mode doesn't have this issue

maniacx commented 1 year ago

Seems like the culprit is here, but I dont know the solution. need to figure this out.

https://github.com/daitj/gnome-display-brightness-ddcutil/blob/1ec328568382f170d0cc74e20e9c6b4f389f2ff0/display-brightness-ddcutil%40themightydeity.github.com/indicator.js#L17

https://github.com/daitj/gnome-display-brightness-ddcutil/blob/1ec328568382f170d0cc74e20e9c6b4f389f2ff0/display-brightness-ddcutil%40themightydeity.github.com/indicator.js#L355

https://github.com/daitj/gnome-display-brightness-ddcutil/blob/1ec328568382f170d0cc74e20e9c6b4f389f2ff0/display-brightness-ddcutil%40themightydeity.github.com/indicator.js#L360

Razer0123 commented 11 months ago

Any news? just updated to f39 and the extension is not working

daitj commented 11 months ago

Sorry for the delay, I was away from digital life.

I have submitted new version for review now, lets see what happens after that.

Thanks @maniacx for the pull request.

ovitters commented 11 months ago

I have submitted new version for review now, lets see what happens after that.

New version is available already. It's working nicely! Thanks for the extension and thanks @maniacx for the pull request!

Razer0123 commented 11 months ago

Sorry for the delay, I was away from digital life.

I have submitted new version for review now, lets see what happens after that.

Thanks @maniacx for the pull request.

Thanks for your work, it's highly appreciated

Hope you are doing fine in life, wish you the best

Tamas-Toth-ebola commented 11 months ago

The fresh version 46 (from extensions.gnome.org) works fine for me on the latest Ubuntu 23.10 with 'GNOME 45'...

Thanks for your efforts!

(It is interesting that your latest version here is the 47, with the corresponding fixes in theory, while from GNOME extensions library we can download the version 46, as the latest, but which is also fresh... but basically the point is that the 'latest available' is working fine...)

daitj commented 11 months ago

@Tamas-Toth-ebola yeah it was my mistake, v46 wasn't uploaded to GSE, so when I uploaded v47, it automatically set it to v46. I have reverted the version in the repo now.

and I am closing this issue now.