Add RTC driver #177

ghost · 2017-12-13T10:21:29Z

Driver to use the internal RTC:

Enable/disable RTC
Set/Get time & calendar
Enable/disable alarm
Attach/detach alarm callback

This driver is linked with the STM32RTC library.

LMESTM

One comment / proposal about the callback type but overall looks good to me

LMESTM · 2017-12-13T13:02:50Z

variants/NUCLEO_F207ZG/stm32f2xx_hal_conf.h

@@ -175,7 +175,7 @@

 /* Section 2: PHY configuration section */

-/* DP83848 PHY Address*/ 
+/* DP83848 PHY Address*/


You're mixing up 2 things in the commit - clean-up of various white spaces with the actual changes for RTC.
In such cases, please try to make 2 separate commits instead ...

LMESTM · 2017-12-13T13:13:26Z

cores/arduino/stm32/rtc.c

+  * @param func: pointer to the callback
+  * @retval None
+  */
+void attachAlarmCallback(void (*func)(void))


rather than
void (*func)(void)
it could be useful from application to offer a callback of type
void (func)(void data)

you would mean: void (*func)(void*)?

What information would you like to pass in parameter? rtc handler? other?

This is up to application . It could the time when alarm was set, or a pointer to the led to be turned on. The application provides a pointer and you simply pass it back to application. This is useful for callback mecanisms. It would be particularly useful if setting several alarms.

It is not the application that choice this parameter. It is the driver which can return a parameter.
If you don't say me the parameter to return, I think return a void* is useless.
Or I must change the prototype of attachAlarmCallback to take account another parameter with the data pointer (also useless).

sorry if unclear, the user data is passed from application to driver, stored in driver and then passed back from driver to application. The driver doesn't know which informaition it contains.

This can be useful for coding generic callback where you don't need to store application data as globals or in context.
https://arduino.stackexchange.com/questions/23063/user-callback-functions
https://stackoverflow.com/questions/14270846/passing-a-parameter-in-a-callback-in-c

In the case of RTC alarm, this can be useful if application can set 2 different alarms in future, then application could pass the programmed time, or a different LED or whatever it needs

It is clearer thank you. And done.

fpistm

This driver must handle all possible RTC clock config.

fpistm · 2018-01-23T09:01:27Z

cores/arduino/stm32/rtc.c

+/* RTC prediv are set to the default values to match with the LSE clock source.
+  If the clock source is modified, those prescalers must be changed */
+/* Synchonuous prediv  */
+#define PREDIV_S                  0x00FF


Main restriction of the driver is the RTC clock configuration.
The driver have to be able to dela with all possible source (HSE, LSE,LSI).
Refer to AN4759 Overview of the STM32 MCU advanced RTC
§ 1.1.4 RTC clock configuration

fpistm · 2018-01-23T09:01:42Z

cores/arduino/stm32/rtc.c

+  *        the backup registers) and RCC_CSR register are set to their reset values.
+  * @retval None
+  */
+void HAL_RTC_MspInit(RTC_HandleTypeDef *hrtc)


fpistm · 2018-01-23T09:02:16Z

cores/arduino/stm32/rtc.c

+  * @param format: enable the RTC in 12 or 24 hours mode
+  * @retval None
+  */
+void RTC_init(hourFormat_t format)


Have to mange RTC clock config

ghost · 2018-01-24T13:18:31Z

RTC driver updated with clock source selection.
The RTC clock source can be set: LSE, LSI or HSE.
User can set the prescaler values for his own configuration. The user clock configuration is not overwritten.
Default prescaler values are provided for the most common hardware clock configuration.

Compilation OK.
Tests OK for LSE and LSI. HSE not tested.
Tested only on boards NUCLEO L0, L4, F0, F1, F3, F4.
L1, F2 & F7 missing.

fpistm

It should be possible to define [HSE|LSE|LSI]_A?SYNC_PREDIVas for each variant clock values are defined.
Ex:

Arduino_Core_STM32/variants/NUCLEO_L476RG/stm32l4xx_hal_conf.h

Line 159 in 2415879

    
           #define LSE_VALUE    ((uint32_t)32768U) /*!< Value of the External oscillator in Hz*/

fpistm · 2018-01-29T15:55:00Z

cores/arduino/stm32/rtc.h

+                                 ((WEEKDAY) == RTC_WEEKDAY_SATURDAY)  || \
+                                 ((WEEKDAY) == RTC_WEEKDAY_SUNDAY))
+
+#define IS_RTC_HOUR12(HOUR)      IS_RTC_HOUR24(HOUR)


Seems F1 do not define it. Probably because it do not handle 12h format.
A comment could be added to give the reason.

fpistm · 2018-02-07T06:54:23Z

cores/arduino/stm32/rtc.c

+  }
+  RtcHandle.Init.OutPut = RTC_OUTPUT_DISABLE;
+  if(source == LSE_CLOCK) {
+    RtcHandle.Init.AsynchPrediv = LSE_ASYNCH_PREDIV;


[HSE|LSE|LSI]_A?SYNC_PREDIV should not be hard-coded. User should have a way to dynamically change them without add a dedicated define. At least an API to change the default value.
Anyway, as mentioned in rtc.h, it should be possible to define the best PREDIV

fpistm · 2018-02-07T06:56:51Z

cores/arduino/stm32/rtc.h

+  - LSE = 32.768 kHz
+  - HSE = 8 MHz
+
+ Prescalers should be set by user in case the clock configuration is different.


It should be possible to define [HSE|LSE|LSI]_A?SYNC_PREDIV as for each variant clock values are defined.
Ex:

Arduino_Core_STM32/variants/NUCLEO_L476RG/stm32l4xx_hal_conf.h

Line 159 in 2415879

#define LSE_VALUE ((uint32_t)32768U) /*!< Value of the External oscillator in Hz*/

lacklustrlabs · 2018-02-08T09:54:21Z

Have you considered adding an attachAlarmCallback(std::function) function?
I guess that will introduce an additional, potentially costly, C/C++ interfacing layer.
What is your opinion?

fpistm · 2018-02-09T07:49:57Z

@lacklustrlabs
I do not considered this but it could. This has been done for the interrupt.
I'm not very aware about that and it's use/utility but why not.

lacklustrlabs · 2018-02-12T09:34:55Z

@fpistm
As you know std::function is a generic way of handling callable objects in C++ (functions, lambda expressions, bind expressions).

I guess std::function callbacks can be added at any time later on, the only thing that should be considered now is if std::function callbacks are good (fast and memory efficient) enough to replace C-style callbacks. If so, the switch-over should be made now, before the API is released.

If you want plain C-style callbacks in the API, the std::function callbacks can be added at any time later. The downside of this option is a, possible, duplicated callback handling.

… quakesense branch - Add RTC driver for STM32 boards - Fix some conflicts in cores/arduino/board.h

fpistm · 2018-03-06T09:52:04Z

Hi @fprwi6labs
any news on this PR?

fpistm · 2018-03-06T09:52:42Z

@lacklustrlabs
It could be considered.
Maybe @fprwi6labs have an opinion on this

Signed-off-by: fpr <[email protected]>

ghost · 2018-03-15T14:37:30Z

Hi @fpistm
Prescalers are now dynamically calculated.
Compilation OK for all available boards but not tested with HSE clock source.

@lacklustrlabs
Is it possible to use std::function<void(void *)>? I tried but without success and I didn't find example.
So I will not implement this feature for now.

fpistm

Some issues and restrictions found during the review.
Those have been fixed in #235

fpistm · 2018-04-24T12:20:17Z

#235 include this PR rebased on top of the master.
This on is abandoned.

ghost requested review from LMESTM, fpistm and VVESTM December 13, 2017 10:21

LMESTM mentioned this pull request Dec 13, 2017

Add STM32RTC library source files stm32duino/STM32RTC#1

Merged

LMESTM reviewed Dec 13, 2017

View reviewed changes

fpistm added the new feature label Jan 11, 2018

fpistm requested changes Jan 23, 2018

View reviewed changes

ghost requested a review from fpistm January 24, 2018 13:19

fpistm self-assigned this Jan 29, 2018

fpistm added the review on going label Jan 29, 2018

fpistm added this to the Next release milestone Feb 1, 2018

fpistm reviewed Feb 7, 2018

View reviewed changes

biagiom added a commit to biagiom/Arduino_Core_STM32 that referenced this pull request Feb 12, 2018

Merge pull request stm32duino#177 from fprwi6labs/add_rtc_driver into…

d273fe4

… quakesense branch - Add RTC driver for STM32 boards - Fix some conflicts in cores/arduino/board.h

fpistm added the waiting feedback Further information is required label Mar 7, 2018

fpr added 2 commits March 15, 2018 14:16

Add RTC driver

cc1e10e

Signed-off-by: fpr <[email protected]>

Enable HAL RTC for all variants

3ff74c6

Signed-off-by: fpr <[email protected]>

ghost requested review from LMESTM and fpistm March 15, 2018 14:37

fpistm modified the milestones: Next release, 1.2.1 Mar 23, 2018

biagiom mentioned this pull request Apr 3, 2018

Add Low Power driver #183

Closed

fpistm removed the waiting feedback Further information is required label Apr 5, 2018

fpistm mentioned this pull request Apr 24, 2018

PR 177 review: Add RTC driver #235

Merged

fpistm reviewed Apr 24, 2018

View reviewed changes

fpistm approved these changes Apr 24, 2018

View reviewed changes

fpistm added abandoned No more work on this and removed review on going labels Apr 24, 2018

fpistm closed this Apr 24, 2018

fpistm removed this from the 1.2.1/1.3.0 milestone May 28, 2018

fpistm mentioned this pull request Jun 6, 2023

gh pr list do not show some PR cli/cli#7538

Closed

fpistm added enhancement New feature or request and removed New feature labels Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RTC driver #177

Add RTC driver #177

ghost commented Dec 13, 2017

LMESTM left a comment

LMESTM Dec 13, 2017

ghost Dec 14, 2017

LMESTM Dec 13, 2017

fpistm Dec 13, 2017

ghost Dec 14, 2017

LMESTM Dec 14, 2017

ghost Dec 14, 2017

LMESTM Dec 14, 2017

ghost Dec 14, 2017

fpistm left a comment

fpistm Jan 23, 2018

fpistm Jan 23, 2018

fpistm Jan 23, 2018

ghost commented Jan 24, 2018

fpistm left a comment

fpistm Jan 29, 2018

fpistm Feb 7, 2018

fpistm Feb 7, 2018

lacklustrlabs commented Feb 8, 2018

fpistm commented Feb 9, 2018

lacklustrlabs commented Feb 12, 2018

fpistm commented Mar 6, 2018

fpistm commented Mar 6, 2018

ghost commented Mar 15, 2018 •

edited by ghost

Loading

fpistm left a comment

fpistm commented Apr 24, 2018

Add RTC driver #177

Add RTC driver #177

Conversation

ghost commented Dec 13, 2017

LMESTM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpistm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jan 24, 2018

fpistm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lacklustrlabs commented Feb 8, 2018

fpistm commented Feb 9, 2018

lacklustrlabs commented Feb 12, 2018

fpistm commented Mar 6, 2018

fpistm commented Mar 6, 2018

ghost commented Mar 15, 2018 • edited by ghost Loading

fpistm left a comment

Choose a reason for hiding this comment

fpistm commented Apr 24, 2018

ghost commented Mar 15, 2018 •

edited by ghost

Loading