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

Refactors HID connections to support busy response #699

Merged
merged 5 commits into from
Sep 5, 2024

Commits on Aug 30, 2024

  1. Refactors HID connections to support busy response

    The main intended difference is that OpenSK now responds with ERR_CHANNEL_BUSY
    when it receives a packet while waiting for touch in in a CTAP2 command.
    
    To support this more elegantly, we changed `HidConnection` to have a
    `send` and `recv` instead of `send_and_maybe_recv` for a cleaner API.
    This also resolves an older TODO to respond to incoming channels on
    the other USB endpoint.
    
    It doesn't process the incoming traffic correctly, but unblock the host
    at least, and tells it to come back later. We still only allow one
    active channel at the same time.
    
    We now don't need the `TRANSMIT_AND_RECEIVE` syscall anymore. I didn't
    remove it from the Tock pack for simplicity, but cleaned up
    libtock-drivers.
    
    The Env API changes from having one connection per endpoint, to having
    one send function that takes an endpoint, and a receive function that
    receives on all endpoints at the same time.
    The `HidConnection` already received on all endpoints before, which was
    inconsistent with the existance of, e.g., `VendorHidConnection` being
    able to receive on the main endpoint.
    
    Since we now have a cleaner send and receive API, we use this in
    `src/main.rs` and simplify it a bit, while making it more consistent
    with other calls to the HID API.
    
    I found an additional inaccuracy in our implementation while
    refactoring: We want to send keepalives every 100 ms. To do so, we first
    wait for a button callback for 100 ms. Then we send the keepalive and
    check if a cancel packet appeared. What should have happened instead is
    that we listen for HID packets and button presses at the same time
    during these 100 ms. If nothing happens that stops the loop, we send the
    keepalive.
    
    The old implementation would, in some implementations, wait 200 ms for
    each keepalive: First 100 for touch, then 100 for `send_and_maybe_receive`.
    
    The new implementation can loop faster in case there is a lot of unrelated
    HID traffic. To make the touch timeout last 30 seconds, we introduce an
    extra timer and loop until time is up. This might still make us blink
    the LEDs too fast, but that is already the case in the main loop, and
    generally would need a bigger refactoring.
    The only simple workaround to make the LEDs slightly more precise is to
    wait for touch and packets until it is time to flip the LEDs, and if we
    return too early within some thresholds, wait again. If we care enough,
    we can fix that in a later PR.
    
    Not sure how to make the test work that I removed in ctap/mod.rs. I'd
    need to advance the time while the loop is running. Setting a user
    presence function that advances time seems hard with the Rust borrowing
    rules.
    kaczmarczyck committed Aug 30, 2024
    Configuration menu
    Copy the full SHA
    214b004 View commit details
    Browse the repository at this point in the history

Commits on Sep 3, 2024

  1. Refactors status and error codes

    There are a few small logic fixes hidden in this comit:
    
    - We always call `check_complete`, even if we received a cancel.
    - We only accept Cancel packets from the active channel.
    - Send timeouts now return `Ctap2StatusCode::CTAP1_ERR_TIMEOUT`.
    kaczmarczyck committed Sep 3, 2024
    Configuration menu
    Copy the full SHA
    01d7ee3 View commit details
    Browse the repository at this point in the history

Commits on Sep 5, 2024

  1. Stable blinking pattern

    Before, if the device gets a CTAP2 UP request and another channel sends
    packets, the blinking pattern would have sped up.
    
    The patch for the Tock USB implementation fixes a panic after a timeout
    in send.
    kaczmarczyck committed Sep 5, 2024
    Configuration menu
    Copy the full SHA
    269578b View commit details
    Browse the repository at this point in the history
  2. Better TestClock API

    kaczmarczyck committed Sep 5, 2024
    Configuration menu
    Copy the full SHA
    f4d3969 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    744ab17 View commit details
    Browse the repository at this point in the history