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

Add Sharing Data with Interrupts #8

Merged
merged 4 commits into from
Mar 8, 2019
Merged

Add Sharing Data with Interrupts #8

merged 4 commits into from
Mar 8, 2019

Conversation

jamesmunns
Copy link
Member

Category

Is this PR a:

  • New Not Yet Awesome item?
  • A WIP project addressing an open item?
  • Removing a Not Yet Awesome item?

New Not Yet Awesome item checklist

  • Is the request clearly stated, linking to relevant documentation, such as a whitepaper, protocol definition, datasheet, etc.?
  • Are the "Success Criteria" defined?
  • Is this request possible using today's Rust (not blocked by LLVM impl, rustc impl, etc.)?
  • Is this request broken into reasonable work packages, such as "Create HAL for XYZ chip", not "support all boards from ABC vendor"?

@jamesmunns jamesmunns requested a review from a team as a code owner March 7, 2019 00:05
@rust-highfive
Copy link

r? @korken89

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources labels Mar 7, 2019
@jamesmunns
Copy link
Member Author

CC rust-embedded/wg#294

README.md Outdated
* Not all data can be initialized in a `const` context, so it's often necessary to use an `Option<T>`
* Global variables aren't typically idiomatic Rust.

Tools like [cortex-m-rtfm] achieve this in a zero cost fashion by using a Domain Specific Language to automatically provide safe access to shared resources between tasks and interrupts, however these tools can not be used by applications not using RTFM, or by libraries such as HAL or BSP crates.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also mention atomics? Or it could go in the patterns book. Atomics in statics are fine if you scope them; you can use them in libraries as well. For example

// crate: my-hal-impl

mod timer {
    // not global
    static DONE: AtomicBool = AtomicBool::new(false);

    impl Timer {
        fn wait(&mut self, cycles: u32) {
            DONE.store(false, Ordering::Relaxed);

            // set a timeout

            while !DONE.load(Ordering::Relaxed) {
                asm::wfi();
            }
        }
    }

    #[interrupt]
    fn TIM2() {
        // clear interrupt flag

        DONE.store(true, Ordering::Relaxed);
    }
}

Copy link
Member

@japaric japaric Mar 7, 2019

Choose a reason for hiding this comment

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

(Also I would change "tools like ..." to "frameworks like ..." (in the text))

Copy link
Member

Choose a reason for hiding this comment

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

Maybe also mention atomics?

Or you could reword the request to say "share non-atomic data ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

(Also I would change "tools like ..." to "frameworks like ..." (in the text))

I actually went back and forth on that exact naming difference! Thanks for the deciding factor.

Maybe also mention atomics? Or it could go in the patterns book. Atomics in statics are fine if you scope them; you can use them in libraries as well.

I'll add a bit about this, once there is a section about atomics in the patterns book, I'd be happy to link it here as well.

I think we will need to make a folder of "detailed requests" sooner than later - I'd like to keep the README from becoming 10k-100k lines long, so at some point we should probably move each NYA item to it's own MD file, and just have a title and 1 paragraph summary (and a link to the child MD) on the main readme.

@jamesmunns
Copy link
Member Author

Addressed @japaric's comments, this is ready for re-review!

@mathk
Copy link

mathk commented Mar 8, 2019

Coudn't this kind of issue be solved with a messaging bus system ?

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM

@jamesmunns
Copy link
Member Author

@mathk - yes, but you would still need to get a handle to the message bus system (producer, consumer, etc) to the interrupt!

@jamesmunns
Copy link
Member Author

bors r=therealprof

bors bot added a commit that referenced this pull request Mar 8, 2019
8: Add Sharing Data with Interrupts r=therealprof a=jamesmunns

## Category

Is this PR a:

- [x] New Not Yet Awesome item?
- [ ] A WIP project addressing an open item?
- [ ] Removing a Not Yet Awesome item?

## New Not Yet Awesome item checklist

- [x] Is the request clearly stated, linking to relevant documentation, such as a whitepaper, protocol definition, datasheet, etc.?
- [x] Are the "Success Criteria" defined?
- [x] Is this request possible using today's Rust (not blocked by LLVM impl, rustc impl, etc.)?
- [x] Is this request broken into reasonable work packages, such as "Create HAL for XYZ chip", not "support all boards from ABC vendor"?


Co-authored-by: James Munns <james.munns@ferrous-systems.com>
@bors
Copy link
Contributor

bors bot commented Mar 8, 2019

Build succeeded

@bors bors bot merged commit dac52ce into master Mar 8, 2019
@bors bors bot deleted the interrupt-data branch March 8, 2019 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants