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

Chart example: add gap between old and new data #2565

Merged
merged 3 commits into from Sep 27, 2021

Conversation

kisvegabor
Copy link
Member

Description of the feature or fix

Demonstrate how to add gap between the newest and oldest points of a circular line chart.

@amirgon I was trying to add the MP version too but couldn't figure out how to convert this to get an array:

    lv_coord_t * a = lv_chart_get_y_array(chart, ser);

Checkpoints

There was a missing bullet if the previous point was LV_CHART_POINT_NONE
@amirgon
Copy link
Contributor

amirgon commented Sep 17, 2021

@amirgon I was trying to add the MP version too but couldn't figure out how to convert this to get an array:

    lv_coord_t * a = lv_chart_get_y_array(chart, ser);

Currently returning an entire array (by returning a pointer to the first element of the array) is not supported in Micropython Bindings.
(The other direction is supported - you can set an array by sending a Python List as an argument)

That's because -

  • It is not clear how many elements are in the array. How do we prevent the user from accessing past the last element? Currently in case of overflow the system would crash of behave unexpectedly.
  • It is rarely used. In LVGL such array is usually (always?) set by the user, so the user already has access to that array without the need to read it from LVGL API.

Could you explain why reading chart coords is needed? (Or why some other array getter is needed?)

If this is important, I may be able to implement this under the assumption that the user would never try to access elements out of bound, or preferably, if we have some way to detect the size of the array.


btw, how is the missing bullet fix related to demonstrating how to add gaps?

@kisvegabor
Copy link
Member Author

If this is important, I may be able to implement this under the assumption that the user would never try to access elements out of bound, or preferably, if we have some way to detect the size of the array.

IMO it'd be better than nothing. We could implement an lv_array_t or so but IMO it'd be overkill and very cumbersome for a very little gain.

Could you explain why reading chart coords is needed? (Or why some other array getter is needed?)

To manually set some empty points as gap.

btw, how is the missing bullet fix related to demonstrating how to add gaps?

I forgot the push the really relevant commit. 😅

@amirgon
Copy link
Contributor

amirgon commented Sep 17, 2021

Could you explain why reading chart coords is needed? (Or why some other array getter is needed?)

To manually set some empty points as gap.

I still don't get it.
The user is the one who sets the array coords, right?
So the user knows (and can keep reference of) the array coords of his chart.
So why does the user need to read the array coords from LVGL if he already knows them, because he was the one who set them in the first place?

@kisvegabor
Copy link
Member Author

kisvegabor commented Sep 17, 2021

The user is the one who sets the array coords, right?

By default LVGL allocates and reallocates the "coord arrays" based on the set point count of the chart. It's only an option to use an external array e.g. if

  • you already have the data in an array
  • you store the array in a "special memory" e.g. external RAM or flash or so

@amirgon
Copy link
Contributor

amirgon commented Sep 17, 2021

By default LVGL allocates and reallocates the "coord arrays" based on the set point count of the chart.

But it's still the user who sets the data, so he could keep track of it.

Anyway, If I implement this, could we agree on a way to tell the array size? Maybe by some new convention?
For example, a new C function with _array_size suffix for each function that returns an array:

size_t lv_chart_get_y_array_size(const lv_obj_t * obj, lv_chart_series_t * ser);

That way the binding could query array size for each array it is handling and verify the user is not overflowing.

@kisvegabor
Copy link
Member Author

But it's still the user who sets the data, so he could keep track of it.

Yes, but usually one by one. Only set the next value. So don't have to keep track with the whole array. E.g. you measure the temperature in every 5 minutes and just push the measured value to the chart.

Anyway, If I implement this, could we agree on a way to tell the array size? Maybe by some new convention?
For example, a new C function with _array_size suffix for each function that returns an array:

Unfortunately it's not that straight forward here. Both the x and y arrays have point_count size
set by lv_chart_set_point_count(). So I don't see a good logical way to add function(s) to get the array size.

I'd go with the unsafe version (no boundary check) or I can implement it as @uraich did here.

@amirgon
Copy link
Contributor

amirgon commented Sep 18, 2021

Unfortunately it's not that straight forward here. Both the x and y arrays have point_count size
set by lv_chart_set_point_count(). So I don't see a good logical way to add function(s) to get the array size.

Could you please explain this?
I don't understand the problem. Why can't lv_chart_get_y_array_size(... return whatever was set by lv_chart_set_point_count()?

@kisvegabor
Copy link
Member Author

It could, but it'd be redundant to add 2 new functions with exactly the same purpose as lv_chart_set_point_count().

@amirgon
Copy link
Contributor

amirgon commented Sep 20, 2021

I'm looking into array access (without boundary checks).
Initial work on lvgl/lv_binding_micropython#179, still requires some work to be usable.

@kisvegabor
Copy link
Member Author

Great!

@amirgon
Copy link
Contributor

amirgon commented Sep 21, 2021

@kisvegabor I've implemented array access.
See details here: lvgl/lv_binding_micropython#179 (comment)

Please try to add the MP example you were working on with array access and let me know if there are any issues.

@kisvegabor
Copy link
Member Author

Great, thank you! It's still not available in the online simulator right?

@amirgon
Copy link
Contributor

amirgon commented Sep 22, 2021

It's still not available in the online simulator right?

Actually it is available.

@embeddedt took care of automatic deployment whenever lv_micropython is updated.

@kisvegabor
Copy link
Member Author

Great! I made a quick test and it was working:
LINK

@kisvegabor
Copy link
Member Author

@amirgon I've implemented the Python version here.

Can you take look if it looks Pythonic enough? 🙂

@uraich
Copy link
Contributor

uraich commented Sep 24, 2021

Looks nice to me. In fact, these calls make treatment of the point array quite a bit simpler. When I did the examples, I had to treat the point array myself, outside the chart.
So, I think this is quite an improvement.

@amirgon
Copy link
Contributor

amirgon commented Sep 25, 2021

@amirgon I've implemented the Python version here.

Can you take look if it looks Pythonic enough?

Looks good to me .

I just don't understand why you need three assignment to CHART_POINT. NONE

@Karijn
Copy link
Contributor

Karijn commented Sep 25, 2021

Hmmm, I think that I was to fast with my pull request for MicroPython sample lv_example_chart_9.py

@amirgon
Copy link
Contributor

amirgon commented Sep 26, 2021

my pull request for MicroPython sample

Ok, added question there.

@kisvegabor
Copy link
Member Author

Okay, I merge it then. Let's continue the MP part in #2604

@kisvegabor kisvegabor merged commit 5565d59 into master Sep 27, 2021
@kisvegabor kisvegabor deleted the feat/chart-example-9 branch September 27, 2021 09:01
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

4 participants