Refactor battery driver (#25550)

builddefs/build_test.mk

@@ -62,6 +62,7 @@ include $(BUILDDEFS_PATH)/common_features.mk
 include $(BUILDDEFS_PATH)/generic_features.mk
 include $(PLATFORM_PATH)/common.mk
 include $(TMK_PATH)/protocol.mk
+include $(QUANTUM_PATH)/battery/tests/rules.mk
 include $(QUANTUM_PATH)/debounce/tests/rules.mk
 include $(QUANTUM_PATH)/encoder/tests/rules.mk
 include $(QUANTUM_PATH)/os_detection/tests/rules.mk

builddefs/common_features.mk

@@ -945,21 +945,25 @@ ifeq ($(strip $(DIP_SWITCH_ENABLE)), yes)
     endif
 endif
 
+ifeq ($(strip $(BATTERY_ENABLE)), yes)
+    BATTERY_DRIVER_REQUIRED := yes
+endif
+
 VALID_BATTERY_DRIVER_TYPES := adc custom vendor
 
-BATTERY_DRIVER ?= adc
+BATTERY_DRIVER ?= none
 ifeq ($(strip $(BATTERY_DRIVER_REQUIRED)), yes)
     ifeq ($(filter $(BATTERY_DRIVER),$(VALID_BATTERY_DRIVER_TYPES)),)
         $(call CATASTROPHIC_ERROR,Invalid BATTERY_DRIVER,BATTERY_DRIVER="$(BATTERY_DRIVER)" is not a valid battery driver)
     endif
 
-    OPT_DEFS += -DBATTERY_DRIVER
-    OPT_DEFS += -DBATTERY_$(strip $(shell echo $(BATTERY_DRIVER) | tr '[:lower:]' '[:upper:]'))
+    OPT_DEFS += -DBATTERY_DRIVER_$(strip $(shell echo $(BATTERY_DRIVER) | tr '[:lower:]' '[:upper:]'))
 
     COMMON_VPATH += $(DRIVER_PATH)/battery
 
-    SRC += battery.c
-    SRC += battery_$(strip $(BATTERY_DRIVER)).c
+    ifneq ($(strip $(BATTERY_DRIVER)), custom)
+        SRC += battery_$(strip $(BATTERY_DRIVER)).c
+    endif
 
     # add extra deps
     ifeq ($(strip $(BATTERY_DRIVER)), adc)

builddefs/generic_features.mk

@@ -21,6 +21,7 @@ SPACE_CADET_ENABLE ?= yes
 GENERIC_FEATURES = \
     AUTO_SHIFT \
     AUTOCORRECT \
+    BATTERY \
     BOOTMAGIC \
     CAPS_WORD \
     COMBO \

builddefs/testlist.mk

@@ -1,6 +1,7 @@
 TEST_LIST = $(sort $(patsubst %/test.mk,%, $(shell find $(ROOT_DIR)tests -type f -name test.mk)))
 FULL_TESTS := $(notdir $(TEST_LIST))
 
+include $(QUANTUM_PATH)/battery/tests/testlist.mk
 include $(QUANTUM_PATH)/debounce/tests/testlist.mk
 include $(QUANTUM_PATH)/encoder/tests/testlist.mk
 include $(QUANTUM_PATH)/os_detection/tests/testlist.mk

data/mappings/info_config.hjson

@@ -43,6 +43,14 @@
     "BOOTMAGIC_ROW": {"info_key": "bootmagic.matrix.0", "value_type": "int"},
     "BOOTMAGIC_ROW_RIGHT": {"info_key": "split.bootmagic.matrix.0", "value_type": "int"},
 
+    // Battery
+    "BATTERY_SAMPLE_INTERVAL": {"info_key": "battery.sample_interval", "value_type": "int"},
+    "BATTERY_ADC_PIN": {"info_key": "battery.adc.pin"},
+    "BATTERY_ADC_REF_VOLTAGE_MV": {"info_key": "battery.adc.reference_voltage", "value_type": "int"},
+    "BATTERY_ADC_VOLTAGE_DIVIDER_R1": {"info_key": "battery.adc.divider_r1", "value_type": "int"},
+    "BATTERY_ADC_VOLTAGE_DIVIDER_R2": {"info_key": "battery.adc.divider_r2", "value_type": "int"},
+    "BATTERY_ADC_RESOLUTION": {"info_key": "battery.adc.resolution", "value_type": "int"},
+
     // Caps Word
     "BOTH_SHIFTS_TURNS_ON_CAPS_WORD": {"info_key": "caps_word.both_shifts_turns_on", "value_type": "flag"},
     "CAPS_WORD_IDLE_TIMEOUT": {"info_key": "caps_word.idle_timeout", "value_type": "int"},

data/mappings/info_rules.hjson

@@ -13,6 +13,7 @@
 
     "AUDIO_DRIVER": {"info_key": "audio.driver"},
     "BACKLIGHT_DRIVER": {"info_key": "backlight.driver"},
+    "BATTERY_DRIVER": {"info_key": "battery.driver"},
     "BLUETOOTH_DRIVER": {"info_key": "bluetooth.driver"},
     "BOARD": {"info_key": "board"},
     "BOOTLOADER": {"info_key": "bootloader", "warn_duplicate": false},

data/schemas/keyboard.jsonschema

@@ -188,6 +188,28 @@
                 "as_caps_lock": {"type": "boolean"}
             }
         },
+        "battery": {
+            "type": "object",
+            "additionalProperties": false,
+            "properties": {
+                "driver": {
+                    "type": "string",
+                    "enum": ["adc", "custom", "vendor"]
+                },
+                "adc": {
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "pin": {"$ref": "./definitions.jsonschema#/mcu_pin"},
+                        "reference_voltage": {"type": "integer"},
+                        "divider_r1": {"type": "integer"},
+                        "divider_r2": {"type": "integer"},
+                        "resolution": {"type": "integer"}
+                    }
+                },
+                "sample_interval": {"type": "integer"}
+            }
+        },
         "bluetooth": {
             "type": "object",
             "additionalProperties": false,

docs/_sidebar.json

@@ -175,6 +175,7 @@
                         ]
                     },
                     { "text": "Audio", "link": "/features/audio" },
+                    { "text": "Battery", "link": "/features/battery" },
                     { "text": "Bootmagic", "link": "/features/bootmagic" },
                     { "text": "Converters", "link": "/feature_converters" },
                     { "text": "Custom Matrix", "link": "/custom_matrix" },

docs/drivers/battery.md

@@ -1,6 +1,6 @@
 # Battery Driver
 
-This driver provides support for sampling battery level.
+This driver provides support for directly sampling battery level.
 
 ## Usage
 
@@ -10,21 +10,17 @@ To use this driver, add the following to your `rules.mk`:
 BATTERY_DRIVER_REQUIRED = yes
 ```
 
-## Basic Configuration {#basic-configuration}
-
-Add the following to your `config.h`:
-
-|Define                    |Default |Description                                       |
-|--------------------------|--------|--------------------------------------------------|
-|`BATTERY_SAMPLE_INTERVAL` |`30000` |The time between battery samples in milliseconds. |
+::::info Note
+This is already configured for you if you are using the [Battery](../features/battery) feature.
+::::
 
 ## Driver Configuration {#driver-configuration}
 
-Driver selection can be configured in `rules.mk` as `BATTERY_DRIVER`. Valid values are `adc` (default), `vendor`, or `custom`. See below for information on individual drivers.
+Driver selection can be configured in `rules.mk` as `BATTERY_DRIVER`. Valid values are `adc`, `vendor`, or `custom`. See below for information on individual drivers.
 
 ### ADC Driver {#adc-driver}
 
-This is the default battery driver. The default configuration assumes the battery is connected to a ADC capable pin through a voltage divider.
+The default configuration assumes the battery is connected to a ADC capable pin through a voltage divider.
 
 ```make
 BATTERY_DRIVER = adc
@@ -32,42 +28,25 @@ BATTERY_DRIVER = adc
 
 The following `#define`s apply only to the `adc` driver:
 
-|Define                       |Default       |Description                                                   |
-|-----------------------------|--------------|--------------------------------------------------------------|
-|`BATTERY_PIN`                |*Not defined* |The GPIO pin connected to the voltage divider.                |
-|`BATTERY_REF_VOLTAGE_MV`     |`3300`        |The ADC reverence voltage, in millivolts.                     |
-|`BATTERY_VOLTAGE_DIVIDER_R1` |`100`         |The voltage divider resistance, in kOhm. Set to 0 to disable. |
-|`BATTERY_VOLTAGE_DIVIDER_R2` |`100`         |The voltage divider resistance, in kOhm. Set to 0 to disable. |
-|`BATTERY_ADC_RESOLUTION`     |`10`          |The ADC resolution configured for the ADC Driver.             |
-
-## Functions
-
-### `uint8_t battery_get_percent(void)` {#api-battery-get-percent}
-
-Sample battery level.
-
-#### Return Value {#api-battery-get-percent-return}
-
-The battery percentage, in the range 0-100.
+|Define                           |Default       |Description                                                   |
+|---------------------------------|--------------|--------------------------------------------------------------|
+|`BATTERY_ADC_PIN`                |*Not defined* |The GPIO pin connected to the voltage divider.                |
+|`BATTERY_ADC_REF_VOLTAGE_MV`     |`3300`        |The ADC reverence voltage, in millivolts.                     |
+|`BATTERY_ADC_VOLTAGE_DIVIDER_R1` |`100`         |The voltage divider resistance, in kOhm. Set to 0 to disable. |
+|`BATTERY_ADC_VOLTAGE_DIVIDER_R2` |`100`         |The voltage divider resistance, in kOhm. Set to 0 to disable. |
+|`BATTERY_ADC_RESOLUTION`         |`10`          |The ADC resolution configured for the ADC Driver.             |
 
-## Callbacks
+### Custom Driver {#custom-driver}
 
-### `void battery_percent_changed_user(uint8_t level)` {#api-battery-percent-changed-user}
+A custom driver is expected to implement the following interface:
 
-User hook called when battery level changed.
+```c
+void battery_driver_init(void) {
+    // Perform any initialisation here
+}
 
-### Arguments {#api-battery-percent-changed-user-arguments}
-
- - `uint8_t level`  
-   The battery percentage, in the range 0-100.
-
----
-
-### `void battery_percent_changed_kb(uint8_t level)` {#api-battery-percent-changed-kb}
-
-Keyboard hook called when battery level changed.
-
-### Arguments {#api-battery-percent-changed-kb-arguments}
-
- - `uint8_t level`  
-   The battery percentage, in the range 0-100.
+uint8_t battery_driver_sample_percent(void) {
+    // Read and return current state here
+    return value;
+}
+```

docs/features/battery.md

@@ -0,0 +1,55 @@
+# Battery
+
+This feature provides the high level abstraction for sampling battery level.
+
+## Usage
+
+To use this driver, add the following to your `rules.mk`:
+
+```make
+BATTERY_ENABLE = yes
+```
+
+## Basic Configuration {#basic-configuration}
+
+Add the following to your `config.h`:
+
+|Define                    |Default |Description                                       |
+|--------------------------|--------|--------------------------------------------------|
+|`BATTERY_SAMPLE_INTERVAL` |`30000` |The time between battery samples in milliseconds. |
+
+## Driver Configuration {#driver-configuration}
+
+See the [Battery Driver](../drivers/battery) documentation for more information.
+
+## Functions
+
+### `uint8_t battery_get_percent(void)` {#api-battery-get-percent}
+
+Sample battery level.
+
+#### Return Value {#api-battery-get-percent-return}
+
+The battery percentage, in the range 0-100.
+
+## Callbacks
+
+### `void battery_percent_changed_user(uint8_t level)` {#api-battery-percent-changed-user}
+
+User hook called when battery level changed.
+
+### Arguments {#api-battery-percent-changed-user-arguments}
+
+ - `uint8_t level`  
+   The battery percentage, in the range 0-100.
+
+---
+
+### `void battery_percent_changed_kb(uint8_t level)` {#api-battery-percent-changed-kb}
+
+Keyboard hook called when battery level changed.
+
+### Arguments {#api-battery-percent-changed-kb-arguments}
+
+ - `uint8_t level`  
+   The battery percentage, in the range 0-100.

docs/reference_info_json.md

@@ -179,6 +179,32 @@ Configures the [Backlight](features/backlight) feature.
     * `pins` <Badge type="info">Array: Pin</Badge>
         * A list of GPIO pins connected to the backlight LEDs (`software` and `timer` drivers only).
 
+## Battery
+
+Configures the [Battery](features/battery) feature.
+
+* `battery`
+    * `adc`
+        * `pin` <Badge type="info">Pin</Badge> <Badge>Required</Badge>
+            * The GPIO pin connected to the voltage divider.
+        * `reference_voltage` <Badge type="info">Number</Badge>
+            * The ADC reverence voltage, in millivolts. 
+            * Default: `3300`
+        * `divider_r1` <Badge type="info">Number</Badge>
+            * The voltage divider resistance, in kOhm. Set to 0 to disable.
+            * Default: `100`
+        * `divider_r2` <Badge type="info">Number</Badge>
+            * The voltage divider resistance, in kOhm. Set to 0 to disable.
+            * Default: `100`
+        * `resolution` <Badge type="info">Number</Badge>
+            * The ADC resolution configured for the ADC Driver.
+            * Default: `10`
+    * `driver` <Badge type="info">String</Badge> <Badge>Required</Badge>
+        * The driver to use. Must be one of `adc`, `custom`, `vendor`.
+    * `sample_interval` <Badge type="info">Number</Badge>
+        * The delay between sampling the battery in milliseconds.
+        * Default: `30000` (30 s)
+
 ## Wireless/Bluetooth {#bluetooth}
 
 Configures the [Wireless](features/wireless) feature.

drivers/battery/battery_adc.c

@@ -1,23 +1,24 @@
 // Copyright 2025 QMK
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include "battery_driver.h"
 #include "analog.h"
 #include "gpio.h"
 
-#ifndef BATTERY_PIN
-#    error("BATTERY_PIN not configured!")
+#ifndef BATTERY_ADC_PIN
+#    error("BATTERY_ADC_PIN not configured!")
 #endif
 
-#ifndef BATTERY_REF_VOLTAGE_MV
-#    define BATTERY_REF_VOLTAGE_MV 3300
+#ifndef BATTERY_ADC_REF_VOLTAGE_MV
+#    define BATTERY_ADC_REF_VOLTAGE_MV 3300
 #endif
 
-#ifndef BATTERY_VOLTAGE_DIVIDER_R1
+#ifndef BATTERY_ADC_VOLTAGE_DIVIDER_R1
 #    define BATTERY_VOLTAGE_DIVIDER_R1 100
 #endif
 
-#ifndef BATTERY_VOLTAGE_DIVIDER_R2
-#    define BATTERY_VOLTAGE_DIVIDER_R2 100
+#ifndef BATTERY_ADC_VOLTAGE_DIVIDER_R2
+#    define BATTERY_ADC_VOLTAGE_DIVIDER_R2 100
 #endif
 
 // TODO: infer from adc config?
@@ -26,16 +27,16 @@
 #endif
 
 void battery_driver_init(void) {
-    gpio_set_pin_input(BATTERY_PIN);
+    gpio_set_pin_input(BATTERY_ADC_PIN);
 }
 
 uint16_t battery_driver_get_mv(void) {
-    uint32_t raw = analogReadPin(BATTERY_PIN);
+    uint32_t raw = analogReadPin(BATTERY_ADC_PIN);
 
-    uint32_t bat_mv = raw * BATTERY_REF_VOLTAGE_MV / (1 << BATTERY_ADC_RESOLUTION);
+    uint32_t bat_mv = raw * BATTERY_ADC_REF_VOLTAGE_MV / (1 << BATTERY_ADC_RESOLUTION);
 
-#if BATTERY_VOLTAGE_DIVIDER_R1 > 0 && BATTERY_VOLTAGE_DIVIDER_R2 > 0
-    bat_mv = bat_mv * (BATTERY_VOLTAGE_DIVIDER_R1 + BATTERY_VOLTAGE_DIVIDER_R2) / BATTERY_VOLTAGE_DIVIDER_R2;
+#if BATTERY_VOLTAGE_DIVIDER_R1 > 0 && BATTERY_ADC_VOLTAGE_DIVIDER_R2 > 0
+    bat_mv = bat_mv * (BATTERY_VOLTAGE_DIVIDER_R1 + BATTERY_ADC_VOLTAGE_DIVIDER_R2) / BATTERY_ADC_VOLTAGE_DIVIDER_R2;
 #endif
 
     return bat_mv;

keyboards/handwired/onekey/keymaps/battery/config.h

@@ -3,4 +3,4 @@
 
 #pragma once
 
-#define BATTERY_PIN ADC_PIN
+#define BATTERY_ADC_PIN ADC_PIN

keyboards/handwired/onekey/keymaps/battery/keymap.c

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include QMK_KEYBOARD_H
-#include "battery.h"
 
 const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
     LAYOUT_ortho_1x1(KC_A)
@@ -14,8 +13,6 @@ void keyboard_post_init_user(void) {
 //   debug_matrix=false;
 //   debug_keyboard=true;
 //   debug_mouse=false;
-
-  battery_init();
 }
 
 void housekeeping_task_user(void) {

keyboards/handwired/onekey/keymaps/battery/keymap.json

@@ -1,6 +1,10 @@
 {
     "config": {
+        "battery": {
+            "driver": "adc"
+        },
         "features": {
+            "battery": true,
             "console": true
         }
     }

keyboards/handwired/onekey/keymaps/battery/rules.mk

@@ -1 +0,0 @@
-BATTERY_DRIVER_REQUIRED = yes

keyboards/handwired/promethium/config.h

@@ -64,8 +64,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #define PS2_MOUSE_INIT_DELAY 2000
 
-#define BATTERY_PIN B5
-
 #ifndef __ASSEMBLER__  // assembler doesn't like enum in .h file
 enum led_sequence {
     LED_IND_LINUX,

keyboards/handwired/promethium/keyboard.json

@@ -10,6 +10,12 @@
   "ws2812": {
     "pin": "B5"
   },
+  "battery": {
+    "driver": "adc",
+    "adc": {
+        "pin": "B5"
+    }
+  },
   "bluetooth": {
     "driver": "bluefruit_le"
   },
@@ -22,6 +28,7 @@
     "nkro": true,
     "ps2_mouse": true,
     "ps2": true,
+    "battery": true,
     "bluetooth": true
   },
   "build": {

keyboards/handwired/promethium/rules.mk

@@ -5,7 +5,6 @@ PS2_DRIVER = interrupt
 CUSTOM_MATRIX = yes
 
 WS2812_DRIVER_REQUIRED = yes
-BATTERY_DRIVER_REQUIRED = yes
 
 SRC += rgbsps.c
 SRC += matrix.c

quantum/battery/tests/battery_tests.cpp

@@ -0,0 +1,97 @@
+// Copyright 2025 QMK
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+using testing::_;
+
+class BatteryDriverMock {
+   public:
+    virtual ~BatteryDriverMock() {}
+
+    // mock methods
+    MOCK_METHOD0(battery_driver_init, void(void));
+    MOCK_METHOD0(battery_driver_sample_percent, uint8_t(void));
+    MOCK_METHOD1(battery_percent_changed_kb, void(uint8_t));
+};
+
+class BatteryTest : public ::testing::Test {
+   public:
+    BatteryTest() {
+        _batteryDriverMock.reset(new ::testing::NiceMock<BatteryDriverMock>());
+    }
+    virtual ~BatteryTest() {
+        _batteryDriverMock.reset();
+    }
+
+    static std::unique_ptr<BatteryDriverMock> _batteryDriverMock;
+};
+
+std::unique_ptr<BatteryDriverMock> BatteryTest::_batteryDriverMock;
+
+extern "C" {
+#include "quantum/battery/battery.h"
+#include "timer.h"
+
+void advance_time(uint32_t ms);
+
+void battery_driver_init(void) {
+    if (BatteryTest::_batteryDriverMock) {
+        BatteryTest::_batteryDriverMock->battery_driver_init();
+    }
+}
+
+uint8_t battery_driver_sample_percent(void) {
+    if (BatteryTest::_batteryDriverMock) {
+        return BatteryTest::_batteryDriverMock->battery_driver_sample_percent();
+    }
+    return 255;
+}
+
+void battery_percent_changed_kb(uint8_t level) {
+    if (BatteryTest::_batteryDriverMock) {
+        BatteryTest::_batteryDriverMock->battery_percent_changed_kb(level);
+    }
+}
+}
+
+TEST_F(BatteryTest, TestInit) {
+    // init driver and initial sample
+    EXPECT_CALL(*_batteryDriverMock, battery_driver_init()).Times(1);
+    EXPECT_CALL(*_batteryDriverMock, battery_driver_sample_percent()).Times(1);
+
+    battery_init();
+}
+
+TEST_F(BatteryTest, TestSampleCached) {
+    // sample before timeout
+    EXPECT_CALL(*_batteryDriverMock, battery_driver_sample_percent()).Times(0);
+
+    advance_time(1);
+    battery_task();
+}
+
+TEST_F(BatteryTest, TestSampleNotCached) {
+    // sample after timeout
+    EXPECT_CALL(*_batteryDriverMock, battery_driver_sample_percent()).Times(1);
+
+    advance_time(60000);
+    battery_task();
+}
+
+TEST_F(BatteryTest, TestGet) {
+    // sample does not directly sample
+    EXPECT_CALL(*_batteryDriverMock, battery_driver_sample_percent()).Times(0);
+
+    battery_get_percent();
+}
+
+TEST_F(BatteryTest, TestChanged) {
+    // callbacks on value changed
+    EXPECT_CALL(*_batteryDriverMock, battery_percent_changed_kb(_)).Times(1);
+
+    battery_task();
+    advance_time(60000);
+    battery_task();
+}

quantum/battery/tests/rules.mk

@@ -0,0 +1,7 @@
+VPATH += $(DRIVER_PATH)/battery
+
+battery_SRC := \
+    $(PLATFORM_PATH)/timer.c \
+    $(PLATFORM_PATH)/$(PLATFORM_KEY)/timer.c \
+	$(QUANTUM_PATH)/battery/battery.c \
+	$(QUANTUM_PATH)/battery/tests/battery_tests.cpp \

quantum/battery/tests/testlist.mk

@@ -0,0 +1,2 @@
+TEST_LIST += \
+	battery \

quantum/keyboard.c

@@ -122,7 +122,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifdef SPLIT_KEYBOARD
 #    include "split_util.h"
 #endif
-#ifdef BATTERY_DRIVER
+#ifdef BATTERY_ENABLE
 #    include "battery.h"
 #endif
 #ifdef BLUETOOTH_ENABLE
@@ -532,7 +532,7 @@ void keyboard_init(void) {
     // init after split init
     pointing_device_init();
 #endif
-#ifdef BATTERY_DRIVER
+#ifdef BATTERY_ENABLE
     battery_init();
 #endif
 #ifdef BLUETOOTH_ENABLE
@@ -779,7 +779,7 @@ void keyboard_task(void) {
     joystick_task();
 #endif
 
-#ifdef BATTERY_DRIVER
+#ifdef BATTERY_ENABLE
     battery_task();
 #endif
 

quantum/quantum.h

@@ -63,6 +63,10 @@
 #    include "bootmagic.h"
 #endif
 
+#ifdef BATTERY_ENABLE
+#    include "battery.h"
+#endif
+
 #ifdef DEFERRED_EXEC_ENABLE
 #    include "deferred_exec.h"
 #endif