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

Type error when using picoserve::routing::Router::nest #49

Open
Sycrosity opened this issue Aug 16, 2024 · 15 comments
Open

Type error when using picoserve::routing::Router::nest #49

Sycrosity opened this issue Aug 16, 2024 · 15 comments

Comments

@Sycrosity
Copy link
Contributor

While attempting to nest a router like so: (simplified example)

pub(crate) fn app_router() -> picoserve::Router<AppRouter, GlobalState> {
    picoserve::Router::new()
        .nest("/api", api_router())
        .route(
            "/",
            get_service(picoserve::response::File::html(include_str!(
                "../dist/index.html"
            ))),
        )
}

pub(crate) fn api_router() -> picoserve::Router<Apiouter, GlobalState> {
    picoserve::Router::new().route(
        ("/", parse_path_segment()),
        picoserve::routing::get(
            |on_off| async move {
                info!("Setting to {}", if on_off { "ON" } else { "OFF" });

                DebugValue(on_off)
            },
        ),
    )
}

I encountered the following error:

error[E0308]: mismatched types
    --> src/net.rs:192:23
     |
156  | type AppRouter = impl picoserve::routing::PathRouter<GlobalState>;
     |                  ------------------------------------------------ the found opaque type
...
192  |         .nest("/api", api_router())
     |          ----         ^^^^^^^^^^^^ expected `Router<_>`, found `Router<AppRouter, GlobalState>`
     |          |
     |          arguments to this method are incorrect
     |
     = note: expected struct `picoserve::Router<_, ()>`
                found struct `picoserve::Router<net::AppRouter, net::GlobalState>`
note: method defined here
    --> /Users/louis/.cargo/registry/src/index.crates.io-6f17d22bba15001f/picoserve-0.12.2/src/routing.rs:1185:12
     |
1185 |     pub fn nest<PD: PathDescription<CurrentPathParameters>>(
     |            ^^^^

which happens regardless of if the impl picoserve::routing::PathRouter<GlobalState> is different for each router or not.

Is this a bug, and if not what have I done wrong here? an example of how to use nesting could be useful to work out what should be done instead.

Amazing project btw!

@sammhicks
Copy link
Owner

There is indeed a bug! Good find!

I unfortunately don't have time to do a proper patch and release, but I've committed the fix to nest_fix.

I also suggest you change your app_router and api_router functions to have a return type of

picoserve::Router<impl picoserve::routing::PathRouter<GlobalState>, GlobalState>

@Sycrosity
Copy link
Contributor Author

Thank you for the fix! It's working perfectly now - don't worry about the patch/release, take your time :)

I tried to change the return types, however since I'm using embassy, it needs a concrete type signature, so I kept the

pub type AppRouter = impl picoserve::routing::PathRouter<GlobalState>

annotation for the app_router, but changed it for the api_router.

Should I close the issue, or should that be done when the release is out?

@Sycrosity Sycrosity changed the title Add an example for picoserve::routing::Router::nest / possible bug Type error when using picoserve::routing::Router::nest Aug 31, 2024
@sammhicks
Copy link
Owner

I'll close the issue when the release is out.

@JuliDi
Copy link

JuliDi commented Sep 3, 2024

@Sycrosity which version of the Rust compiler are you using and how do you pass the pub type AppRouter into embassy task arguments?
I can't get rid of errors like

error: item does not constrain `AppRouter::{opaque#0}`, but has it in its signature
  --> src/main.rs:54:1
   |
54 | #[embassy_executor::task(pool_size = WEB_TASK_POOL_SIZE)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: consider moving the opaque type's declaration and defining uses into a separate module

Any ideas?
I'm using the latest versions of rustc nightly, embassy and the nest_fix branch of this crate.

@Sycrosity
Copy link
Contributor Author

Any ideas?

I'm using version 1.80.0-nightly (the espup toolchain for esp32 devices)

I passed it like this:

pub type AppRouter = impl picoserve::routing::PathRouter<GlobalState>;

#[task(pool_size = WEB_TASK_POOL_SIZE)]
pub async fn site_task(
    id: usize,
    // uuid: Uuid,
    stack: &'static Stack<WifiDevice<'static, WifiApDevice>>,
    app: &'static picoserve::Router<AppRouter, GlobalState>,
    config: &'static picoserve::Config<Duration>,
    state: GlobalState,
) -> ! {
    .....

I'd likely need to see an example of your code or a repo link to see what you may have done differently.

@JuliDi
Copy link

JuliDi commented Sep 4, 2024

Thanks!
I will give it a try later, maybe I can get it to work with what you shared (first of all, I should use the same toolchain to avoid other possibly breaking changes on nightly).
I've also uploaded the current main.rs here https://gist.github.com/JuliDi/284280e4b702cafda70e76f9c8ee406a if you want to have a look.

@Sycrosity
Copy link
Contributor Author

@JuliDi I'm a bit confused as to what MyRouter is on lines 150-153. In my equivalent of the make_app function, I have this:

pub fn app_router() -> picoserve::Router<AppRouter, GlobalState> {

    picoserve::Router::from_service(NotFound)
        .route(
            "/",
            get_service(picoserve::response::File::html(include_str!(
                "../dist/index.html"
            ))),
        )
        .route(
            "/favicon.svg",
            get_service(picoserve::response::File::with_content_type(
                "text/plain; charset=utf-8",
                include_str!("../dist/favicon.svg").as_bytes(),
            )),
        )
        .nest("/api", api_router())
}

@JuliDi
Copy link

JuliDi commented Sep 5, 2024

No special reason, I may have forgotten to revert some changes as I kept editing due to the overwhelming number of errors with opaque type stuff...

Btw, what is your GlobalState?

Thanks for the example, I'll give it a try.
But I am unsure whether it solves the opaque type errors. Maybe this is also caused by newer versions of embassy? Which one are you using?

Also, is there a way to use picoserve without nightly features? I feel like these things might break more often in the future.

@Sycrosity
Copy link
Contributor Author

GlobalState is

pub struct GlobalState {
    pub wifi_creds: WifiCredentialsState,
}
...
pub struct WifiCredentialsState(pub &'static Mutex<CriticalSectionRawMutex, WifiCredentials>);
...
pub struct WifiCredentials {
    ssid: heapless::String<32>,
    password: heapless::String<64>,
}

I am also on the latest version of embassy so I don't believe that is it either

@JuliDi
Copy link

JuliDi commented Sep 6, 2024

Interesting, thanks! I will give it a try next week. Probably some rather trivial mistake after all...

@Sycrosity
Copy link
Contributor Author

@Sycrosity which version of the Rust compiler are you using and how do you pass the pub type AppRouter into embassy task arguments? I can't get rid of errors like

error: item does not constrain `AppRouter::{opaque#0}`, but has it in its signature
  --> src/main.rs:54:1
   |
54 | #[embassy_executor::task(pool_size = WEB_TASK_POOL_SIZE)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: consider moving the opaque type's declaration and defining uses into a separate module

On rust version 1.81.0, I now have this error too - seems to be due to something in rust 1.81.0

@Sycrosity
Copy link
Contributor Author

Sycrosity commented Sep 14, 2024

I feel rather stupid - sammhicks' initial suggestion was all that was needed, with no type State = ... needed at all.

@JuliDi
Copy link

JuliDi commented Sep 14, 2024

What do you mean? Apparently this is a problem with the new TAIT requirements

@jcorrie
Copy link

jcorrie commented Sep 15, 2024

@Sycrosity which version of the Rust compiler are you using and how do you pass the pub type AppRouter into embassy task arguments? I can't get rid of errors like

error: item does not constrain `AppRouter::{opaque#0}`, but has it in its signature
  --> src/main.rs:54:1
   |
54 | #[embassy_executor::task(pool_size = WEB_TASK_POOL_SIZE)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: consider moving the opaque type's declaration and defining uses into a separate module

On rust version 1.81.0, I now have this error too - seems to be due to something in rust 1.81.0

I've been battling this for a while - using 1.80.0-nightly has resolved the issue, but with anything later I'm encountering this error.

@JuliDi
Copy link

JuliDi commented Sep 15, 2024

As suggested on the Embassy Matrix chat, you need to wrap the type into in its own module like so

mod approuter {
    use picoserve::routing::get;

    pub struct GlobalState {
        pub led_on: bool,
    }

    pub type AppRouter = impl picoserve::routing::PathRouter<GlobalState>;
    pub fn make_app() -> picoserve::Router<AppRouter, GlobalState> {
        picoserve::Router::new().route("/", get(|| async move { "Hello World" }))
    }
}


// and then use it like this e.g.

#[embassy_executor::task(pool_size = WEB_TASK_POOL_SIZE)]
async fn web_task(
    id: usize,
    stack: &'static embassy_net::Stack<Device<'static>>,
    app: &'static picoserve::Router<approuter::AppRouter, approuter::GlobalState>,
    config: &'static picoserve::Config<Duration>,
    state: approuter::GlobalState,
) -> ! {
//...

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