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

Fix: Continue iteration after recoverable errors in SdrIter #23

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

lubinszARM
Copy link
Contributor

Hi dear @datdenkikniet

This PR improve the SdrIter implementation where the iterator would stop iterating upon encountering a recoverable error.

Key changes in this PR:

1 Fix recoverable error handling:

When a recoverable error (IpmiError::ParsingFailed) is encountered, the iterator now skips the current record and continues iterating over subsequent records.
The next_id is updated with the next valid record ID, ensuring the iteration proceeds.

2 Add a limit to recoverable errors:

To prevent potential infinite loops in extreme cases where consecutive recoverable errors occur, a MAX_RECOVERABLE_ERRORS constant is introduced. If the iterator encounters too many recoverable errors, it will log an error and terminate safely.

@lubinszARM
Copy link
Contributor Author

Using the get_info example:
When querying all the SDRs, the iteration will stop in the event of a Recoverable error, as shown in the figure below.
image
After applying this patch, the result looks good:
image
Then, we can get the information of Total_Power successfully.
image

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@datdenkikniet
Copy link
Owner

Thank you very much for your contribution :) Have some minor changes, but overall this is a great change (and much more in the spirit of what I had envisioned it to do :P)

src/lib.rs Outdated Show resolved Hide resolved
@datdenkikniet
Copy link
Owner

One more nit, and please be so kind to add a note in the CHANGELOG.md. For the rest it looks great & about ready to merge!

@lubinszARM
Copy link
Contributor Author

One more nit, and please be so kind to add a note in the CHANGELOG.md. For the rest it looks great & about ready to merge!

Done

@datdenkikniet datdenkikniet merged commit 7ce6a6a into datdenkikniet:main Jan 6, 2025
3 checks passed
@datdenkikniet
Copy link
Owner

Thank you for your contribution :) Merged!

@datdenkikniet
Copy link
Owner

datdenkikniet commented Jan 6, 2025

Released as v0.3.1 on crates.io

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

Successfully merging this pull request may close these issues.

2 participants