BUG: HAL_DMA_PollForTransfer: Type of CompleteLevel should be HAL_DMA_LevelCompleteTypeDef.

Morty Morty · ‎2017-10-10

Posted on October 10, 2017 at 16:07

The title says it all.

#bug

Note: this post was migrated and contained many threaded conversations, some content may be missing.

Nesrine M_O · ‎2017-10-11

Posted on October 11, 2017 at 15:44

Hi

moritz.struebe

,

The title says it all.

What are you using as STM32 product?

Could you precise in which HAL library you have found the issue. So, we can verify it.

-Nesrine-

Tesla DeLorean · ‎2017-10-11

Posted on October 11, 2017 at 18:11

Post history suggests STM32Cube_FW_F3_V1.9.0

Tips, Buy me a coffee, or three.. PayPal Venmo
Up vote any posts that you find helpful, it shows what's working..

Morty Morty · ‎2017-10-12

Posted on October 12, 2017 at 08:43

Yep that's right. But I'm getting so fed up with the quality of the code that I'm getting less and less motivated to write decent bug reports[1]. Especially as I'm only working with this code for a few weeks now. And how many lines are there that contain the words HAL_DMA_PollForTransfer and CompleteLevel. Oh wait - first level support, right? regexp is a magic.

[1] Just found out, that the SPI-HAL might busy wait for up to 100ms

:(

in an interrupt

:(

. Seriously, ST?

waclawek.jan · ‎2017-10-12

Posted on October 12, 2017 at 10:09

I'm getting so fed up with the quality of the code...working with this code for a few weeks now...

Okay, so you've made the preliminary assessment and found it inadequate. Then why do you use it at all now?

Just found out, that the SPI-HAL might busy wait for up to 100ms in an interrupt .

Sounds scary, but isn't that a last-resort measure, for the the 'this never happens unless serious bug in user code' leg?

JW

Morty Morty · ‎2017-10-12

Posted on October 12, 2017 at 10:45

waclawek.jan wrote:

I'm getting so fed up with the quality of the code...working with this code for a few weeks now...

Okay, so you've made the preliminary assessment and found it inadequate. Then why do you use it at all now?

Not really my choice. Also I'm a big fan of using libraries / HALs. Especially parameter checking can save hours of debugging. But to me it seems like the HAL was specified and then implemented by hiring some code monkeys, who were not allowed to revise the spec in case they noted that there might be a better way to do it.

Just found out, that the SPI-HAL might busy wait for up to 100ms in an interrupt .

Sounds scary, but isn't that a last-resort measure, for the the 'this never happens unless serious bug in user code' leg?

No, one must not wait in an interrupt - unless one really knows what he/she is doing. If this is done in a library, there must be a big, big warning in the documentation. In the case I'm looking at the DMA-Interrupt callback waits for the SPI to finish up. So yes, it's unlikely that the SPI will take more than a ms. But the fact that there is a loop gives me a very bad feeling. I've seen quite a few projects that caused random problems because of someone looping in an interrupt. And as there is no comment in the code 'Looping is no problem, because this can not take more than n cycles', I assume that no one really put though into this.

AvaTar · ‎2017-10-12

Posted on October 12, 2017 at 11:29

But the fact that there is a loop gives me a very bad feeling.

It should.

And it should give you an idea of the level of understanding on the side of the developer, code reviewer and tester at ST.

Perhaps you reconsider your decision.

Churing out some working prototype doesn't mean production-ready.

And, AFAIK, ST never really promises that, apart from maketing blather.

Tesla DeLorean · ‎2017-10-12

Posted on October 12, 2017 at 16:40

I've been pointing these kind of blocking situations for a long while, so long I get tired of it, and the abuse from some who just think I'm some old curmudgeon. What I have is experience, and a knack for seeing bad and broken code when I look at it. I was good at these things as a teenager, I'm now old enough to speak to stupidity when I see it, because it saves a lot of time and nonsense.

My observation is the code was not written by people with experience in multi-threaded and asynchronous driver stacks, but rather some rather green ones who only have linear/sequential mindset. Spinning in loops for millions of cycles is never productive. This is compounded by poor testing and lack of real world application.

My recommendation is to code your own routines where timing/behaviour is critical, the linker/compiler should discard the parts of the HAL you don't utilize.

Tips, Buy me a coffee, or three.. PayPal Venmo
Up vote any posts that you find helpful, it shows what's working..

Nesrine M_O · ‎2017-10-13

Posted on October 13, 2017 at 12:53

Hi

,

inHAL_DMA_PollForTransfer function the Type of CompleteLevel should be HAL_DMA_LevelCompleteTypeDef, this issue is reported internally to our team for check.

[1] Just found out, that the SPI-HAL might busy wait for up to 100msin an interrupt. Seriously, ST?

Could you please add further details about this issue,So, we can verify it.

You are talking about the HAL_SPI_Abort_IT function ,right?

HAL_StatusTypeDef HAL_SPI_Abort_IT(SPI_HandleTypeDef *hspi){ HAL_StatusTypeDef errorcode; uint32_t abortcplt ; __IO uint32_t count, resetcount; /* Initialized local variable */ errorcode = HAL_OK; abortcplt = 1U; resetcount = SPI_DEFAULT_TIMEOUT * (SystemCoreClock / 24U / 1000U); count = resetcount; /* Change Rx and Tx Irq Handler to Disable TXEIE, RXNEIE and ERRIE interrupts */ if (HAL_IS_BIT_SET(hspi->Instance->CR2, SPI_CR2_TXEIE)) { hspi->TxISR = SPI_AbortTx_ISR; /* Wait HAL_SPI_STATE_ABORT state */ do { if (count-- == 0U) { SET_BIT(hspi->ErrorCode, HAL_SPI_ERROR_ABORT); break; } } while (hspi->State != HAL_SPI_STATE_ABORT); /* Reset Timeout Counter */ count = resetcount; }�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

-Nesrine-

Morty Morty · ‎2017-10-13

Posted on October 13, 2017 at 15:51

ELMHIRI.Syrine

‌: That function is a good example for bad code (resetcount is derived from SystemCoreClock, but the loop is not related to the clock at all and there is a good chance that the code hangs in the while loop forever if something is wrong with interrupts), but it is normally not called from an interrupt context.

The code path I'm referring to is:

(Interrupt context)->SPI_DMATransmitReceiveCplt()->SPI_EndRxTxTransaction() with three chances of waiting up to 100ms.

P.s: Wanna pay me for a code review? It seems like any code I'm looking at contains some issues in one way or the other.