Merge lp:~seb128/ubuntu-system-settings/battery-full-charge-info into lp:ubuntu-system-settings

[Sebastien Bacher]

battery: display the last full charge

battery: display the last full charge

Some comments for the reviewer(s):

- in timeDeltaString(): I'm not sure how to/if we can do plurial in i18n.tr() (we have a 2 arguments format already which is used to specify a domain)

- the isCharging property is needed because chargeState is not a property (it takes a battery number as parameter) so it doesn't get dynamic update

- changing the line to be a bit thicker, that works better on the device

- I'm keeping the QDebug include while working on the panel at least, it's annoying to comment it/put it back for every commit

- getLastFullCharge() iterates on an higher timerange than getHistory

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:259
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/217/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-saucy-amd64-ci/217

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-system-settings-ci/217/rebuild

review: Approve (continuous-integration)

[Iain Lane (laney)]

Hey, looks mostly good. It seems to be correctly dynamic which is great to see.

I agree it's not the most beautiful but I don't have ideas about how to fix the plural stuff.

Couple of comments:

        if (timeDelta > 3600 && timeDelta < 86400)
            return i18n.tr("%1 hours ago").arg(Math.round(timeDelta/3600))

If timedelta is 86399 then this will show "24 hours ago" instead of "1 day ago" which you want, won't it?

And similarly for Math.round(3601/3600) = 1 → "1 hours ago"?

I guess this algorithm can be improved by splitting the rounding and display logic.

158 + if (up_history_item_get_state(item) == UP_DEVICE_STATE_FULLY_CHARGED) {
159 + m_lastFullCharge = (int)(((gint32) up_history_item_get_time(item) - offset)*-1);
160 + Q_EMIT(lastFullChargeChanged());
161 + return;

What's the -1 for (comment it) and also can the repeated copies of this code be factored out?

review: Needs Fixing

[Sebastien Bacher (seb128)]

Thanks for the review

> I guess this algorithm can be improved by splitting the rounding and display logic.

working on that

> What's the -1 for (comment it)

doing that

> can the repeated copies of this code be factored out?

you are speaking about the loop iterating on the charge datas I guess? Not easily, but I guess I can add a parameter to the function, make the code in the middle do if/else and call different functions

[lp:~seb128/ubuntu-system-settings/battery-full-charge-info]

260. By Sebastien Bacher on 2013-08-20

better computer the time delta labels

261. By Sebastien Bacher on 2013-08-20

don't work with negative offsets, easier to read this way

262. By Sebastien Bacher on 2013-08-20

use a new function for the lastfullcharge update

263. By Sebastien Bacher on 2013-08-20

don't leak values

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:263
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/225/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-saucy-amd64-ci/225

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-system-settings-ci/225/rebuild

review: Approve (continuous-integration)

[lp:~seb128/ubuntu-system-settings/battery-full-charge-info]

264. By Sebastien Bacher on 2013-08-20

use the correct rounding values

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:264
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/226/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-saucy-amd64-ci/226

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-system-settings-ci/226/rebuild

review: Approve (continuous-integration)

[Iain Lane (laney)]

OK, it'd be good to comment what those numbers are but seems good

thanks!

review: Approve

=== modified file 'plugins/battery/PageComponent.qml'
--- plugins/battery/PageComponent.qml	2013-08-12 17:39:21 +0000
+++ plugins/battery/PageComponent.qml	2013-08-20 14:54:26 +0000
@@ -32,6 +32,27 @@
     title: i18n.tr("Battery")
     flickable: scrollWidget
 
+    property bool isCharging
+
+    function timeDeltaString(timeDelta) {
+        if (timeDelta === 1)
+            return i18n.tr("1 second ago")
+        if (timeDelta < 60)
+            return i18n.tr("%1 seconds ago").arg(timeDelta)
+        if (timeDelta >= 60 && timeDelta < 90)
+            return i18n.tr("1 minute ago")
+        if (timeDelta >= 90 && timeDelta < 3570)
+            return i18n.tr("%1 minutes ago").arg(Math.round(timeDelta/60))
+        if (timeDelta >= 3570 && timeDelta < 5400)
+            return i18n.tr("1 hour ago")
+        if (timeDelta >= 5400 && timeDelta < 84600)
+            return i18n.tr("%1 hours ago").arg(Math.round(timeDelta/3600))
+        if (timeDelta >= 84600 && timeDelta < 129600)
+            return i18n.tr("1 day ago")
+        if (timeDelta >= 129600)
+            return i18n.tr("%1 days ago").arg(Math.round(timeDelta/86400))
+    }
+
     GSettings {
         id: powerSettings
         schema.id: batteryBackend.powerdRunning ? "com.canonical.powerd" : "org.gnome.settings-daemon.plugins.power"
@@ -46,12 +67,15 @@
         onChargingStateChanged: {
             if (state === BatteryInfo.Charging) {
                 chargingEntry.text = i18n.tr("Charging now")
-                chargingEntry.value = ""
-
+                isCharging = true
             }
-            else {
+            else if (state === BatteryInfo.Discharging) {
                 chargingEntry.text = i18n.tr("Last full charge")
-                chargingEntry.value = i18n.tr("N/A")  // TODO: find a way to get that information
+                isCharging = false
+            }
+            else if (state === BatteryInfo.Full || state === BatteryInfo.NotCharging) {
+                chargingEntry.text = i18n.tr("Fully charged")
+                isCharging = true
             }
         }
         onRemainingCapacityChanged: {
@@ -88,6 +112,8 @@
 
             ListItem.SingleValue {
                 id: chargingEntry
+                value: isCharging ?
+                           "" : (batteryBackend.lastFullCharge ? timeDeltaString(batteryBackend.lastFullCharge) : i18n.tr("N/A"))
                 showDivider: false
             }
 
@@ -112,7 +138,7 @@
                     ctx.stroke()
 
                     /* Display the charge history in orange color */
-                    ctx.lineWidth = units.dp(1)
+                    ctx.lineWidth = units.dp(2)
                     ctx.strokeStyle = UbuntuColors.orange
 
                     /* Get infos from battery0, on a day (60*24*24=86400 seconds), with 150 points on the graph */
@@ -121,9 +147,9 @@
                     /* time is the offset in seconds compared to the current time (negative value)
                        we display the charge on a day, which is 86400 seconds, the value is the %
                        the coordinates are adjusted to the canvas, top,left is 0,0 */
-                    ctx.moveTo((86400+chargeDatas[0].time)/86400*canvas.width, (1-chargeDatas[0].value/100)*canvas.height)
+                    ctx.moveTo((86400-chargeDatas[0].time)/86400*canvas.width, (1-chargeDatas[0].value/100)*canvas.height)
                     for (var i=1; i < chargeDatas.length; i++) {
-                        ctx.lineTo((86400+chargeDatas[i].time)/86400*canvas.width, (1-chargeDatas[i].value/100)*canvas.height)
+                        ctx.lineTo((86400-chargeDatas[i].time)/86400*canvas.width, (1-chargeDatas[i].value/100)*canvas.height)
                     }
                     ctx.stroke()
                     ctx.restore();

=== modified file 'plugins/battery/battery.cpp'
--- plugins/battery/battery.cpp	2013-08-12 20:01:37 +0000
+++ plugins/battery/battery.cpp	2013-08-20 14:54:26 +0000
@@ -23,7 +23,7 @@
 #include <libupower-glib/upower.h>
 #include <QEvent>
 #include <QDBusReply>
-#include <QPair>
+#include <QtCore/QDebug>
 
 Battery::Battery(QObject *parent) :
     QObject(parent),
@@ -31,10 +31,14 @@
     m_powerdIface ("com.canonical.powerd",
                    "/com/canonical/powerd",
                    "com.canonical.powerd",
-                   m_systemBusConnection)
+                   m_systemBusConnection),
+    m_deviceString("")
 {
     m_device = up_device_new();
 
+    buildDeviceString();
+    getLastFullCharge();
+
     if (!m_powerdIface.isValid()) {
         m_powerdRunning = false;
         return;
@@ -48,7 +52,7 @@
     return m_powerdRunning;
 }
 
-QString Battery::deviceString() {
+void Battery::buildDeviceString() {
     UpClient *client;
     gboolean returnIsOk;
     GPtrArray *devices;
@@ -59,7 +63,7 @@
     returnIsOk = up_client_enumerate_devices_sync(client, NULL, NULL);
 
     if(!returnIsOk)
-        return QString("");
+        return;
 
     devices = up_client_get_devices(client);
 
@@ -67,20 +71,58 @@
         device = (UpDevice *)g_ptr_array_index(devices, i);
         g_object_get(device, "kind", &kind, NULL);
         if (kind == UP_DEVICE_KIND_BATTERY) {
-            QString deviceId(up_device_get_object_path(device));
-            g_ptr_array_unref(devices);
-            g_object_unref(client);
-            return deviceId;
+            m_deviceString = QString(up_device_get_object_path(device));
         }
     }
 
     g_ptr_array_unref(devices);
     g_object_unref(client);
-    return QString("");
+}
+
+QString Battery::deviceString()
+{
+    return m_deviceString;
+}
+
+int Battery::lastFullCharge()
+{
+    return m_lastFullCharge;
+}
+
+bool Battery::updateLastFullCharge(UpHistoryItem *item, int offset)
+{
+    if (up_history_item_get_state(item) == UP_DEVICE_STATE_FULLY_CHARGED) {
+        m_lastFullCharge = (int)((offset - (gint32) up_history_item_get_time(item)));
+        Q_EMIT(lastFullChargeChanged());
+        return true; /* return true if the charge value got updated */
+    }
+    return false;
+}
+
+void Battery::getLastFullCharge()
+{
+    UpHistoryItem *item;
+    GPtrArray *values;
+    gint32 offset = 0;
+    GTimeVal timeval;
+
+    g_get_current_time(&timeval);
+    offset = timeval.tv_sec;
+    up_device_set_object_path_sync(m_device, m_deviceString.toStdString().c_str(), NULL, NULL);
+    values = up_device_get_history_sync(m_device, "charge", 864000, 1000, NULL, NULL);
+    for (uint i=values->len-1; i > 0; i--) {
+        item = (UpHistoryItem *) g_ptr_array_index(values, i);
+
+        if (updateLastFullCharge(item, offset) == true) {
+            g_ptr_array_unref (values);
+            return;
+        }
+    }
+    g_ptr_array_unref (values);
 }
 
 /* TODO: refresh values over time for dynamic update */
-QVariantList Battery::getHistory(const QString &deviceString, const int timespan, const int resolution) const
+QVariantList Battery::getHistory(const QString &deviceString, const int timespan, const int resolution)
 {
     UpHistoryItem *item;
     GPtrArray *values;
@@ -99,11 +141,13 @@
         if (up_history_item_get_state(item) == UP_DEVICE_STATE_UNKNOWN)
             continue;
 
-        listItem.insert("time",((gint32) up_history_item_get_time(item) - offset));
+        updateLastFullCharge(item, offset);
+
+        listItem.insert("time",(offset - (gint32) up_history_item_get_time(item)));
         listItem.insert("value",up_history_item_get_value(item));
         listValues += listItem;
     }
-
+    g_ptr_array_unref (values);
     return listValues;
 }
 

=== modified file 'plugins/battery/battery.h'
--- plugins/battery/battery.h	2013-08-12 19:59:57 +0000
+++ plugins/battery/battery.h	2013-08-20 14:54:26 +0000
@@ -38,12 +38,20 @@
                 READ deviceString
                 CONSTANT)
 
+    Q_PROPERTY( int lastFullCharge
+                READ lastFullCharge
+                NOTIFY lastFullChargeChanged)
+
 public:
     explicit Battery(QObject *parent = 0);
     ~Battery();
     bool powerdRunning();
     QString deviceString();
-    Q_INVOKABLE QVariantList getHistory(const QString &deviceString, const int timespan, const int resolution) const;
+    int lastFullCharge();
+    Q_INVOKABLE QVariantList getHistory(const QString &deviceString, const int timespan, const int resolution);
+
+Q_SIGNALS:
+    void lastFullChargeChanged();
 
 private:
     QDBusConnection m_systemBusConnection;
@@ -51,6 +59,11 @@
     QDBusInterface m_powerdIface;
     bool m_powerdRunning;
     UpDevice *m_device;
+    QString m_deviceString;
+    int m_lastFullCharge;
+    void buildDeviceString();
+    void getLastFullCharge();
+    bool updateLastFullCharge(UpHistoryItem *item, int offset);
 };
 
 #endif // BATTERY_H

=== modified file 'po/ubuntu-system-settings.pot'
--- po/ubuntu-system-settings.pot	2013-08-13 13:24:21 +0000
+++ po/ubuntu-system-settings.pot	2013-08-20 14:54:26 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: ubuntu-system-settings\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2013-08-13 09:22-0400\n"
+"POT-Creation-Date: 2013-08-14 15:54+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -26,8 +26,8 @@
 msgstr ""
 
 #: ../plugins/about/PageComponent.qml:77 ../plugins/about/PageComponent.qml:84
-#: ../plugins/battery/PageComponent.qml:54
-#: ../plugins/battery/PageComponent.qml:85
+#: ../plugins/battery/PageComponent.qml:109
+#: ../plugins/battery/PageComponent.qml:116
 #: ../plugins/cellular/PageComponent.qml:75
 msgid "N/A"
 msgstr ""
@@ -45,8 +45,8 @@
 msgstr ""
 
 #: ../plugins/about/PageComponent.qml:98
-#: ../plugins/battery/PageComponent.qml:180
-#: ../plugins/battery/PageComponent.qml:185
+#: ../plugins/battery/PageComponent.qml:206
+#: ../plugins/battery/PageComponent.qml:211
 #: ../plugins/battery/SleepValues.qml:76
 msgid "Never"
 msgstr ""
@@ -153,38 +153,78 @@
 msgid "Battery"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:48
+#: ../plugins/battery/PageComponent.qml:39
+msgid "1 second ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:41
+#, qt-format
+msgid "%1 seconds ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:43
+msgid "1 minute ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:45
+#, qt-format
+msgid "%1 minutes ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:47
+msgid "1 hour ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:49
+#, qt-format
+msgid "%1 hours ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:51
+msgid "1 day ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:53
+#, qt-format
+msgid "%1 days ago"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:69
 msgid "Charging now"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:53
+#: ../plugins/battery/PageComponent.qml:73
 msgid "Last full charge"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:59
+#: ../plugins/battery/PageComponent.qml:77
+msgid "Fully charged"
+msgstr ""
+
+#: ../plugins/battery/PageComponent.qml:83
 #, qt-format
 msgid "%1 %"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:84
+#: ../plugins/battery/PageComponent.qml:108
 msgid "Charge level"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:134
+#: ../plugins/battery/PageComponent.qml:160
 msgid "Ways to reduce battery use:"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:138
+#: ../plugins/battery/PageComponent.qml:164
 msgid "Display brightness"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:175
+#: ../plugins/battery/PageComponent.qml:201
 #: ../plugins/battery/SleepValues.qml:31
 msgid "Auto sleep"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:179
-#: ../plugins/battery/PageComponent.qml:184
+#: ../plugins/battery/PageComponent.qml:205
+#: ../plugins/battery/PageComponent.qml:210
 #: ../plugins/battery/SleepValues.qml:71 ../plugins/battery/SleepValues.qml:72
 #: ../plugins/battery/SleepValues.qml:73 ../plugins/battery/SleepValues.qml:74
 #: ../plugins/battery/SleepValues.qml:75
@@ -192,19 +232,19 @@
 msgid "After %1 minutes"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:192 ../.build/settings.js:19
+#: ../plugins/battery/PageComponent.qml:218 ../.build/settings.js:19
 msgid "Wi-Fi"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:200 ../.build/settings.js:5
+#: ../plugins/battery/PageComponent.qml:226 ../.build/settings.js:5
 msgid "Bluetooth"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:208 ../.build/settings.js:10
+#: ../plugins/battery/PageComponent.qml:234 ../.build/settings.js:10
 msgid "GPS"
 msgstr ""
 
-#: ../plugins/battery/PageComponent.qml:216
+#: ../plugins/battery/PageComponent.qml:242
 msgid "Accurate location detection requires GPS and/or Wi-Fi."
 msgstr ""
 
@@ -221,11 +261,11 @@
 msgid "Carrier"
 msgstr ""
 
-#: ../plugins/cellular/ChooseCarrier.qml:83
+#: ../plugins/cellular/ChooseCarrier.qml:80
 msgid "Refresh"
 msgstr ""
 
-#: ../plugins/cellular/ChooseCarrier.qml:100
+#: ../plugins/cellular/ChooseCarrier.qml:97
 msgid "Searching"
 msgstr ""