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

Add Bin, Idf, Uf2 support to cargo flash #1765

Merged
merged 17 commits into from
Sep 18, 2023
Merged

Add Bin, Idf, Uf2 support to cargo flash #1765

merged 17 commits into from
Sep 18, 2023

Conversation

tommy-gilligan
Copy link
Contributor

#1758

This is very rough so far. In trying to add UF2 support to cargo flash, it helped to bring across some stuff that was only available on probe-rs so far ie. Bin and Idf support. Is this desirable?

I've 'cheated' with errors for now through unwrap. Should I create more error types or use anyhow? For now I've just deleted a now unused error type. 44d69bfdbec2d768aa388dbb3f142b3b7cb033af

I've used my own crate for UF2 but I'm not married to it. I haven't evaluated other options very well yet and my crate does not have very good support for multiple sections.

@tommy-gilligan tommy-gilligan mentioned this pull request Sep 17, 2023
Copy link
Member

@Yatekii Yatekii left a comment

Choose a reason for hiding this comment

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

Thanks a lot for looking into this!

I added two comments :)

I would add a new error type at least for when the UF2 file does not have a loadable section. The conversion from usize to u32 can stay an unwrap imo :)

I am fine using your UF2 crate if you intend to maintain it. If it ever changes, we can easily take maintainership :)

probe-rs/src/flashing/loader.rs Outdated Show resolved Hide resolved
@tommy-gilligan
Copy link
Contributor Author

Thank you for your quick review!

@Yatekii
Copy link
Member

Yatekii commented Sep 18, 2023

I am pretty much good to merge this after the open question is resolved we can undraft and get this merged :)

@tommy-gilligan
Copy link
Contributor Author

I'm going to do a small second release of uf2 crate ahead of this PR being ready.

@tommy-gilligan
Copy link
Contributor Author

Not sure why cargo deny is unhappy. Can't see any changes that would impact it. Otherwise this is ready now. Let me know if you'd like me to squash things down.

@tommy-gilligan tommy-gilligan marked this pull request as ready for review September 18, 2023 14:18
Copy link
Member

@Yatekii Yatekii left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Yatekii Yatekii added this pull request to the merge queue Sep 18, 2023
Merged via the queue into probe-rs:master with commit 67c2efb Sep 18, 2023
10 checks passed
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

2 participants