Make GetHistory() array order consistent

When requesting fewer history elements than we actually have, fix the
interpolation loop to not reverse the returned elements; this already does not
happen if we request more elements than available, which led to the returned
list order depending on the history size.

Now the first array element is always the most recent one. Update documentation
accordingly.

Add test case to reproduce the problem. We now add three sample points to be
able to request a subset and still assert its correct order, and make the
charge values be further apart to ensure correct interpolation.

https://bugs.freedesktop.org/show_bug.cgi?id=68384

3 files changed, 51 insertions, 12 deletions

diff --git a/libupower-glib/up-device.c b/libupower-glib/up-device.c
index 8b66bde..b8768d8 100644
--- a/libupower-glib/up-device.c
+++ b/libupower-glib/up-device.c
@@ -592,7 +592,9 @@ out:
  *
  * Gets the device history.
  *
- * Return value: (transfer full): an array of #UpHistoryItem's, else #NULL and @error is used
+ * Return value: (transfer full): an array of #UpHistoryItem's, with the most
+ *               recent one being first; %NULL if @error is set or @device is
+ *               invalid
  *
  * Since: 0.9.0
  **/
diff --git a/src/up-history.c b/src/up-history.c
index cd16f36..c16b3c1 100644
--- a/src/up-history.c
+++ b/src/up-history.c
@@ -121,7 +121,7 @@ up_history_array_limit_resolution (GPtrArray *array, guint max_num)
 	UpHistoryItem *item_new;
 	gfloat division;
 	guint length;
-	gint i;
+	guint i;
 	guint last;
 	guint first;
 	GPtrArray *new;
@@ -157,7 +157,7 @@ up_history_array_limit_resolution (GPtrArray *array, guint max_num)
 	/* Reduces the number of points to a pre-set level using a time
 	 * division algorithm so we don't keep diluting the previous
 	 * data with a conventional 1-in-x type algorithm. */
-	for (i=length-1; i>=0; i--) {
+	for (i = 0; i < length; i++) {
 		item = (UpHistoryItem *) g_ptr_array_index (array, i);
 		preset = last + (division * (gfloat) step);
 
diff --git a/src/up-self-test.c b/src/up-self-test.c
index d19734e..edcae99 100644
--- a/src/up-self-test.c
+++ b/src/up-self-test.c
@@ -149,7 +149,7 @@ up_test_history_func (void)
 	gboolean ret;
 	GPtrArray *array;
 	gchar *filename;
-	UpHistoryItem *item;
+	UpHistoryItem *item, *item2, *item3;
 
 	history = up_history_new ();
 	g_assert (history != NULL);
@@ -171,17 +171,23 @@ up_test_history_func (void)
 	array = up_history_get_data (history, UP_HISTORY_TYPE_CHARGE, 10, 100);
 	g_assert (array != NULL);
 	g_assert_cmpint (array->len, ==, 0);
+	g_ptr_array_unref (array);
 
-	/* setup some fake device */
+	/* setup some fake device and three data points */
 	up_history_set_state (history, UP_DEVICE_STATE_CHARGING);
+	up_history_set_charge_data (history, 85);
+	up_history_set_rate_data (history, 0.99f);
+	up_history_set_time_empty_data (history, 12346);
+	up_history_set_time_full_data (history, 54322);
+
+	g_usleep (2 * G_USEC_PER_SEC);
 	up_history_set_charge_data (history, 90);
 	up_history_set_rate_data (history, 1.00f);
 	up_history_set_time_empty_data (history, 12345);
 	up_history_set_time_full_data (history, 54321);
 
-	/* sleep for a little bit */
-	g_usleep (3 * G_USEC_PER_SEC);
-	up_history_set_charge_data (history, 91);
+	g_usleep (2 * G_USEC_PER_SEC);
+	up_history_set_charge_data (history, 95);
 	up_history_set_rate_data (history, 1.01f);
 	up_history_set_time_empty_data (history, 12344);
 	up_history_set_time_full_data (history, 54320);
@@ -189,13 +195,44 @@ up_test_history_func (void)
 	/* get data for last 10 seconds */
 	array = up_history_get_data (history, UP_HISTORY_TYPE_CHARGE, 10, 100);
 	g_assert (array != NULL);
-	g_assert_cmpint (array->len, ==, 2);
+	g_assert_cmpint (array->len, ==, 3);
 
 	/* get the first item, which should be the most recent */
 	item = g_ptr_array_index (array, 0);
 	g_assert (item != NULL);
-	g_assert_cmpint (up_history_item_get_value (item), ==, 91);
+	g_assert_cmpint (up_history_item_get_value (item), ==, 95);
+	g_assert_cmpint (up_history_item_get_time (item), >, 1000000);
+
+        /* the second one ought to be older */
+	item2 = g_ptr_array_index (array, 1);
+	g_assert (item2 != NULL);
+	g_assert_cmpint (up_history_item_get_value (item2), ==, 90);
+	g_assert_cmpint (up_history_item_get_time (item2), <, up_history_item_get_time (item));
+
+        /* third one is the oldest */
+	item3 = g_ptr_array_index (array, 2);
+	g_assert (item3 != NULL);
+	g_assert_cmpint (up_history_item_get_value (item3), ==, 85);
+	g_assert_cmpint (up_history_item_get_time (item3), <, up_history_item_get_time (item2));
+
+	g_ptr_array_unref (array);
+
+        /* request fewer items than we have in our history; should have the
+         * same order: first one is the most recent, and the data gets
+         * interpolated */
+	array = up_history_get_data (history, UP_HISTORY_TYPE_CHARGE, 10, 2);
+	g_assert (array != NULL);
+	g_assert_cmpint (array->len, ==, 2);
+
+	item = g_ptr_array_index (array, 0);
+	g_assert (item != NULL);
+	item2 = g_ptr_array_index (array, 1);
+	g_assert (item2 != NULL);
+
 	g_assert_cmpint (up_history_item_get_time (item), >, 1000000);
+	g_assert_cmpint (up_history_item_get_value (item), ==, 95);
+	g_assert_cmpint (up_history_item_get_value (item2), ==, 87);
+
 	g_ptr_array_unref (array);
 
 	/* force a save to disk */
@@ -216,10 +253,10 @@ up_test_history_func (void)
 	/* get data for last 10 seconds */
 	array = up_history_get_data (history, UP_HISTORY_TYPE_CHARGE, 10, 100);
 	g_assert (array != NULL);
-	g_assert_cmpint (array->len, ==, 3); /* we have inserted an unknown as the first entry */
+	g_assert_cmpint (array->len, ==, 4); /* we have inserted an unknown as the first entry */
 	item = g_ptr_array_index (array, 1);
 	g_assert (item != NULL);
-	g_assert_cmpint (up_history_item_get_value (item), ==, 91);
+	g_assert_cmpint (up_history_item_get_value (item), ==, 95);
 	g_assert_cmpint (up_history_item_get_time (item), >, 1000000);
 	g_ptr_array_unref (array);