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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

arc initial unit tests #2617

Merged
merged 9 commits into from Oct 7, 2021
Merged

arc initial unit tests #2617

merged 9 commits into from Oct 7, 2021

Conversation

C47D
Copy link
Contributor

@C47D C47D commented Sep 29, 2021

Description of the feature or fix

What the title says, if you need me to add any special test please let me know 馃樃

Checkpoints

@C47D
Copy link
Contributor Author

C47D commented Sep 29, 2021

test_arc_should_truncate_to_min_range_when_new_value_is_inferior fails because the expected min_value after creation is 1 (from the docs), not 0. Please let me know if this is not the case.

There are some refactor opportunities that could be done in lv_arc.c, but achieving a high 100% test coverage before hand would be very useful. That way we have a test harness that would let us know if we break something (a test) with the refactor, what do you think @kisvegabor @embeddedt?

@cmumford Thanks for adding coverage report, it does look very nice 馃樃 , I need to research how to 'visualize' the coverage info in the source files within WSL tho.
imagen

@kisvegabor
Copy link
Member

test_arc_should_truncate_to_min_range_when_new_value_is_inferior fails because the expected min_value after creation is 1 (from the docs), not 0. Please let me know if this is not the case.

The docs was incorrect. I've updated it to say 0..100.

There are some refactor opportunities that could be done in lv_arc.c

Can you mention some of these opportunities?

@C47D
Copy link
Contributor Author

C47D commented Sep 29, 2021

If you have more tests ideas please let me know. How can I trigger lv_arc_event? That function has almost no coverage.

The docs was incorrect. I've updated it to say 0..100.

Thanks, I've updated the test accordingly

Can you mention some of these opportunities?

It's just a cleanup on the value_update function, but having it covered with tests we're now able to change it and have immediate feedback to refactor it.

diff --git a/src/widgets/lv_arc.c b/src/widgets/lv_arc.c
index 4df3dc015..0cc1446f0 100644
--- a/src/widgets/lv_arc.c
+++ b/src/widgets/lv_arc.c
@@ -816,26 +816,24 @@ static void value_update(lv_obj_t * obj)
     /*If the value is still not set to any value do not update*/
     if(arc->value == VALUE_UNSET) return;

-    int16_t bg_midpoint, range_midpoint, bg_end = arc->bg_angle_end;
-    if(arc->bg_angle_end < arc->bg_angle_start) bg_end = arc->bg_angle_end + 360;
+    int16_t bg_end = arc->bg_angle_end;
+    if(arc->bg_angle_end < arc->bg_angle_start) bg_end += 360;

     int16_t angle;
     switch(arc->type) {
-        case LV_ARC_MODE_SYMMETRICAL:
-            bg_midpoint = (arc->bg_angle_start + bg_end) / 2;
-            range_midpoint = (int32_t)(arc->min_value + arc->max_value) / 2;
+        case LV_ARC_MODE_SYMMETRICAL: {
+            int16_t bg_midpoint = (arc->bg_angle_start + bg_end) / 2;
+            int16_t range_midpoint = (int16_t)(arc->min_value + arc->max_value) / 2;

             if(arc->value < range_midpoint) {
                 angle = lv_map(arc->value, arc->min_value, range_midpoint, arc->bg_angle_start, bg_midpoint);
-                lv_arc_set_start_angle(obj, angle);
-                lv_arc_set_end_angle(obj, bg_midpoint);
+                lv_arc_set_angles(obj, angle, bg_midpoint);
             }
             else {
                 angle = lv_map(arc->value, range_midpoint, arc->max_value, bg_midpoint, bg_end);
-                lv_arc_set_start_angle(obj, bg_midpoint);
-                lv_arc_set_end_angle(obj, angle);
+                lv_arc_set_angles(obj, bg_midpoint, angle);
             }
-            break;
+        }   break;
         case LV_ARC_MODE_REVERSE:
             angle = lv_map(arc->value, arc->min_value, arc->max_value, arc->bg_angle_start, bg_end);
             lv_arc_set_angles(obj, angle, arc->bg_angle_end);

@C47D
Copy link
Contributor Author

C47D commented Sep 30, 2021

OFFTOPIC
Why is the API documented in both header and source file? It seems like the docs are being generated with the latter, and in the picture the docs are wrong.
imagen

in the source file:
imagen

in the header file:
imagen

@C47D C47D marked this pull request as ready for review October 1, 2021 04:32
@C47D
Copy link
Contributor Author

C47D commented Oct 1, 2021

How can I trigger lv_arc_event? That function has almost no coverage.

I left that function without tests on purpose.

@kisvegabor
Copy link
Member

kisvegabor commented Oct 4, 2021

OFFTOPIC
Why is the API documented in both header and source file? It seems like the docs are being generated with the latter, and in the picture the docs are wrong.

API comments should be only in the headers and a lot of files are already updated but still not all. I've pushed a commit to fix it case of the arc.

How can I trigger lv_arc_event? That function has almost no coverage.

The functions from lv_test_indev.c should help to send pressing, release and key events. A rendering test can help to trigger draw related events.

@C47D
Copy link
Contributor Author

C47D commented Oct 4, 2021

The functions from lv_test_indev.c should help to send pressing, release and key events. A rendering test can help to trigger draw related events.

I tried to trigger the VALUE_CHANGED event but I wasn't able to do so. I will try again later.

@C47D
Copy link
Contributor Author

C47D commented Oct 6, 2021

Can we merge this and keep the event test for later?

@kisvegabor
Copy link
Member

Can we merge this and keep the event test for later?

Sure, every bit of test is a treasure. 馃檪

Thank you very much!

@kisvegabor kisvegabor merged commit acf915b into lvgl:master Oct 7, 2021
@C47D C47D deleted the test/arc_initial_tests branch October 8, 2021 03:03
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.

None yet

2 participants