Skip to content

feat(asic-rs): use asic-rs components to help classify errors#363

Open
b-rowan wants to merge 1 commit into
block:mainfrom
b-rowan:feat/use-error-component
Open

feat(asic-rs): use asic-rs components to help classify errors#363
b-rowan wants to merge 1 commit into
block:mainfrom
b-rowan:feat/use-error-component

Conversation

@b-rowan
Copy link
Copy Markdown
Contributor

@b-rowan b-rowan commented Jun 2, 2026

Updates error classification to use the new MinerComponent to help classify errors when they come from asic-rs.

Also adds some tests for different edge cases in this new scenario, such as errors with or without components, and with or without error messages.

Fixes: #312

@b-rowan b-rowan requested a review from a team as a code owner June 2, 2026 17:01
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file server labels Jun 2, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d962217e0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugin/asicrs/src/device.rs Outdated
@b-rowan
Copy link
Copy Markdown
Contributor Author

b-rowan commented Jun 2, 2026

Worth mentioning this does lose some very slight granularity on error levels, namely, messages from asic-rs will no longer be classed as Major, only Critical, Minor, and Info. Up for some discussion on this part, since this could possibly be reverted to parsing directly from the message strings.

@b-rowan b-rowan force-pushed the feat/use-error-component branch from d962217 to 24f0537 Compare June 2, 2026 17:07
Fixes: block#312

Signed-off-by: Brett Rowan <121075405+b-rowan@users.noreply.github.com>
@b-rowan b-rowan force-pushed the feat/use-error-component branch from 24f0537 to 624a008 Compare June 2, 2026 17:24
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 624a008090

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1078 to +1080
Some(MinerComponent::HashBoard { chip_idx: None, .. }) => {
pb::MinerError::HashboardNotPresent
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don't turn every hashboard component into a missing-board error

When asic-rs supplies component: Some(MinerComponent::HashBoard { chip_idx: None, .. }), this identifies the component the vendor message belongs to, but it does not mean the board is absent. With this branch, any hashboard-scoped message that does not contain the few early text keywords—e.g. a board voltage/current/protection warning or another vendor-specific board fault—gets reported as HashboardNotPresent, which is a different diagnosis and can send operators toward replacing/reseating a board instead of seeing an unmapped board error. Use the component to set component_type, but only choose HashboardNotPresent when the message/code actually indicates absence.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this, I suppose it depends on the error code, but likely not a better way to classify errors. Can take a more in depth look into the different error types, but not sure if there is a better option here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

asicrs plugin loses per-chip / per-slot error detail because component_id is never populated

1 participant