zephyr: tests: Test most of remaining ocpc.c code

Add coverage to much of ocpc_config_secondary_charger() and
ocpc_calc_resistances()

BUG=b:276805061
TEST=./twister -T zephyr/test/ocpc

Change-Id: I78ca222fc44c087807d1419ae1dd15e1875b1b2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4504794
Commit-Queue: Tristan Honscheid <honscheid@google.com>
Tested-by: Tristan Honscheid <honscheid@google.com>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>

6 files changed, 315 insertions, 6 deletions

diff --git a/common/battery_fuel_gauge.c b/common/battery_fuel_gauge.c
index 710c854145..8d8396c4c0 100644
--- a/common/battery_fuel_gauge.c
+++ b/common/battery_fuel_gauge.c
@@ -276,7 +276,7 @@ static enum ec_error_list battery_get_fet_status_regval(int type, int *regval)
 	return rv;
 }
 
-int battery_is_charge_fet_disabled(void)
+test_mockable int battery_is_charge_fet_disabled(void)
 {
 	int rv;
 	int reg;
diff --git a/common/charger.c b/common/charger.c
index a9cee0decf..12e1961f4b 100644
--- a/common/charger.c
+++ b/common/charger.c
@@ -767,9 +767,9 @@ int chg_ramp_get_current_limit(void)
 }
 #endif
 
-enum ec_error_list charger_set_vsys_compensation(int chgnum,
-						 struct ocpc_data *ocpc,
-						 int current_ma, int voltage_mv)
+test_mockable enum ec_error_list
+charger_set_vsys_compensation(int chgnum, struct ocpc_data *ocpc,
+			      int current_ma, int voltage_mv)
 {
 	if ((chgnum < 0) || (chgnum >= board_get_charger_chip_count())) {
 		CPRINTS("%s(%d) Invalid charger!", __func__, chgnum);
diff --git a/common/ocpc.c b/common/ocpc.c
index 6ed15b1b1b..5d2d97dc2c 100644
--- a/common/ocpc.c
+++ b/common/ocpc.c
@@ -166,13 +166,25 @@ static bool is_within_range(struct ocpc_data *ocpc, int combined, int rbatt,
 enum ec_error_list ocpc_calc_resistances(struct ocpc_data *ocpc,
 					 struct batt_params *battery)
 {
-	int act_chg = ocpc->active_chg_chip;
+	int act_chg;
 	static bool seeded;
 	static int initial_samples;
 	int combined;
 	int rsys = -1;
 	int rbatt = -1;
 
+#ifdef TEST_BUILD
+	/* Mechanism for resetting static vars in tests */
+	if (!ocpc && !battery) {
+		seeded = false;
+		initial_samples = 0;
+
+		return EC_SUCCESS;
+	}
+#endif
+
+	act_chg = ocpc->active_chg_chip;
+
 	/*
 	 * In order to actually calculate the resistance, we need to make sure
 	 * we're actually charging the battery at a significant rate.  The LSB
@@ -810,4 +822,19 @@ int test_ocpc_get_debug_output(void)
 {
 	return debug_output;
 }
+void test_ocpc_reset_resistance_state(void)
+{
+	for (int i = 0; i < NUM_RESISTANCE_SAMPLES; i++) {
+		resistance_tbl[i][0] = CONFIG_OCPC_DEF_RBATT_MOHMS;
+		resistance_tbl[i][1] = CONFIG_OCPC_DEF_RBATT_MOHMS;
+		resistance_tbl[i][2] = 0;
+	}
+
+	resistance_tbl_idx = 0;
+
+	memset(mean_resistance, 0, sizeof(mean_resistance));
+	memset(stddev_resistance, 0, sizeof(stddev_resistance));
+	memset(ub, 0, sizeof(ub));
+	memset(lb, 0, sizeof(lb));
+}
 #endif
diff --git a/include/ocpc.h b/include/ocpc.h
index 3b95556fc1..050a40c099 100644
--- a/include/ocpc.h
+++ b/include/ocpc.h
@@ -10,6 +10,8 @@
 #ifndef __CROS_EC_OCPC_H_
 #define __CROS_EC_OCPC_H_
 
+#include "battery.h"
+
 #define OCPC_UNINIT 0xdededede
 
 struct ocpc_data {
@@ -106,6 +108,21 @@ int test_ocpc_get_viz_output(void);
  * @return int
  */
 int test_ocpc_get_debug_output(void);
+
+/**
+ * @brief Reset state used to track resistance calculations.
+ */
+void test_ocpc_reset_resistance_state(void);
+
+/**
+ * @brief Calculate the system impedance
+ *
+ * @param ocpc OCPC data struct pointer
+ * @param battery Battery params pointer
+ * @return enum ec_error_list Success or error code
+ */
+enum ec_error_list ocpc_calc_resistances(struct ocpc_data *ocpc,
+					 struct batt_params *battery);
 #endif
 
 #endif /* __CROS_EC_OCPC_H */
diff --git a/zephyr/emul/emul_isl923x.c b/zephyr/emul/emul_isl923x.c
index 74a2edfbf5..ef52704f4e 100644
--- a/zephyr/emul/emul_isl923x.c
+++ b/zephyr/emul/emul_isl923x.c
@@ -114,6 +114,12 @@ struct isl923x_emul_data {
 	uint16_t ac_prochot_reg;
 	/** Emulated DC PROCHOT register */
 	uint16_t dc_prochot_reg;
+	/* Emulated RAA489000_REG_ADC_INPUT_CURRENT */
+	uint16_t adc_input_current_reg;
+	/* Emulated RAA489000_REG_ADC_CHARGE_CURRENT */
+	uint16_t adc_charge_current_reg;
+	/* Emulated RAA489000_REG_ADC_VSYS */
+	uint16_t adc_vsys_reg;
 	/** Emulated ADC vbus register */
 	uint16_t adc_vbus_reg;
 	/** Emulated input voltage register */
@@ -289,6 +295,15 @@ static int isl923x_emul_read_byte(const struct emul *emul, int reg,
 	case ISL923X_REG_PROCHOT_DC:
 		READ_REG_16(data->dc_prochot_reg, bytes, val);
 		break;
+	case RAA489000_REG_ADC_INPUT_CURRENT:
+		READ_REG_16(data->adc_input_current_reg, bytes, val);
+		break;
+	case RAA489000_REG_ADC_CHARGE_CURRENT:
+		READ_REG_16(data->adc_charge_current_reg, bytes, val);
+		break;
+	case RAA489000_REG_ADC_VSYS:
+		READ_REG_16(data->adc_vsys_reg, bytes, val);
+		break;
 	case RAA489000_REG_ADC_VBUS:
 		READ_REG_16(data->adc_vbus_reg, bytes, val);
 		break;
@@ -377,6 +392,24 @@ static int isl923x_emul_write_byte(const struct emul *emul, int reg,
 		WRITE_REG_16(data->control_10_reg, bytes, val,
 			     REG_CONTROL10_MASK);
 		break;
+	case RAA489000_REG_ADC_INPUT_CURRENT:
+		__ASSERT(
+			false,
+			"Write to read-only reg RAA489000_REG_ADC_INPUT_CURRENT");
+		break;
+	case RAA489000_REG_ADC_CHARGE_CURRENT:
+		__ASSERT(
+			false,
+			"Write to read-only reg RAA489000_REG_ADC_CHARGE_CURRENT");
+		break;
+	case RAA489000_REG_ADC_VSYS:
+		__ASSERT(false,
+			 "Write to read-only reg RAA489000_REG_ADC_VSYS");
+		break;
+	case RAA489000_REG_ADC_VBUS:
+		__ASSERT(false,
+			 "Write to read-only reg RAA489000_REG_ADC_VBUS");
+		break;
 	case ISL9238_REG_INFO2:
 		__ASSERT(false, "Write to read-only reg ISL9238_REG_INFO2");
 		break;
diff --git a/zephyr/test/ocpc/src/ocpc.c b/zephyr/test/ocpc/src/ocpc.c
index 004d0c2408..dd7e2733e7 100644
--- a/zephyr/test/ocpc/src/ocpc.c
+++ b/zephyr/test/ocpc/src/ocpc.c
@@ -2,6 +2,7 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
+#include "charge_state_v2.h"
 #include "console.h"
 #include "host_command.h"
 #include "ocpc.h"
@@ -127,12 +128,239 @@ ZTEST_USER(ocpc, test_consolecmd_ocpcdebug)
 		      shell_execute_cmd(get_ec_shell(), "ocpcdebug"));
 }
 
+ZTEST(ocpc, test_ocpc_config_secondary_charger__with_primary_charger)
+{
+	/* Should immediately return if a non-secondary charger is active,
+	 * which is the default.
+	 */
+
+	charge_set_active_chg_chip(CHARGER_PRIMARY);
+
+	zassert_equal(EC_ERROR_INVAL,
+		      ocpc_config_secondary_charger(NULL, NULL, 0, 0));
+}
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__zero_desired_batt_curr)
+{
+	int expected_vsys_voltage = battery_get_info()->voltage_min;
+	int desired_vsys_voltage = expected_vsys_voltage - 1;
+	struct ocpc_data test_ocpc = {
+		.last_vsys = expected_vsys_voltage - 10,
+	};
+
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	zassert_equal(EC_SUCCESS,
+		      ocpc_config_secondary_charger(NULL, &test_ocpc,
+						    desired_vsys_voltage, 0));
+
+	/* Vsys should have been clamped to voltage_min */
+	zassert_equal(test_ocpc.last_vsys, expected_vsys_voltage);
+}
+
+FAKE_VALUE_FUNC(int, battery_is_charge_fet_disabled);
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__fet_disabled)
+{
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	/* A disabled FET should cause the function to abort */
+	battery_is_charge_fet_disabled_fake.return_val = true;
+
+	/* Use an arbitrary non-zero desired_batt_current_ma */
+	zassert_equal(EC_ERROR_INVALID_CONFIG,
+		      ocpc_config_secondary_charger(NULL, NULL, 0, 1000));
+
+	/* Try again and we should hit the rate limiter */
+	battery_is_charge_fet_disabled_fake.return_val = false;
+
+	zassert_equal(EC_ERROR_BUSY,
+		      ocpc_config_secondary_charger(NULL, NULL, 0, 1000));
+
+	/* Allow the block to expire */
+	k_sleep(K_SECONDS(6));
+}
+
+FAKE_VALUE_FUNC(enum ec_error_list, charger_set_vsys_compensation, int,
+		struct ocpc_data *, int, int);
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__happy)
+{
+	int desired_batt_voltage_mv = 123;
+	int desired_batt_current_ma = 456;
+
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	charger_set_vsys_compensation_fake.return_val = EC_SUCCESS;
+
+	/* charger_set_vsys_compensation() will succeed and we will be
+	 * done. Again use an arbitrary non-zero desired_current.
+	 */
+	zassert_equal(EC_SUCCESS, ocpc_config_secondary_charger(
+					  NULL, NULL, desired_batt_voltage_mv,
+					  desired_batt_current_ma));
+
+	zassert_equal(1, charger_set_vsys_compensation_fake.call_count);
+	zassert_equal(desired_batt_current_ma,
+		      charger_set_vsys_compensation_fake.arg2_history[0]);
+	zassert_equal(desired_batt_voltage_mv,
+		      charger_set_vsys_compensation_fake.arg3_history[0]);
+}
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__unknown_return_code)
+{
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	charger_set_vsys_compensation_fake.return_val = 999;
+
+	/* charger_set_vsys_compensation() will return an unhandled return
+	 * value.
+	 */
+	zassert_equal(999, ocpc_config_secondary_charger(NULL, NULL, 123, 456));
+}
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__unimpl)
+{
+	int desired_charger_input_current;
+	int desired_batt_voltage_mv = 10000;
+	int desired_batt_current_ma = 1000;
+	struct ocpc_data test_ocpc = {
+		/* First run through loop */
+		.last_vsys = OCPC_UNINIT,
+	};
+
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	/* Need to manually adjust Vsys */
+	charger_set_vsys_compensation_fake.return_val = EC_ERROR_UNIMPLEMENTED;
+
+	/* charger_set_vsys_compensation() will succeed and we will be
+	 * done. Again use an arbitrary non-zero desired_current.
+	 */
+	zassert_equal(EC_SUCCESS, ocpc_config_secondary_charger(
+					  &desired_charger_input_current,
+					  &test_ocpc, desired_batt_voltage_mv,
+					  desired_batt_current_ma));
+}
+
+ZTEST(ocpc, test_ocpc_config_secondary_charger__second_loop)
+{
+	int desired_charger_input_current = 2;
+	int desired_batt_voltage_mv = 10000;
+	int desired_batt_current_ma = 1000;
+	int initial_integral = 123;
+	struct ocpc_data test_ocpc = {
+		/* Non-first run through loop */
+		.last_vsys = 0,
+		.integral = initial_integral,
+	};
+
+	/* Proportional controller only */
+	test_ki = 0;
+	test_ki_div = 1;
+	test_kd = 0;
+	test_kd_div = 1;
+	ocpc_set_pid_constants();
+
+	charge_set_active_chg_chip(CHARGER_SECONDARY);
+
+	/* Need to manually adjust Vsys */
+	charger_set_vsys_compensation_fake.return_val = EC_ERROR_UNIMPLEMENTED;
+
+	/* charger_set_vsys_compensation() will succeed and we will be
+	 * done. Again use an arbitrary non-zero desired_current.
+	 */
+	zassert_equal(EC_SUCCESS, ocpc_config_secondary_charger(
+					  &desired_charger_input_current,
+					  &test_ocpc, desired_batt_voltage_mv,
+					  desired_batt_current_ma));
+
+	/* Make sure the integral got updated */
+	int expected_last_error =
+		desired_charger_input_current - test_ocpc.secondary_ibus_ma;
+
+	zassert_equal(expected_last_error, test_ocpc.last_error,
+		      "Actual: %d, expected: %d", test_ocpc.last_error,
+		      expected_last_error);
+	zassert_equal(expected_last_error + initial_integral,
+		      test_ocpc.integral, "Actual: %d, expected: %d",
+		      test_ocpc.integral,
+		      expected_last_error + initial_integral);
+}
+
+ZTEST(ocpc, test_ocpc_calc_resistances__not_charging)
+{
+	struct ocpc_data test_ocpc;
+	struct batt_params test_batt_params;
+
+	/* There are multiple conditions to exercise that qualify as charging */
+
+	/* Battery current below 1666 */
+	test_ocpc = (struct ocpc_data){ 0 };
+	test_batt_params = (struct batt_params){
+		.current = 0,
+	};
+	zassert_equal(EC_ERROR_INVALID_CONFIG,
+		      ocpc_calc_resistances(&test_ocpc, &test_batt_params));
+
+	/* Isys <= 0 */
+	test_ocpc = (struct ocpc_data){ 0 };
+	test_batt_params = (struct batt_params){
+		.current = 1667,
+	};
+	zassert_equal(EC_ERROR_INVALID_CONFIG,
+		      ocpc_calc_resistances(&test_ocpc, &test_batt_params));
+}
+
+ZTEST(ocpc, test_ocpc_calc_resistances__separate)
+{
+	struct ocpc_data test_ocpc;
+	struct batt_params test_batt_params;
+
+	/* Make Rsys = 1, Rbatt = 2 */
+	test_ocpc = (struct ocpc_data){
+		.vsys_aux_mv = 2005,
+		.vsys_mv = 2000,
+		.isys_ma = 1000,
+	};
+	test_batt_params = (struct batt_params){
+		.current = 2000,
+		.voltage = 1950,
+	};
+
+	/* Run enough times to become seeded. */
+	for (int i = 0; i < 17; i++) {
+		zassert_equal(EC_SUCCESS,
+			      ocpc_calc_resistances(&test_ocpc,
+						    &test_batt_params));
+	}
+
+	int expected_rbatt = (test_ocpc.vsys_mv - test_batt_params.voltage) *
+			     1000 / test_batt_params.current;
+
+	zassert_equal(expected_rbatt, test_ocpc.rbatt_mo,
+		      "Actual: %d, expected: %d", test_ocpc.rbatt_mo,
+		      expected_rbatt);
+
+	int expected_rsys = (test_ocpc.vsys_aux_mv - test_ocpc.vsys_mv) * 1000 /
+			    test_ocpc.isys_ma;
+
+	zassert_equal(expected_rsys, test_ocpc.rsys_mo,
+		      "Actual: %d, expected: %d", test_ocpc.rsys_mo,
+		      expected_rsys);
+}
+
 static void reset(void *fixture)
 {
 	ARG_UNUSED(fixture);
 
-	/* Reset */
+	charge_set_active_chg_chip(CHARGER_PRIMARY);
+	trigger_ocpc_reset();
+
+	/* Reset fakes */
 	RESET_FAKE(ocpc_get_pid_constants);
+	RESET_FAKE(battery_is_charge_fet_disabled);
+	RESET_FAKE(charger_set_vsys_compensation);
 
 	/* Load values that match ocpc.c's defaults */
 	test_kp = 1;
@@ -146,6 +374,10 @@ static void reset(void *fixture)
 
 	/* Force an update which will use the above parameters. */
 	ocpc_set_pid_constants();
+
+	/* Reset the resistance calculation state */
+	ocpc_calc_resistances(NULL, NULL);
+	test_ocpc_reset_resistance_state();
 }
 
 ZTEST_SUITE(ocpc, NULL, NULL, reset, reset, NULL);