search

lvgl / lv_binding_rust

LVGL bindings for Rust. A powerful and easy-to-use embedded GUI with many widgets, advanced visual effects (opacity, antialiasing, animations) and low memory requirements (16K RAM, 64K Flash).
MIT License
687 stars 71 forks source link

Rust bindings API review #51

Closed rafaelcaricio closed 1 year ago

rafaelcaricio commented 3 years ago

This PR makes the Rust bindings usage more ergonomic and flexible.

nia-e commented 1 year ago

What's the status on this PR? I was considering attempting to upgrade LVGL to 8.x and it seems like this is necessary. I can rebase on current master and flesh out any missing features if that would be desirable.

rafaelcaricio commented 1 year ago

@nia-e As described in the README of this project. I've moved on from it, since I couldn't find anyone to help me with the development. That means this PR is in its "final" state, as I don't plan to work further on it. I'm pleased that you seem to have interest in this project, and if you think this can or should be merged, please go ahead!

I think this PR has some things that need a second opinion. I've added some comments to things I wanted to highlight. The public API changes here, and we remove explicit dependency on embedded_graphics (it is optional).

There is a part of this PR that I stopped working on before making use of it, which is in the code generation crate. I think we could do much better there. Just keep in mind that I've started this project to help me "learn Rust", so take with a grain of salt the things this library does. If you see something that seems wrong to you, it is because it is probably wrong indeed. 👀

nia-e commented 1 year ago

To my utter surprise, a rebase and minimal fixes later, this compiles. I'd rather test more things before I do anything but it seems like my job is easier than expected. Should I open a new PR or push to this branch? I squashed all commits.

rafaelcaricio commented 1 year ago

@nia-e Awesome, Rust is great. lol

You can commit to this branch and continue this PR.

nia-e commented 1 year ago

What were the plans for lvgl_codegen? It has some unused code and behavior is missing. I could try to finish implementing whatever the original idea was.

nia-e commented 1 year ago

Also; further digging on the Mutexes revealed that there is Probably No Way (that I could find) to get rid of them short of marking this entire library as non-thread-safe which doesn't feel great. I'd love to be proven wrong, though. parking_lot is a very small crate though and this shouldn't be a huge deal - certainly far better than pulling in all of embedded-graphics at all times like before.

rafaelcaricio commented 1 year ago

@nia-e The idea with the lvgl-codegen crate was to be able to generate the entirety of the LVGL C API instead of manually writing bindings for every single API function LVGL exposes. I wanted to have in the lvgl crate the "core abstractions" that would be used by the codegen. This was a suggestion I got at the begging of this project. That thread might be a good source of what I was thinking at the time too (it has been a while ago 😬 ).

LVGL is not thread-safe itself, so making this crate non-thread-safe shouldn't be a bid deal? 👀 For us to make this crate "thread-safe" we would need to (have and) lock a global Mutex every time we call LVGL C API, not sure if that is a good idea!?

nia-e commented 1 year ago

Perfect, okay. I'll drop the Mutexes and read through that thread afterwards.

nia-e commented 1 year ago

Seems like extending lvgl-codegen is a can I can safely kick down the road. All in all, I think this is complete and ready for review. Squashed commits. One more thing I need to fix then it'll be ready.

nia-e commented 1 year ago

Okay, now it's ready. Give this a look when you have some time @rafaelcaricio. Also this hopefully beings to close #42

rafaelcaricio commented 1 year ago

LGTM. I cannot approve as I originally opened the PR. Feel free to merge ;)