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.

Sign up for GitHub

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

Jump to bottom

gimlet-seq: retry I2C errors, or die trying #1611

Merged
merged 3 commits into from
Feb 14, 2024
Merged

gimlet-seq: retry I2C errors, or die trying #1611

merged 3 commits into from
Feb 14, 2024

Commits on Feb 14, 2024

  1. gimlet-seq: retry I2C errors, or die trying (#1611) 

    Currently, the sequencer task uses `unwrap()` on I2C bus operations.
However, these may fail due to transient bus errors, or fail permanently
for reasons wholly outside the sequencer task's control. As discussed in
issue #1205, it's incorrect for the sequencer task to unwap these
errors, as panicking results in the task being restarted. On fatal bus
errors, this means we crash loop.

Instead, this commit changes the `gimlet-seq` task to retry failed I2C
reads/writes up to three times, and, if they still fail, transition to a
permanent failed state, setting the FAULT net to Ignition to indicate
that the Gimlet must be power-cycled.

Fixes #1205
    @hawkw
    hawkw committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    dbe70d5 View commit details
    Browse the repository at this point in the history

  2. gimlet-seq: Move init to ServerImpl::init (#1611) 

    This way, we only construct a `ServerImpl` when the sequencer task is in
a state to serve IPC requests. This isn't all that important, but it's
nice to make it a bit harder to accidentally start a server without
finishing init.
    @hawkw
    hawkw committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    eda7479 View commit details
    Browse the repository at this point in the history

  3. gimlet-seq: Die more politely (#1611) 

    Sleeping for 100ms in a loop is not a particularly efficient way for a
task to die permanently. Thanks to suggestions from @labbott and
@cbiffle, I've changed this so that the sequencer task now dies by
waiting for a notification with an empty notification mask, essentially
waiting forever for a notification that should never happen. This is
much more respectful of other tasks that might still be trying to do
stuff, and would like to be scheduled to do that stuff.

Also, I've cleaned up a couple other things based on some of Cliff's
suggestions. Because we no longer set the state when dying, we can pass
the `jefe` handle into `init` by value, which is a bit nicer.
    @hawkw
    hawkw committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    72c99f3 View commit details
    Browse the repository at this point in the history