Skip to content

Commit f514c04

Browse files
fwk: reapply an explicitly requested static charge limit
Another control path can disable or replace the live sustainer while the cached requested percentage stays unchanged. Re-sending that percentage then succeeds without restoring the requested static thresholds. Invalidate the cache on an accepted explicit SET in stage 0. Preserve the caps of active Battery Extender stages and leave timer/GET behavior alone. Add a host regression to the existing utility runner and export optional GCC coverage. Use Zephyr time units and the repository formatter to pass source checks. Keep minute/hour arithmetic 64-bit so the accepted 9999-minute reset interval is not truncated. Exercise extender deadlines, configuration round-trips and console timer controls as well as charge-limit recovery. Related: FrameworkComputer/SoftwareFirmwareIssueTracker#284 BUG=None TEST=python3 util/test_framework_battery_extender.py (10 pass) TEST=bash util/run_tests.sh (pass) TEST=lotus RO/RW build with Zephyr SDK 0.17.2 (pass) Change-Id: I3d0946955ea6b9c401674b42ae65ded7e531dc54
1 parent c6575ef commit f514c04

3 files changed

Lines changed: 454 additions & 76 deletions

File tree

‎util/run_tests.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ cd "$(dirname "$(realpath -e "${BASH_SOURCE[0]}")")"/..
1717

1818
# Run shell tests
1919
cd util
20+
python3 test_framework_battery_extender.py
2021
./test-inject-keys.sh
2122

2223
# Run the Zephyr config tests
Lines changed: 351 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,351 @@
1+
#!/usr/bin/env python3
2+
# Copyright 2026 The ChromiumOS Authors
3+
# Use of this source code is governed by a BSD-style license that can be
4+
# found in the LICENSE file.
5+
6+
"""Host regression for Framework's charge-limit command.
7+
8+
Compile the complete production translation unit and real protocol headers.
9+
Mock only its EC services. No hardware or firmware flashing is involved.
10+
Run with: python3 util/test_framework_battery_extender.py
11+
Requires Python 3.8+ and a C compiler (cc, or set CC). Coverage needs GCC/gcov.
12+
"""
13+
14+
import argparse
15+
import os
16+
from pathlib import Path
17+
import subprocess
18+
import sys
19+
import tempfile
20+
import unittest
21+
22+
23+
ROOT = Path(__file__).resolve().parents[1]
24+
SOURCE = ROOT / "zephyr/program/framework/src/battery_extender.c"
25+
STUB_HEADERS = (
26+
"battery.h battery_smart.h board_charger.h battery_fuel_gauge.h "
27+
"board_function.h charge_state.h charger.h console.h "
28+
"customized_shared_memory.h extpower.h factory.h host_command.h "
29+
"system.h timer.h util.h hooks.h zephyr/sys/time_units.h"
30+
).split()
31+
32+
HARNESS = r"""
33+
#include <stdbool.h>
34+
#include <stdint.h>
35+
#include <stdio.h>
36+
#include <stdlib.h>
37+
#include <string.h>
38+
#include "ec_commands.h"
39+
#include "board_host_command.h"
40+
41+
#define USEC_PER_SEC 1000000U
42+
#define SEC_PER_MIN 60U
43+
#define SEC_PER_HOUR 3600U
44+
/* Match legacy timer.h when running this regression against the baseline. */
45+
#define SECOND 1000000
46+
#define MINUTE 60000000
47+
#define HOUR 3600000000ULL
48+
#define MAX(a, b) ((a) > (b) ? (a) : (b))
49+
#define MIN(a, b) ((a) < (b) ? (a) : (b))
50+
#define EC_SUCCESS 0
51+
#define EC_ERROR_PARAM1 1
52+
#define EC_ERROR_PARAM2 2
53+
#define EC_ERROR_PARAM3 3
54+
#define EC_ERROR_PARAM_COUNT 4
55+
#define SYSTEM_BBRAM_IDX_CHARGE_LIMIT_MAX 0
56+
#define DECLARE_HOOK(...)
57+
#define DECLARE_HOST_COMMAND(...)
58+
#define DECLARE_CONSOLE_COMMAND(...)
59+
#define cprints(...) ((void)0)
60+
#define cprintf(...) ((void)0)
61+
62+
typedef union { uint64_t val; } timestamp_t;
63+
struct host_cmd_handler_args {
64+
const void *params;
65+
void *response;
66+
int response_size;
67+
int version;
68+
};
69+
static uint8_t saved_limit = 80;
70+
static int lower = -1, upper = -1, mode;
71+
static uint64_t now;
72+
static int ac_present = 1;
73+
static timestamp_t get_time(void) { return (timestamp_t){ .val = now }; }
74+
static bool timestamp_expired(timestamp_t deadline, const timestamp_t *time)
75+
{ return deadline.val <= time->val; }
76+
static int system_get_bbram(int idx, uint8_t *value)
77+
{ *value = saved_limit; return 0; }
78+
static int system_set_bbram(int idx, uint8_t value)
79+
{ saved_limit = value; return 0; }
80+
static bool battery_sustainer_enabled(void)
81+
{ return lower != -1 && upper != -1; }
82+
static int battery_sustainer_set(int8_t lo, int8_t hi)
83+
{ lower = lo; upper = hi; return 0; }
84+
static int set_chg_ctrl_mode(enum ec_charge_control_mode value)
85+
{ mode = value; return 0; }
86+
static int factory_status(void) { return 0; }
87+
static int extpower_is_present(void) { return ac_present; }
88+
static int strtoi(const char *s, char **e, int base)
89+
{ return strtol(s, e, base); }
90+
static int parse_bool(const char *s, int *value)
91+
{ *value = !strcmp(s, "enable"); return 1; }
92+
93+
#include "PRODUCTION_SOURCE"
94+
95+
#define CHECK(condition, message) do { \
96+
if (!(condition)) { fprintf(stderr, "%s\n", message); return 1; } \
97+
} while (0)
98+
99+
static enum ec_status send(uint8_t operation, uint8_t limit)
100+
{
101+
struct ec_params_ec_chg_limit_control p = {
102+
.modes = operation, .max_percentage = limit
103+
};
104+
struct ec_response_chg_limit_control r = {0};
105+
struct host_cmd_handler_args args = { .params = &p, .response = &r };
106+
return cmd_charging_limit_control(&args);
107+
}
108+
109+
int main(int argc, char **argv)
110+
{
111+
extender_init();
112+
batt_extender_disable = true;
113+
CHECK(send(CHG_LIMIT_SET_LIMIT, 80) == EC_RES_SUCCESS, "initial SET failed");
114+
CHECK(lower == 75 && upper == 80, "initial SET did not apply 75..80");
115+
if (!strcmp(argv[1], "disabled")) {
116+
battery_sustainer_set(-1, -1);
117+
send(CHG_LIMIT_SET_LIMIT, 80);
118+
CHECK(lower == 75 && upper == 80,
119+
"same-limit SET left the sustainer disabled");
120+
} else if (!strcmp(argv[1], "replaced")) {
121+
battery_sustainer_set(90, 95);
122+
send(CHG_LIMIT_SET_LIMIT, 80);
123+
CHECK(lower == 75 && upper == 80,
124+
"same-limit SET kept another policy's thresholds");
125+
} else if (!strcmp(argv[1], "host_override")) {
126+
battery_sustainer_set(-1, -1);
127+
battery_extender();
128+
send(CHG_LIMIT_GET_LIMIT, 0);
129+
CHECK(!battery_sustainer_enabled(),
130+
"timer or GET cancelled intentional host control");
131+
} else if (!strncmp(argv[1], "stage", 5)) {
132+
send(CHG_LIMIT_SET_LIMIT, 98);
133+
batt_extender_disable = false;
134+
stage = !strcmp(argv[1], "stage1") ?
135+
BATT_EXTENDER_STAGE_1 : BATT_EXTENDER_STAGE_2;
136+
int cap_lower = stage == BATT_EXTENDER_STAGE_1 ? 90 : 85;
137+
int cap_upper = stage == BATT_EXTENDER_STAGE_1 ? 95 : 87;
138+
sustainer_lower = 93;
139+
sustainer_upper = 98;
140+
batt_extender_deadline.val = 0;
141+
batt_extender_deadline_stage2.val = 0;
142+
battery_sustainer_set(cap_lower, cap_upper);
143+
send(CHG_LIMIT_SET_LIMIT, 98);
144+
CHECK(lower == cap_lower && upper == cap_upper,
145+
"same-limit SET removed the active extender cap");
146+
battery_extender();
147+
CHECK(lower == cap_lower && upper == cap_upper,
148+
"timer removed the active extender cap");
149+
} else if (!strcmp(argv[1], "full_and_invalid")) {
150+
CHECK(send(CHG_LIMIT_SET_LIMIT, 19) == EC_RES_ERROR,
151+
"invalid SET was accepted");
152+
CHECK(lower == 75 && upper == 80, "invalid SET changed thresholds");
153+
send(CHG_LIMIT_SET_LIMIT, 100);
154+
CHECK(!battery_sustainer_enabled(), "100 percent did not disable limiting");
155+
send(CHG_LIMIT_SET_LIMIT, 80);
156+
CHECK(lower == 75 && upper == 80, "changed-limit SET failed");
157+
} else if (!strcmp(argv[1], "extender_timing")) {
158+
send(CHG_LIMIT_SET_LIMIT, 100);
159+
batt_extender_disable = false;
160+
batt_extender_deadline.val = 1000000;
161+
batt_extender_deadline_stage2.val = 2000000;
162+
now = 999999;
163+
battery_extender();
164+
CHECK(stage == BATT_EXTENDER_STAGE_0, "stage one began early");
165+
now = 1000000;
166+
battery_extender();
167+
CHECK(stage == BATT_EXTENDER_STAGE_1 && lower == 90 && upper == 95,
168+
"stage one missed its deadline or cap");
169+
now = 2000000;
170+
battery_extender();
171+
CHECK(stage == BATT_EXTENDER_STAGE_2 && lower == 85 && upper == 87,
172+
"stage two missed its deadline or cap");
173+
ac_present = 0;
174+
now = reset_deadline.val;
175+
battery_extender();
176+
CHECK(stage == BATT_EXTENDER_STAGE_0 && !battery_sustainer_enabled(),
177+
"AC absence did not reset the extender after its interval");
178+
} else if (!strcmp(argv[1], "configuration")) {
179+
struct ec_params_battery_extender p = {
180+
.cmd = BATT_EXTENDER_WRITE_CMD, .trigger_days = 99,
181+
.reset_minutes = 9999, .disable = 0
182+
};
183+
struct ec_response_battery_extender r = {0};
184+
struct host_cmd_handler_args args = { .params = &p, .response = &r };
185+
CHECK(battery_extender_hc(&args) == EC_SUCCESS, "configuration failed");
186+
CHECK(battery_extender_trigger == 8553600000000ULL,
187+
"99-day trigger was truncated");
188+
CHECK(battery_extender_reset == 599940000000ULL,
189+
"9999-minute reset was truncated");
190+
p.cmd = BATT_EXTENDER_READ_CMD;
191+
CHECK(battery_extender_hc(&args) == EC_SUCCESS, "readback failed");
192+
CHECK(r.trigger_days == 99 && r.reset_minutes == 9999,
193+
"configuration did not round-trip");
194+
} else if (!strcmp(argv[1], "console_timing")) {
195+
const char *args[] = { "batt_extender", "trigger", "2", "s" };
196+
CHECK(cmd_parse_timestamp(4, args) == 2000000ULL,
197+
"seconds changed during time-unit migration");
198+
args[3] = "m";
199+
CHECK(cmd_parse_timestamp(4, args) == 120000000ULL, "minutes changed");
200+
args[3] = "h";
201+
CHECK(cmd_parse_timestamp(4, args) == 7200000000ULL, "hours changed");
202+
args[3] = "d";
203+
CHECK(cmd_batt_extender(4, args) == EC_SUCCESS, "console trigger failed");
204+
CHECK(battery_extender_trigger == 172800000000ULL, "days changed");
205+
CHECK(cmd_batt_extender(1, args) == EC_SUCCESS, "console status failed");
206+
} else if (!strcmp(argv[1], "console_controls")) {
207+
const char *args[] = { "batt_extender", "enable", "2", "m" };
208+
now = 123;
209+
CHECK(cmd_batt_extender(2, args) == EC_SUCCESS,
210+
"console enable failed");
211+
CHECK(!batt_extender_disable && batt_extender_deadline.val > now,
212+
"enable did not arm the extender");
213+
args[1] = "timeext";
214+
CHECK(cmd_batt_extender(4, args) == EC_SUCCESS, "stage-one timer failed");
215+
CHECK(batt_extender_deadline.val == 120000123ULL,
216+
"stage-one timer used the wrong unit");
217+
args[1] = "timeext2";
218+
CHECK(cmd_batt_extender(4, args) == EC_SUCCESS, "stage-two timer failed");
219+
CHECK(batt_extender_deadline_stage2.val == 120000123ULL,
220+
"stage-two timer used the wrong unit");
221+
args[1] = "timerst";
222+
CHECK(cmd_batt_extender(4, args) == EC_SUCCESS, "reset timer failed");
223+
CHECK(reset_deadline.val == 120000123ULL,
224+
"reset timer used the wrong unit");
225+
args[1] = "reset";
226+
args[3] = "h";
227+
CHECK(cmd_batt_extender(4, args) == EC_SUCCESS, "reset setting failed");
228+
CHECK(battery_extender_reset == 7200000000ULL,
229+
"reset setting used the wrong unit");
230+
args[1] = "disable";
231+
CHECK(cmd_batt_extender(2, args) == EC_SUCCESS, "disable failed");
232+
CHECK(batt_extender_disable && stage == BATT_EXTENDER_STAGE_0,
233+
"disable did not return to stage zero");
234+
} else {
235+
return 2;
236+
}
237+
return 0;
238+
}
239+
"""
240+
241+
242+
class ChargeLimitTest(unittest.TestCase):
243+
"""Exercise explicit SET and the policies it must preserve."""
244+
245+
coverage_dir = None
246+
247+
@classmethod
248+
def setUpClass(cls):
249+
# unittest owns and closes this directory after the class finishes.
250+
# pylint: disable=consider-using-with
251+
temporary = tempfile.TemporaryDirectory()
252+
# pylint: enable=consider-using-with
253+
cls.addClassCleanup(temporary.cleanup)
254+
path = Path(temporary.name)
255+
for header in STUB_HEADERS:
256+
(path / header).parent.mkdir(parents=True, exist_ok=True)
257+
(path / header).write_text(
258+
"/* EC service mocked by the host test. */\n"
259+
)
260+
c = path / "regression.c"
261+
c.write_text(HARNESS.replace("PRODUCTION_SOURCE", SOURCE.as_posix()))
262+
cls.binary = path / "regression"
263+
coverage_flags = ["--coverage"] if cls.coverage_dir else []
264+
subprocess.run(
265+
[
266+
os.environ.get("CC", "cc"),
267+
"-std=gnu11",
268+
"-Wall",
269+
"-Werror",
270+
*coverage_flags,
271+
"-Wno-unused-function",
272+
"-Wno-unused-variable",
273+
"-I",
274+
str(path),
275+
"-I",
276+
str(ROOT / "zephyr/program/framework/include"),
277+
"-I",
278+
str(ROOT / "include"),
279+
str(c),
280+
"-o",
281+
str(cls.binary),
282+
],
283+
check=True,
284+
)
285+
if cls.coverage_dir:
286+
cls.coverage_dir.mkdir(parents=True, exist_ok=True)
287+
cls.addClassCleanup(cls.export_coverage, c)
288+
289+
@classmethod
290+
def export_coverage(cls, source):
291+
"""Save GCC's coverage data before the temporary files are removed."""
292+
subprocess.run(
293+
["gcov", "--json-format", "--branch-probabilities", str(source)],
294+
cwd=cls.coverage_dir,
295+
check=True,
296+
)
297+
298+
def check_case(self, case):
299+
"""Run an isolated production-code scenario and expose its assertion."""
300+
result = subprocess.run(
301+
[self.binary, case], capture_output=True, text=True, check=False
302+
)
303+
self.assertEqual(result.returncode, 0, result.stderr)
304+
305+
def test_same_limit_restores_disabled_sustainer(self):
306+
"""An explicit repeated SET must restore disabled static thresholds."""
307+
self.check_case("disabled")
308+
309+
def test_same_limit_restores_replaced_thresholds(self):
310+
"""A repeated static SET must replace another policy's thresholds."""
311+
self.check_case("replaced")
312+
313+
def test_timer_and_get_preserve_host_override(self):
314+
"""Readback and timer activity must not cancel intentional host control."""
315+
self.check_case("host_override")
316+
317+
def test_same_limit_preserves_stage_one_cap(self):
318+
"""A repeated SET must preserve the first active extender cap."""
319+
self.check_case("stage1")
320+
321+
def test_same_limit_preserves_stage_two_cap(self):
322+
"""A repeated SET must preserve the second active extender cap."""
323+
self.check_case("stage2")
324+
325+
def test_full_charge_and_invalid_limit(self):
326+
"""Reject invalid limits and retain full-charge/static transitions."""
327+
self.check_case("full_and_invalid")
328+
329+
def test_extender_deadlines_and_ac_reset(self):
330+
"""Keep both cap deadlines and the AC-absence reset interval."""
331+
self.check_case("extender_timing")
332+
333+
def test_configuration_round_trips_maximum_intervals(self):
334+
"""Keep valid multi-day/minute intervals wider than 32 bits."""
335+
self.check_case("configuration")
336+
337+
def test_console_time_units(self):
338+
"""Keep seconds, minutes, hours and days in microseconds."""
339+
self.check_case("console_timing")
340+
341+
def test_console_enable_disable_and_timers(self):
342+
"""Keep public console controls and their configured intervals."""
343+
self.check_case("console_controls")
344+
345+
346+
if __name__ == "__main__":
347+
parser = argparse.ArgumentParser(add_help=False)
348+
parser.add_argument("--coverage-dir", type=Path)
349+
options, test_args = parser.parse_known_args()
350+
ChargeLimitTest.coverage_dir = options.coverage_dir
351+
unittest.main(argv=[sys.argv[0], *test_args], verbosity=2)

0 commit comments

Comments
 (0)