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.

Sign up for GitHub

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

Fix: knob loses diff event (BSP-592) #450

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+112 −31
Open

Fix: knob loses diff event (BSP-592) #450

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ba025e
Fix: knob loses diff event
espressif2022 Dec 4, 2024
96eff84
fix: knob lost event
espressif2022 Jan 7, 2025
6a50832
fix: calculate diff in callback
espressif2022 Jan 9, 2025
f5ff636
fix: If the direction is reversed, clear the count
espressif2022 Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/esp_lvgl_port/idf_component.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version: "2.4.3"
version: "2.4.4"
description: ESP LVGL port
url: https://github.com/espressif/esp-bsp/tree/master/components/esp_lvgl_port
dependencies:
Expand Down
44 changes: 40 additions & 4 deletions components/esp_lvgl_port/src/lvgl8/esp_lvgl_port_knob.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -19,7 +19,8 @@ typedef struct {
knob_handle_t knob_handle; /* Encoder knob handlers */
button_handle_t btn_handle; /* Encoder button handlers */
lv_indev_drv_t indev_drv; /* LVGL input device driver */
bool btn_enter; /* Encoder button enter state */
bool btn_enter; /* Encoder button enter state */
knob_event_t event; /* Encoder event */
} lvgl_port_encoder_ctx_t;

/*******************************************************************************
Expand All @@ -29,6 +30,8 @@ typedef struct {
static void lvgl_port_encoder_read(lv_indev_drv_t *indev_drv, lv_indev_data_t *data);
static void lvgl_port_encoder_btn_down_handler(void *arg, void *arg2);
static void lvgl_port_encoder_btn_up_handler(void *arg, void *arg2);
static void lvgl_port_encoder_left_handler(void *arg, void *arg2);
static void lvgl_port_encoder_right_handler(void *arg, void *arg2);

/*******************************************************************************
* Public API functions
Expand All @@ -54,6 +57,9 @@ lv_indev_t *lvgl_port_add_encoder(const lvgl_port_encoder_cfg_t *encoder_cfg)
ESP_GOTO_ON_FALSE(encoder_ctx->knob_handle, ESP_ERR_NO_MEM, err, TAG, "Not enough memory for knob create!");
}

ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_LEFT, lvgl_port_encoder_left_handler, encoder_ctx));
ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_RIGHT, lvgl_port_encoder_right_handler, encoder_ctx));

/* Encoder Enter */
if (encoder_cfg->encoder_enter != NULL) {
encoder_ctx->btn_handle = iot_button_create(encoder_cfg->encoder_enter);
Expand Down Expand Up @@ -125,11 +131,17 @@ static void lvgl_port_encoder_read(lv_indev_drv_t *indev_drv, lv_indev_data_t *d
assert(ctx);

int32_t invd = iot_knob_get_count_value(ctx->knob_handle);
knob_event_t event = iot_knob_get_event(ctx->knob_handle);
knob_event_t event = ctx->event;

if (last_v ^ invd) {

int32_t diff = (int32_t)((uint32_t)invd - (uint32_t)last_v);

diff += (event == KNOB_RIGHT && invd < last_v) ? CONFIG_KNOB_HIGH_LIMIT :
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we need this?
event is only last event and when knob rotate to right by some steps then to left, the event will left, but more steps can be still right.

Copy link
Contributor Author

@espressif2022 espressif2022 Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, you're right. In some case, as follows:
Starting at 10, rotating right by 10 [20] (not captured by LVGL this time), and then rotating left by 5 [15].

This would result in a rotate left event, but with a positive diff, leading to an abnormal diff for the left rotation.
Should we ignore the diff?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In your example case, there is still direction (diff as you naimed). I think, the direction is only in sign minus or plus value. That's all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use interrupts to record the current diff value in real time.
Accumulate the value when rotating in the same direction; reset to zero and start recalculating when the direction reverses.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you reset value when changed direction? I thought, it is enough to add or sub from latest value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the encoder is rotated 10 steps to the right, but lvgl_port_encoder_read does not capture it (assuming the UI is experiencing severe lag at this moment), and then it is rotated 5 steps to the left, I think a result of 5 steps to the left is better than 5 steps to the right.
Therefore, the count for the different direction should be reset.

(event == KNOB_LEFT && invd > last_v) ? CONFIG_KNOB_LOW_LIMIT : 0;

data->enc_diff = diff;
last_v = invd;
data->enc_diff = (KNOB_LEFT == event) ? (-1) : ((KNOB_RIGHT == event) ? (1) : (0));
} else {
data->enc_diff = 0;
}
Expand Down Expand Up @@ -159,3 +171,27 @@ static void lvgl_port_encoder_btn_up_handler(void *arg, void *arg2)
}
}
}

static void lvgl_port_encoder_left_handler(void *arg, void *arg2)
{
lvgl_port_encoder_ctx_t *ctx = (lvgl_port_encoder_ctx_t *) arg2;
knob_handle_t knob = (knob_handle_t)arg;
if (ctx && knob) {
/* LEFT */
if (knob == ctx->knob_handle) {
ctx->event = KNOB_LEFT;
}
}
}

static void lvgl_port_encoder_right_handler(void *arg, void *arg2)
{
lvgl_port_encoder_ctx_t *ctx = (lvgl_port_encoder_ctx_t *) arg2;
knob_handle_t knob = (knob_handle_t)arg;
if (ctx && knob) {
/* RIGHT */
if (knob == ctx->knob_handle) {
ctx->event = KNOB_RIGHT;
}
}
}
48 changes: 39 additions & 9 deletions components/esp_lvgl_port/src/lvgl9/esp_lvgl_port_knob.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -20,6 +20,7 @@ typedef struct {
button_handle_t btn_handle; /* Encoder button handlers */
lv_indev_t *indev; /* LVGL input device driver */
bool btn_enter; /* Encoder button enter state */
knob_event_t event; /* Encoder event */
} lvgl_port_encoder_ctx_t;

/*******************************************************************************
Expand All @@ -29,7 +30,8 @@ typedef struct {
static void lvgl_port_encoder_read(lv_indev_t *indev_drv, lv_indev_data_t *data);
static void lvgl_port_encoder_btn_down_handler(void *arg, void *arg2);
static void lvgl_port_encoder_btn_up_handler(void *arg, void *arg2);
static void lvgl_port_encoder_knob_handler(void *arg, void *arg2);
static void lvgl_port_encoder_left_handler(void *arg, void *arg2);
static void lvgl_port_encoder_right_handler(void *arg, void *arg2);

/*******************************************************************************
* Public API functions
Expand All @@ -54,8 +56,8 @@ lv_indev_t *lvgl_port_add_encoder(const lvgl_port_encoder_cfg_t *encoder_cfg)
encoder_ctx->knob_handle = iot_knob_create(encoder_cfg->encoder_a_b);
ESP_GOTO_ON_FALSE(encoder_ctx->knob_handle, ESP_ERR_NO_MEM, err, TAG, "Not enough memory for knob create!");

ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_LEFT, lvgl_port_encoder_knob_handler, encoder_ctx));
ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_RIGHT, lvgl_port_encoder_knob_handler, encoder_ctx));
ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_LEFT, lvgl_port_encoder_left_handler, encoder_ctx));
ESP_ERROR_CHECK(iot_knob_register_cb(encoder_ctx->knob_handle, KNOB_RIGHT, lvgl_port_encoder_right_handler, encoder_ctx));
}

/* Encoder Enter */
Expand Down Expand Up @@ -131,16 +133,23 @@ esp_err_t lvgl_port_remove_encoder(lv_indev_t *encoder)
static void lvgl_port_encoder_read(lv_indev_t *indev_drv, lv_indev_data_t *data)
{
static int32_t last_v = 0;

assert(indev_drv);
lvgl_port_encoder_ctx_t *ctx = (lvgl_port_encoder_ctx_t *)lv_indev_get_user_data(indev_drv);
assert(ctx);

int32_t invd = iot_knob_get_count_value(ctx->knob_handle);
knob_event_t event = iot_knob_get_event(ctx->knob_handle);
knob_event_t event = ctx->event;

if (last_v ^ invd) {

int32_t diff = (int32_t)((uint32_t)invd - (uint32_t)last_v);

diff += (event == KNOB_RIGHT && invd < last_v) ? CONFIG_KNOB_HIGH_LIMIT :
(event == KNOB_LEFT && invd > last_v) ? CONFIG_KNOB_LOW_LIMIT : 0;

data->enc_diff = diff;
last_v = invd;
data->enc_diff = (KNOB_LEFT == event) ? (-1) : ((KNOB_RIGHT == event) ? (1) : (0));
} else {
data->enc_diff = 0;
}
Expand Down Expand Up @@ -177,9 +186,30 @@ static void lvgl_port_encoder_btn_up_handler(void *arg, void *arg2)
lvgl_port_task_wake(LVGL_PORT_EVENT_TOUCH, ctx->indev);
}

static void lvgl_port_encoder_knob_handler(void *arg, void *arg2)
static void lvgl_port_encoder_left_handler(void *arg, void *arg2)
{
lvgl_port_encoder_ctx_t *ctx = (lvgl_port_encoder_ctx_t *) arg2;
/* Wake LVGL task, if needed */
lvgl_port_task_wake(LVGL_PORT_EVENT_TOUCH, ctx->indev);
knob_handle_t knob = (knob_handle_t)arg;
if (ctx && knob) {
/* LEFT */
if (knob == ctx->knob_handle) {
ctx->event = KNOB_LEFT;
}
/* Wake LVGL task, if needed */
lvgl_port_task_wake(LVGL_PORT_EVENT_TOUCH, ctx->indev);
}
}

static void lvgl_port_encoder_right_handler(void *arg, void *arg2)
{
lvgl_port_encoder_ctx_t *ctx = (lvgl_port_encoder_ctx_t *) arg2;
knob_handle_t knob = (knob_handle_t)arg;
if (ctx && knob) {
/* RIGHT */
if (knob == ctx->knob_handle) {
ctx->event = KNOB_RIGHT;
}
/* Wake LVGL task, if needed */
lvgl_port_task_wake(LVGL_PORT_EVENT_TOUCH, ctx->indev);
}
}
Loading