Suggestion for more flexibility

[Vadiml1024]

bms-firmware/drivers/bms_ic/bq769x2/bq769x2.c

Line 480 in ad92b6a

static int bq769x2_init_config(const struct device *dev)

It seems that this initialisation assumes very specific BMS board.
In my version I've added 2 fields to the config structure:

/**
 * BQ769x2 voltage resolution options in [mV]
 */
enum bq769x2_config_voltage_resolution {
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_1 = 0,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_10 = 1,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_MAX = 1,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_COUNT = 2
};

/**
 * BQ769x2 current resolution options in [mA]
 */
enum bq769x2_config_current_resolution {
	BQ769x2_CONFIG_CURRENT_RESOLUTION_01 = 0,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_1 = 1,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_10 = 2,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_100 = 3,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_MAX = 3,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_COUNT = 4
};

bq769x2_config {
  ....
	enum bq769x2_config_voltage_resolution voltage_resolution;
	enum bq769x2_config_current_resolution current_resolution;
....
}

And modified the
bq769x2_init_config

...
	/* Set resolution for current and voltage measurements */
	uint16_t da_value = config->current_resolution |
			    (config->voltage_resolution << 2);
	err |= bq769x2_datamem_write_u1(dev, BQ769X2_SET_CONF_DA, da_value);

	/*
	 * See bq769x2 technical reference manual pages 27-28 for details on the calculation
	*/
	/* Shunt value based on nominal value of VREF2 (could be improved by calibration) */
	float cc_gain = 7568.4F / config->shunt_resistor_uohm;
	err |= bq769x2_datamem_write_f4(dev, BQ769X2_CAL_CURR_CC_GAIN, cc_gain);

	/* Set capacity cc gain */
	float cap_gain = cc_gain * 298261.6178F;
	err |= bq769x2_datamem_write_f4(dev, BQ769X2_CAL_CURR_CAP_GAIN,
					cap_gain);

...

[martinjaeger]

Makes sense to make this more generic. How would you set the config, though? Did you also add these parameters to Devicetree?

[Vadiml1024]

I actually removed all Devicetree related stuff, i don't have it in my environment (pigweed). Because of this i cant simple send you patch file.
But the idea is following:

/**
 * BQ769x2 voltage resolution options in [mV]
 */
enum bq769x2_config_voltage_resolution {
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_1 = 0,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_10 = 1,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_MAX = 1,
	BQ769x2_CONFIG_VOLTAGE_RESOLUTION_COUNT = 2
};

/**
 * BQ769x2 current resolution options in [mA]
 */
enum bq769x2_config_current_resolution {
	BQ769x2_CONFIG_CURRENT_RESOLUTION_01 = 0,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_1 = 1,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_10 = 2,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_100 = 3,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_MAX = 3,
	BQ769x2_CONFIG_CURRENT_RESOLUTION_COUNT = 4
};

/* read-only driver configuration */
struct bq769x2_config {
	uint32_t shunt_resistor_uohm;
	uint32_t board_max_current;
	uint16_t used_cell_channels;
	enum bq769x2_config_voltage_resolution voltage_resolution;
	enum bq769x2_config_current_resolution current_resolution;
	uint8_t pin_config[9];
	uint8_t cell_temp_pins[CONFIG_BQ769x2_MAX_THERMISTORS];
	uint8_t num_cell_temps;
	uint8_t fet_temp_pin;
	bool crc_enabled;
	bool auto_pdsg;
	uint8_t reg12_config;
	bq769x2_write_bytes_t write_bytes_no_crc;
	bq769x2_read_bytes_t read_bytes_no_crc;
	uint8_t i2c_addr;
	void (*usleep)(uint32_t usec);
};

This also imposes modifications in voltage and current reading functions:

static const float VOLTAGE_FACTORS[BQ769x2_CONFIG_VOLTAGE_RESOLUTION_COUNT] = {
		0.001F, 0.01F
	};



static int bq769x2_detect_cells(const struct bq769x2 *dev)
{
	const struct bq769x2_config *config = &dev->config;
	uint8_t conn_cells = 0;
	uint16_t vcell_mode = 0;
	int16_t voltage = INT16_MAX; /* init with implausible value */
	uint32_t stack_voltage_calc = 0;
	int err = 0;

	err |= bq769x2_direct_read_i2(dev, BQ769X2_CMD_VOLTAGE_STACK, &voltage);
	uint32_t voltage_in_1mVs = 1000u * voltage * VOLTAGE_FACTORS[config->voltage_resolution];
	uint32_t stack_voltage_meas = voltage_in_1mVs;

	int last_cell = find_msb_set(config->used_cell_channels);
	for (int i = 0; i < last_cell; i++) {
		if (config->used_cell_channels & (1 << i)) {
			err |= bq769x2_direct_read_i2(
				dev, BQ769X2_CMD_VOLTAGE_CELL_1 + i * 2,
				&voltage);
			if (voltage > 500) {
				/* Only voltages > 500 mV are considered as
				 * connected cells
				 */
				vcell_mode |= 1 << i;
				stack_voltage_calc += voltage;
				conn_cells++;
			}
		}
	}

	/*
	 * Check for open wires by comparing measured stack voltage with sum of
	 * cell voltages.
	 * Deviation of +-50 mV per cell are accepted (stack voltage is not very
	 * precise if uncalibrated).
	 */
	if (!IN_RANGE(stack_voltage_calc, stack_voltage_meas - conn_cells * 50,
		      stack_voltage_meas + conn_cells * 50)) {
		PW_LOG_ERROR(
			"Sum of cell voltages (%u mV) != stack voltage (%u mV)",
			stack_voltage_calc, stack_voltage_meas);
		return -EINVAL;
	}

	err |= bq769x2_datamem_write_u2(dev, BQ769X2_SET_CONF_VCELL_MODE,
					vcell_mode);

	PW_LOG_INFO("Detected %d cells", conn_cells);

	return err == 0 ? 0 : -EIO;
}

static int bq769x2_read_total_voltages(const struct bq769x2 *dev,
				       struct bq769x2_data *ic_data)
{
	int16_t voltage = 0;
	int err;

	err = bq769x2_direct_read_i2(dev, BQ769X2_CMD_VOLTAGE_STACK, &voltage);
	ic_data->total_voltage =
		voltage * VOLTAGE_FACTORS[dev->config.voltage_resolution & 1];

#ifdef CONFIG_BQ769x2_SWITCHES
	err |= bq769x2_direct_read_i2(dev, BQ769X2_CMD_VOLTAGE_PACK, &voltage);
	ic_data->external_voltage =
		voltage * VOLTAGE_FACTORS[dev->config.voltage_resolution & 1];
#endif

	return err == 0 ? 0 : -EIO;
}

#ifdef CONFIG_BQ769x2_CURRENT_MONITORING

static int bq769x2_read_current(const struct bq769x2 *dev,
				struct bq769x2_data *ic_data)
{
	int16_t current = 0;
	int err;

	err = bq769x2_direct_read_i2(dev, BQ769X2_CMD_CURRENT_CC2, &current);

	static const float factors[BQ769x2_CONFIG_CURRENT_RESOLUTION_COUNT] = {
		0.0001F, 0.001F, 0.01F, 0.1F
	};

	ic_data->current = current * factors[dev->config.current_resolution];

	return err;
}
#endif