ci(slider): add unit test (#3198)

* slider: Refactor LV_SLIDER_KNOB_COORD and draw_knob

The `hor` parameter of this macro was always true, ending up always using the
left side of the ternary operator, with this refactor we eliminate the `hor`
parameter and remove the ternary operator it was used for.

This macro is only used in the `draw_knob` event handler, so it helps with its
initial cleanup.

* test(slider): Test documented default properties

* test(slider): Tests for LV_EVENT_KEY with valid and invalid keys

* feat(slider): Add helper to know if slider is horizontal

* chore(slider): Cleanup LV_EVENT_PRESSING handler

Reduce scope of variables where possible and use LV_CLAMP

* chore(slider): Cleanup draw_knob

* test(slider): Add tests for LV_EVENT_RELEASED

* test(slider): Add note about left_knob_focus

* test(slider): Initial test for hit test

* test(slider): Add tests for different slider modes

* chore(slider): cleanup draw_knob

* chore(slider): Cleanup position_knob

* test(slider): Move group configuration to setUp

* test(slider): Refactor tests with feedback

* add test_ranged_mode_adjust_with_encoder

* code formatting

* tests(slider): Remove direct usages of left_knob_focus

* test(slider): Remove unused variable

* Revert "test(slider): Refactor tests with feedback"

This reverts commit b274f829ecd83c894569b6f0535c7edebce5cbc9.

* test(slider): Add missing variable

Co-authored-by: Gabor Kiss-Vamosi <kisvegabor@gmail.com>

src/widgets/lv_slider.c

@@ -22,7 +22,7 @@
  *********************/
 #define MY_CLASS &lv_slider_class
 
-#define LV_SLIDER_KNOB_COORD(hor, is_rtl, area) (hor ? (is_rtl ? area.x1 : area.x2) : (is_rtl ? area.y1 : area.y2))
+#define LV_SLIDER_KNOB_COORD(is_rtl, area) (is_rtl ? area.x1 : area.x2)
 
 /**********************
  *      TYPEDEFS
@@ -33,8 +33,9 @@
  **********************/
 static void lv_slider_constructor(const lv_obj_class_t * class_p, lv_obj_t * obj);
 static void lv_slider_event(const lv_obj_class_t * class_p, lv_event_t * e);
-static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, lv_coord_t knob_size, bool hor);
+static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, const lv_coord_t knob_size, const bool hor);
 static void draw_knob(lv_event_t * e);
+static bool is_slider_horizontal(lv_obj_t * obj);
 
 /**********************
  *  STATIC VARIABLES
@@ -83,8 +84,8 @@ static void lv_slider_constructor(const lv_obj_class_t * class_p, lv_obj_t * obj
 
     /*Initialize the allocated 'slider'*/
     slider->value_to_set = NULL;
-    slider->dragging = 0;
+    slider->dragging = 0U;
-    slider->left_knob_focus = 0;
+    slider->left_knob_focus = 0U;
 
     lv_obj_clear_flag(obj, LV_OBJ_FLAG_SCROLL_CHAIN);
     lv_obj_clear_flag(obj, LV_OBJ_FLAG_SCROLLABLE);
@@ -194,40 +195,40 @@ static void lv_slider_event(const lv_obj_class_t * class_p, lv_event_t * e)
 
         lv_point_t p;
         lv_indev_get_point(indev, &p);
-        lv_base_dir_t base_dir = lv_obj_get_style_base_dir(obj, LV_PART_MAIN);
-
-        lv_coord_t w = lv_obj_get_width(obj);
-        lv_coord_t h = lv_obj_get_height(obj);
-
-        lv_coord_t bg_left = lv_obj_get_style_pad_left(obj,   LV_PART_MAIN);
-        lv_coord_t bg_right = lv_obj_get_style_pad_right(obj,  LV_PART_MAIN);
-        lv_coord_t bg_top = lv_obj_get_style_pad_top(obj,    LV_PART_MAIN);
-        lv_coord_t bg_bottom = lv_obj_get_style_pad_bottom(obj, LV_PART_MAIN);
-
-        int32_t range = slider->bar.max_value - slider->bar.min_value;
         int32_t new_value = 0;
-        int32_t real_max_value = slider->bar.max_value;
-        int32_t real_min_value = slider->bar.min_value;
 
-        if(w >= h) {
+        const int32_t range = slider->bar.max_value - slider->bar.min_value;
-            lv_coord_t indic_w = w - bg_left - bg_right;
+        if(is_slider_horizontal(obj)) {
-            if(base_dir == LV_BASE_DIR_RTL) {
+            const lv_coord_t bg_left = lv_obj_get_style_pad_left(obj, LV_PART_MAIN);
-                new_value = (obj->coords.x2 - bg_right) - p.x; /*Make the point relative to the indicator*/
+            const lv_coord_t bg_right = lv_obj_get_style_pad_right(obj, LV_PART_MAIN);
+            const lv_coord_t w = lv_obj_get_width(obj);
+            const lv_coord_t indic_w = w - bg_left - bg_right;
+
+            if(lv_obj_get_style_base_dir(obj, LV_PART_MAIN) == LV_BASE_DIR_RTL) {
+                /*Make the point relative to the indicator*/
+                new_value = (obj->coords.x2 - bg_right) - p.x;
             }
             else {
-                new_value = p.x - (obj->coords.x1 + bg_left); /*Make the point relative to the indicator*/
+                /*Make the point relative to the indicator*/
+                new_value = p.x - (obj->coords.x1 + bg_left);
             }
             new_value = (new_value * range) / indic_w;
             new_value += slider->bar.min_value;
         }
         else {
-            lv_coord_t indic_h = h - bg_bottom - bg_top;
+            const lv_coord_t bg_top = lv_obj_get_style_pad_top(obj, LV_PART_MAIN);
-            new_value = p.y - (obj->coords.y2 + bg_bottom); /*Make the point relative to the indicator*/
+            const lv_coord_t bg_bottom = lv_obj_get_style_pad_bottom(obj, LV_PART_MAIN);
+            const lv_coord_t h = lv_obj_get_height(obj);
+            const lv_coord_t indic_h = h - bg_bottom - bg_top;
+
+            /*Make the point relative to the indicator*/
+            new_value = p.y - (obj->coords.y2 + bg_bottom);
             new_value = (-new_value * range) / indic_h;
             new_value += slider->bar.min_value;
-
         }
 
+        int32_t real_max_value = slider->bar.max_value;
+        int32_t real_min_value = slider->bar.min_value;
         /*Figure out the min. and max. for this mode*/
         if(slider->value_to_set == &slider->bar.start_value) {
             real_max_value = slider->bar.cur_value;
@@ -236,8 +237,8 @@ static void lv_slider_event(const lv_obj_class_t * class_p, lv_event_t * e)
             real_min_value = slider->bar.start_value;
         }
 
-        if(new_value < real_min_value) new_value = real_min_value;
+        new_value = LV_CLAMP(real_min_value, new_value, real_max_value);
-        else if(new_value > real_max_value) new_value = real_max_value;
+
         if(*slider->value_to_set != new_value) {
             *slider->value_to_set = new_value;
             lv_obj_invalidate(obj);
@@ -308,17 +309,17 @@ static void lv_slider_event(const lv_obj_class_t * class_p, lv_event_t * e)
         if(c == LV_KEY_RIGHT || c == LV_KEY_UP) {
             if(!slider->left_knob_focus) lv_slider_set_value(obj, lv_slider_get_value(obj) + 1, LV_ANIM_ON);
             else lv_slider_set_left_value(obj, lv_slider_get_left_value(obj) + 1, LV_ANIM_ON);
-
-            res = lv_event_send(obj, LV_EVENT_VALUE_CHANGED, NULL);
-            if(res != LV_RES_OK) return;
         }
         else if(c == LV_KEY_LEFT || c == LV_KEY_DOWN) {
             if(!slider->left_knob_focus) lv_slider_set_value(obj, lv_slider_get_value(obj) - 1, LV_ANIM_ON);
             else lv_slider_set_left_value(obj, lv_slider_get_left_value(obj) - 1, LV_ANIM_ON);
-
-            res = lv_event_send(obj, LV_EVENT_VALUE_CHANGED, NULL);
-            if(res != LV_RES_OK) return;
         }
+        else {
+            return;
+        }
+
+        res = lv_event_send(obj, LV_EVENT_VALUE_CHANGED, NULL);
+        if(res != LV_RES_OK) return;
     }
     else if(code == LV_EVENT_DRAW_MAIN) {
         draw_knob(e);
@@ -330,51 +331,32 @@ static void draw_knob(lv_event_t * e)
     lv_obj_t * obj = lv_event_get_target(e);
     lv_slider_t * slider = (lv_slider_t *)obj;
     lv_draw_ctx_t * draw_ctx = lv_event_get_draw_ctx(e);
-    lv_base_dir_t base_dir = lv_obj_get_style_base_dir(obj, LV_PART_MAIN);
 
-    lv_coord_t objw = lv_obj_get_width(obj);
+    const bool is_rtl = LV_BASE_DIR_RTL == lv_obj_get_style_base_dir(obj, LV_PART_MAIN);
-    lv_coord_t objh = lv_obj_get_height(obj);
+    const bool is_horizontal = is_slider_horizontal(obj);
-    bool hor = objw >= objh ? true : false;
-    lv_coord_t knob_size = hor ? objh : objw;
-    bool sym = false;
-    if(slider->bar.mode == LV_BAR_MODE_SYMMETRICAL && slider->bar.min_value < 0 && slider->bar.max_value > 0) sym = true;
 
     lv_area_t knob_area;
-
+    lv_coord_t knob_size;
-    /*Horizontal*/
+    if(is_horizontal) {
-    if(hor) {
+        knob_size = lv_obj_get_height(obj);
-        if(!sym) {
+        knob_area.x1 = LV_SLIDER_KNOB_COORD(is_rtl, slider->bar.indic_area);
-            knob_area.x1 = LV_SLIDER_KNOB_COORD(hor, base_dir == LV_BASE_DIR_RTL, slider->bar.indic_area);
-        }
-        else {
-            if(slider->bar.cur_value >= 0) {
-                knob_area.x1 = LV_SLIDER_KNOB_COORD(hor, base_dir == LV_BASE_DIR_RTL, slider->bar.indic_area);
-            }
-            else {
-                knob_area.x1 = LV_SLIDER_KNOB_COORD(hor, base_dir != LV_BASE_DIR_RTL, slider->bar.indic_area);
-            }
-        }
     }
-    /*Vertical*/
     else {
-        if(!sym) {
+        bool is_symmetrical = false;
-            knob_area.y1 = slider->bar.indic_area.y1;
+        if(slider->bar.mode == LV_BAR_MODE_SYMMETRICAL && slider->bar.min_value < 0 &&
-        }
+           slider->bar.max_value > 0) is_symmetrical = true;
-        else {
+
-            if(slider->bar.cur_value >= 0) {
+        knob_size = lv_obj_get_width(obj);
-                knob_area.y1 = slider->bar.indic_area.y1;
+        if(is_symmetrical && slider->bar.cur_value < 0) knob_area.y1 = slider->bar.indic_area.y2;
-            }
+        else knob_area.y1 = slider->bar.indic_area.y1;
-            else {
-                knob_area.y1 = slider->bar.indic_area.y2;
-            }
-        }
     }
 
     lv_draw_rect_dsc_t knob_rect_dsc;
     lv_draw_rect_dsc_init(&knob_rect_dsc);
     lv_obj_init_draw_rect_dsc(obj, LV_PART_KNOB, &knob_rect_dsc);
-
+    /* Update knob area with knob style */
-    position_knob(obj, &knob_area, knob_size, hor);
+    position_knob(obj, &knob_area, knob_size, is_horizontal);
+    /* Update right knob area with calculated knob area */
     lv_area_copy(&slider->right_knob_area, &knob_area);
 
     lv_obj_draw_part_dsc_t part_draw_dsc;
@@ -395,19 +377,19 @@ static void draw_knob(lv_event_t * e)
         /*Save the draw part_draw_dsc. because it can be modified in the event*/
         lv_draw_rect_dsc_t knob_rect_dsc_tmp;
         lv_memcpy(&knob_rect_dsc_tmp, &knob_rect_dsc, sizeof(lv_draw_rect_dsc_t));
-
+        /* Draw the right knob */
         lv_event_send(obj, LV_EVENT_DRAW_PART_BEGIN, &part_draw_dsc);
         lv_draw_rect(draw_ctx, &knob_rect_dsc, &slider->right_knob_area);
         lv_event_send(obj, LV_EVENT_DRAW_PART_END, &part_draw_dsc);
 
-        /*Draw a second knob for the start_value side*/
+        /*Calculate the second knob area*/
-        if(hor) {
+        if(is_horizontal) {
-            knob_area.x1 = LV_SLIDER_KNOB_COORD(hor, base_dir != LV_BASE_DIR_RTL, slider->bar.indic_area);
+            knob_area.x1 = LV_SLIDER_KNOB_COORD(is_rtl, slider->bar.indic_area);
         }
         else {
             knob_area.y1 = slider->bar.indic_area.y2;
         }
-        position_knob(obj, &knob_area, knob_size, hor);
+        position_knob(obj, &knob_area, knob_size, is_horizontal);
         lv_area_copy(&slider->left_knob_area, &knob_area);
 
         lv_memcpy(&knob_rect_dsc, &knob_rect_dsc_tmp, sizeof(lv_draw_rect_dsc_t));
@@ -422,9 +404,8 @@ static void draw_knob(lv_event_t * e)
     }
 }
 
-static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, lv_coord_t knob_size, bool hor)
+static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, const lv_coord_t knob_size, const bool hor)
 {
-
     if(hor) {
         knob_area->x1 -= (knob_size >> 1);
         knob_area->x2 = knob_area->x1 + knob_size - 1;
@@ -438,9 +419,9 @@ static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, lv_coord_t knob
         knob_area->x2 = obj->coords.x2;
     }
 
-    lv_coord_t knob_left = lv_obj_get_style_pad_left(obj,   LV_PART_KNOB);
+    lv_coord_t knob_left = lv_obj_get_style_pad_left(obj, LV_PART_KNOB);
-    lv_coord_t knob_right = lv_obj_get_style_pad_right(obj,  LV_PART_KNOB);
+    lv_coord_t knob_right = lv_obj_get_style_pad_right(obj, LV_PART_KNOB);
-    lv_coord_t knob_top = lv_obj_get_style_pad_top(obj,    LV_PART_KNOB);
+    lv_coord_t knob_top = lv_obj_get_style_pad_top(obj, LV_PART_KNOB);
     lv_coord_t knob_bottom = lv_obj_get_style_pad_bottom(obj, LV_PART_KNOB);
 
     lv_coord_t transf_w = lv_obj_get_style_transform_width(obj, LV_PART_KNOB);
@@ -453,4 +434,9 @@ static void position_knob(lv_obj_t * obj, lv_area_t * knob_area, lv_coord_t knob
     knob_area->y2 += knob_bottom + transf_h;
 }
 
+static bool is_slider_horizontal(lv_obj_t * obj)
+{
+    return lv_obj_get_width(obj) >= lv_obj_get_height(obj);
+}
+
 #endif

tests/src/test_cases/test_slider.c

@@ -0,0 +1,214 @@
+#if LV_BUILD_TEST
+#include "../lvgl.h"
+
+#include "unity/unity.h"
+#include "lv_test_indev.h"
+
+static lv_obj_t * active_screen = NULL;
+static lv_obj_t * slider = NULL;
+static lv_obj_t * sliderRangeMode = NULL;
+static lv_obj_t * sliderNormalMode = NULL;
+static lv_obj_t * sliderSymmetricalMode = NULL;
+static lv_group_t * g = NULL;
+
+void setUp(void)
+{
+    active_screen = lv_scr_act();
+    slider = lv_slider_create(active_screen);
+    sliderRangeMode = lv_slider_create(active_screen);
+    sliderNormalMode = lv_slider_create(active_screen);
+    sliderSymmetricalMode = lv_slider_create(active_screen);
+
+    lv_slider_set_mode(sliderRangeMode, LV_SLIDER_MODE_RANGE);
+    lv_slider_set_mode(sliderNormalMode, LV_SLIDER_MODE_NORMAL);
+    lv_slider_set_mode(sliderSymmetricalMode, LV_SLIDER_MODE_SYMMETRICAL);
+
+    g = lv_group_create();
+    lv_indev_set_group(lv_test_encoder_indev, g);
+}
+
+void tearDown(void)
+{
+    lv_obj_clean(active_screen);
+}
+
+void test_textarea_should_have_valid_documented_default_values(void)
+{
+    lv_coord_t objw = lv_obj_get_width(slider);
+    lv_coord_t objh = lv_obj_get_height(slider);
+
+    /* Horizontal slider */
+    TEST_ASSERT_TRUE(objw >= objh);
+    TEST_ASSERT_FALSE(lv_obj_has_flag(slider, LV_OBJ_FLAG_SCROLL_CHAIN));
+    TEST_ASSERT_FALSE(lv_obj_has_flag(slider, LV_OBJ_FLAG_SCROLLABLE));
+}
+
+void test_slider_event_keys_right_and_up_increment_value_by_one(void)
+{
+    char key = LV_KEY_RIGHT;
+    lv_slider_set_value(slider, 10, LV_ANIM_OFF);
+    int32_t value = lv_slider_get_value(slider);
+
+    lv_event_send(slider, LV_EVENT_KEY, (void *) &key);
+
+    int32_t new_value = lv_slider_get_value(slider);
+    TEST_ASSERT_EQUAL_INT32(value + 1, new_value);
+
+    key = LV_KEY_UP;
+    lv_event_send(slider, LV_EVENT_KEY, (void *) &key);
+    TEST_ASSERT_EQUAL_INT32(new_value + 1, lv_slider_get_value(slider));
+}
+
+void test_slider_event_keys_left_and_down_decrement_value_by_one(void)
+{
+    char key = LV_KEY_LEFT;
+    lv_slider_set_value(slider, 10, LV_ANIM_OFF);
+    int32_t value = lv_slider_get_value(slider);
+
+    lv_event_send(slider, LV_EVENT_KEY, (void *) &key);
+
+    int32_t new_value = lv_slider_get_value(slider);
+    TEST_ASSERT_EQUAL_INT32(value - 1, new_value);
+
+    key = LV_KEY_DOWN;
+    lv_event_send(slider, LV_EVENT_KEY, (void *) &key);
+    TEST_ASSERT_EQUAL_INT32(new_value - 1, lv_slider_get_value(slider));
+}
+
+void test_slider_event_invalid_key_should_not_change_values(void)
+{
+    char key = LV_KEY_ENTER;
+    lv_slider_set_value(slider, 10, LV_ANIM_OFF);
+    int32_t value = lv_slider_get_value(slider);
+
+    lv_event_send(slider, LV_EVENT_KEY, (void *) &key);
+
+    TEST_ASSERT_EQUAL_INT32(value, lv_slider_get_value(slider));
+}
+
+void test_slider_range_mode_should_leave_edit_mode_if_released(void)
+{
+    lv_slider_t * ptr = (lv_slider_t *) sliderRangeMode;
+
+    /* Setup group and encoder indev */
+    lv_group_add_obj(g, sliderNormalMode);
+    lv_group_set_editing(g, true);
+
+    lv_test_encoder_click();
+
+    /* Always executed when handling LV_EVENT_RELEASED or
+     * LV_EVENT_PRESS_LOST */
+    TEST_ASSERT_FALSE(ptr->dragging);
+    TEST_ASSERT_NULL(ptr->value_to_set);
+    TEST_ASSERT_EQUAL(0U, ptr->left_knob_focus);
+
+    /* Group leaved edit mode */
+    TEST_ASSERT_FALSE(lv_group_get_editing(g));
+}
+
+void test_slider_range_mode_should_not_leave_edit_mode_if_released_with_no_left_knob_focus(void)
+{
+    lv_slider_t * ptr = (lv_slider_t *) sliderRangeMode;
+
+    /* Setup group and encoder indev */
+    lv_group_add_obj(g, sliderRangeMode);
+    lv_group_set_editing(g, true);
+
+    lv_test_encoder_release();
+    lv_test_indev_wait(50);
+
+    /* Always executed when handling LV_EVENT_RELEASED or
+     * LV_EVENT_PRESS_LOST */
+    TEST_ASSERT_FALSE(ptr->dragging);
+    TEST_ASSERT_NULL(ptr->value_to_set);
+
+    TEST_ASSERT(lv_group_get_editing(g));
+}
+
+void test_slider_normal_mode_should_leave_edit_mode_if_released(void)
+{
+    lv_slider_t * ptr = (lv_slider_t *) sliderNormalMode;
+    ptr->left_knob_focus = 1;
+
+    /* Setup group and encoder indev */
+    lv_group_add_obj(g, sliderNormalMode);
+    lv_group_set_editing(g, true);
+
+    lv_test_encoder_click();
+
+    /* Always executed when handling LV_EVENT_RELEASED or
+     * LV_EVENT_PRESS_LOST */
+    TEST_ASSERT_FALSE(ptr->dragging);
+    TEST_ASSERT_NULL(ptr->value_to_set);
+    TEST_ASSERT_EQUAL(0U, ptr->left_knob_focus);
+
+    /* Group leaved edit mode */
+    TEST_ASSERT_FALSE(lv_group_get_editing(g));
+}
+
+void test_ranged_mode_adjust_with_encoder(void)
+{
+    lv_slider_set_value(sliderRangeMode, 90, LV_ANIM_OFF);
+    lv_slider_set_left_value(sliderRangeMode, 10, LV_ANIM_OFF);
+
+    /* Setup group and encoder indev */
+    lv_group_add_obj(g, sliderRangeMode);
+    lv_group_set_editing(g, false);
+
+    /*Go the edit mode*/
+    lv_test_encoder_click();
+
+    /*Adjust the right knob*/
+    lv_test_encoder_turn(-10);
+    TEST_ASSERT_EQUAL(80, lv_slider_get_value(sliderRangeMode));  /*Updated?*/
+    TEST_ASSERT_EQUAL(10, lv_slider_get_left_value(sliderRangeMode));     /*Maintained?*/
+
+    /*Focus the left knob*/
+    lv_test_encoder_click();
+
+    /*Adjust the left knob*/
+    lv_test_encoder_turn(5);
+    TEST_ASSERT_EQUAL(80, lv_slider_get_value(sliderRangeMode));  /*Maintained?*/
+    TEST_ASSERT_EQUAL(15, lv_slider_get_left_value(sliderRangeMode));  /*Updated?*/
+
+}
+
+void test_normal_mode_slider_hit_test(void)
+{
+    /* Validate if point 0,0 can click in the slider */
+    lv_point_t point = {
+        .x = 0,
+        .y = 0
+    };
+
+    lv_hit_test_info_t info = {
+        .res = false,
+        .point = &point
+    };
+
+    lv_slider_set_value(sliderNormalMode, 100, LV_ANIM_OFF);
+    lv_event_send(sliderNormalMode, LV_EVENT_HIT_TEST, (void *) &info);
+
+    /* point can click slider */
+    TEST_ASSERT(info.res);
+}
+
+void test_slider_range_event_hit_test(void)
+{
+    /* Validate if point 0,0 can click in the slider */
+    lv_point_t point = {
+        .x = 0,
+        .y = 0
+    };
+
+    lv_hit_test_info_t info = {
+        .res = false,
+        .point = &point
+    };
+    lv_event_send(sliderRangeMode, LV_EVENT_HIT_TEST, (void *) &info);
+
+    /* point can click slider in the left knob */
+    TEST_ASSERT(info.res);
+}
+
+#endif