Skip to content

Added alarm features for DS3231 #758

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

mfulz
Copy link

@mfulz mfulz commented May 11, 2025

I've implemented missing features for DS3231 RTC similar like RTClib

These can be used to set Alarm1 (alarm2 is missing), read alarm1, activate alarm1, use sqw for interrupts, etc.

@deadprogram deadprogram changed the base branch from release to dev May 20, 2025 13:33
@deadprogram deadprogram requested a review from conejoninja May 20, 2025 13:35
@deadprogram
Copy link
Member

@conejoninja PTAL 😸

@conejoninja
Copy link
Member

it's on my backlog, i'll review it eventually

Copy link
Member

@conejoninja conejoninja left a comment

Choose a reason for hiding this comment

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

Hello, thanks for your contribution, since you already did the work for Alarm1, does it make sense to add Alarm2 too?

Also, since we are at it, I would add a new example under /examples/ds3231 for the alarms, let's keep the one already as "basic" and another for the alarms. It would be great to show how to set an interrupt on TinyGo.

@@ -148,6 +149,105 @@ func (d *Device) ReadTime() (dt time.Time, err error) {
return
}

func (d *Device) GetSqwPinMode() SqwPinMode {
Copy link
Member

Choose a reason for hiding this comment

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

Exported functions need documentation

return SqwPinMode(data[0])
}

func (d *Device) SetSqwPinMode(mode SqwPinMode) error {
Copy link
Member

Choose a reason for hiding this comment

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

This too, exported functions need documentation

// ALarm1 Modes
type Alarm1Mode uint8

const (
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be add Alarm2 too?

@mfulz
Copy link
Author

mfulz commented May 30, 2025

Hello, thanks for your contribution, since you already did the work for Alarm1, does it make sense to add Alarm2 too?

Also, since we are at it, I would add a new example under /examples/ds3231 for the alarms, let's keep the one already as "basic" and another for the alarms. It would be great to show how to set an interrupt on TinyGo.

Hi sure can do that stuff, if it's fine around 2-3 weeks as timeline. And of course alarm2, examples, docs will be added. Just wanted to make sure, that the PR will be accepted before doing the whole work :)

@conejoninja
Copy link
Member

Sure, we welcome every contribution, sometimes we are a bit slower reviewing changes, but we eventually get them done.

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

Successfully merging this pull request may close these issues.

3 participants