Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DS3232RTC::alarmInterrupt issue #102

Open
amazed-2022 opened this issue Mar 19, 2023 · 4 comments
Open

DS3232RTC::alarmInterrupt issue #102

amazed-2022 opened this issue Mar 19, 2023 · 4 comments

Comments

@amazed-2022
Copy link

alarmInterrupt(ALARM_NBR_t alarmNumber, bool alarmEnabled)
Description

Enable or disable an alarm "interrupt". Note that this "interrupt" just causes the RTC's INT pin to be asserted. To use this signal as an actual interrupt to (for example) a microcontroller, the RTC "interrupt" pin needs to be connected to the microcontroller and the microcontroller application firmware must properly configure the interrupt and also provide for handling it.

In case of alarmEnabled = true is requested for any of the alarms, the INTCN bit in the control register could be also set, to make sure that the INT pin is really asserted. (calling this method will not result the described behavior, in case of INTCN was set to 0 previously).

@amazed-2022 amazed-2022 changed the title Using DS3232RTC::alarmInterrupt will not assert the INT pin in case of INTCN was previously set to 0 DS3232RTC::alarmInterrupt issue Mar 19, 2023
@JChristensen
Copy link
Owner

JChristensen commented Mar 19, 2023

Is there no scenario where I want to set A2IE and/or A1IE and not set INTCN, or perhaps not set it simultaneously?

It may be better to control INTCN independently of A1IE and A2IE. Calling squareWave(DS3232RTC::SQWAVE_NONE) will set INTCN. Calling it with any other valid argument will clear INTCN.

I can see where the documentation for alarmInterrupt() and also for squareWave() could be improved.

@amazed-2022
Copy link
Author

Is there no scenario where I want to set A2IE and/or A1IE and not set INTCN, or perhaps not set it simultaneously?

I can't think of such scenario. Setting A1IE and/or A2IE without INTCN will do nothing.

@JChristensen
Copy link
Owner

I can't think of such scenario.

Off the top, I am not sure that I can either, but it could be coded that way. There may be a small risk that if alarmInterrupt() is changed to always set INTCN, it could break something for someone.

The suggestion here is a convenience feature; it would avoid a call to squareWave(). Perhaps the best approach would be to implement a new method that is like alarmInterrupt() but that sets INTCN as well as AxIE. That way, backwards compatibility is ensured.

@amazed-2022
Copy link
Author

There may be a small risk that if alarmInterrupt() is changed to always set INTCN, it could break something for someone.

I understand your concern. However if anyone used alarmInterrupt() for arm an interrupt before, he/she already had INTCN bit set to 1 (the default value when power is first applied), otherwise their code wouldn't work, at least not as they intended.

The suggestion here is a convenience feature; it would avoid a call to squareWave().

I don't think it's convenience, it would just make more sense that alarmInterrupt() can really arm the interrupt. Currently, viewing from application level, it's like squareWave(SQWAVE_NONE) has a hidden feature, enabling/preparing INT pin toggling.

Anyway, I thought this minor code/documentation (as you mentioned earlier) issue is worth mentioning. To anyone who is familiar with the device's documentation and knows the how to set/reset register bits, this wouldn't be a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants