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

drivers: add support for DS3231 RTC #15245

Merged
merged 2 commits into from
Nov 12, 2020

Conversation

haukepetersen
Copy link
Contributor

Contribution description

For a current project I was in the need for a battery-backed real-time clock and so I ended up with some ds3231 modules. This PR adds basic support for these devices to RIOT. Unfortunately my modules to not provide access to the 32khz nor the interrupt/square wave pins, so I was not able to test that functionality. In the end I decided to also omit the alarm and square wave code from this PR altogether. If somebody is interested that code can be found here: https://github.com/haukepetersen/RIOT/tree/add_driver_ds3231_alarm_sqw

Note: there is some slight overlap with the existing ds3234 driver, that would be worth of being merged together at some point. For now I decided to keep this PR independent as I want to get a little better overview on the other devices of the ds323x family before merging the code bases...

Testing procedure

Connect a ds3231 device to a board of your choice and run the test shell command from the provided test application.

Issues/PRs references

none

@haukepetersen haukepetersen added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Oct 19, 2020
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

ACK. Code looks good, and given that the DS3234 driver only implements only one function it makes sense to implement a new driver (with the potential of merging them later also with other variants). Tested with samr21-xpro, all functions work as expected.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 9, 2020
@jdavid
Copy link
Contributor

jdavid commented Nov 9, 2020

Tested with the feather-m0 board and https://www.adafruit.com/product/3028

$ BOARD=feather-m0 make -C tests/driver_ds3231 all flash test
[...]
Welcome to pyterm!
Type '/exit' to exit.
This is Rma
shell: command not found: main():
> test
 
>  test
testing device now
OK

make: Leaving directory '/home/jdavid/sandboxes/UiO/RIOT/tests/driver_ds3231'

@miri64
Copy link
Member

miri64 commented Nov 9, 2020

Ah yeah, apart from the test command I also tested the other commands provided in the test, to increase the coverage of my tests. :-)

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 9, 2020
@haukepetersen
Copy link
Contributor Author

pushed some minor static fixes, as pointed out by murdock, and squashed.

@miri64
Copy link
Member

miri64 commented Nov 9, 2020

ACK upholds.

@miri64
Copy link
Member

miri64 commented Nov 9, 2020

There are still some boards for which the test is to small: arduino-leonardo, nucleo-l011k4

@haukepetersen
Copy link
Contributor Author

updated blacklisted boards, hopefully i caught them all...

@miri64
Copy link
Member

miri64 commented Nov 12, 2020

Then let's go! :-)

@miri64 miri64 merged commit 71d970c into RIOT-OS:master Nov 12, 2020
@haukepetersen haukepetersen deleted the add_driver_ds3231 branch November 12, 2020 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants