-
Notifications
You must be signed in to change notification settings - Fork 2.1k
pkg/lvgl: add initial support for LittlevGL to RIOT #13124
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
Conversation
I tested this in the PineTime. Seems to work, with just minor modifications!
I have a picture, but somehow my Gnome/Wayland doesn't let me paste it here. ;( |
5c87f76
to
9793864
Compare
It's not that simple without patching lvgl. The
Probably doable, with get/set functions. |
Hm, looking at the "porting" example here, they first initialize the driver struct (using BTW, at this point, does it make sense to introduce |
I see the merits of having a
or:
|
The abstraction design has no external dependency so each display driver can provide the required driver functions (only one for the moment). If we reuse the abstraction API that comes from lvgl, then this introduce a hard dependency to it. I would prefer to avoid this. So for now I would use option B, as proposed by @bergzand: keep the disp_dev header and improve later. Regarding the while loop in the task thread, I agree it's not nice, especially because of power consumption. while (1) {
/*Normal operation (no sleep) in < 1 sec inactivity*/
if (lv_disp_get_inactive_time(NULL) < 1000) {
lv_task_handler();
}
/* Block after 1 sec inactivity */
else {
msg_t msg;
msg_receive(&msg);
}
xtimer_usleep(5 * US_PER_MS);
} Then the screen could be explicitly restarted from user code with a simple |
ok |
better use thread flags. Cannot get lost (e.g., if set from ISR), doesn't block caller. |
01598dc
to
4852d0a
Compare
@kaspar030, I just pushed a couple of commits that implement a mechanism to allow stopping the lvgl task handler thread after some inactivity period (defined by the LVGL_ACTIVITY_PERIOD macro). The glue code now provides an extra function In the sysmon example, this function is called each time the sysmon_task is called: this ensures the chart is always refreshed. But if you tweak the REFR_TIME macro in main.c to be greater than LVGL_ACTIVITY_PERIOD, then lvgl is stopped after LVGL_ACTIVITY_PERIOD is elapsed and an explicit call to lvgl_wakeup is needed to restart it (this can be done in a gpio callback for example). What do you think of the approach ? |
I added a bit of documentation. @kaspar030, if you think the thread_flags is good enough, this PR is close to be mergeable I think. |
e62b015
to
e8e7954
Compare
e8e7954
to
2ddf58c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first round
pkg/lvgl/contrib/lvgl.c
Outdated
#define LVGL_TASK_HANDLER_DELAY (5 * US_PER_MS) | ||
#endif | ||
|
||
static char _task_thread_stack[THREAD_STACKSIZE_MAIN]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be "THREAD_STACKSIZE_DEFAULT" (or maybe _MEDIUM or _LARGE?).
A user might have a large main and thus overrides "THREAD_STACKSIZE_MAIN", which shouldn't affect lvgl's thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to THREAD_STACKSIZE_LARGE, otherwise it doesnt fit.
pkg/lvgl/contrib/lvgl.c
Outdated
static lv_color_t buf[LVGL_COLOR_BUF_SIZE]; | ||
static disp_dev_t *_dev = NULL; | ||
|
||
void *_task_thread(void *arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all fixed
pkg/lvgl/contrib/lvgl.c
Outdated
return NULL; | ||
} | ||
|
||
void _disp_map(lv_disp_drv_t *drv, const lv_area_t *area, lv_color_t *color_p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static?
tests/pkg_lvgl/Makefile
Outdated
|
||
USEPKG += lvgl | ||
USEMODULE += ili9341 | ||
USEMODULE += xtimer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lvgl already depends on xtimer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a left-over from previous versions, removed.
2ddf58c
to
325fc1e
Compare
9434f1d
to
96297fb
Compare
I think I addressed all your comments @bergzand :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second round of reviewing
96297fb
to
8d4379e
Compare
Comments addressed @bergzand :) |
ping @bergzand :) |
c8620e3
to
5e4610c
Compare
5e4610c
to
e7189aa
Compare
Please squash after addressing my last comment |
90a116c
to
6b7be34
Compare
6b7be34
to
ae63826
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
Awesome ! Thanks a lot for the reviews and tests :) |
Contribution description
This PR adds support to the LittlevGL package to RIOT. I've been looking to it for quite some time and now that the ili9341 driver is in, I think it's time to move forward with this.
This PR also introduces a generic (and still basic) API for display devices, it follows the same scheme as netdev.
There's also a test application that display a system monitor on a screen. It was tested on the stm32f429i-disc1 board.
I know other people in the community (@bergzand, @fjmolinas) have already done some work with this package and maybe they can provide some good advice here so we can move forward faster.
Ideas for future improvements:
Testing procedure
Grab an stm32f429i-disc1 board and run the following command
Issues/PRs references
Based on ##13262