Ensure the PAC crates and other private crates are never exposed publicly

[MabezDev]

The PACs will probably never be 1.0, therefore we need to keep it out of the public API in esp-hal.

This is currently exposed in a few places

• interrupt::enable* takes pac::Interrupt
• Some Instance traits expose * const peripheral::RegisterBlock
• esp_hal::peripherals::* implement Deref, which deref to the pac types

[bugadani]

Some Instance traits expose * const peripheral::RegisterBlock

This is annoying because even if we newtyped the RegisterBlock, if third parties try to write a driver using the same Instance (for example an SCCB driver over I2C), they would need access to the PAC.

I guess if we can live with multiple PACs in the same project, we can "just" expose the address, and make the user convert it to "a" PAC peripheral, but I'm worried this might cause problems.

[MabezDev]

I guess if we can live with multiple PACs in the same project, we can "just" expose the address, and make the user convert it to "a" PAC peripheral, but I'm worried this might cause problems.

I think upstream rust would have a stroke at this comment, this completely eliminates pointer provenance :D.

I think this is a use-case we want to support though, SCCB is the perfect example tbh.

[bugadani]

I think upstream rust would have a stroke at this comment, this completely eliminates pointer provenance :D.

svd2rust does that already, we're casting a random number to a typed pointer in the PACs.

[MabezDev]

svd2rust does that already, we're casting a random number to a typed pointer in the PACs.

True, it's no different. I guess it just feels less bad when machine-generated code is doing it 😅.

[Dominaezzz]

Maybe just mark the PAC exposing APIs as unstable indefinitely?

I feel like the moment one needs to write a 3rd party driver, they need to touch the PAC, period.
Obviously hal drivers should prevent the need for 3rd party drivers were possible and expose sensible customization points were practical, but ultimately it's hard to predict everything users may ever want to customize.
(Perhaps we need a good definition of what a 3rd party driver is, as I see SCCB to be a wrapper rather than a 3rd party driver.)
There should probably be some guidelines around how 3rd party drivers interact with the hal.

Adding to the list of exposed PAC is https://github.com/esp-rs/esp-hal/blob/main/esp-hal/src/pcnt/channel.rs#L12 . (My IDE never seems to do the right thing with this, so I'll be happy to see it refactored)

[jessebraham]

I think this issue needs to be expanded, as this is not specific only to the PACs. We additionally re-export riscv/esp-riscv-rt and xtensa-lx/xtensa-lx-rt as well, none of which are stable releases. There may be additional packages for which this is the case as well, will need to do some further investigation.

[bugadani]

Just because the list contains a ?: We're reexporting (almost) all peripherals because the create_peripheral! macro creates types that deref to the PAC peripherals. This means almost all register-related API is publicly exposed.

[MabezDev]

I think this issue needs to be expanded, as this is not specific only to the PACs. We additionally re-export riscv/esp-riscv-rt and xtensa-lx/xtensa-lx-rt as well, none of which are stable releases. There may be additional packages for which this is the case as well, will need to do some further investigation.

I opened #2589. As far as I know, the rt crates aren't exposed in the public API (which is what this issue is tracking), but I agree there are some interactions we need to be aware of with these dependencies.

[bjoernQ]

I think this issue needs to be expanded, as this is not specific only to the PACs. We additionally re-export riscv/esp-riscv-rt and xtensa-lx/xtensa-lx-rt as well, none of which are stable releases. There may be additional packages for which this is the case as well, will need to do some further investigation.

I opened #2589. As far as I know, the rt crates aren't exposed in the public API (which is what this issue is tracking), but I agree there are some interactions we need to be aware of with these dependencies.

TrapFrame / Context is re-exported by the HAL (and needed by esp-wifi) - we could just doc-hide it

[bugadani]

I think we should formulate our stability guarantees before we invest much more time in the "Hide everything PAC-related" question.

Every breaking PAC update will itself be a breaking HAL update as well, because there can only ever be one PAC version in a project. If someone is using PAC version 0.N, but we update the HAL in a minor release to PAC version 0.N+1, projects will stop to build, even while strictly speaking we can say we're semver compatible. In my mind this looks like the PAC is pretty much part of the API surface offered by esp-hal, even if in a somewhat roundabout way.

Can we accept "semver-compatible update breaks something else" as part of our stability guarantees?

[jessebraham]

I'm not sure I follow, we do not make any guarantees with regards to external dependencies. If users are relying on the PAC in addition to the HAL this does not impact semver guarantees of the HAL.

[bugadani]

The issue is that just changing the HAL might break things.

[playfulFence]

I'll go through the items we're stabilizing just to take a look if there's something we reexport from pacs.

Also, this will help me to double-check if we've marked everything what needs to be unstable in our stable modules in #2900 (because I already found the lp_i2c module without the unstable tag haha 😄 )

[playfulFence]

I'll just leave some notes here:

• fugit - still a big problem here. Should we contact its' owner, maybe we can offer some of our resources to make 1.0 possible faster?
• PACs - newtype?

[bjoernQ]

The issue is that just changing the HAL might break things.

I agree with Jesse - as long as the user doesn't add the PAC as a dependency nothing can happen.

If they add a dependency on the PAC they add a dependency on something unstable and have to deal with it

[bugadani]

If they add a dependency on the PAC they add a dependency on something unstable and have to deal with it

This implies that we can - unstably - expose the PAC and that would even make users' lives simpler.

[bjoernQ]

If they add a dependency on the PAC they add a dependency on something unstable and have to deal with it

This implies that we can - unstably - expose the PAC and that would even make users' lives simpler.

Yes - I think so