Skip to content

RFC: Introducing a common interface for counters. #762

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.

Sign up for GitHub

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
Ben-PH wants to merge 10 commits into
base: master
Choose a base branch
from
Open

RFC: Introducing a common interface for counters. #762

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1cae262
Initial RFC draft
Ben-PH May 22, 2024
f8ee374
Add comment contributions by James Munn
Ben-PH May 23, 2024
fb1e268
Include early prototype of concept.
Ben-PH May 23, 2024
8359971
State the core principal
Ben-PH May 23, 2024
29ddfd8
Minor writing changes
Ben-PH May 23, 2024
304ebba
Design details work
Ben-PH May 23, 2024
ec2c659
Archive of base-and-extend
Ben-PH May 23, 2024
84d29a4
Unresolved questions work
Ben-PH May 23, 2024
cdcae5e
Correct rendering issues
Ben-PH May 23, 2024
e4bdb37
Merge pull request #1 from Ben-PH/counter-rfc-stage
Ben-PH May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Archive of base-and-extend
  • Loading branch information
Ben PHL committed May 23, 2024
commit ec2c6591b0e8db1ad9740e55bcdc627ee0ec85fc
9 changes: 3 additions & 6 deletions text/0000-common-counter-interface.md
View file
Open in desktop
Copy link
Member

@jamesmunns jamesmunns May 22, 2024

Choose a reason for hiding this comment

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

I appreciate you writing this up! In particular, I would very much like to see the following items, which I don't see in the current draft:

  • discussion of potential wraparound conditions, particularly if u64 is not a mandatory representation of the counter value
  • discussion of how you expect platforms with limited range timers (e.g. 16-bit only timers) to participate for u32 and u64 based counter values
  • If necessary (particularly for increasing smaller timers to larger ranges), any external resources, such as interrupts, that may be necessary for implementation
  • How you expect sharing of these clock/counter resources to work, for example if multiple drivers need to use them to provide timing capabilities
  • Any additional API surface, such as how "wait for timer to complete" would look

I'd also really really like to see:

  • The concrete traits you are proposing, for example in a a published crate (on GitHub or crates-io)
  • At least one example of implementing these traits, for your hardware platform or HAL of choice, likely in a fork of an existing HAL
  • At least one example of a downstream driver crate, that uses this trait
  • At least one example of an application, that links the HAL-provided trait impl and the trait-consuming driver, e.g. an "end to end example"

These examples are vitally necessary for evaluation - both for you to determine if the proposed interface(s) are reasonable in practice, as well as for others to comprehend the full extent of what you are proposing.

I'm not on the HAL team, but I imagine all the items I have listed here (and in comments) will be strong points of discussion, so for your RFC to be accepted it should likely address or dismiss these concerns.

Copy link
Author

@Ben-PH Ben-PH May 22, 2024

Choose a reason for hiding this comment

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

Your first group of points illustrate the need to ensure that everything that is implementation detail be left to the implementor. If a hal author of a specific platform or usecase is in any way hampered in their ability for their implementation to be encapsulated appropriately, then that is a failure of the trait definitions. For example, platforms with 16 bit counters could track wrappings themselves, or they could just say "out counter wraps" or whatever they choose to do.

The proposal in this RFC is to standardise an interface, with the only restriction on implementation detail being at the type constraint level.

Copy link
Member

@jamesmunns jamesmunns May 22, 2024

Choose a reason for hiding this comment

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

Yes, however, it is necessary to at least demonstrate that your proposed interface is practically usable before agreeing on that - and the best way we have today is by providing at least one (ideally a few diverse) implementations.

We've had many designs in embedded-hal that SEEMED like good ideas, or were good in isolation, but did not interact well beyond basic "hello world" interactions. This is particularly true in SPI and I2C, where it became clear between 0.2 and 1.0 that it was necessary to address sharing of a bus, as many practical systems had more than one device on the bus.

Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,9 @@ pub trait TimeWrap: TimeCount {
```
</details>



Up to this point, I've taken the concept of `Clock` and generalised it to `Counter`. I beleive that the `Clock` trait fits
in by being a trait extension of a `Counter` trait. Such an extension would further constrain the type that is being measured
with an aditional requirement that it is a representation of time (e.g. MilliSecs<...>), and perhaps add methods that are
clock-specific.
This RFC initially proposed a base `Counter` trait, where a `Clock` trait would extend
it in a manner to handle time. This has changed. For background context, the initial
illustration of this concept is included here.

```rust
pub trait Clock: Counter
Expand Down