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

F haptic disable when usb not configured or suspended #12692

Conversation

purdeaandrei
Copy link
Contributor

@purdeaandrei purdeaandrei commented Apr 25, 2021

Description

This also add support for specifying a LED pin to indicate haptic status,
and also adds support for a haptic-enable pin, which is useful to turn off
the boost converter on the solenoid driver.

This has been tested on chibios running on an STM32F446.
Also tested on atmega32u4, and also on atmega32u2.

The following pull-request was a pre-requisite: (it's now merged)
#12691

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@purdeaandrei
Copy link
Contributor Author

Note: the prerequisite PR is included here as the first commit. I may or may not need to do a force push once the prerequisite is merged, in order for github to not show the content of the prerequisite in the changes.

@drashna drashna added the awaiting_pr Relies on another PR to be merged first label Apr 26, 2021
@purdeaandrei
Copy link
Contributor Author

purdeaandrei commented May 1, 2021

Now also tested, and working as expected on atmega32u4, and atmega32u2, in addition to STM32F446.

An interesting observation is that on atmega32u4 the solenoid clicks on a keypress that would bring the system out of suspend. This appears to be because of ordering of events, first USB comes out of the suspend state, and only after is the haptic feedback processed.
If I remember right, when I tested this on STM32F446 the solenoid didn't click when exiting suspend.
These differences are probably related to how the USB drivers are implemented, but I can confirm, that I checked that the solenoid is indeed disabled during suspend, the DC-DC converter is powered down, and I see the output capacitor of the DC-DC converter discharging.

@stale
Copy link

stale bot commented Jun 16, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch from c984838 to 347a621 Compare June 21, 2021 19:35
@stale stale bot removed the awaiting changes label Jun 21, 2021
@stale
Copy link

stale bot commented Aug 6, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch from 347a621 to 40bfcfa Compare August 22, 2021 23:40
@stale stale bot removed the awaiting changes label Aug 22, 2021
@purdeaandrei
Copy link
Contributor Author

Rebased.
Still awaiting other PR.
Not yet tested after rebase.

@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch 2 times, most recently from f13e878 to 88b531a Compare August 26, 2021 19:59
@purdeaandrei
Copy link
Contributor Author

Rebased on top of the latest power tracking PR, and tested again with an STM32F446-based keyboard

@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch from 88b531a to 2116f5e Compare September 16, 2021 22:32
@purdeaandrei
Copy link
Contributor Author

Rebased to fix conflict. Still awaiting other PR.

@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch from 2116f5e to d3b80d7 Compare September 30, 2021 21:09
@purdeaandrei
Copy link
Contributor Author

Not awaiting any other PR's anymore.
Please remove the awaiting_pr label.
(Also I rebased it again, to fix conflict, and to force github to not show the previous dependency as part of the changes anymore)

@drashna drashna requested a review from a team October 11, 2021 00:04
@zvecr zvecr removed the awaiting_pr Relies on another PR to be merged first label Oct 11, 2021
@tzarc
Copy link
Member

tzarc commented Nov 1, 2021

If we can get the conflicts resolved, this should be good to go.

…ended.

This also add support for specifying a LED pin to indicate haptic status,
and also adds support for a haptic-enable pin, which is useful to turn off
the boost converter on the solenoid driver.
@purdeaandrei purdeaandrei force-pushed the f_haptic_disable_when_usb_not_configured_or_suspended branch from d3b80d7 to 51b2d2f Compare November 2, 2021 05:04
@purdeaandrei
Copy link
Contributor Author

@tzarc Conflict fixed, just needed a rebase

@tzarc tzarc merged commit 76fb544 into qmk:develop Nov 2, 2021
@purdeaandrei purdeaandrei deleted the f_haptic_disable_when_usb_not_configured_or_suspended branch November 2, 2021 05:55
ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
…ended. (qmk#12692)

This also add support for specifying a LED pin to indicate haptic status,
and also adds support for a haptic-enable pin, which is useful to turn off
the boost converter on the solenoid driver.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants