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

Example comms-01 doesn't call ping_handler on embedded device #75

Open
davidmpye opened this issue Jan 4, 2025 · 2 comments
Open

Example comms-01 doesn't call ping_handler on embedded device #75

davidmpye opened this issue Jan 4, 2025 · 2 comments

Comments

@davidmpye
Copy link

davidmpye commented Jan 4, 2025

Hi,

I have been testing the comms-01 example (both client and fw) on a standard Pi Pico.

The examples compile and run, and appear to work fine at first glance:


Pinging with 0... got 0!
Pinging with 1... got 1!
Pinging with 2... got 2!
Pinging with 3... got 3!
Pinging with 4... got 4!
Pinging with 5... got 5!
Pinging with 6... got 6!
Pinging with 7... got 7!
Pinging with 8... got 8!
Pinging with 9... got 9!
App is done

However, if I edit the Pico firmware to modify the ping_handler so it either returns a fixed value, or even just loop{}s, the client still reports 9 successful incrementing pings.

eg

fn ping_handler(_context: &mut Context, _header: VarHeader, rqst: u32) -> u32 {

    info!("ping");
     4
}

or even :


fn ping_handler(_context: &mut Context, _header: VarHeader, rqst: u32) -> u32 {
    loop{}
}

As far as I can see, the ping_handler function doesn't appear to be getting run, but the client still thinks it's receiving an incrementing result. Any ideas welcome, as I am somewhat baffled...!

David

@davidmpye
Copy link
Author

Ah, I think I've figured this out.

It appears there's a built in :
standard_icd::{PingEndpoint, WireError, ERROR_PATH},

that is the one that the Workbook Client is using, so it isn't actually using the Ping Endpoint provided by the ping_handler in the client code.

I'll have a think about how to make it clearer that that's what is happening and maybe try to format it into a pull req.

@jamesmunns
Copy link
Owner

Ah! Yes, sorry, that should definitely be more clear. There are currently two "baked in" handlers:

  • Ping
  • Schema discovery

For Poststation, there's also a mandatory handler, "get unique id", but that is not "baked in".

We should definitely remove the PingHandler in the example - that was a mistake not to remove when I moved that to be "baked in". Let me know if you still plan to PR this, or I will remove it soon.

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

2 participants