Skip to content

drivers/rtt_rtc: add RTT based RTC implementation, enable it for cpu/cc2538, nrf5x_common#13519

Merged
gschorcht merged 6 commits intoRIOT-OS:masterfrom
benpicco:rtt_rtc
Mar 19, 2020
Merged

drivers/rtt_rtc: add RTT based RTC implementation, enable it for cpu/cc2538, nrf5x_common#13519
gschorcht merged 6 commits intoRIOT-OS:masterfrom
benpicco:rtt_rtc

Conversation

@benpicco
Copy link
Contributor

@benpicco benpicco commented Feb 29, 2020

Contribution description

This adds an emulated RTC based on top of the RTT implementation.
It is rather basic in that it only allows alarms to be set one full period of the RTT in the future.
For a 32bit RTT running at 32kHz, this means you can set alarms at most 1½ days in the future.

Testing procedure

Run tests/periph_rtc on e.g. openmote-b:

2020-02-29 16:39:38,199 # This test will display 'Alarm!' every 2 seconds for 4 times
2020-02-29 16:39:38,200 #   Setting clock to   2011-12-13 14:15:57
2020-02-29 16:39:38,211 # Clock value is now   2011-12-13 14:15:57
2020-02-29 16:39:38,212 #   Setting alarm to   2011-12-13 14:15:59
2020-02-29 16:39:38,212 #    Alarm is set to   2011-12-13 14:15:59
2020-02-29 16:39:38,213 # 
2020-02-29 16:39:40,211 # Alarm!
2020-02-29 16:39:42,211 # Alarm!
2020-02-29 16:39:44,210 # Alarm!
2020-02-29 16:39:46,210 # Alarm!

Issues/PRs references

I moved the additions to the RTT driver to #13630
They are not needed anymore.

@benpicco benpicco added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Feb 29, 2020
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 29, 2020
@benpicco benpicco added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 7, 2020
@benpicco benpicco force-pushed the rtt_rtc branch 3 times, most recently from 0f37191 to f6aee80 Compare March 8, 2020 19:50
@fjmolinas
Copy link
Contributor

@benpicco why is this wip now?

@benpicco benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 13, 2020
@benpicco
Copy link
Contributor Author

Because the previous version was adding up rounding errors by dropping the sub-seconds and the real solution is much simpler.

@benpicco benpicco changed the title drivers/rtt_rtc: add RTT based RTC implementation, enable it for cpu/cc2538 drivers/rtt_rtc: add RTT based RTC implementation, enable it for cpu/cc2538, nrf5x_common Mar 13, 2020
@benpicco benpicco added CI: run tests If set, CI server will run tests on hardware for the labeled PR 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 Mar 13, 2020
@benpicco benpicco force-pushed the rtt_rtc branch 4 times, most recently from f31ee9a to 08a68d9 Compare March 13, 2020 13:14
@benpicco benpicco 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 Mar 13, 2020
@gschorcht
Copy link
Contributor

gschorcht commented Mar 16, 2020

@benpicco I like this PR because I could use it right away for the ESP8266, now that I have implemented an RTT (PR #13640). So I'm really interested to have it soon.

@gschorcht gschorcht added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 17, 2020
@gschorcht
Copy link
Contributor

gschorcht commented Mar 17, 2020

I have added Waiting for other PR since it depends now on PR #13646.

Forget it 😎

@gschorcht gschorcht removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 17, 2020
Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

Please fix the typo and squash.

Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

ACK

@gschorcht gschorcht merged commit df19c6d into RIOT-OS:master Mar 19, 2020
@benpicco benpicco deleted the rtt_rtc branch March 19, 2020 16:19
@gschorcht
Copy link
Contributor

@benpicco Thanks for this contribution.

@gschorcht
Copy link
Contributor

@benpicco One question, should a CPU that use rtt_trc module to provide periph_rtc also define

FEATURES_PROVIDED += periph_rtc

in its Makefile.features? The reason for my question is that some programs such as tests/periph_pm use

FEATURES_OPTIONAL += periph_rtc

which doesn't pull in module rtt_rtc because the feature is only optional and not declared as provided.

@benpicco
Copy link
Contributor Author

If the RTT doesn't depend on a board feature (low frequency external oscillator), it can be added together with FEATURES_PROVIDED += periph_rtt.

@gschorcht
Copy link
Contributor

it can be added together with FEATURES_PROVIDED += periph_rtt.

OK, then there's another question. If an application would claim

FEATURES_REQUIRED += periph_rtt
FEATURES_REQUIRED += periph_rtc

a problem will occur. For that purpose, we have FEATURES_CONFLICT variable. But adding

FEATURES_CONFLICT += periph_rtc:periph_rtc

would always yield the conflict waring. Any idea?

@benpicco
Copy link
Contributor Author

I noticed that problem too and was hoping no application would use both RTT and RTC…
I don't know of a way how to resolve this with the means currently provided by our build system.

@gschorcht
Copy link
Contributor

Maybe, we could realize it by defining a peripheral periph_rtt_base that implements the RTT and can be used either by periph_rtt or by periph_rtc (alias rtt_rtc).

FEATURES_PROVIDED += periph_rtt_base
FEATURES_PROVIDED += periph_rtt
FEATURES_PROVIDED += periph_rtc

FEATURES_CONFLICT += periph_rtt:periph_rtc
ifneq (,$(filter rtt_rtc,$(USEMODULE)))
   FEATURES_REQUIRED += periph_rtt_base
endif

@benpicco
Copy link
Contributor Author

benpicco commented Mar 20, 2020

That would be a solution. I like the idea since it could also be used e.g. on ATmega where periph_rtt emulates a 24 bit counter when the hardware only provides 8 bit counters.

rtt_rtc can deal with fast overflowing 8 bit counters just fine, using the 24 bit emulated RTT would be needless overhead. So that could also make use of a simple periph_rtt_base module that just exposes the real hardware features.

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 CI: run tests If set, CI server will run tests on hardware 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.

4 participants