Skip to content

Conversation

@MartinBroers
Copy link

@MartinBroers MartinBroers commented Dec 24, 2025

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • [x ] I have updated existing examples or added new ones (if applicable).
  • [ x] I have used cargo fmt command to ensure that all changed code is formatted correctly.
  • I have used cargo clippy command to ensure that all changed code passes latest Clippy nightly lints.
  • [x ] My changes were added to the CHANGELOG.md in the proper section.

Pull Request Details 📖

Description

In rust, we should never crash in a Drop implementation. The unwrap in the SpiDriver Drop implementation causes a crash when the SPI is not correctly initialized (in my case, when initializing the W5500 ethernet peripheral, the spi device is partially initialized, but the ethernet driver unwraps when it encounters an error calling this Drop).

So I changed the unwrap into an error log. I am not sure though how exactly we should handle this. So I am open to changing this :)

Testing

I have tested this with an ESP without W55000 connected and happy flow with one connected.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Jan 1, 2026

@MartinBroers Thanks for this PR.

I suggest to turn the PR the other way around:

  • If the SPI Driver cannot be initialized correctly, its new constructor should return an error, by also properly disposing all C driver resources.

We should not allow partially initialized driver in the first place, which is the crux of the issue.

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