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

tool: Long running led_save causes probe to fail #178

Open
ids1024 opened this issue Apr 16, 2021 · 3 comments
Open

tool: Long running led_save causes probe to fail #178

ids1024 opened this issue Apr 16, 2021 · 3 comments

Comments

@ids1024
Copy link
Member

ids1024 commented Apr 16, 2021

I've noticed this in the Configurator with multiple per-key color setting, and tracked down exactly what is needed to reproduce the issue. (The configurator uses probe() to test if a keyboard has been detached).

use ectool::{AccessHid, Ec};
use hidapi::HidApi;

fn main() {
    let api = HidApi::new().unwrap();
    let info = api.device_list().find(|x|
        (x.vendor_id(), x.product_id(), x.interface_number()) == (0x3384, 0x0001, 1)).unwrap();
    let device = info.open_device(&api).unwrap();

    let access = AccessHid::new(device, 10, 100).unwrap();
    let mut ec = unsafe { Ec::new(access).unwrap() };

    for i in 0..=83 {
        let (r, g, b) = if unsafe { ec.led_get_color(i) }.unwrap() == (255, 255, 255) {
            (0, 0, 0)
        } else {
            (255, 255, 255)
        };
        unsafe { ec.led_set_color(i, r, g, b) }.unwrap();
    }
    unsafe { ec.led_save() }.unwrap();
    unsafe { ec.probe() }.unwrap();
}

This outputs:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Signature((0, 0))', src/main.rs:22:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Not sure what's going on, since my understanding was that led_save should block until complete, so there shouldn't be an issue sending commands after it.

@jackpot51
Copy link
Member

Is this specific to Launch or does it also happen on laptops?

@ids1024
Copy link
Member Author

ids1024 commented Apr 16, 2021

It should be Launch specific since led_save currently isn't supported in the ec firmware.

@ids1024
Copy link
Member Author

ids1024 commented Apr 19, 2021

I seem to have figured out what is going on. When setting all the keys like this, led_save (unsurprisingly) takes longer than the 100ms timeout. So it is retried 3 times, before it receives a response. The probe call then ends up receiving the response one of the led_save calls, which naturally doesn't have the right value.

A higher timeout results in led_save succeeding the first time and probe working correctly.

I guess the retries functionality in AccessHid isn't working correctly.

ids1024 added a commit to pop-os/keyboard-configurator that referenced this issue Apr 19, 2021
This sometimes timed out with `led_save`, which caused issues:
system76/ec#178
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