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

drop #[exception] and #[interrupt] syntax in favor of #[task(binds = ..)] #207

Closed
japaric opened this issue Jun 19, 2019 · 9 comments
Closed
Labels
RFC This issue needs you input! S-accepted This RFC has been accepted but not yet implemented
Milestone

Comments

@japaric
Copy link
Collaborator

japaric commented Jun 19, 2019

Current behavior

Two attributes are used to declare hardware tasks: #[exception] and
#[interrupt]. The difference between the two is subtle: #[exception] is for
declaring hardware tasks bound to one of the ARM Cortex-M exceptions (e.g.
SVCall, SysTick), whereas #[interrupt] is for declaring hardware tasks
bound to device specific interrupts.

This distinction has been inherited from cortex-m-rt-land where two attributes
are required to implement the "does this interrupt exist?" check -- the issue
there is that the enumeration of valid exceptions / interrupts are defined in
different crates (cortex-m and PAC, respectively) and the #[exception] and
#[interrupt] attributes can be used in any context (application or library) --
handlers can even be defined in multiple crates.

Proposal

Drop the #[exception] and #[interrupt] attributes from the syntax and use
the #[task] attribute to declare both software and hardware tasks.

Rationale

In RTFM applications hardware tasks are defined within the contents of the
#[rtfm::app] macro so there's no real need for the user to distinguish between
hardware tasks bound to Cortex-M exceptions and hardware tasks bounds to
device-specific interrupts -- the framework can tell these apart without user
input (the set of Cortex-M exceptions is known at macro expansion time).

Apart from the #[init]-ialization function and the background #[idle]
context RTFM only has tasks: these may be software tasks issued on demand by the
software or hardware tasks started by some hardware event (interrupt signal).
It seems appropriate to use the #[task] attribute for both kind of tasks
rather than to use two additional attributes to refer to one particular kind
of task.

Lastly, this shrinks the syntax of the RTFM meta-language which simplifies the
parser and shrinks the test suite -- there would be no need to test so many
combinations of attributes and arguments.

Detailed design

Reject uses of the the #[exception] and #[interrupt] attributes within a
#[rtfm::app] specification. To declare a hardware task the programmer will use
the existing #[task] attribute but include the binds argument.

#[rtfm::app(device = ..)]
const APP: () = {
    // this is a hardware task
    #[task(binds = EXTI0)]
    fn on_button_pressed(_: on_button_pressed::Context) {
        // ..
    }

    // this is a software task
    #[task]
    fn process_packet(_: process_packet::Context, packet: Packet) {
        // ..
    }
};

Additionally, the framework will reject #[task]s where both the binds and
capacity arguments have been specified -- a hardware task has no associated
message queue and thus no capacity.

#[rtfm::app(device = ..)]
const APP: () = {
    #[task(binds = EXTI0, capacity = 2)]
    fn on_button_pressed(_: on_button_pressed::Context) {
    // ~~~~~~~~~~~~~~~~~ error: hardware tasks can't be assigned a capacity
        // ..
    }
};

Drawbacks

The #[exception] and #[interrupt] attributes are well established in the
wider Cortex-M ecosystem so newcomers to RTFM may find them familiar; that
familiarity would be lost with this proposal. However, I'm not sure this
familiarity is that useful because the attributes used within #[rtfm::app]
don't have the same syntax as the cortex_m_rt::{exception,interrupt}
attributes.

Alternatives

If it seems useful to visualize tell apart hardware tasks from software tasks we
could use a single attribute for them. For example, we could use
#[interrupt] for all of them (instead of a mixture of #[exception] and
#[interrupt]).

@japaric japaric added the RFC This issue needs you input! label Jun 19, 2019
@japaric japaric added this to the v0.5.0 milestone Jun 19, 2019
@placrosse
Copy link

Well-reasoned.

bors bot referenced this issue in rtic-rs/rtic-syntax Jun 20, 2019
12: drop #[exception] and #[interrupt] syntax; use #[task(binds=..)] for … r=japaric a=japaric

…hardware tasks

this implements RFC japaric/cortex-m-rtfm#207

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
japaric added a commit that referenced this issue Jun 20, 2019
@little-arhat
Copy link

Is it possible to add some attributes to help with https://github.com/japaric/cortex-m-rtfm/issues/197 ?

@korken89
Copy link
Collaborator

This is indeed a good step forward and a simplification that helps readability.
STGM!

@japaric
Copy link
Collaborator Author

japaric commented Jun 29, 2019

This seems uncontroversial let's give a 1 week-ish FCP with disposition to merge.

@TeXitoi
Copy link
Collaborator

TeXitoi commented Jun 29, 2019

A great simplification.

OK for me

@jordens
Copy link
Contributor

jordens commented Jul 2, 2019

How would the irqn argument and the HardFaultTrampoline be handled/passed?
Is the never type return ok for (non-idle) tasks already or would that be an addition?

fn DefaultHandler(irqn: i16);
fn HardFault(ef: &cortex_m_rt::ExceptionFrame) -> !;

@japaric
Copy link
Collaborator Author

japaric commented Jul 4, 2019

@jordens neither of those two handlers can be managed by RTFM; they are rejected by the DSL if you try to bind a task to them. Only interrupts / exceptions with configurable priorities can be used as tasks.

(Accessing a resource (lock API) from any of those two handlers would be unsound. Many instances of DefaultHandler can run concurrently that could produce multiple mutable references to resource data, and the HardFault handler can't be masked)

@japaric
Copy link
Collaborator Author

japaric commented Jul 8, 2019

🎉 This RFC has been formally approved. Implementation is in PR #205 (I'm going to keep this open until the PR lands)

@japaric japaric added S-accepted This RFC has been accepted but not yet implemented and removed disposition-merge labels Jul 8, 2019
bors bot added a commit that referenced this issue Sep 15, 2019
205: rtfm-syntax refactor + heterogeneous multi-core support r=japaric a=japaric

this PR implements RFCs #178, #198, #199, #200, #201, #203 (only the refactor
part), #204, #207, #211 and #212.

most cfail tests have been removed because the test suite of `rtfm-syntax`
already tests what was being tested here. The `rtfm-syntax` crate also has tests
for the analysis pass which we didn't have here -- that test suite contains a
regression test for #183.

the remaining cfail tests have been upgraded into UI test so we can more
thoroughly check / test the error message presented to the end user.

the cpass tests have been converted into plain examples

EDIT: I forgot, there are some examples of the multi-core API for the LPC541xx in [this repository](https://github.com/japaric/lpcxpresso54114)

people that would like to try out this API but have no hardware can try out the
x86_64 [Linux port] which also has multi-core support.

[Linux port]: https://github.com/japaric/linux-rtfm

closes #178 #198 #199 #200 #201 #203 #204 #207 #211 #212 
closes #163 
cc #209 (documents how to deal with errors)

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
bors bot added a commit that referenced this issue Sep 15, 2019
205: rtfm-syntax refactor + heterogeneous multi-core support r=japaric a=japaric

this PR implements RFCs #178, #198, #199, #200, #201, #203 (only the refactor
part), #204, #207, #211 and #212.

most cfail tests have been removed because the test suite of `rtfm-syntax`
already tests what was being tested here. The `rtfm-syntax` crate also has tests
for the analysis pass which we didn't have here -- that test suite contains a
regression test for #183.

the remaining cfail tests have been upgraded into UI test so we can more
thoroughly check / test the error message presented to the end user.

the cpass tests have been converted into plain examples

EDIT: I forgot, there are some examples of the multi-core API for the LPC541xx in [this repository](https://github.com/japaric/lpcxpresso54114)

people that would like to try out this API but have no hardware can try out the
x86_64 [Linux port] which also has multi-core support.

[Linux port]: https://github.com/japaric/linux-rtfm

closes #178 #198 #199 #200 #201 #203 #204 #207 #211 #212 
closes #163 
cc #209 (documents how to deal with errors)

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
@japaric
Copy link
Collaborator Author

japaric commented Sep 15, 2019

Done in PR #205

@japaric japaric closed this as completed Sep 15, 2019
andrewgazelka pushed a commit to andrewgazelka/cortex-m-rtic that referenced this issue Nov 3, 2021
207: Rejig link.x to include more lables to help the linker lay out the ob… r=thejpster a=therealprof

…jects

Tested with 1.34.0 and 1.38.0 and careful inspection of the linker map
generated on the previously failing
https://github.com/rust-lang/rust/files/3722440/minimal-rust-lld-issue.zip

Closes rtic-rs#188 (I believe)
Closes rust-lang/rust#65391

Signed-off-by: Daniel Egger <daniel@eggers-club.de>

Co-authored-by: Daniel Egger <daniel@eggers-club.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC This issue needs you input! S-accepted This RFC has been accepted but not yet implemented
Projects
None yet
Development

No branches or pull requests

6 participants