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

Allow lower values for touch threshold #4081

Merged
merged 2 commits into from
Aug 18, 2024
Merged

Conversation

RobinMeis
Copy link
Contributor

Solved issue #4076 by allowing lower values for touch threshold

@blazoncek
Copy link
Collaborator

Once @DedeHai gets back from vacation I hope he'll review and approve.

Copy link
Collaborator

@DedeHai DedeHai left a comment

Choose a reason for hiding this comment

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

Need to test, may lead to ghost detections

@RobinMeis
Copy link
Contributor Author

Need to test, may lead to ghost detections

For me it doesn't but shouldn't be a too big issue as you already can get ghost detections on ESP32-Wroom with inappropriate settings.

@DedeHai
Copy link
Collaborator

DedeHai commented Aug 18, 2024

I tested the change, low values lead to ghost detections but that is probably ok, I did not notice any crashes in my tests (tested on S2).
@RobinMeis your PR is missing changes to the second interrupt attachment here:

touchAttachInterrupt(btnPin[i], touchButtonISR, 256 + (touchThreshold << 4)); // threshold on Touch V2 is much higher (1500 is a value given by Espressif example, I measured changes of over 5000)

apart from that I am ok with removing the 256 as a minimum base value.

@RobinMeis
Copy link
Contributor Author

I have fixed this. For me, I also get ghost detections for very low values but didn't get any ghost detections within the last two weeks on two lamps with reasonable values.

@DedeHai
Copy link
Collaborator

DedeHai commented Aug 18, 2024

looks good, gets my approval.

@blazoncek blazoncek merged commit cc298f5 into Aircoookie:0_15 Aug 18, 2024
18 checks passed
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