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

Fixed VectActive::from() returning incorrect value #500

Open
wants to merge 6 commits into
base: v0.7.x
Choose a base branch
from

Conversation

skibon02
Copy link

Fixes #499

Fixed VectActive::from(u8) now returns correct value for Interrupt variant
Refactored SCB::vect_active() to use VectActive::from(u8) and get rid of code duplication

@skibon02 skibon02 requested a review from a team as a code owner December 14, 2023 12:58
@newAM
Copy link
Member

newAM commented Dec 14, 2023

Can you rebase this? The CI failure is from an old MSRV

@skibon02
Copy link
Author

@newAM, Could you please explain a little more? I forked the repo and made this commit from the v0.7.x branch.

@newAM
Copy link
Member

newAM commented Dec 14, 2023

Oh, sorry, I didn't see that this was targeting v0.7.x! That makes more sense then. The problem is that serde_json isn't pinned, and the latest version requires edition 2021. I think specifying >=1.00,<=1.0.100 for serde_json should fix the build failure.

@skibon02
Copy link
Author

Oh, unwrap_unchecked seems to not be available on rustc version 1.38.0. It forces me to write a reaction to the case, when icsr register returns unexpected value. Is panic appropriate in this case? Or maybe we should execute udf instruction?

Assuming correct implementation for VectActive::from for all cortex-m processors, this problem can only appear when incorrect target was selected by user, and it is impossible to check this in compile time.

@skibon02
Copy link
Author

Adding these ugly lines in Cargo.toml to fix deps versions did help compiling on 1.38

Or should we better just bump MSRV to 1.60?

@jonathanpallant
Copy link
Contributor

cortex-m-rt says:

This crate is guaranteed to compile on stable Rust 1.59.0 and up

@skibon02
Copy link
Author

But this is cortex-m, not cortex-m-rt
latest update 0.7.7 was published 1 year ago

From v0.7.x branch Readme:

This crate is guaranteed to compile on stable Rust 1.38 and up. It might compile with older versions but that may change in any new patch release.

@jonathanpallant
Copy link
Contributor

But how many people use cortex-m without using cortex-m-rt?

@skibon02
Copy link
Author

cortex-m has twice as many downloads compared to cortex-m-rt
It seems that there isn't much sense in supporting these old versions of the Rust compiler. Moreover, a significant portion of people even uses Nightly.

Should I bump the MSRV to 1.60 and remove fixed dependency versions?

@jonathanpallant
Copy link
Contributor

1.59 makes sense, so they are the same.

unwrap_unchecked first appeared in 1.58 so that's fine.

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.

None yet

4 participants