Consider moving boilerplate into its own module/crate?

embeddedt commented

2020-06-19 12:53:12 +00:00

(Migrated from github.com)

I'm reading the examples to gain an understanding of how this works and I notice that the boilerplate for calling lv_task_handler and lv_tick_inc is duplicated across many of the examples. It might be worth moving this to a common crate where it can be reused.

b85226aada/examples/gauge.rs (L60-L107)

I'm reading the examples to gain an understanding of how this works and I notice that the boilerplate for calling `lv_task_handler` and `lv_tick_inc` is duplicated across many of the examples. It might be worth moving this to a common crate where it can be reused. https://github.com/rafaelcaricio/lvgl-rs/blob/b85226aadac7b9eeed6bebcb34f2008ea803eaa4/examples/gauge.rs#L60-L107

rafaelcaricio commented

2020-06-19 13:13:55 +00:00

(Migrated from github.com)

@embeddedt True, that is a valid concern. I just think we need to focus on showing how to use the library. If we start designing too much the examples, it will be difficult for people to understand what is going on.

Do you have an example in C that uses LVGL without threads? I was thinking we could remove this Mutex from the examples and thus simplify all the examples.

@embeddedt True, that is a valid concern. I just think we need to focus on showing how to use the library. If we start designing too much the examples, it will be difficult for people to understand what is going on. Do you have an example in C that uses LVGL without threads? I was thinking we could remove this `Mutex` from the examples and thus simplify all the examples.

embeddedt commented

2020-06-19 13:18:11 +00:00

(Migrated from github.com)

None of the example code itself uses threads (as the library is not threaded), however, most of the platform implementations run lv_tick_inc using a thread or an interrupt, to make sure it doesn't block lv_task_handler.

For nearly all the platforms, lv_task_handler gets run in a while(1) loop inside main.

Does Rust have a function that returns the number of milliseconds since system startup? If so, we can use that, and not even bother calling lv_tick_inc. That way, the Rust examples can work identically to the C examples.

I just think we need to focus on showing how to use the library. If we start designing too much the examples, it will be difficult for people to understand what is going on.

Exactly. I know that the code is really just calling lv_task_handler and lv_tick_inc, but there is lots happening in order to do that that I don't really understand yet.

OT: Can you explain why you need to create an arc to call lv_task_handler? Is this some type of syntactic sugar?

let threaded_ui = Arc::new(Mutex::new(ui));
threaded_ui.lock().unwrap().task_handler();

None of the example code itself uses threads (as the library is not threaded), however, most of the platform implementations run `lv_tick_inc` using a thread or an interrupt, to make sure it doesn't block `lv_task_handler`. For nearly all the platforms, `lv_task_handler` gets run in a `while(1)` loop inside `main`. Does Rust have a function that returns the number of milliseconds since system startup? If so, we can use that, and not even bother calling `lv_tick_inc`. That way, the Rust examples can work identically to the C examples. > I just think we need to focus on showing how to use the library. If we start designing too much the examples, it will be difficult for people to understand what is going on. Exactly. I know that the code is really just calling `lv_task_handler` and `lv_tick_inc`, but there is lots happening in order to do that that I don't really understand yet. OT: Can you explain why you need to create an arc to call `lv_task_handler`? Is this some type of syntactic sugar? ```rust let threaded_ui = Arc::new(Mutex::new(ui)); threaded_ui.lock().unwrap().task_handler(); ```

rafaelcaricio commented

2020-06-19 13:33:46 +00:00

(Migrated from github.com)

I just updated the one example to remove the Mutex and Arc completely (7466815586). I think I wrote that way initially due to my lack of experience using the LVGL library before. 🤦

An Arc in Rust means Atomic Reference Counter, it is a way to share a reference to a variable with multiple threads. Better described at https://doc.rust-lang.org/std/sync/struct.Arc.html

I just updated the one example to remove the `Mutex` and `Arc` completely (https://github.com/rafaelcaricio/lvgl-rs/pull/23/commits/7466815586dc648a695b63ccb115b1d0eea66288). I think I wrote that way initially due to my lack of experience using the LVGL library before. :facepalm: An `Arc` in Rust means `Atomic Reference Counter`, it is a way to share a reference to a variable with multiple threads. Better described at https://doc.rust-lang.org/std/sync/struct.Arc.html

embeddedt commented

2020-06-19 13:42:54 +00:00

(Migrated from github.com)

That definition of Arc makes a lot more sense. I thought it was this type of arc. 🙂

I've installed Rust on my computer so hopefully I can give this a try at some point and see how it compares to using C directly.

I just have two more suggestions.

This method of passing a string is a bit awkward. Can LVGL APIs that want a string accept a Rust string as well as a C string?
Maybe we can use Instant::now with the LV_TICK_CUSTOM options directly. That way, we don't need to deal with lv_tick_inc at all in the code.

That definition of `Arc` makes a lot more sense. I thought it was [this type of arc](https://docs.lvgl.io/latest/en/html/widgets/arc.html). :slightly_smiling_face: I've installed Rust on my computer so hopefully I can give this a try at some point and see how it compares to using C directly. I just have two more suggestions. * [This method of passing a string](https://github.com/rafaelcaricio/lvgl-rs/blob/7466815586dc648a695b63ccb115b1d0eea66288/examples/bar.rs#L51) is a bit awkward. Can LVGL APIs that want a string accept a Rust string as well as a C string? * Maybe we can use [`Instant::now`](https://doc.rust-lang.org/std/time/struct.Instant.html#method.now) with the [`LV_TICK_CUSTOM` options](https://github.com/lvgl/lvgl/blob/0f4aeede79e93f3a57c510c4dfb970eddc39d901/lv_conf_template.h#L263-L269) directly. That way, we don't need to deal with `lv_tick_inc` at all in the code.

❤️ 1

rafaelcaricio commented

2020-06-19 14:05:03 +00:00

(Migrated from github.com)

I've updated all examples removing the threads. 👍

On the other suggestions, it's complicated...

Strings in Rust are a long topic, that's why you will find many string types. From std and from other crates ( https://docs.rs/cstr_core/0.2.0/cstr_core/struct.CStr.html , https://japaric.github.io/heapless/heapless/struct.String.html ). Each have their perks and use cases. In case of embedded Rust, we cannot use anything under std, since it is built upon the OS abstractions. We work here in #[no_std] Rust universe, so we are restricted to the Rust core. I'm thinking of ways to improve this part. Maybe creating our own String type that uses the lv_mem_* mechanism to manipulate the string bytes. All we need is to append \0 character at the end of the string, Rust str don't contain \0 at the end.
This is very interesting to know, it is totally possible to expose a Rust function to LVGL with those requirements. BUT, as I mentioned in the previous point, we don't have access to std in embedded Rust. What I can think of here is a way to enable users of lvgl-rs to register themselves a pure Rust function to be called to calculate time. I think it's a valid feature to add. Do you mind opening a new ticket just for this one?

Thanks for your comments! This helps a lot. 😃

I've updated all examples removing the threads. :+1: On the other suggestions, it's complicated... - Strings in Rust are a long topic, that's why you will find many string types. From [`std`](https://doc.rust-lang.org/std/string/struct.String.html?search=String) and from other crates ( https://docs.rs/cstr_core/0.2.0/cstr_core/struct.CStr.html , https://japaric.github.io/heapless/heapless/struct.String.html ). Each have their perks and use cases. In case of embedded Rust, we cannot use anything under `std`, since it is built upon the OS abstractions. We work here in `#[no_std]` Rust universe, so we are restricted to the Rust [`core`](https://doc.rust-lang.org/core/index.html). I'm thinking of ways to improve this part. Maybe creating our own `String` type that uses the `lv_mem_*` mechanism to manipulate the string bytes. All we need is to append `\0` character at the end of the string, Rust `str` don't contain `\0` at the end. - This is very interesting to know, it is totally possible to expose a Rust function to LVGL with those requirements. BUT, as I mentioned in the previous point, we don't have access to `std` in embedded Rust. What I can think of here is a way to enable users of `lvgl-rs` to register themselves a pure Rust function to be called to calculate time. I think it's a valid feature to add. Do you mind opening a new ticket just for this one? Thanks for your comments! This helps a lot. :smiley:

embeddedt commented

2020-06-19 14:10:28 +00:00

(Migrated from github.com)

I'm thinking of ways to improve this part. Maybe creating our own String type that uses the lv_mem_* mechanism to manipulate the string bytes. All we need is to append \0 character at the end of the string, Rust str don't contain \0 at the end.

In my opinion, the best thing to do here is to make sure that a string literal "just works" (TM) without needing any extra wrapper around it in the user's code.

> I'm thinking of ways to improve this part. Maybe creating our own String type that uses the lv_mem_* mechanism to manipulate the string bytes. All we need is to append \0 character at the end of the string, Rust str don't contain \0 at the end. In my opinion, the best thing to do here is to make sure that a string literal "just works" (TM) without needing any extra wrapper around it in the user's code.

rafaelcaricio commented

2020-06-19 14:29:40 +00:00

(Migrated from github.com)

Yes, that is what I meant. Rust "string literals" are static strings, so to append a nul byte at the end I need to, internally, have a sort of dynamic array and copy the static string data from Rust in there and add a \0 at the end so it is a valid C string. Thus the internal CStr type just for that, it wouldn't be exposed by the crate to external users.

Yes, that is what I meant. Rust "string literals" are static strings, so to append a nul byte at the end I need to, internally, have a sort of dynamic array and copy the static string data from Rust in there and add a `\0` at the end so it is a valid C string. Thus the internal `CStr` type just for that, it wouldn't be exposed by the crate to external users.

embeddedt commented

2020-06-19 14:32:29 +00:00

(Migrated from github.com)

I see. This is an unfortunate limitation, because it makes passing strings to C quite inefficient. It would be nice if the Rust compiler had a flag that would make it generate the strings with \0 on the end automatically.

I see. This is an unfortunate limitation, because it makes passing strings to C quite inefficient. It would be nice if the Rust compiler had a flag that would make it generate the strings with `\0` on the end automatically.

rafaelcaricio commented

2020-06-19 14:41:15 +00:00

(Migrated from github.com)

Yeah, that doesn't exist. Best thing is to let users pass a instance of cstr_core::CStr so no need to copy as it already contains the nul byte at the end.

Yeah, that doesn't exist. Best thing is to let users pass a instance of `cstr_core::CStr` so no need to copy as it already contains the nul byte at the end.

rafaelcaricio commented

2021-06-03 17:47:22 +00:00

(Migrated from github.com)

Now see a better way to deal with this. If the users the the lib enable alloc we can support native Rust strings (&str and String, and others) by generating a method that accepts impl AsRef<str> object. We would have always a method that accepts a ready CStr and another would be available if the user enables alloc feature of LVGL-rs.

fn set_text_cstr(&mut self, text: impl AsRef<cstr_core::CStr>) -> Result<(), Error> { 
    // Calls LVGL FFI 
}

#[cfg(feature = "alloc")]
fn set_text(&mut self, text: impl AsRef<str>) -> Result<(), Error> { 
    // Converts to a CStr here to have the `\0` at the end
    self.set_text_cstr(&CString::new(text.as_ref())?.as_c_str())
}

If the alloc feature is enabled, then the LVGL-rs API would be much nicer to call with strings.

loading_lbl.set_text("Loading...")?;

let text = String::new("Click me!");
button_lbl.set_text(text)?;

Now see a better way to deal with this. If the users the the lib enable `alloc` we can support native Rust strings (`&str` and `String`, and others) by generating a method that accepts `impl AsRef<str>` object. We would have always a method that accepts a ready `CStr` and another would be available if the user enables `alloc` feature of LVGL-rs. ```rust fn set_text_cstr(&mut self, text: impl AsRef<cstr_core::CStr>) -> Result<(), Error> { // Calls LVGL FFI } #[cfg(feature = "alloc")] fn set_text(&mut self, text: impl AsRef<str>) -> Result<(), Error> { // Converts to a CStr here to have the `\0` at the end self.set_text_cstr(&CString::new(text.as_ref())?.as_c_str()) } ``` If the `alloc` feature is enabled, then the LVGL-rs API would be much nicer to call with strings. ```rust loading_lbl.set_text("Loading...")?; let text = String::new("Click me!"); button_lbl.set_text(text)?; ```

Consider moving boilerplate into its own module/crate? #24