stm32duino / STM32RTC

Arduino RTC library for STM32.
125 stars 48 forks source link

Add simultaneous support for RTC_ALARM_A and RTC_ALARM_B #74

Closed batkov closed 1 year ago

batkov commented 1 year ago

Hi,

I'm trying to run few RTC Alarms on one chip simultaneously. I've found that RTC_ALARM_A (rtc.c) is used by default and I see no way to run it along with RTC_ALARM_B on chip supporting such a feature.

It seems like RTC_HandleTypeDef RtcHandle initialized once and would be able to run only one type of alarm at one point of time.

Can you add support for RTC_ALARM_B? Can you suggest how to add such a support?

fpistm commented 1 year ago

Hi @batkov sorry for the delay (was in vacations). You're right, RTC_ALARM_B is currently not supported as this library was designed originally to be compliant with Arduino RTCZero library which only support one alarm IIRW. Moreover not all STM32 series have a second alarm. Anyway, it should be fine to add it. Unfortunately I cannot give a date for this. Any help are welcome. Maybe a way to add it would be to check if we can extend the alarm API's to select the alarm with RTC_ALARM_A by default to keep backward compatibility and of course available only for STM32 series with RTC_ALARM_B support.

batkov commented 1 year ago

Yes, I already implemented something similar to what you've mentioned. Will fix backward compatibility issues and create PR soon. I have a question though. I've changed RTC methods to receive alarm type, but kept old ones for compatibility. I've added v2 prefixes for new methods. Probably, @fpistm, you'll have better idea how to name those methods.

fpistm commented 1 year ago

Well for example you can do something like that (not tested):

    uint32_t getAlarmSubSeconds(uint32_t alarm = RTC_ALARM_A);
    uint8_t getAlarmSeconds(uint32_t alarm = RTC_ALARM_A );
    uint8_t getAlarmMinutes(uint32_t alarm = RTC_ALARM_A );
    uint8_t getAlarmHours(AM_PM *period = nullptr, uint32_t alarm = RTC_ALARM_A);
    uint8_t getAlarmHours(uint32_t alarm = RTC_ALARM_A);
batkov commented 1 year ago

@fpistm please check https://github.com/stm32duino/STM32RTC/pull/75

fpistm commented 1 year ago

Hi @batkov I will review it soon. As a first feedback I would not use v2_ suffix.

batkov commented 1 year ago

Hi @fpistm , Yeah, but I did not come with anything better. Please propose something, and I'll update the PR.

JeroenIoT commented 1 year ago

Hi @batkov,

Would it be an idea to add 1 extra function to set the alarm which we are configuring, setting and getting (by default this is alarmA). This function would only be valid for number of processors capable of handling alarm A / B.

void RTC_StartAlarm(uint8_t day, uint8_t hours, uint8_t minutes, uint8_t seconds, uint32_t subSeconds, hourAM_PM_t period, uint8_t mask) {
   |
if (alarmtype == ALARM_B)
{   RTC_AlarmStructure.Alarm = RTC_ALARM_B;}
else
{   RTC_AlarmStructure.Alarm = RTC_ALARM_A; }
   |
}

At a first glance the following to be changed to handle both alarmA and alarmB (depending on alarmType set by used: