Is checking return value of HAL_Transmit/Receive_IT overkill?

[Marcury1]

Hello everyone, I have a lot (too much) question regarding code optimisation, the safety of a code and some return value of the HAL.

Firstly, I am currently using HAL and CMSIS RTOS v2.  I am using a spi bus protected with a mutexe. Assuming that I am correctly using the mutexe, is it possible for HAL_Transmit_IT or HAL_Receive_IT to actually return HAL_Busy?

What I want to know is:

1) Am I making too much verification? Afterall a if/else bloc is require some ressources from the core, even if it's very little.

2) I read on various post that using assert was a good practice in embbeded system but I'm still strugling to know when to use it and when not. I also read that it was recommanded to always check a return value of a function when it has one. 

Am I using correctly the assert() function? Is there too much usage of it? Not enough? Where is it really required and when is it not?

Here is a sample of a basic function for a driver I am currently working on. I'm following the same code structure in all my other function so it gives a good idea of how I use insert and HAL return checking in other parts of my code.

osStatus_t ADXL375_set(ADXL375_handler_t* handler, ADXL375_register_t register_adress, uint8_t new_data) {
	//Parameter check
	assert(handler != NULL);

	osStatus_t driver_status = osMutexAcquire(handler->spi_mutex, osWaitForever);
	if (driver_status == osOK) {
		uint8_t command[2];
		command[0] = register_adress;
		command[1] = new_data;
		HAL_GPIO_WritePin(handler->cs_gpio, handler->cs_pin, GPIO_PIN_RESET);
		HAL_StatusTypeDef spi_status = HAL_SPI_Transmit_IT(handler->hspi, command, 2);
		if (spi_status == HAL_OK) {
			driver_status = osOK;
			//The flag tell the system that it is this driver that used the SPI bus.
			osEventFlagsSet(handler->read_sensors_event, handler->event_flag_number);
		} else {
			//Should never return HAL busy or error since parameter are checked and mutex is used.
			assert(0);
		}
	} else {
		//Error on Mutex. It should not happend so I guess it is critical enough to restart the program?
		assert(0);
	}
	return driver_status;
}

 
Don't hesistate to tell me if my question is unclear or if it needs precision.

[Tesla DeLorean · Accepted solution]

Depends if you want to be able to diagnose failure when it occurs.

The _IT and _DMA calls can only have a single instance "in-flight" at any given time, so you have to catch those failures, or errors sent to the callbacks, etc. There can be multiple exit paths, so if you get out-of-sync you could end up with a situation where communications stall or just stop completely. Basically a resource leak.

asserts can be good for a debug/diagnostic build as they can catch a lot of parameter passing errors. Probably too much for a customer release.

Customer States "Doesn't Work"  : From a support perspective getting no feedback on the cause/nature of a failure is very unhelpful.

You'll have to decide at what level you want to catch and report errors, having HardFault_Handler() and Error_Handler() provide some context will help. On the other hand reporting every HAL interaction, might be too much, but you want it to propagate the error upward, to a point where you can report it, or attempt recovery.