Undefined behaviour in LTC681x_run_adc_overlap()

[gudnimg]

This includes LTC681x.cpp, LTC6813.cpp, LTC6812.cpp.

See below LTC681x.cpp case:

for (int cic = 0; cic<total_ic; cic++)
{
	measure_delta = (int32_t)ic[cic].cells.c_codes[6]-(int32_t)ic[cic].cells.c_codes[7];
		
	if ((measure_delta>failure_pos_limit) || (measure_delta<failure_neg_limit))
	{
	        error = error | (1<<(cic-1)); // <-- when cic = 0 we have a negative bitwise shift which is undefined behaviour
	}
}

On top of this I think the error counter should only count the errors.

for (int cic = 0; cic<total_ic; cic++)
{
	measure_delta = (int32_t)ic[cic].cells.c_codes[6]-(int32_t)ic[cic].cells.c_codes[7];
		
	if ((measure_delta>failure_pos_limit) || (measure_delta<failure_neg_limit))
	{
	        error += 1;
	}
}

[gudnimg]

To add to this issue. We should remove these two variable definitions int32_t a, b; from LTC681x_run_adc_overlap() since they are never used for anything. Let's call it cleanup 👍

I'm also thinking if we can somehow use this function for LTC6813/LTC6812 as well? They have an additional overlap test. I'm thinking of a solution that looks something like this: Notice the #ifdef (LTC6813 || LTC6812)

I do realise this would require a systematic change across the library. Shouldn't be too much work though if there's time.

/*  Runs the ADC overlap test for the IC */
uint16_t LTC681x_run_adc_overlap(uint8_t total_ic, // Number of ICs in the system
								cell_asic *ic // A two dimensional array that will store the data
								)
{
	uint16_t error = 0;
	int32_t measure_delta =0;
	int16_t failure_pos_limit = 20;   //  20mV
	int16_t failure_neg_limit = -20; // -20mV
	uint32_t conv_time=0;  
	wakeup_idle(total_ic);
	LTC681x_adol(MD_7KHZ_3KHZ,DCP_DISABLED);
	conv_time = LTC681x_pollAdc();

	wakeup_idle(total_ic);
	error = LTC681x_rdcv(0, total_ic,ic);
	for (int cic = 0; cic<total_ic; cic++)
	{
      
		measure_delta = (int32_t)ic[cic].cells.c_codes[6]-(int32_t)ic[cic].cells.c_codes[7]; 
		if ((measure_delta>failure_pos_limit) || (measure_delta<failure_neg_limit))
		{
		  error += 1
		}
                 
                #ifdef (LTC6813 || LTC6812)
		measure_delta = (int32_t)ic[cic].cells.c_codes[12]-(int32_t)ic[cic].cells.c_codes[13]; 
		if ((measure_delta>failure_pos_limit) || (measure_delta<failure_neg_limit))
		{
		        error += 1
		}
	        #endif
        }
	return(error);
}

[gudnimg]

Another quick point is the header doxygen comment for the function

/*!
 Helper Function that runs the ADC Overlap test 
 @return uint16_t, error
  0: Pass
 -1: False, Error detected 
 */

It should look something like this:

/*!
 Helper Function that runs the ADC Overlap test 
 @return uint16_t, error
  0: Pass, No errors detected.
  Else returns the number of errors detected > 0.
 */

This makes things inline with how this function is used for example for the LTC6811 evaluation board DC2259A.