Implement most of chrono::high_resolution_clock

[ckormanyos]

Fixes #9 via impl of chrono::high_resolution_clock

[salkinium]

Do you want to add a similiar system_clock as chrono::milliseconds while you're at it? Then targets without a microsecond timer can also reuse this.
Just not sure which one should be the steady_clock then… perhaps the default could be a defaulted macro?

[ckormanyos]

Do you want to add a similiar system_clock as chrono::milliseconds while you're at it? Then targets without a microsecond timer can also reuse this.

Yes. That is a good idea. I need a few days to implement and test both, maybe throw in a couple of examples.

Just not sure which one should be the steady_clock then… perhaps the default could be a defaulted macro?

I like the idea of macro(s) for the clocks. I'll draft all these things out in this PR.

[rleh]

Now that #12 is merged it would be good if you rebase your branch onto to the current master branch. Then the examples and the new code will get compiled by the CI.

[chris-durand]

Do you want to add a similiar system_clock as chrono::milliseconds while you're at it? Then targets without a microsecond timer can also reuse this.
Just not sure which one should be the steady_clock then… perhaps the default could be a defaulted macro?

The standard mandates that "Objects of type system_­clock represent wall clock time". Generally they can't be the same since steady_clock, you might have guessed it, has to be steady.

[ckormanyos]

would be good if you rebase your branch onto to the current master branch

Thx @rleh. Done.

[ckormanyos]

Do you want to add a similiar system_clock as chrono::milliseconds while you're at it? Then targets without a microsecond timer can also reuse this.

Yes. That is a good idea. I need a few days to implement and test both, maybe throw in a couple of examples.

I am sort of in the middle of this.

As an initial step, I think it is a good first progress to:

1. Add one clock (high_resolution_clock) to <chrono>.
2. Make 1 other clock configurable via macro to be aliased to high_resolution_clock.
3. Make the timebase of high_resolution_clock configurable via macro.
4. Add an example of using <chrono>.

Thoughts ... ?

The recent push of mine addresses 1,2,4. Point 3 is still open.

Let's see how CI plays out...

Cc: @chris-durand and @salkinium and @rleh

[ckormanyos]

Hi @rleh I can't really see where to add examples/chrono/chrono.cpp to the CI Makefile

Any tips...?

[chris-durand]

Hi @rleh I can't really see where to add examples/chrono/chrono.cpp to the CI Makefile

Any tips...?

You can add your own Makefile to the examples/chrono subfolder and the CI job will run it.

[ckormanyos]

Note: I added CI runs to push in addition to pull-request.

Anyway, this thing should be ready for further discussion/review. CI and chrono-example-build are running green in my fork's CI.

[ckormanyos]

add your own Makefile to the examples/chrono subfolder

Done. Thx @rleh

[rleh]

I believe this chrono example becomes more useful if we replace the mcal::gpt::* and reinterpret_cast<> stuff with the (well known) avr-libc syntax (DDRB |= (1<<5u)/...).

[ckormanyos]

this chrono example becomes more useful if we replace the mcal::gpt::* and reinterpret_cast<> stuff with the (well known)

Oh sure. I'll do that. Got to remember the name of that header at the moment...

Thanks for reviewing.

[ckormanyos]

would have allowed the error to be seen much more quickly.

I was not aware of an error at this time. Did you detect one?

[ckormanyos]

Is there a reason to have the CI running on every push?

Good question.

How active is your project? How many cycles do you want to burn?

It depends on the activity and quality-depth of your project. i work in some projects in which even the tinyest defect can lead to big problems. So we run tests on syntax, unit tests, etc. --- the whole CI on each and every push, branch push, PR and sometimes even scheduled builds.

After a bit of thought, I might recommend:

1. Let's say there is an active phase in avr-libstdcpp for a while, during which we maybe add a few examples and docs. Then it might make sense to run CI on both push and PR.
2. If this activity burst slows down a bit (as expected), then remove CI on pushes if you like.
3. I see no need for nightlies or similar on this repo because it seems to be rather slow-changing.

On the other hand regarding points 1 and 2 (i.e., to run CI on pushes/or-not). Since CI on this project runs in about a minute, why not...?

I would also recommend setting up a test matrix that tests a few C++ standards, maybe start with C++17 and C++20.

It remains to be seen if a a significant number of tests might be added. i would, however, have a bunch of simple ideas if you'd like to pursue these.

[ckormanyos]

OK folks,

Many, many thanks for the excellent, detailed and very thoughtful review comments.

I believe the recent checkins address all review comments. if anything else is found in further review rounds, please don't hesitate to post mention of it.

I added a test matrix to CI which tests on -std=c++17 and -std=c++20. I also added the -Werror flag to the chrono example's CI, as there are some neat new warnings in C++20 available in GCC.

As a final comment, I retained the running of CI on pushes, as I really think this is best if/as/when we may add a few more tests/examples to this repo. I can, however, be convinced otherwise, but running a minute or so on CI, i would like that added security as a contributor to this/these repos.

Cc: @chris-durand and @rleh and @salkinium

[salkinium]

I feel like this is too convoluted an example, it focusses on things like namespacing to hint at a larger HAL library that isn't really there or auto main() -> int that is just forced because C++ can do it, but really what value does it add?

I would focus on the chrono now function, and take out the WDT, remove the static_casts in favor of C-style access, move int main() to the bottom with a visual //==== separator to find it easier.

This example should be very simple, to convince someone to integrate this into their C code without scaring them away with some unfortunate C++ bloat? What do you think?

[ckormanyos]

This example should be very simple, to convince someone to integrate this into their C code without scaring them away with some unfortunate C++ bloat? What do you think?

@salkinium and guys. Decide which you want. The classic example is a 3-to-5 liner that measures the time of a code sequence.

It might be better to go for that when showcasing only the functionality of <chrono>.

This is a larger refactor/complete-re-design. Please decide. I'm happy to go either way. This post would basically render those above moot. Might be the easiest way to proceed... We could go that way and wrap up <chrono> lickety split if that's the decision...?

HJust provide me your feedback. I'm neutral on it...

Cc: @salkinium and @rleh and @chris-durand

[salkinium]

The classic example is a 3-to-5 liner that measures the time of a code sequence.

Let's go with that. Simpler and easier is better.

[ckormanyos]

The classic example is a 3-to-5 liner that measures the time of a code sequence.

Let's go with that. Simpler and easier is better.

You bet. Coming right up.

Compiling it on CI. Just for the sake of completion, here are a couple of those C++20 volatile warnings. I'll suppress these and move forward.

[ckormanyos]

OK CI green.

Pls review.... I'm happy to tune at your request(s)...

I stubbed the clock here.

Cc: @salkinium and @rleh and @chris-durand

[salkinium]

Ok, that's perhaps a little too minimal, I would still keep the TIMER0_OVF code, that was pretty cool with guarding the rollover via a double read.

[ckormanyos]

Ok, that's perhaps a little too minimal, I would still keep the TIMER0_OVF code, that was pretty cool with guarding the rollover via a double read.

Ok. That was one of my open questions in my mind also...
I'll draft that out tomorrow and run it on a board... To-be-continued...

[ckormanyos]

OK folks the next iteration of the <chrono>-example's refactoring is available.

This iteration of refactoring has/does:

• Very simple, readable form. Clarity hopefully improved now.
• Restore the actual use of system tick at 1MHz.
• Incorporate most review points to use avr/whatever.h-stuff.
• Include watchdog (but only deactivates it, as delivery state of Arduino has WDT active, I think).
• Still open is toggle mechanism (PINB or logical xor?).
• Also if WDT deactivate ramins, then I need help with pneumonic for this line.

Pls. take a look and comment as needed/felt.

Cc: @salkinium and @rleh and @chris-durand

[salkinium]

Awesome, thanks!

[chris-durand]

Nice! Thank you!

[ckormanyos]

Hi guys, this one is, for the moment, done and good to go, in my opinion.

Tabs as line indentations to be handled in #13. Or should I handle tabs as line indentations here in this PR?

[ckormanyos]

OK I decided to:

• Handle line indentation as tabs here in this PR.
• Use wdt_reset() which is found to be defined as asm volatile("wdr") in <avr/wdt.h>.

Sorry about the late changes.

[salkinium]

Squash?

[ckormanyos]

Squash?

Hi Niklas (@salkinium), if this comment is for me, I'm not exactly sure how to do it...

Could you please explain more details?

[salkinium]

The commits in this PR should be squashed into one commit, so that the git history is clean and understandable.

[ckormanyos]

The commits in this PR should be squashed into one commit

Well Niklas (@salkinium) I think I did this, or something with essentially the same result. I do admit, though, that I'm not experienced with Git squashing methods.

[jwakely]

Please consider contributing this clock impl back upstream.

Edit: Ah, on further inspection, it seems that this just adds a declaration, and leaves the now() function undefined. i.e. a hook allowing user code to provide its own definition. Something like that could still make sense upstream, and the example implementation could be optionally provided.

[ckormanyos]

Please consider contributing this clock impl back upstream

Hi Jonathon (@jwakely) this is a good idea. We would need to negotiate on the resolution of the clock(s), as I believe this needs to be specified when laying out a concrete implementation.

As time allows, we can/should discuss this matter. I can help where needed and if wanted.