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

[RFC] drop the timer-queue Cargo feature #199

Closed
japaric opened this issue Jun 14, 2019 · 6 comments
Closed

[RFC] drop the timer-queue Cargo feature #199

japaric opened this issue Jun 14, 2019 · 6 comments
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 14, 2019

Current behavior

To use the schedule API one needs to first enable the timer-queue Cargo
feature. Enabling this feature changes the types of the fields of the
rtfm::Peripherals struct.

Proposal

Remove the timer-queue feature; one can always use the schedule API in an
RTFM application. The fields of the rtfm::Peripherals struct never change when
one uses the schedule API in an application.

Rationale

The timer-queue feature doesn't work with the thumbv6m-none-eabi target. To
support the schedule API on heterogeneous multi-core devices where one core is
ARMv6-M the user would have to compile the cortex-m-rtfm crate with a
different set of Cargo features for each core (compilation target). Not only
this is not currently supported by cargo-microamp but it would also result in
re-compiling the cortex-m-rtfm-macros crate, the biggest dependency, at least
twice.

Detailed design

The timer-queue Cargo feature will be removed. The cortex-m-rtfm-macros
crate will codegen timer queue related data structures only when the application
makes use of the schedule API.

The rtfm::Peripherals struct, which represents the Cortex-M peripherals not
used by the framework, will change its fields to match the fields of
cortex_m::Peripherals minus the SYST (system timer) field. All fields will
be there by value so the user can turn them into resources and use them from
any task -- ownership over the SCB was specifically requested in #165 (cc
@azasypkin).

The type of the core field of the init::Context struct will change
depending on whether the application uses the schedule API or not. When the
schedule API is used the core field will have type rtfm::Peripherals
(SYST field not included); when the schedule API is not used the field
will have type cortex_m::Peripherals (the SYST field will be included). In
other words, the system timer (SysTick) will only be exposed to the end user
when it's not used by the framework. Likewise, the end user will not be able
to use the SysTick as a hardware task when the application uses the schedule
API.

// app doesn't use the schedule API
#[rtfm::app(device = ..)]
const APP: () = {
    #[init]
    fn init(c: init::Context) {
        let core: cortex_m::Peripherals = c.core;
        // OK
        let syst: SYST = core.SYST;
    }

    // OK
    #[exception]
    fn SysTick(c: SysTick::Context) {}
};
// app may use the schedule API
#[rtfm::app(device = ..)]
const APP: () = {
    #[init(schedule = [foo])]
    fn init(c: init::Context) {
        let core: rtfm::Peripherals = c.core;
        // let syst: SYST = core.SYST;
        //~^ error: no field named `SYST` in struct
    }

    // #[exception]
    // fn SysTick(c: SysTick::Context) {}
    //~^ this would also be an error
};
@japaric japaric added the RFC This issue needs you input! label Jun 14, 2019
@japaric japaric added this to the v0.5.0 milestone Jun 14, 2019
@korken89
Copy link
Collaborator

I quite like this design, however it might cause confusion for users that do not know that this is automatic. I assume there will be very descriptive compile errors if one does not adhere to this?

@japaric
Copy link
Collaborator Author

japaric commented Jun 29, 2019

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


however it might cause confusion for users that do not know that this is automatic.

Could you give examples of situations where users may find this confusing?

I assume there will be very descriptive compile errors if one does not adhere to this?

Paired with RFC #200 the error messages currently are

  • used the schedule API but didn't specify a monotonic timer
error: a `monotonic` timer must be specified to use the `schedule` API

with span pointing to the #[app] attribute

  • bound a task to SysTick and used the schedule API
error: this exception can't be used because it's being used by the runtime

with span pointing to the SysTick task

  • tried to access SYST peripheral while using the schedule API
error[E0609]: no field `SYST` on type `rtfm::Peripherals`

the type is a good hint, I think

  • passed context.core to a function that expected cortex_m::Peripherals while using the schedule API
error[E0308]: mismatched types

expected struct `cortex_m::peripheral::Peripherals`, found struct `rtfm::Peripherals`

I think the path difference is a good hint

For the last two errors we can't do much to improve the error message.

@TeXitoi
Copy link
Collaborator

TeXitoi commented Jun 29, 2019

OK for me

@korken89
Copy link
Collaborator

korken89 commented Jul 2, 2019

Thanks for the examples @japaric !

The type of errors you have specified are good and descriptive. I was a bit worried about the following kind of errors that was in the example:

// let syst: SYST = core.SYST;
//~^ error: no field named `SYST` in struct

When they look like this intimate knowledge of the internal workings are needed, but (just an example) some thing like this would be more descriptive:

// let syst: SYST = core.SYST;
//~^ error: no field named `SYST` in struct, this is reserved by the monotonic timer in RTFM ...

I just want to keep us away from the first type of error messages, though this is an constructed example.

@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
199: v0.6.10 r=korken89 a=Disasm



Co-authored-by: Vadim Kaushan <admin@disasm.info>
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

3 participants