ec icon indicating copy to clipboard operation
ec copied to clipboard

tool: Long running `led_save` causes probe to fail

Open ids1024 opened this issue 3 years ago • 3 comments

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.

ids1024 avatar Apr 16 '21 21:04 ids1024

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

jackpot51 avatar Apr 16 '21 21:04 jackpot51

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

ids1024 avatar Apr 16 '21 21:04 ids1024

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 avatar Apr 19 '21 16:04 ids1024