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

Can't use early, shared resource in init in 0.6.0-alpha.0 #429

Closed
bradleyharden opened this issue Nov 25, 2020 · 16 comments
Closed

Can't use early, shared resource in init in 0.6.0-alpha.0 #429

bradleyharden opened this issue Nov 25, 2020 · 16 comments

Comments

@bradleyharden
Copy link

The following example gives cannot find value priority in this scope.

#![no_std]
#![no_main]

use panic_halt as _;

#[rtic::app(device = atsamd51n)]
mod app {
    #[resources]
    struct Resources {
        #[init(Default::default())]
        ping_buf: [i16; 8],
        #[init(Default::default())]
        pong_buf: [i16; 8],
    }

    #[init(resources = [ping_buf, pong_buf])]
    fn init(mut ctx: init::Context) -> init::LateResources {
        init::LateResources{}
    }
}

It looks like the generated code is trying to assign a priority for the lock, but there is no priority.

    impl<'a> initResources<'a> {
        #[inline(always)]
        pub unsafe fn new() -> Self {
            initResources {
                ping_buf: resources::ping_buf::new(priority),
                pong_buf: resources::pong_buf::new(priority),
            }
        }
    }

That leads to another question. Why are locks necessary for early resources in init in the first place?

In the meantime, do you have any suggested workarounds? I need the static address of the buffers in init. I guess I could do it myself, outside of the Resources struct, but I'd rather not have to go that far.

@korken89
Copy link
Collaborator

Hi,

More or less this support is in limbo in 0.6.0-alpha.0.
It's still under discussion how to do this, but the current way is to use the safe static mut transform instead of using resources.

What you did encounter is a bug though. :)

@bradleyharden
Copy link
Author

Ok. If this isn't yet supported, it would be nice to add an error message indicating that's the case. Most of the macro error messages have been helpful, so I'm surprised that isn't the case here.

@korken89 korken89 changed the title Can't use early, shared resource in init Can't use early, shared resource in init in 0.6.0-alpha.0 Nov 27, 2020
@korken89
Copy link
Collaborator

Hi, it was supported but it in the major rewrite of the resource handling module, it broke.
It will be fixed in 0.6.0-alpha.1, for now you can use the static mut transform.

@korken89 korken89 added the bug label Nov 27, 2020
@bradleyharden
Copy link
Author

Sorry, I must still be misunderstanding something then. In chat you said

No, in 0.5 it will work fine, as long as you do not use this resource in any other task. The interface is asymmetric here

What would happen in 0.5 if I tried to use an early resource in both init and in a task? It sounds like that would not work fine, but how would it fail?

In 0.6 you will most likely have to use a #[task_local] resource in init to get the same effect

#[task_local] doesn't really meet my needs here, unfortunately. If possible, it would be nice to allow init to share resources. In this case, I'm creating a pair of static buffers. I need the static addresses during init, and the buffers need to be shared between two tasks in the actual code.

For now, I'm using the static mut transform and sharing references to the buffers as resources. It works, but it seems like it should be possible to use early resources in init yet still share them.

@TeXitoi
Copy link
Collaborator

TeXitoi commented Nov 27, 2020

Something like (not tested)

#![no_std]
#![no_main]

use panic_halt as _;

#[rtic::app(device = atsamd51n)]
mod app {
    #[resources]
    struct Resources {
        ping_buf: &'static mut [i16; 8],
        pong_buf: &'static mut [i16; 8],
    }

    #[init]
    fn init(mut ctx: init::Context) -> init::LateResources {
        static mut PING_BUF: [i16; 8] = [0; 8];
        static mut PONG_BUF: [i16; 8] = [0; 8];
        init::LateResources{
            ping_buf: &mut PING_BUF,
            pong_buf: &mut PONG_BUF,
        }
    }
}

Edit: Mmm, it must be what you mean by "the static mut transform"?

@bradleyharden
Copy link
Author

@TeXitoi, yes, as I said this is precisely what I'm doing now. I'm just trying to understand why the limitation is there and how the previous version of RTIC would have handled my code. I would try 0.5 myself, but I'm not in front of my computer right now.

@TeXitoi
Copy link
Collaborator

TeXitoi commented Nov 27, 2020

This static mut transform is working on 0.5

@bradleyharden
Copy link
Author

Yes, I understand. I used the static mut transform before I ever migrated to 0.6.

My question is about the resources struct. If you tried to create an early shared resource and use it in both init and some other task, what would have happened? Would there have been an error? Would it have silently failed?

@korken89
Copy link
Collaborator

korken89 commented Nov 27, 2020

@bradleyharden The thing that changes is how lifetimes are generated.
If it is only used in #[init], the lifetime of the resource will be 'static (in 0.5, 0.6 will also require the #[task_local] attribute), while if you use it in multiple tasks the lifetime will be 'a as the access to the resource can happen multiple times via re-entrance of the task using the resource.
This means that, for example, you can't store it inside an object as 'a does not live long enough.
So if you want to be able to store the buffers as references in other objects you need the 'static version, if not - there are no issues.

I hope this helps :)

To directly answer the 2 questions:

Would there have been an error?

Not from RTIC, the rust compiler might give you lifetime errors though.

Would it have silently failed?

No, it would not. If it compiles, it is fine.

@bradleyharden
Copy link
Author

bradleyharden commented Nov 28, 2020

@korken89, ok, so it's just a question of lifetimes. This sort of ties into my question in #417. It sounds like you'll give a 'static reference to a #[task_local] resource in init but not to a normal task. Is that right?

In this case, I don't want to put the reference inside a structure. I need to access the buffers from two different normal tasks, so it needs to be shared; and I need access in init, so I can create a raw pointer for a DMA descriptor. An 'a lifetime would be perfectly fine for my purposes here.

For now, the static mut transform is fine.

@korken89
Copy link
Collaborator

It sounds like you'll give a 'static reference to a #[task_local] resource in init but not to a normal task. Is that right?

This is correct, only #[init] can have 'static lifetimes as it is the only task that is guaranteed to never re-enter.
For doing a "DMA dance" there are several ways, one way I implemented is in the UART FrameSender/FrameReceiver of the STM32L4 HAL, this might be some inspiration. :)

@bradleyharden
Copy link
Author

Thanks for the tip. I took a look at your implementation. I think my case is even simpler. For my situation, everything is a fixed size at a known frequency, so I don't need anything fancy. Two static buffers that I switch between is all I really need. That's why I was hoping to do it with RTIC resources. Hopefully the bug will be fixed soon and I can use an 'a lifetime.

@korken89
Copy link
Collaborator

Hi, that sounds fine.
You can expect this to be fixed in 0.6.0-alpha.1.

@korken89 korken89 added this to the v0.6.0-alpha.1 milestone Nov 30, 2020
@MabezDev
Copy link
Contributor

I don't believe this was fixed in 0.6.0-alpha.1? I am trying to fill a free queue in init which I have initialized via #[init(Queue(heapless::i::Queue::new()))], I was surprised I needed locks in init, but I figured that was just part of the symmetrical lock changes.

I guess I can just make them late resources for now, but what is the proposed solution to this?

@korken89
Copy link
Collaborator

Hi, unfortunately not.
In alpha.1 we only got the new monotonic, the new resource syntax and fixes will start now after the cancel/reschedule PR.

@korken89
Copy link
Collaborator

korken89 commented Aug 19, 2021

Hi, the recommended approach is now:

  1. Make an early local to init, e.g. #[init(local = [my_struct: MyStruct = MyStruct::const_new()])]
  2. Move it by &mut cx.local.my_struct to the shared resource in init

Then you will get the old functionality with minimal overhead.
The old interface was more straight forward, but it does unfortunately not play well with the new resource syntax.

@korken89 korken89 removed the bug label Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants