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

0.4 HAL releases may break RAL users #116

Open
mciantyre opened this issue Jan 8, 2022 · 5 comments
Open

0.4 HAL releases may break RAL users #116

mciantyre opened this issue Jan 8, 2022 · 5 comments

Comments

@mciantyre
Copy link
Member

When we release new imxrt-hal peripherals in a backwards-compatible package, we risk breaking users who prefer to mix the HAL with imxrt-ral.

use imxrt_hal as hal;
use imxrt_ral as ral;

// The HAL doesn't support an ADC driver yet.
// We maintain our own, custom driver.
mod custom_adc;

let peripheral = hal::Peripherals::take().unwrap();
let adc = custom_adc::new(
  ral::adc::ADC1::take().unwrap() // <-- panics if the HAL takes the ADC driver for itself.
);

The 0.4.2 HAL release introduced an ADC peripheral. Suppose this user designed to the 0.4.1 HAL, and wanted to implement their own ADC peripheral. When this user upgrades from 0.4.1 to 0.4.2, they observe a panic!() at the indicated call site.

This just hit me while reviewing #115. Looks like the 0.4.2 release is the only example of this breakage in the 0.4 series.

Any thoughts on how to proceed?

@mciantyre
Copy link
Member Author

I think something resembling #92 would be a solution to this issue. If the HAL were a collection of modules designed to the RAL, users could pick and choose which HAL modules they'd use. The HAL could add support for new peripherals in a truly backwards-compatible change.

@mciantyre
Copy link
Member Author

mciantyre commented Jan 8, 2022

Under the current design, we could release new peripherals as breaking changes. This burdens BSP maintainers / end users who aren't mixing the HAL and RAL. They're required to manually update their dependencies to receive new HAL peripherals.

@mciantyre
Copy link
Member Author

mciantyre commented Jan 8, 2022

We could also do nothing and keep going as we have (at least for the 0.4 series). We state that the RAL is our implementation detail, and we're free to break users in this manner.

@shumpohl
Copy link

How about feature-gating new peripherals in backward compatible releases behind a non-default feature?

@mciantyre
Copy link
Member Author

Great idea; HAL features should work perfectly. Thanks for chiming in.

If a BSP simply forwards the HAL's interface, the BSP user is free to enable the HAL's features during build time. (The user could also directly depend on the HAL and enable the feature.) These kinds of HAL features add no extra burden for that BSP maintainer.

# Building firmware that (in)directly depends on imxrt-hal...
cargo build --features=imxrt-hal/my-new-peripheral

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

No branches or pull requests

2 participants