2024-08-27 11:27 AM - edited 2024-08-27 01:06 PM
Can you see the glaring bug, or is it? I left a clue *** some fail returns do not unlock HAL
HAL_StatusTypeDef HAL_I2C_Mem_Write(I2C_HandleTypeDef *hi2c, uint16_t DevAddress, uint16_t MemAddress,
uint16_t MemAddSize, uint8_t *pData, uint16_t Size, uint32_t Timeout)
{
uint32_t tickstart;
/* Check the parameters */
assert_param(IS_I2C_MEMADD_SIZE(MemAddSize));
if (hi2c->State == HAL_I2C_STATE_READY)
{
if ((pData == NULL) || (Size == 0U))
{
hi2c->ErrorCode = HAL_I2C_ERROR_INVALID_PARAM;
return HAL_ERROR;
}
/* Process Locked ***/
__HAL_LOCK(hi2c);
/* Init tickstart for timeout management*/
tickstart = HAL_GetTick();
if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BUSY, SET, I2C_TIMEOUT_BUSY, tickstart) != HAL_OK)
{
return HAL_ERROR;
}
hi2c->State = HAL_I2C_STATE_BUSY_TX;
hi2c->Mode = HAL_I2C_MODE_MEM;
hi2c->ErrorCode = HAL_I2C_ERROR_NONE;
/* Prepare transfer parameters */
hi2c->pBuffPtr = pData;
hi2c->XferCount = Size;
hi2c->XferISR = NULL;
/* Send Slave Address and Memory Address */
if (I2C_RequestMemoryWrite(hi2c, DevAddress, MemAddress, MemAddSize, Timeout, tickstart) != HAL_OK)
{
/* Process Unlocked */
__HAL_UNLOCK(hi2c);
return HAL_ERROR;
}
...
Solved! Go to Solution.
2024-09-04 02:18 AM - edited 2024-09-04 02:19 AM
The function has not return specification, on top of that one path returns nothing, the other a status value, so how does the compiler even compile it, it would have to assume int for example, then assume to return 0 for the path with no return.
Do you think that complies with C Standards, or that its safe programming?
Of course different compilers/parsers may do different things, but my point is that a $16 billion turnover company like STM should not be pushing sloppy code on their commercial clients.
Or do are you a sloppy Joe programmer? 🙂
2024-09-05 04:46 AM
That allows recovery, but standard practice is to leave interface in the same state as when entered, and not depend on external routines to recover the status.
You've also overlooked the failing in the HAL_Lock code as #defined, where the return type is not specified, and one path return a status value, which prompts a GNU warning.
2024-09-05 05:13 AM - edited 2024-09-05 05:13 AM
@Robmar wrote:in the HAL_Lock code as #defined, where the return type is not specified
Because __HAL_Lock is just a macro - not a function.
Therefore the return within the macro acts as a return for the function in which the macro is used. It's the enclosing function which provides the return type.
Whether this is a great coding style is another question ...
2024-09-05 07:04 PM
I wonder if you're a bot, because you are ignoring the fact the the GNU compiler is report a warning that the function is void yet returns a value.
Maybe you know better than compiler software engineers lol!