Add STM32RTC library source files #1

ghost · 2017-12-13T10:14:49Z

Source files of the RTC library.

This library is based on the RTCZero library.
We tried to keep the compatibility as far as possible.

Tested on NUCLEO-L476RG and NUCLEO-L053R8.
Compilation test OK on all variant boards.

Requires the validation of this PR.

LMESTM · 2017-12-13T10:18:46Z

@fprwi6labs
thanks for the PR.
Please edit and update the Pull Request title to something more precise than "Add source files"

LMESTM · 2017-12-13T13:00:11Z

@fpistm just told me there is a dependency to stm32duino/Arduino_Core_STM32#177
Would be good to mention it in the PR description here, I was looking for services like RTC_Init and was not finding them ...

LMESTM

few questions and comments

LMESTM · 2017-12-13T10:20:43Z

src/STM32RTC.cpp

@@ -0,0 +1,814 @@
+/**
+  ******************************************************************************


Please edit the commit title to something specific => Like STM32 RTC library

LMESTM · 2017-12-13T10:37:09Z

src/STM32RTC.cpp

+  ******************************************************************************
+  */
+
+#include <time.h>


what is being used from time.h ?

Epoch conversion needs time functions.

LMESTM · 2017-12-13T10:38:00Z

src/STM32RTC.cpp

+#define EPOCH_TIME_YEAR_OFF 100        // years since 1900
+
+// Initialize static variable
+bool STM32RTC::_configured = false;


couldn't this be initialized in the STM32RTC::STM32RTC(void) constructor?

No, it couldn't because we use a static variable to avoid the overwrite of a previous RTC configuration in case the user create is own object and a library call another. It is the case with the future low power library.

LMESTM · 2017-12-13T10:41:13Z

src/STM32RTC.cpp

+  * @retval None
+  */
+void STM32RTC::begin(hourFormat_t format)
+{


hourFormat_t is a type of the core - it would be easier to read to have a type defined in the library that will map to the one of the core.

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

src/STM32RTC.h

+#endif
+
+typedef void(*voidFuncPtr)(void);
+


as proposed in the RTC PR of the core, could make sense to add a (void* data) parameter to the callback.

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

src/STM32RTC.cpp

+        RTC_StopAlarm();
+        break;
+      case MATCH_YYMMDDHHMMSS://kept for compatibility
+      case MATCH_MMDDHHMMSS:  //kept for compatibility


Where is the limitation about year / months described ?

Inside the header file.

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

src/STM32RTC.cpp

+// Kept for compatibility.
+void STM32RTC::setAlarmMonth(uint8_t month)
+{
+  UNUSED(month);


please explain and document the implication of not using / supporting the month

LMESTM · 2017-12-13T13:27:56Z

src/STM32RTC.cpp

+// Kept for compatibility.
+void STM32RTC::setAlarmYear(uint8_t year)
+{
+  UNUSED(year);


please explain and document the implication of not using / supporting the year

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

src/STM32RTC.cpp

+
+// Kept for compatibility.
+void STM32RTC::setAlarmDate(uint8_t day, uint8_t month, uint8_t year)
+{


why not setting the day ?

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

src/STM32RTC.h

+#define HOUR_24 HOUR_FORMAT_24
+
+//Hour AM/PM definition
+#define HOUR_AM AM    //AM or 24 hours


do you mean that hourAM_PM_t will always be HOUR_AM when hourFormat_t is HOUR_24 ?

Exactly. It is documented in board datasheet:

Bit 22 PM: AM/PM notation 0: AM or 24-hour format 1: PM

At least I would like to have all information in this file so if you can add better comments of each type definition in this header file that would help users. Otherwise they don't know which define corresponds to which type.

LMESTM · 2017-12-14T15:49:32Z

README.md

+The following functions have been added to support specific STM32 RTC features:
+
+_RTC hours mode (12 or 24)_
+* **`void begin(hourFormat_t format)`**


should it be RTCHourFormats_t ?

Good catch!

LMESTM · 2017-12-14T15:50:19Z

README.md

+
+_Hour format (AM or PM)_
+* **`uint8_t getHours(hourAM_PM_t *format)`**
+* **`void setHours(uint8_t hours, hourAM_PM_t format)`**


should it be Hour12_AM_PM_t now ?
same in below functions ?

LMESTM

@fprwi6labs Looks all good to me now - thanks for all the quick changes ! Approved from my side.

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

src/STM32RTC.cpp

+  * @param format: hour format: HOUR_12 or HOUR_24(default)
+  * @retval None
+  */
+void STM32RTC::begin(RTCHourFormats_t format)


Should provide a way to select RTC clock source (HSE, LSI, LSE)
Even if all LP is not reachable depending of the clock source.
Not All Nucleo board provide LSE by default.

Need to handle RTC clock config. Currently fixed config.

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

ghost · 2018-01-24T13:13:25Z

Library updated.
The RTC clock source can be selected.
By default, LSI is used as RTC clock source.

fpistm

It should be fine to add one example with clock source selection.

fpistm · 2018-01-30T10:06:29Z

examples/SimpleRTC/SimpleRTC.ino

@@ -0,0 +1,107 @@
+/**
+  ******************************************************************************
+  * @file    simpleRTC.ino


Typo: sSimpleRTC.ino

fpistm · 2018-01-30T10:06:58Z

examples/simpleRTCAlarm/simpleRTCAlarm.ino

@@ -0,0 +1,77 @@
+/**
+  ******************************************************************************
+  * @file    Epoch.ino


simpleRTCAlarm.ino

fpistm · 2018-01-30T10:10:47Z

README.md

+This library is based on the Arduino RTCZero library.
+You can take control of the internal RTC of the STM32 boards.
+
+The following functions are not supported:


Please add the reason why not supported

fpistm · 2018-01-30T10:11:36Z

README.md

+# API
+
+This library is based on the Arduino RTCZero library.
+You can take control of the internal RTC of the STM32 boards.


Please, use passive form to avoid use of 'you'

fpistm · 2018-01-30T10:12:19Z

README.md

+* **`void setWeekDay(uint8_t weekDay)`**
+* **`void setDate(uint8_t weekDay, uint8_t day, uint8_t month, uint8_t year)`**
+
+Refer you to the Arduino RTC documentation for the other functions  


remove 'you'

fpistm · 2018-01-30T10:12:53Z

README.md

+
+## Source
+
+You can find the source files at  


passive form:

Source file available at:

fpistm · 2018-01-30T10:41:44Z

examples/Epoch/Epoch.ino

+
+#include <STM32RTC.h>
+
+/* Create an rtc object */


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

ghost · 2018-03-15T08:56:34Z

@fpistm
Fix done.
Example with clock selection added.

fpistm · 2018-04-24T12:36:29Z

This PR is corrected and extended by #2

Add library properties

7d38f81

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

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

ghost mentioned this pull request Dec 13, 2017

Add RTC driver stm32duino/Arduino_Core_STM32#177

Closed

ghost changed the title ~~Add source files~~ Add STM32RTC library source files Dec 13, 2017

LMESTM reviewed Dec 13, 2017

View reviewed changes

LMESTM reviewed Dec 14, 2017

View reviewed changes

LMESTM previously approved these changes Dec 14, 2017

View reviewed changes

fpistm requested changes Jan 23, 2018

View reviewed changes

fpr added 2 commits January 24, 2018 14:08

Add syntax coloring map

0da46b5

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

Add source files

4223643

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

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

fpistm requested changes Jan 30, 2018

View reviewed changes

fpistm reviewed Jan 30, 2018

View reviewed changes

examples/Epoch/Epoch.ino Outdated

#include <STM32RTC.h>

/* Create an rtc object */

Copy link

Member

fpistm Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a rtc

fpr added 2 commits March 15, 2018 09:53

Add examples

78990ed

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

Update readme

d28089d

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

ghost requested review from LMESTM and fpistm March 15, 2018 08:56

fpistm mentioned this pull request Apr 24, 2018

PR1 review: Add STM32RTC library sources files #2

Merged

fpistm approved these changes Apr 24, 2018

View reviewed changes

fpistm assigned ghost Apr 24, 2018

fpistm merged commit 6cb058c into stm32duino:master Apr 25, 2018

		@@ -0,0 +1,814 @@
		/**
		******************************************************************************

		#endif

		typedef void(*voidFuncPtr)(void);

Add STM32RTC library source files #1

Add STM32RTC library source files #1

Conversation

ghost commented Dec 13, 2017 • edited by ghost Loading

LMESTM commented Dec 13, 2017

LMESTM 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

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

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

LMESTM left a 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

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 Mar 15, 2018

fpistm commented Apr 24, 2018 • edited Loading

ghost commented Dec 13, 2017 •

edited by ghost

Loading

fpistm commented Apr 24, 2018 •

edited

Loading