Rust bindings API review #51

Open
rafaelcaricio wants to merge 19 commits from api-review into master
2 changed files with 28 additions and 10 deletions
Showing only changes of commit 93c68afc0e - Show all commits

View file

@ -79,7 +79,7 @@ impl<const N: usize> DrawBuffer<N> {
rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.
rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.
if self.initialized.swap_and_check() {
// TODO: needs to be 'static somehow
// Cannot be in the DrawBuffer struct because the type `lv_disp_buf_t` contains a raw
// pointer and raw pointers are not Sync and consequently cannot be in `static` variables.
rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.
// pointer and raw pointers are not Send and consequently cannot be in `static` variables.
rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.
let mut inner: MaybeUninit<lvgl_sys::lv_disp_buf_t> = MaybeUninit::uninit();
rafaelcaricio commented 2021-06-08 20:51:59 +00:00 (Migrated from github.com)
Review

I would like to find a way to make this memory statically allocated. This seems to be a requirement for LVGL v8.

I would like to find a way to make this memory statically allocated. This seems to be [a requirement for LVGL v8](https://docs.lvgl.io/8.0/get-started/quick-overview.html#add-lvgl-into-your-project).
let primary_buffer_guard = self.refresh_buffer.lock();
let draw_buf = unsafe {

rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.
rafaelcaricio commented 2021-06-04 20:41:26 +00:00 (Migrated from github.com)
Review

TODO: Remove this code duplication.

TODO: Remove this code duplication.
rafaelcaricio commented 2021-06-04 20:43:14 +00:00 (Migrated from github.com)
Review

I use a Mutex here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

I use a `Mutex` here, to satisfy the Rust guarantees. But maybe it is not necessary... some food for thought.

View file

@ -1,9 +1,6 @@
use crate::widgets::Label;
use crate::{LvResult, NativeObject};
#[cfg(feature = "alloc")]
use cstr_core::CString;
impl Label {
pub fn set_label_align(&mut self, align: LabelAlign) -> LvResult<()> {
unsafe {
@ -14,15 +11,36 @@ impl Label {
}
rafaelcaricio commented 2021-06-04 20:38:52 +00:00 (Migrated from github.com)
Review

This is a test of what is possible with alloc feature enabled. In reality, the From trait implementation should use the TryFrom trait implementation and just call .unwrap() here. People will decide which guarantees they want to have.

This is a test of what is possible with `alloc` feature enabled. In reality, the `From` trait implementation should use the `TryFrom` trait implementation and just call `.unwrap()` here. People will decide which guarantees they want to have.
#[cfg(feature = "alloc")]
impl<S: AsRef<str>> From<S> for Label {
fn from(text: S) -> Self {
let text_cstr = CString::new(text.as_ref()).unwrap();
let mut label = Label::new().unwrap();
label.set_text(text_cstr.as_c_str()).unwrap();
label
mod alloc_imp {
use crate::widgets::Label;
use crate::LvError;
use cstr_core::CString;
use core::convert::TryFrom;
impl<S: AsRef<str>> From<S> for Label {
fn from(text: S) -> Self {
// text.try_into().unwrap()
let text_cstr = CString::new(text.as_ref()).unwrap();
let mut label = Label::new().unwrap();
label.set_text(text_cstr.as_c_str()).unwrap();
label
}
}
// Issue link: https://github.com/rust-lang/rust/issues/50133
//
// impl<S: AsRef<str>> TryFrom<S> for Label {
// type Error = LvError;
// fn try_from(text: S) -> Result<Self, Self::Error> {
// let text_cstr = CString::new(text.as_ref())?;
// let mut label = Label::new()?;
// label.set_text(text_cstr.as_c_str())?;
// Ok(label)
// }
// }
}
#[derive(Debug, Copy, Clone, PartialEq)]
#[repr(u8)]
pub enum LabelAlign {