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

Add support for LVGL transitions #23020

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

treitmayr
Copy link
Contributor

@treitmayr treitmayr commented Feb 17, 2025

Description:

This PR enables support for the LVGL transition feature which creates continuous adjustments from one style to another to be performed over a given time duration.

For this the object type lv.style_transition_dsc was enabled, and it can be fully initialized via its constructor (which in turn calls the native LVGL function lv_style_transition_dsc()). The PR currently only includes the actual code changes as well as an updated LVGL berry example, but does not include the changed generated files. I can add the generated files as soon as all kinks and quirks found in the code during the review were straightened out.

One thing to observe is that passing a style_transition_dsc object to lv.style.set_transition() does not add a reference to the object, therefore it also has to be kept in some variable to prevent its garbage collection. I am not sure how this is usually solved.

Also please check for any issues in the lv_berry.c code because I implemented this glue code mostly be looking up examples in other Tasmota source files. In a stripped down version we could leave out the function list_to_zbytes() and require the user to create an lv.style_prop_arr object as the first parameter for initialization.

As for the example, it works very well with just copy/pasting it to the Berry console. It also shows the two different ways of creating the lv.style_transition_dsc object.

Related issue (if applicable): none

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.8
  • The code change is tested and works with Tasmota core ESP32 V.3.1.1.250203
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@s-hadinger
Copy link
Collaborator

Wow, you are mastering the LVGL mapping. Impressive. I skimmed through the code and saw nothing abnormal.

I'm ok to merge now, anyways it does not interfere with other functions.

@s-hadinger
Copy link
Collaborator

It will need to wait for 1 day or 2. Version 14.5.0 is getting out, I will merge right after.

@treitmayr
Copy link
Contributor Author

Thank you for the quick review! No hurry, please - good to see a new release coming.

At some point maybe you could let me know about your opinion regarding the list_to_zbytes() code path. It adds convenience at the cost of more code. And although it took me quite a while to finish this function, it may not be worth it after all when it is only used in the (probably rare) case someone uses transitions.
Before the merge please also let me know if I shall provide the updated generated files or you do that yourself (not sure how that usually is done).

@@ -35,6 +35,7 @@
lv_dir = ct.u8
lv_coord_t = ct.i32
lv_value_precise = ct.i32
lv_style_prop_t = ct.u8
Copy link
Collaborator

@s-hadinger s-hadinger Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not actually useful, this is only needed for structure mapping in this python file.
Please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it, you are right, it is not of any use.

This commit includes the actual code changes as well as updated
LVGL berry examples, but does not include the changed generated files.
@treitmayr
Copy link
Contributor Author

I just realized that there is a second example for LVGL transitions ("gummy button") which I now adjusted as well.

@@ -112,10 +112,10 @@ end
class lv_style_prop_arr : bytes
def init(l)
if type(l) != 'instance' || !isinstance(l, list) raise "value_error", "argument must be a list" end
super(self).init(size(l) * 4)
super(self).init(size(l))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a bug indeed

@s-hadinger
Copy link
Collaborator

Overall excellent job. My main concern is that class lv_style_transition_dsc already exists as a bytes buffer. Hence creating the C class instead masks the access to the internal fields.

Ideally I would like to call your constructor on top of the bytes() mapping.

I'm wondering if this could be even more simplified by doing a super class in Berry just to override the constructor.

Stay tuned

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.

2 participants