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

Make SPI Error support defmt::Format #522

Closed
wants to merge 2 commits into from

Conversation

henrikssn
Copy link
Contributor

Summary

Make SPI Error support the defmt::Format trait (behind the defmt feature)

Checklist

  • CHANGELOG.md for the BSP or HAL updated
  • All new or modified code is well documented, especially public items
  • No new warnings or clippy suggestions have been introduced (see CI or check locally)

@TDHolmes
Copy link
Contributor

Can we do this for thumbv7em too, as well as perhaps other peripheral's Error enums?

@bradleyharden
Copy link
Contributor

#467 will unify the implementation for the different chips, so that won't be an issue anymore. It might make sense to merge that one, since it's ready, and apply this PR to the new code.

@henrikssn
Copy link
Contributor Author

#467 will unify the implementation for the different chips, so that won't be an issue anymore. It might make sense to merge that one, since it's ready, and apply this PR to the new code.

Sure, I'll rebase when that is merged.

Can we do this for thumbv7em too, as well as perhaps other peripheral's Error enums?

Let's keep this to SPI for now (since that is what I need), we can open separate PR(s) for the rest on an as-needed basis.

@bradleyharden
Copy link
Contributor

@henrikssn, you probably won't be able to rebase. Might be easier to just add it fresh. But #467 has been merged now

@henrikssn
Copy link
Contributor Author

@henrikssn, you probably won't be able to rebase. Might be easier to just add it fresh. But #467 has been merged now

Done, please take another look.

@TDHolmes now also added this for v1 SPI, thumbv7m and I2C.

@bradleyharden
Copy link
Contributor

@henrikssn, I think you didn't pull the latest changes when you updated this PR, because your commits modify files in the thumbv6m/sercom/v2 and thumbv7em/sercom/v2 directories, which no longer exist.

If you update it, we can merge

@henrikssn
Copy link
Contributor Author

@henrikssn, I think you didn't pull the latest changes when you updated this PR, because your commits modify files in the thumbv6m/sercom/v2 and thumbv7em/sercom/v2 directories, which no longer exist.

If you update it, we can merge

I have lots of things going on right now, I'll try to get to it "soon". Please bare with me :)

@bradleyharden
Copy link
Contributor

No worries. I didn't even notice this PR for the past 6 weeks, so...

@bradleyharden
Copy link
Contributor

@henrikssn, still interested in following up with this? We're all busy, so 9 months isn't that long of a wait :p

@jbeaurivage jbeaurivage added the duplicate This issue or pull request already exists label May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants