Haptic and solenoid cleanup (#9700)

* solenoid: remove two functions that do nothing.

These functions modify the argument, and so they do nothing.

Note: versions of these functions exist in mtdjr's user folder,
however to core solenoid support and mtdjr user-specific solenoid
support are exclusive (only one can be used at a time).

So removing these confusing functions does no harm.

* solenoid: bugfix: don't allow dwell time to go 1ms below minimum time.

The previous code allowed dwell time to go 1ms below the configured minimum.
This change corrects it.

* solenoid: bugfix: when incrementing above maximum dwell time, jump back to minimum, not to 1

The previous code used to jump back to 1, which might be way under the configured minimum setting.

* solenoid: bugfix: on startup actually use the eeprom-stored dwell-time

This is because the dwell time is stored in two variables.

* solenoid: bugfix: on haptic_reset, actually use the newly set default dwell time.

This is needed because dwell time is configured in two variables.

* solenoid: on HPT_RST set buzz to a default value

* solenoid: buzz: reworked to make more configurable. Previous behaviour maintained.

* solenoid: documentation: clarify meaning of dwell time

* solenoid: add feature SOLENOID_DWELL_STEP_SIZE

* solenoid: documentation: added note about the precision of the solenoid time settings

* haptic: Correctly call haptic_reset when eeprom is corrupt.

* haptic: improve what happens if haptic is enabled without erasing eeprom

* haptic: improve what happens if solenoid is enabled without erasing eeprom

* drivers/haptic: fix compilation issue, when haptic is enabled, but solenoid isn't

docs/feature_haptic_feedback.md

@@ -42,14 +42,21 @@ First you will need a build a circuit to drive the solenoid through a mosfet as
 [Wiring diagram provided by Adafruit](https://playground.arduino.cc/uploads/Learning/solenoid_driver.pdf)
 
 
-| Settings                 | Default       | Description                                           |
+| Settings                   | Default              | Description                                           |
-|--------------------------|---------------|-------------------------------------------------------|
+|----------------------------|----------------------|-------------------------------------------------------|
-|`SOLENOID_PIN`            | *Not defined* |Configures the pin that the Solenoid is connected to.  |
+|`SOLENOID_PIN`              | *Not defined*        |Configures the pin that the Solenoid is connected to.  |
-|`SOLENOID_DEFAULT_DWELL`  | `12` ms       |Configures the default dwell time for the solenoid.    |
+|`SOLENOID_DEFAULT_DWELL`    | `12` ms              |Configures the default dwell time for the solenoid.    |
-|`SOLENOID_MIN_DWELL`      | `4` ms        |Sets the lower limit for the dwell.                    |
+|`SOLENOID_MIN_DWELL`        | `4` ms               |Sets the lower limit for the dwell.                    |
-|`SOLENOID_MAX_DWELL`      | `100` ms      |Sets the upper limit for the dwell.                    |
+|`SOLENOID_MAX_DWELL`        | `100` ms             |Sets the upper limit for the dwell.                    |
+|`SOLENOID_DWELL_STEP_SIZE`  | `1` ms               |The step size to use when `HPT_DWL*` keycodes are sent |
+|`SOLENOID_DEFAULT_BUZZ`     | `0` (disabled)       |On HPT_RST buzz is set "on" if this is "1"             |
+|`SOLENOID_BUZZ_ACTUATED`    | `SOLENOID_MIN_DWELL` |Actuated-time when the solenoid is in buzz mode        |
+|`SOLENOID_BUZZ_NONACTUATED` | `SOLENOID_MIN_DWELL` |Non-Actuated-time when the solenoid is in buzz mode    |
 
-?> Dwell time is how long the "plunger" stays activated.  The dwell time changes how the solenoid sounds.
+* If solenoid buzz is off, then dwell time is how long the "plunger" stays activated. The dwell time changes how the solenoid sounds.
+* If solenoid buzz is on, then dwell time sets the length of the buzz, while `SOLENOID_BUZZ_ACTUATED` and `SOLENOID_BUZZ_NONACTUATED` set the (non-)actuation times withing the buzz period.
+* With the current implementation, for any of the above time settings, the precision of these settings may be affected by how fast the keyboard is able to scan the matrix.
+  Therefore, if the keyboards scanning routine is slow, it may be preferable to set `SOLENOID_DWELL_STEP_SIZE` to a value slightly smaller than the time it takes to scan the keyboard.
 
 Beware that some pins may be powered during bootloader (ie. A13 on the STM32F303 chip) and will result in the solenoid kept in the on state through the whole flashing process. This may overheat and damage the solenoid. If you find that the pin the solenoid is connected to is triggering the solenoid during bootloader/DFU, select another pin.
 

drivers/haptic/haptic.c

@@ -33,11 +33,18 @@ void haptic_init(void) {
         eeconfig_init();
     }
     haptic_config.raw = eeconfig_read_haptic();
-    if (haptic_config.mode < 1) {
+#ifdef SOLENOID_ENABLE
-        haptic_config.mode = 1;
+    solenoid_set_dwell(haptic_config.dwell);
-    }
+#endif
-    if (!haptic_config.mode) {
+    if ((haptic_config.raw == 0)
-        dprintf("No haptic config found in eeprom, setting default configs\n");
+#ifdef SOLENOID_ENABLE
+        || (haptic_config.dwell == 0)
+#endif
+       ) {
+        // this will be called, if the eeprom is not corrupt,
+        // but the previous firmware didn't have haptic enabled,
+        // or the previous firmware didn't have solenoid enabled,
+        // and the current one has solenoid enabled.
         haptic_reset();
     }
 #ifdef SOLENOID_ENABLE
@@ -118,25 +125,37 @@ void haptic_mode_decrease(void) {
 }
 
 void haptic_dwell_increase(void) {
-    uint8_t dwell = haptic_config.dwell + 1;
 #ifdef SOLENOID_ENABLE
+    int16_t next_dwell = ((int16_t)haptic_config.dwell) + SOLENOID_DWELL_STEP_SIZE;
     if (haptic_config.dwell >= SOLENOID_MAX_DWELL) {
-        dwell = 1;
+        // if it's already at max, we wrap back to min
+        next_dwell = SOLENOID_MIN_DWELL;
+    } else if (next_dwell > SOLENOID_MAX_DWELL) {
+        // if we overshoot the max, then cap at max
+        next_dwell = SOLENOID_MAX_DWELL;
     }
-    solenoid_set_dwell(dwell);
+    solenoid_set_dwell(next_dwell);
+#else
+    int16_t next_dwell = ((int16_t)haptic_config.dwell) + 1;
 #endif
-    haptic_set_dwell(dwell);
+    haptic_set_dwell(next_dwell);
 }
 
 void haptic_dwell_decrease(void) {
-    uint8_t dwell = haptic_config.dwell - 1;
 #ifdef SOLENOID_ENABLE
-    if (haptic_config.dwell < SOLENOID_MIN_DWELL) {
+    int16_t next_dwell = ((int16_t)haptic_config.dwell) - SOLENOID_DWELL_STEP_SIZE;
-        dwell = SOLENOID_MAX_DWELL;
+    if (haptic_config.dwell <= SOLENOID_MIN_DWELL) {
+        // if it's already at min, we wrap to max
+        next_dwell = SOLENOID_MAX_DWELL;
+    } else if (next_dwell < SOLENOID_MIN_DWELL) {
+        // if we go below min, then we cap to min
+        next_dwell = SOLENOID_MIN_DWELL;
     }
-    solenoid_set_dwell(dwell);
+    solenoid_set_dwell(next_dwell);
+#else
+    int16_t next_dwell = ((int16_t)haptic_config.dwell) - 1;
 #endif
-    haptic_set_dwell(dwell);
+    haptic_set_dwell(next_dwell);
 }
 
 void haptic_reset(void) {
@@ -150,6 +169,12 @@ void haptic_reset(void) {
 #ifdef SOLENOID_ENABLE
     uint8_t dwell       = SOLENOID_DEFAULT_DWELL;
     haptic_config.dwell = dwell;
+    haptic_config.buzz  = SOLENOID_DEFAULT_BUZZ;
+    solenoid_set_dwell(dwell);
+#else
+    // This is to trigger haptic_reset again, if solenoid is enabled in the future.
+    haptic_config.dwell = 0;
+    haptic_config.buzz  = 0;
 #endif
     eeconfig_update_haptic(haptic_config.raw);
     xprintf("haptic_config.feedback = %u\n", haptic_config.feedback);

drivers/haptic/solenoid.c

@@ -32,14 +32,6 @@ void solenoid_buzz_off(void) { haptic_set_buzz(0); }
 
 void solenoid_set_buzz(int buzz) { haptic_set_buzz(buzz); }
 
-void solenoid_dwell_minus(uint8_t solenoid_dwell) {
-    if (solenoid_dwell > 0) solenoid_dwell--;
-}
-
-void solenoid_dwell_plus(uint8_t solenoid_dwell) {
-    if (solenoid_dwell < SOLENOID_MAX_DWELL) solenoid_dwell++;
-}
-
 void solenoid_set_dwell(uint8_t dwell) { solenoid_dwell = dwell; }
 
 void solenoid_stop(void) {
@@ -73,7 +65,7 @@ void solenoid_check(void) {
 
     // Check whether to buzz the solenoid on and off
     if (haptic_config.buzz) {
-        if (elapsed / SOLENOID_MIN_DWELL % 2 == 0) {
+        if ((elapsed % (SOLENOID_BUZZ_ACTUATED + SOLENOID_BUZZ_NONACTUATED)) < SOLENOID_BUZZ_ACTUATED) {
             if (!solenoid_buzzing) {
                 solenoid_buzzing = true;
                 writePinHigh(SOLENOID_PIN);

drivers/haptic/solenoid.h

@@ -29,6 +29,22 @@
 #    define SOLENOID_MIN_DWELL 4
 #endif
 
+#ifndef SOLENOID_DWELL_STEP_SIZE
+#    define SOLENOID_DWELL_STEP_SIZE 1
+#endif
+
+#ifndef SOLENOID_DEFAULT_BUZZ
+#    define SOLENOID_DEFAULT_BUZZ 0
+#endif
+
+#ifndef SOLENOID_BUZZ_ACTUATED
+#    define SOLENOID_BUZZ_ACTUATED SOLENOID_MIN_DWELL
+#endif
+
+#ifndef SOLENOID_BUZZ_NONACTUATED
+#    define SOLENOID_BUZZ_NONACTUATED SOLENOID_MIN_DWELL
+#endif
+
 #ifndef SOLENOID_PIN
 #    error SOLENOID_PIN not defined
 #endif
@@ -37,8 +53,6 @@ void solenoid_buzz_on(void);
 void solenoid_buzz_off(void);
 void solenoid_set_buzz(int buzz);
 
-void solenoid_dwell_minus(uint8_t solenoid_dwell);
-void solenoid_dwell_plus(uint8_t solenoid_dwell);
 void solenoid_set_dwell(uint8_t dwell);
 
 void solenoid_stop(void);

tmk_core/common/eeconfig.c

@@ -13,6 +13,10 @@
 #    include "eeprom_driver.h"
 #endif
 
+#if defined(HAPTIC_ENABLE)
+#    include "haptic.h"
+#endif
+
 /** \brief eeconfig enable
  *
  * FIXME: needs doc
@@ -65,6 +69,15 @@ void eeconfig_init_quantum(void) {
     eeprom_update_byte(EECONFIG_HANDEDNESS, 0);
 #endif
 
+#if defined(HAPTIC_ENABLE)
+    haptic_reset();
+#else
+    // this is used in case haptic is disabled, but we still want sane defaults
+    // in the haptic configuration eeprom. All zero will trigger a haptic_reset
+    // when a haptic-enabled firmware is loaded onto the keyboard.
+    eeprom_update_dword(EECONFIG_HAPTIC, 0);
+#endif
+
     eeconfig_init_kb();
 }