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

Enable battery widget in Deere skin #2161

Merged
merged 1 commit into from
Aug 14, 2019
Merged

Conversation

rrrapha
Copy link
Contributor

@rrrapha rrrapha commented Jun 12, 2019

Having the widget enabled in skins make testing the feature easier..
The icons are a modified version of those posted here by user "jus":
https://bugs.launchpad.net/mixxx/+bug/1662336

Of course, someone more competent should adjust the style of the widget, this is just a placeholder.

}

if (m_pCurrentPixmap) {
m_pCurrentPixmap->draw(0, 0, &p);
m_pCurrentPixmap->draw(rect(), &p);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really understand this change, it seems to be needed to make the scaling work..

Copy link
Member

Choose a reason for hiding this comment

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

The original version draws a into a rect taken from the prixamp placed at 0,0 of the widget.
The new version takes the whole widget rect as desired.

@ronso0
Copy link
Member

ronso0 commented Jun 12, 2019

Of course, someone more competent should adjust the style of the widget, this is just a placeholder.

for now you can use the icons in
https://github.com/mixxxdj/mixxx/tree/master/res/skins/Tango/graphics/batt
you can also rotate them ccw if that fits better

@rrrapha
Copy link
Contributor Author

rrrapha commented Jun 12, 2019

@ronso0 Thanks, I think it doesn't matter much for now..
(unless the icons from Tango are considered 'final' for Deere)

@daschuer
Copy link
Member

Thank you. Unfortunately this PR mixes a finished bugfix with an unfinished skin change.
Can you do a separate PR only containing the scaling fix?

@rrrapha
Copy link
Contributor Author

rrrapha commented Jun 13, 2019

Can you do a separate PR only containing the scaling fix?

Sure, #2163

@daschuer
Copy link
Member

What is the status of this PR?

@rrrapha
Copy link
Contributor Author

rrrapha commented Jul 29, 2019

I wouldn't mind if it gets merged as is, the icons can still be changed later if desired..

@ronso0
Copy link
Member

ronso0 commented Jul 30, 2019

on my ubuntu studio 18.04 machine the battery status in xfce4 works, but not the widgetin Mixxx:
"Battery status unknown"

upower / libupower are up to date AFAICT 0.99.7-2ubuntu0.18.04.1

same result with locally build master and mixxx-2.3.0-alpha-pre-master-git6883-release-bionic-amd64.deb from http://downloads.mixxx.org/builds/master/release/ , both built with battery=1

@rrrapha
Copy link
Contributor Author

rrrapha commented Jul 30, 2019

@ronso0 Do you have the dbus daemon running?
You can use the command line binary to check if upower works:
$ upower --dump

@ronso0
Copy link
Member

ronso0 commented Jul 30, 2019

dbus is running and $ upower --dump gives me adequate output, also the OS indicator is always running reliably

@rrrapha
Copy link
Contributor Author

rrrapha commented Jul 30, 2019

Strange, it works for me on OpenBSD (with upower 0.99.9).

@daschuer
Copy link
Member

I can confirm the battery status unknown issue with Ubuntu Xenial.
$ upower --dump works.

@rrrapha
Copy link
Contributor Author

rrrapha commented Aug 5, 2019

My guess is that no devices are found with upower <= 0.99.7.
The following code in src/util/battery/batterylinux.cpp looks suspicious:

#if UP_CHECK_VERSION(0, 99, 8)
    GPtrArray* devices = up_client_get_devices2(client);
    VERIFY_OR_DEBUG_ASSERT(devices) {
      return;
    }
#else
    // This deprecated function doesn't set the free function for
    // the array elements so we need to do it!
    // https://bugs.freedesktop.org/show_bug.cgi?id=106740
    // https://gitlab.freedesktop.org/upower/upower/issues/14
    GPtrArray* devices = up_client_get_devices(client);
    VERIFY_OR_DEBUG_ASSERT(devices) {
      return;
    }
    devices = g_ptr_array_new_with_free_func((GDestroyNotify) g_object_unref);
#endif

    for (guint i = 0; i < devices->len; ++i) {
      ...

@ronso0
Copy link
Member

ronso0 commented Aug 7, 2019

As first implementation this is ready to merge IMO.

The lower the battery charge drops the more warning the icons should be colored:
yellow > orange > pink/red
Yes, this will happen in another PR.

@uklotzde uklotzde added this to the 2.3.0 milestone Aug 8, 2019
@daschuer
Copy link
Member

Cool. Thank you. LGTM.

@daschuer daschuer merged commit b91577a into mixxxdj:master Aug 14, 2019
@rrrapha rrrapha deleted the battery-widget branch August 14, 2019 20:54
@Be-ing
Copy link
Contributor

Be-ing commented Sep 10, 2019

This is a nice change. Could you add the battery widget to the other skins too?

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.

5 participants