RTC.setAlarmCallback does not allows matching on more than one field

[dquadros]

The setAlarmCallback method implementation uses "if / else if" so only the first field is used:

ArduinoCore-renesas/libraries/RTC/src/RTC.cpp

Lines 658 to 659 in b82e642

	at.min_match = false;
	at.sec_match = false;

...

ArduinoCore-renesas/libraries/RTC/src/RTC.cpp

Lines 671 to 676 in b82e642

	if(m.isMatchingSecond()) {
	at.sec_match = true;
	}
	else if(m.isMatchingMinute()) {
	at.min_match = true;
	}

---

If you try, for example, to set an alarm for 09:10:04 it will fire every time the second is 4:

#include "RTC.h"
void setup() {
  Serial.begin(115200);
  RTC.begin();
  RTCTime starttime(29, Month::SEPTEMBER, 2023, 14, 00, 00, 
      DayOfWeek::FRIDAY, SaveLight::SAVING_TIME_INACTIVE);
  RTC.setTime(starttime);  
  RTCTime alarme;
  alarme.setHour(9);
  alarme.setMinute(10);
  alarme.setSecond(4);
  AlarmMatch am;
  am.addMatchHour();
  am.addMatchMinute();
  am.addMatchSecond();
  if (!RTC.setAlarmCallback(alarm_cbk, alarme, am)) {
    Serial.println("ERROR: alarm callback not set");
  }
}

bool beep = false;

void alarm_cbk() {
  beep = true;
}

void loop() {
  if (beep) {
    Serial.println("BEEP");
    beep = false;
  }
  delay (100);
}

The setAlarmCallback method could be implemented as follows:

at.min_match = m.isMatchingSecond();               
at.sec_match = m.isMatchingMinute(); 
...

[Fashion-Corp]

Well, That depends on what was the original intent with it! What was the original invariance for the entire set of those functions? To the best of my knowledge it can be implemented both ways and even in further ways. If I get you right you want an array of parallel trigging while the current implementering excludes trigging of the minute if the seconds have been trigged? The key is therefore not to implement it either way or the other which can be tempting. The key is to find out what was the original intent with these methods and then evaluate: 1) Is this the intended functionality as per the originally intended method, the best form of functional feedback? (So: describe the invariance intended) 2) If so, does the current implementation successfully and accurately work as to that end? If not then agree to change it. 3) If your message is then pointing out errors in that implementation then surely it should be corrected. If your message is a comment to that the original intended invariance is not smart then surely the starting point BEFORE changing implementation would prudently be point 1 above, to establish another intended function. 4) Whether the implementation now changes or remains the same, documentation of the usual preconditions, invariances and postconditions would be prudent. Which one is it? What is the right process? Just changing it would be destabilising as implementation is often a matter of taste. Another approach could be to create an early callback, leaving the entire interpretation to the user. If established well, the initialisation of the callback can be given a parameter which will switch other callback off. That way the users like yourself and the original designer can co-inplement each your different designs of how the response should be, and the function would in fact become totally generic and thereby moldable to the application programmer. I shall happily participate in implementation if someone can guide me to how your rules are for: 1. Agreeing to making changes before doing pull requests? Who is in charge of the library coherence? 2. How do YOU here do pull requests? I haven't seen branches here. 3. If you are organising yourselves here in this repository with a configuration manager, like some of us do - who is it? 4. Which rules/practices do you follow in regards to coding and documentation of the code 5. Do you have an approval committee? 6. Do you have an architecture expressed somewhere for your entire system? Maybe an overall roadmap to which we shall adhere? 7. Do you have a place related to the repository where there things are discussed? Sincerely David Svarrer

…

On Fri, 29 Sep 2023, 20:40 Daniel Quadros, ***@***.***> wrote: The setAlarmCallback method implementation uses "if / else if" so only the first field is used: at.min_match = false; at.sec_match = false; ... if(m.isMatchingSecond()) { at.sec_match = true; } else if(m.isMatchingMinute()) { at.min_match = true; } ... If you try, for example, to set an alarm for 09:10:04 it will fire every time the second is 4: #include "RTC.h" void setup() { Serial.begin(115200); RTC.begin(); RTCTime starttime(29, Month::SEPTEMBER, 2023, 14, 00, 00, DayOfWeek::FRIDAY, SaveLight::SAVING_TIME_INACTIVE); RTC.setTime(starttime); RTCTime alarme; alarme.setHour(9); alarme.setMinute(10); alarme.setSecond(4); AlarmMatch am; am.addMatchHour(); am.addMatchMinute(); am.addMatchSecond(); if (!RTC.setAlarmCallback(alarm_cbk, alarme, am)) { Serial.println("ERROR: alarm callback not set"); } } bool beep = false; void alarm_cbk() { beep = true; } void loop() { if (beep) { Serial.println("BEEP"); beep = false; } delay (100); } The setAlarmCallback method could be implemented as follows: at.min_match = m.isMatchingSecond(); at.sec_match = m.isMatchingMinute(); ... — Reply to this email directly, view it on GitHub <#154>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUI6JVRHHOHKHPS2FIDH73DX44BZTANCNFSM6AAAAAA5MZHFRQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[facchinm]

@maidnl can you take a look?