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

InfiniPaint vibrate on colorchange, fix color rotation #803

Merged

Conversation

clemensvonmolo
Copy link
Contributor

This is a very simple pull requests that changes the Paint app in order to vibrate on color change (because It's hard to know if the touch controller actually registered a long press or a small swipe). I also fixed the logic for the color rotation. Before, the color magenta was unreachable and instead black was chosen a second time. The color variable was changed to initialize to 3 instead of 2, so you don't have to press twice on the first rotation.

  • the changes were successfully tested on hardware and showed no problems.

@0x1a8510f2
Copy link

Whoa, didn't even realise the colours could be changed!

@geekbozu
Copy link
Member

geekbozu commented Nov 1, 2021

Not super related to this PR but I would love to see a way to just set a system event for the motor instead of having to pass this around so much....Nice pr tho didn't know you could change the color.

Copy link
Contributor

@Riksu9000 Riksu9000 left a comment

Choose a reason for hiding this comment

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

A proper fix for the color would be to increment the color before the switch and modulo with 8. That way we use 0-7 instead of 1-8. Currently the color variable points to the next color instead of the current one which is strange.

The vibration is a good idea.

src/displayapp/screens/InfiniPaint.cpp Outdated Show resolved Hide resolved
src/displayapp/DisplayApp.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Riksu9000 Riksu9000 left a comment

Choose a reason for hiding this comment

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

Maybe the vibration doesn't have to be so strong. 35 duration is usually used elsewhere.

Other than that this looks good.

@clemensvonmolo
Copy link
Contributor Author

The run duration seems a bit all over the place. (Currently 50, 90, 30, 35 and 15 are used.) Maybe a design guideline with times for strong/medium/weak vibration would help. For now I will change it to 35 since that really is the most common one.

@JF002 JF002 added this to the 1.8.0 milestone Dec 2, 2021
@JF002 JF002 merged commit 1404d01 into InfiniTimeOrg:develop Dec 2, 2021
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.

5 participants