Bug Report: STM32L4 HAL RTC driver

[Ricardo Hassan]

Posted on June 07, 2018 at 20:20

Greetings,

    I believe I have found a bug in the HAL RTC driver.  I am using an STM32L4, and I'm using STM32CubeMX v4.26.0 to generate my project, with firmware package v1.12.0 for STM32L4 series . I observed that using the Set/GetTime API was returning the wrong results - for example, I would see the seconds field go above 60, eventually rolling over at some time later.  The issue is that the RTC registers handle time in BCD format, but the driver treats the number as purely hexadecimal.  In other words, it's treating the 10's digit as base 16 instead of base 10.  I made a fix for my local copy that seems to work.  I will repeat it here, feel free to use part or all of it.  This is in stm32l4xx_hal_rtc.c:

In HAL_RTC_SetTime(), line 795 I replaced this:

tmpreg = (((uint32_t)(sTime->Hours) << 16) | \

((uint32_t)(sTime->Minutes) << 8) | \ ((uint32_t)sTime->Seconds) | \ ((uint32_t)(sTime->TimeFormat) << 16));

with this:

uint8_t ht = sTime->Hours / 10;

uint8_t hu = sTime->Hours - 10 * ht; uint8_t mnt = sTime->Minutes / 10; uint8_t mnu = sTime->Minutes - 10 * mnt; uint8_t st = sTime->Seconds / 10; uint8_t su = sTime->Seconds - 10 * st; tmpreg |= (ht << RTC_TR_HT_Pos) + (hu << RTC_TR_HU_Pos); tmpreg |= (mnt << RTC_TR_MNT_Pos) + (mnu << RTC_TR_MNU_Pos); tmpreg |= (st << RTC_TR_ST_Pos) + (su << RTC_TR_SU_Pos); tmpreg |= sTime->TimeFormat << RTC_TR_PM_Pos;

and in 

HAL_RTC_GetTime(), line 900 I replaced this:

sTime->Hours = (uint8_t)((tmpreg & (RTC_TR_HT | RTC_TR_HU)) >> 16);

sTime->Minutes = (uint8_t)((tmpreg & (RTC_TR_MNT | RTC_TR_MNU)) >>8); sTime->Seconds = (uint8_t)(tmpreg & (RTC_TR_ST | RTC_TR_SU));

with this:

sTime->Hours = (uint8_t)(10 * ((tmpreg & RTC_TR_HT) >> RTC_TR_HT_Pos ) + ((tmpreg & RTC_TR_HU) >> RTC_TR_HU_Pos ));

sTime->Minutes = (uint8_t)(10 * ((tmpreg & RTC_TR_MNT) >> RTC_TR_MNT_Pos ) + ((tmpreg & RTC_TR_MNU) >> RTC_TR_MNU_Pos )); sTime->Seconds = (uint8_t)(10 * ((tmpreg & RTC_TR_ST) >> RTC_TR_ST_Pos ) + ((tmpreg & RTC_TR_SU) >> RTC_TR_SU_Pos ));

My results now look correct.  I hope that ST can confirm or deny my conclusion.  I have attached my modified source file.

Regards,

Ricardo

[Tilen MAJERLE]

Posted on June 08, 2018 at 09:18

Hello Ricardo,

can you please attach your application file where you call SetTime and GetTime functions aswell? Please include entire structure with data you pass as parameter.

Thank you

Best regards,

Tilen

[Ricardo Hassan]

Posted on June 08, 2018 at 18:23

This is basically it.

Ricardo

________________ Attachments : date_time.cpp.zip : https://st--c.eu10.content.force.com/sfc/dist/version/download/?oid=00Db0000000YtG6&ids=0680X000006HxX4&d=%2Fa%2F0X0000000azR%2F5k5f12m2hS1Yw4aD.jNbWj_MMlBa1lSv.C0bO8LSC4c&asPdf=false

[Ricardo Hassan]

Upon further review, I think that this is operating as intended, though I don't care for the intention. I expected to be able to set the hours, minutes, and seconds fields as binary numbers, though in BCD you have to specify the numbers as hex so that the first nibble is the 10s digit and the second nibble is the ones digit. I find this to be extremely counter intuitive, though I may be in the minority. I'd much rather the input struct allow you to specify the 10s and 1s digits separately for BCD. I find it awkward to look at a uint8_t and have to recall that it's not really a uint8_t.