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

Extract picotool binary info to a separate rp-binary-info crate. #830

Merged
merged 2 commits into from
Aug 17, 2024

Conversation

Dirbaio
Copy link
Member

@Dirbaio Dirbaio commented Aug 16, 2024

I'd like to reuse this from Embassy instead of copypasting
it, so as discussed in Matrix I'm opening a PR to etract it to a separate crate.

Having a separate crate causes a few changes to the public API. I think they're
nice on their own:

  • Macros are now exported from the binary_info module, and no longer have a prefix: rp2040_hal::binary_info::rp_program_name!, vs previously rp2040_hal::binary_info_rp_program_name!.
  • The binary-info feature makes the entire rp2040_hal::binary_info unavailable, instead of just not declaring the static (since the crate is not compiled at all).

@thejpster
Copy link
Member

All makes sense to me, but I would like to see a feature to control the static within the new crate - it might be useful to have the crate, but you want to create your own static to put it in a differently named linker section, for example.

@thejpster
Copy link
Member

Also, I've re-worded the README in the bit discussing licenseing, so please could you pick up the new wording? See https://github.com/rp-rs/rp-hal/blob/main/README.md#license

@thejpster
Copy link
Member

This fell over bluss/arrayvec#273 - re-running with 0.7.6

rp2040-hal/src/lib.rs Outdated Show resolved Hide resolved
@thejpster
Copy link
Member

This looks great - thank you! I think the top-level module needs to always be included though (and props to udeps for catching that we had a dependency that was never used, which tipped me off)

@thejpster thejpster merged commit 103e4fa into rp-rs:main Aug 17, 2024
6 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.

2 participants