Skip to content

stm32l1/periph/rtc.c: fixed 24-h clock format#6173

Closed
Cr0s wants to merge 1 commit intoRIOT-OS:masterfrom
Cr0s:patch-1
Closed

stm32l1/periph/rtc.c: fixed 24-h clock format#6173
Cr0s wants to merge 1 commit intoRIOT-OS:masterfrom
Cr0s:patch-1

Conversation

@Cr0s
Copy link
Copy Markdown

@Cr0s Cr0s commented Dec 3, 2016

Hi.

Previously there was actually setting of 12-h clock format with comment "Enabling 24-h format". Possibly typo, fixed with according to ST's RTC AN3371: Table 2, row 6.

How to check that mistake is present:

  • Set time 23:59:55
  • Set alarm to next day's 0:0:0
  • Wait 5 seconds
  • Alarm doesn't occur

If you poll the clock while waiting, you can see transition from 23:59:59 to 24:0:0 without day changing. After applying this fix, the transition becomes from 23:59:59 to 0:0:0 of a next day and alarm will occur 5 seconds later.

Possibly, the same mistake present in STM32F0's RTC driver.

Best regards.

Previously there was actually setting of 12-h clock format with comment "Enabling 24-h format". Possibly typo, fixed with according to ST's RTC AN3371: Table 2, row 6.
@cgundogan cgundogan added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Dec 3, 2016
@OlegHahm OlegHahm added the Area: drivers Area: Device drivers label Dec 8, 2016
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jan 23, 2017

Hi @Cr0s,
I just had a look at your change but didn't try it yet. Have you seen this PR ? Your change is also applied there but goes further by unifying the RTC implementation for the STM32 family.

I propose to close this one in favor of #6448 if you don't mind.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jan 23, 2017

Tested your fix this afternoon : it indeed works but your branch is quite out-of-date comparing to the current master. Note that the actual master is not working at all : RIOT crashes on nucleo-l1 when running the tests/periph_rtc application.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jan 23, 2017

Another comment : the alarms is triggered 6 times instead of the 4 times mentioned by the test.

@Cr0s Cr0s closed this Jan 24, 2017
@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Jan 24, 2017

@Cr0s why?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jan 24, 2017

because of #6448 where the same fix is applied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants