search

nimaltd / gsm_v5

gsm module library for STM32 LL
GNU General Public License v3.0
231 stars 85 forks source link

Inappropriate check to process buffer leads to crash when HAL_GetTick() overflows after Approximatly 49 days #1

Closed danyhm closed 6 years ago

danyhm commented 6 years ago

HAL_GetTick()-Sim80x.UsartRxLastTime check will fail after 49 days of MCU startup.

https://github.com/nimaltd/Sim800_V2/blob/5c76a909f7f69a1f3bea00b078e56c0fb0795abc/Sim80x.c#L1258-L1263

Solution is to change the check overall. first of all 50ms is a constant which is not calculated or reference for a specific UART Baudrate. STM and HAL provide a macro __HAL_UART_GET_FLAG(uarthandle,FLAG). by checking UART_FLAG_IDLE: Idle Line detection flag it is possible to check whether a transmission is undergoing or not. how ever the downside of using this macro is that it's not portable to other MCUs.

danyhm commented 6 years ago

Hello, iIve been following your codes , this library is good and i hope we can improve it, please use comments inside your functions so developers don't have to spend time figuring out the work of a specific code for debugging purposes.

danyhm commented 6 years ago

another method which is near perfect for STM is to use USART DMA circular (or normal?) buffer to minimize system interrupts due to UART. but still not portable to other MCUs.

danyhm commented 6 years ago

HAL_Delay() had the same problem. what if we cast the calculation to uint32 ?

nimaltd commented 6 years ago

Hello and thank you for feedback. (HAL_GetTick()-Sim80x.UsartRxLastTime > 50) and Hal_delay it is safe. if you don't believe it , test it :)
this is uint32_t and always return a positive value.

nimaltd commented 6 years ago

about Usart_DMA: I need last time of receive byte from usart. With DMA i cant Get That.

danyhm commented 6 years ago

@nimaltd well obviously i can't test a board for 49 days, i think it's better to cast the whole calculation to uint just to be sure

nimaltd commented 6 years ago

But you can create another loop an see on debug. Its work fine.

danyhm commented 6 years ago

OK then if you are sure it's that safe , you can close the issue