luna-cycle / vesc_bms_fw

The VESC BMS Firmware
GNU General Public License v3.0
0 stars 0 forks source link

Pointers misalingment #5

Closed diegosarli closed 1 year ago

diegosarli commented 2 years ago

I continue testing the fault behavior. In some occasions, when a OV fault is active the CHG and DSC pins jump from 0 to 1 due to unknown reason. I printed the member request_connection_pack of the bq76940 struct:

image

the values are different than 0 or 1, i re arrange the order inside the struct, before the changes the struct looks like this:

typedef struct { i2c_bb_state m_i2c; bool mutex; bool initialized; uint8_t status; int lrd_pin; float shunt_res; float gain; float offset; float CC; float temp[MAX_TEMP_SENSORS_NUM]; float temp_ic; float cell_mv[MAX_CELL_NUM]; uint32_t padding; float pack_mv; bool m_discharge_state[MAX_CELL_NUM]; bool discharge_enabled; bool charge_enabled; bool is_connected_pack; bool request_connection_pack; fault_data fault; bool is_load_present; bool oc_detected; bool sc_detected; bool UV_detected; bool OV_detected; bool connect_only_charger; uint8_t re_init_retry; } bq76940_t;

after the re arrange:

typedef struct { i2c_bb_state m_i2c; bool mutex; bool initialized; uint8_t status; int lrd_pin; float shunt_res; float gain; float offset; float CC; float temp[MAX_TEMP_SENSORS_NUM]; float temp_ic; float cell_mv[MAX_CELL_NUM]; fault_data fault; uint8_t re_init_retry; uint32_t padding; float pack_mv; bool m_discharge_state[MAX_CELL_NUM]; bool discharge_enabled; bool charge_enabled; bool is_connected_pack; bool request_connection_pack; bool is_load_present; bool oc_detected; bool sc_detected; bool UV_detected; bool OV_detected; bool connect_only_charger;

} bq76940_t;

Note that i place all the mixed types variables (float, int, fault_data ...etc) before the uint32_padding. The issue seems to be solved, but it's make sense? I'm not entirely sure, but i think that the issue is related with pointers misalignment, it make sense?

May be this is also related with the non confirmed issue "hardware over temp"

I will continue testing.

diegosarli commented 2 years ago

The problem was not the pointer misalignment, the issue was related the size of the hw_config array,: https://github.com/luna-cycle/vesc_bms_fw/blob/585776f6087b52da0bd01d23479774f58af421be/datatypes.h#L188

the bq76940 struct does´t fit in 128 bytes https://github.com/luna-cycle/vesc_bms_fw/blob/585776f6087b52da0bd01d23479774f58af421be/drivers/bq76940.c#L79

If it is confirmed it's surprising that we have not had more serious failures

nitrousnrg commented 2 years ago

uggh, good catch. Its not easy to keep track of this kind of bug.

Newer C11 compiler would allow a build-time assert like this

static_assert (sizeof(the struct) > 128,"Struct overflow");

but in our older C99 its not as clean to do something like that