samples: rtc: add sample app for rtc subsystem

[walidbadar]

This sample app sets and periodically reads date/time from the RTC.

Tested with stm32f3 discovery board internal RTC.

[bjarki-andreasen]

Has it been considered to just enabling the RTC shell (I see its manually disabled in the config file)? Time can be both set and gotten in ISO format, and it works with any board and any RTC.
I opted for not creating a sample for RTCs since testing is performed with the RTC test suite, and demoing is achievable with the RTC shell :)

[bjarki-andreasen]

dashes in names don't have spaces:

clock-frequency = <I2C_BITRATE_STANDARD>;

[bjarki-andreasen]

The node should be indented here:

        rtc0: ds1307 @68 {
                compatible = "maxim,ds1307";
                ...
        };

[bjarki-andreasen]

CONFIG_RTC_DS1307=y is automatically selected if the ds1307 exists in the devicetree with status "okay", so it can be removed

[bjarki-andreasen]

CONFIG_I2C=y is automatically selected by the RTC_DS1307 device driver, so can be removed

[bjarki-andreasen]

The default for these options is already =n, so these can be removed

[walidbadar]

I removed all unnecessary Kconfig options and changed the overlay as you suggested.

[walidbadar]

@bjarki-andreasen This sample was created to help anyone quickly learn the RTC API, saving time by skipping the need to sift through documentation. :)

[bjarki-andreasen]

If you think this sample is helpful I thinks it good to add it, but as a sample, its important that it is portable :) That is, can be built for any board with any RTC :) If you look to the test suite in tests/drivers/rtc/rtc_api, it can be built and run for any board, as long as the rtc is pointed to using the alias rtc:

/ {
	aliases {
		rtc = &rtc;
	};
};

The sample should be in samples/drivers/rtc/ :)

[walidbadar]

I have used const struct device *const rtc = DEVICE_DT_GET(DT_ALIAS(rtc)); in source file and also changed the location to samples/drivers/rtc/ .

[bjarki-andreasen]

This sample should be built for an in-tree board which actually has an RTC, rather than a custom board with an external RTC attached, like a b_u585i_iot02a for example, along with an emulated board like the native_sim which has an emulated RTC.

The sample will still work fine with your custom board downstream naturally :)

[bjarki-andreasen]

This check is not required as const struct device *const rtc = DEVICE_DT_GET(DT_ALIAS(rtc)); will result in a compilation error (undefined reference) if this pointer is "NULL", try removing the alias from your overlay, or set status of the rtc to "disabled" and you will see what I mean :)

[bjarki-andreasen]

Initialization is not required, its assigned before read later on, also just use int to match the return value type of the rtc_get_time() API :)

[bjarki-andreasen]

Specific return value is not used, so this can be simplified to:

if (get_date_time(rtc)) {
        printk("Failed to read from rtc\n");
        return 0;
}

and even:

while (get_date_time(rtc) == 0) {
        k_sleep(K_MSEC(1000));
};

[walidbadar]

I have made changes as you requested.

[bjarki-andreasen]

The reason the esp32_devkitc_wroom_proccpu overlay can't be included upstream is that its a modification you have made to your board locally. If someone else has the same board, but has a different RTC on a different bus, the overlay for your board will be applied to their different board if they try to build this sample...

Only boards can be used upstream, not boards with local additions/modifications (with exception for test fixtures like gpio_loopback but that's another story)

[bjarki-andreasen]

For this sample, it should probably always set the time :) so SET_DATE_TIME should be removed. If you really want it, the define should be a Kconfig like CONFIG_TEST_SET_DATE_TIME

See:
https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/rtc/rtc_api/Kconfig

zephyr/tests/drivers/rtc/rtc_api/src/test_alarm.c

Line 19 in c50777a

static const uint16_t test_alarm_time_mask_set = CONFIG_TEST_RTC_ALARM_TIME_MASK;

[walidbadar]

removed #ifdef SET_DATE_TIME and custom overlay for esp32_devkitc_wroom.

[bjarki-andreasen]

The helper get_date_time() already prints that it failed to get the time, including the error code, so adding an additional get_date_time() call and printing printk("Failed to read from rtc\n"); is redundant IMO.

It would produce:

Current RTC time: ...
Cannot read date time: -1
Failed to read from rtc

[bjarki-andreasen]

Additionally, it can then be simplified to:

while (get_date_time(rtc) == 0) {
        k_sleep(K_MSEC(1000));
}

[walidbadar]

Simplified the logic as you suggested. I appreciate your feedback and hope that this pull request is not taking too much of your time. :)

[bjarki-andreasen]

Looks good, nice work!

[kartben]

All sample should have a README, can you please add one? You may use the sample.tmpl template (or other samples' READMEs) as a starting point. Thanks!

[walidbadar]

@kartben Could you please review the PR and let me know if any further changes are needed? After adding the README, all checks have passed, including the Documentation build. 🙂

[walidbadar]

@pdgendt Could you please run the full CI and also review the changes?

[kartben]

great work! a few remaining comments but virtually +1 as soon as they're addressed :) Thanks!

[github-actions]

Hi @walidbadar!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

[BuriKizilkaya]

@walidbadar had you issues to set the month of the rtc with the stm32 driver? I saw during debugging, that the month november would be checked with the value 0x11. the rtc_ll_stm32.c module convert the bin to bcd and the stm32wlxx_ll_rtc.c convert it back to bin and checks with the value 0x11 (what is wrong)...