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.

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

Conversation

espressif2022
Copy link
Contributor

@espressif2022 espressif2022 commented Dec 4, 2024

ESP-BSP Pull Request checklist

Note: For new BSPs create a PR with this link.

  • Version of modified component bumped
  • CI passing

Change description

1: The new component of the knob will clear the flag of knob->event, so we add count_value for conditional checks.

@github-actions github-actions bot changed the title Fix: knob loses diff event Fix: knob loses diff event (BSP-592) Dec 4, 2024
Copy link
Collaborator

@espzav espzav left a comment

Choose a reason for hiding this comment

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

@espressif2022 Thank you for update. I will test it today or tomorrow with HW :-)


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

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.

Copy link
Collaborator

@espzav espzav left a comment

Choose a reason for hiding this comment

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

@espressif2022 Thank you for changes. LGTM
Please, squash commits and update branch before merging.

@tore-espressif
Copy link
Collaborator

@espressif2022 Could you please add a note to CHANGELOG.md too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants