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

Brightness in frame try2 #437

Closed

Conversation

samdroid-apps
Copy link
Contributor

This adds a brightness model and screenshot model (from existing screenshot code). This is then used to create a display frame icon, which houses the brightness and screenshot.

The brightness thing is a bit weird and has limitations. It either manipulates the file directly or if uses GNOME SettingsDaemon over dbus. If none of theses are writable, you get a read only brightness in the frame.

@samdroid-apps
Copy link
Contributor Author

Requires sugarlabs/sugar-artwork#39

@samdroid-apps
Copy link
Contributor Author

@sugarlabs/owners, could this be reviewed for 0.106?

@tchx84
Copy link
Member

tchx84 commented Apr 28, 2015

@samdroid-apps I can review this. Please prepare (or update) the feature page in the wiki.

@tchx84
Copy link
Member

tchx84 commented Apr 28, 2015

Hello @samdroid-apps,

I have reviewed and tested it using sugar-build with Fedora 22 and Gnome.

First thing I noticed is that the DBUS methods you are using only works with unity-settings-daemon. Keep in mind that Ubuntu folks forked gnome-settings-daemon and will be nice to support both.

To give you some examples:

For unity-settings-daemon (the same one you are using):

dbus-send --session --print-reply --dest="org.gnome.SettingsDaemon" /org/gnome/SettingsDaemon/Power org.gnome.SettingsDaemon.Power.Screen.SetPercentage uint32:100

See more details at: http://bazaar.launchpad.net/~unity-settings-daemon-team/unity-settings-daemon/trunk/view/head:/plugins/power/gsd-power-manager.c#L94

For gnome-settings-daemon:

dbus-send --session --print-reply --dest=org.gnome.SettingsDaemon /org/gnome/SettingsDaemon/Power  org.freedesktop.DBus.Properties.Set string:"org.gnome.SettingsDaemon.Power.Screen" string:"Brightness" variant:int32:100

Note there is no SetPercentage, but instead there is a Brightness property. You can see more details on https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-power-manager.c?h=gnome-3-14#n80

Regarding the model code organization, please:

  1. Define a "interface" (basically a class with empty methods) to define this model API.
  2. Create a subclass of this "interface" for each different back-end, ie., one for gnome-settings-daemon, one for unity-settigns-daemon etc. Makes no sense to do this with with IF's since is not like the back-end is going to change in run-time.
  3. Provide a get_model() method to get the proper model for the view. In this method you detect which back-end should be used and return the proper subclass object.

Great work so far!

@quozl
Copy link
Contributor

quozl commented Apr 28, 2015

Subscribed. Reviewed. Great work so far. Agree with comments above.

We need to figure out if there is a way to control brightness without requiring gnome-settings-daemon or unity-settings-daemon, which are not present when Sugar is run as a desktop from lightdm.

To that end I've been looking at how these daemons make the brightness change, but no final result yet. The process that writes to sysfs is the X server, but I don't know how the X server is given the command. It may be one of the X protocol messages. http://dev.laptop.org/ticket/12888 is where I'm tracking this. Won't get to this for a week or so, so if anybody else figures it out, let me know.

@samdroid-apps
Copy link
Contributor Author

@quozl, I use the brightness file, which is writable on the xo

@quozl
Copy link
Contributor

quozl commented Apr 28, 2015

@samdroid-apps, yes, I see that, but it won't work on hardware other than the XO. I'm fine with merge for XO, but not okay with merge for other hardware.

@samdroid-apps
Copy link
Contributor Author

@quozl I do have a read only fallback :)

@quozl
Copy link
Contributor

quozl commented Apr 29, 2015

Heh. Indeed you do. But I'm staring at a very bright screen and reaching for my sunglasses. ;-)

icon_number = math.ceil(float(brightness.get_brightness()) * 3
/ brightness.get_max_brightness()) * 33
if icon_number == 99:
icon_number = 100
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: there is a function in sugar3.graphics.icon that does this

@samdroid-apps
Copy link
Contributor Author

@tchx84 Thanks for the info and review!

I will restructure my code to be MV based.

I am using the GNOME control system in my code, not the Unity one. (I haven't had Ubuntu installed for years). I will look into supporting the Unity version though.

@tchx84
Copy link
Member

tchx84 commented May 1, 2015

@samdroid-apps, you are right! My bad.

But are you able to test it? I think there is something missing because all I get is:

DBusException: org.freedesktop.DBus.Error.ServiceUnknown: The name org.gnome.SettingsDaemon.Power was not provided by any .service files

ie., you can reproduce it by running the shell example I showed you both from your gnome terminal and then from sugar terminal. The later will fail.

There is probably something missing at configuration level.

@samdroid-apps
Copy link
Contributor Author

@tchx84

Yes I am having the same issue.

It may be that sugar-build does not share a dbus with the outside world.

I will look further into it later.

@quozl
Copy link
Contributor

quozl commented May 1, 2015

You have to know where this feature is implemented.

There is no settings-daemon in the scenario I'm looking at; Sugar as a desktop. I suggest not looking into supporting Unity or Ubuntu, but instead understand how both gnome-settings-daemon and unity-settings-daemon implement what they do (for brightness), and reimplement that in Sugar. It need not be a sugar-settings-daemon.

I doubt that sugar-build is a suitable environment for designing or testing brightness control. It is artificial, and is not deployed.

@tchx84
Copy link
Member

tchx84 commented May 5, 2015

@samdroid-apps , think you could make the changes I suggested earlier to organize the model code? I am looking into what James is suggesting, should have news soon.

@quozl
Copy link
Contributor

quozl commented May 6, 2015

@samdroid-apps
Copy link
Contributor Author

We will use a new approach as per the mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants