cancel
Showing results for 
Search instead for 
Did you mean: 

VL53L5CX Linux driver 1.2.0 - Timeout issue in _vl53l5cx_poll_for_answer() function

MFabb.1
Associate II
/**
 * @brief Inner function, not available outside this file. This function is used
 * to wait for an answer from VL53L5CX sensor.
 */
 
static uint8_t _vl53l5cx_poll_for_answer(
		VL53L5CX_Configuration	*p_dev,
		uint8_t					size,
		uint8_t					pos,
		uint16_t				address,
		uint8_t					mask,
		uint8_t					expected_value)
{
	uint8_t status = VL53L5CX_STATUS_OK;
	uint8_t timeout = 0;
 
	do {
		status |= RdMulti(&(p_dev->platform), address,
				p_dev->temp_buffer, size);
		status |= WaitMs(&(p_dev->platform), 10);
 
		if(timeout >= (uint8_t)200)	/* 2s timeout */
		{
			status |= p_dev->temp_buffer[2];
		}else if((size >= (uint8_t)4) 
                         && (p_dev->temp_buffer[2] >= (uint8_t)0x7f))
		{
			status |= VL53L5CX_MCU_ERROR;
		}
		else
		{
			timeout++;
		}
	}while ((p_dev->temp_buffer[pos] & mask) != expected_value);
 
	return status;
}

The variable timeout is incremented and tested if above 200 (2s timeout) but there is not a condition on timeout value to break the while loop.

This could lead to a infinite loop if the while condition below is never true.

(p_dev->temp_buffer[pos] & mask) != expected_value

Could you double check the code?

Thank you

1 ACCEPTED SOLUTION

Accepted Solutions
John E KVAM
ST Employee

The suggested fix is:

if(timeout >= (uint8_t)200)           /* 2s timeout */
{
                        status |= (uint8_t)VL53L5CX_STATUS_TIMEOUT_ERROR;
                        break; // To be tested
}else if((size >= (uint8_t)4) 
                        && (p_dev->temp_buffer[2] >= (uint8_t)0x7f))
{
                        status |= VL53L5CX_MCU_ERROR;
                        break; // To be tested
}
else
{
                        timeout++;
}

It's up to you if you want to implement this before we finish regression tests.

So try it if you want to, or wait for our update.

And thanks for the note.

  • john

If this or any post solves your issue, please mark them as 'Accept as Solution' It really helps. And if you notice anything wrong do not hesitate to 'Report Inappropriate Content'. Someone will review it.

View solution in original post

3 REPLIES 3
John E KVAM
ST Employee

I completely agree. Why even bother to increment a timeout, if you are not going to test for it.

I've forwarded your complaint to the software team.

Nice catch.

  • john

If this or any post solves your issue, please mark them as 'Accept as Solution' It really helps. And if you notice anything wrong do not hesitate to 'Report Inappropriate Content'. Someone will review it.
John E KVAM
ST Employee

The suggested fix is:

if(timeout >= (uint8_t)200)           /* 2s timeout */
{
                        status |= (uint8_t)VL53L5CX_STATUS_TIMEOUT_ERROR;
                        break; // To be tested
}else if((size >= (uint8_t)4) 
                        && (p_dev->temp_buffer[2] >= (uint8_t)0x7f))
{
                        status |= VL53L5CX_MCU_ERROR;
                        break; // To be tested
}
else
{
                        timeout++;
}

It's up to you if you want to implement this before we finish regression tests.

So try it if you want to, or wait for our update.

And thanks for the note.

  • john

If this or any post solves your issue, please mark them as 'Accept as Solution' It really helps. And if you notice anything wrong do not hesitate to 'Report Inappropriate Content'. Someone will review it.
MFabb.1
Associate II

Thank you John.

I am looking forward for an API update.