DEVELOPER-GUIDELINES.md

esp-hal developers' guidelines

About

This is a living document - make sure to check the latest version of this document.

Note

Not all of the currently existing code follows this guideline, yet.

In general, the Rust API Guidelines apply to all projects in the ESP-RS GitHub organization where possible.

• Especially for public API but if possible also for internal APIs.

Amendments to the Rust API Guidelines

• C-RW-VALUE and C-SERDE do not apply.
• C-COMMON-TRAITS: The set of traits to implement depend on the type and use case. In esp-hal, we can highlight a few such use cases and provide recommendations what should be implemented. If nothing here applies, use your best judgement.
  • Driver structures: Debug
  • Driver configuration: Default, Debug, PartialEq/Eq, Clone/Copy, Hash
    • Clone/Copy depends on the size and contents of the structure. They should generally be implemented, unless there is a good reason not to.
    • The Default configuration needs to make sense for a particular driver, and applying the default configuration must not fail.
  • Error types: Debug, PartialEq/Eq, Clone/Copy, Hash, Error, Display

Construction and Destruction of Drivers

• If a driver requires pins, those pins should be configured using fn with_signal_name(self, pin: impl PeripheralInput<'d>) -> Self or fn with_signal_name(self, pin: PeripheralOutput<'d>) -> Self
  • The with_signal_name functions should be placed in an impl block that is generic over the driver mode (i.e. they shouldn't be constrained to just Blocking or just Async) unless there is a technical reason to do otherwise.
• If a driver supports multiple peripheral instances (for example, I2C0 is one such instance):
  • The driver should not be generic over the peripheral instance.
  • The author must to use crate::any_peripheral to define the "any" peripheral instance type.
  • The driver must implement a new constructor that automatically converts the peripheral instance into the any type.
    • The Instance trait should have an IntoAnyPeripheral supertrait. IntoAnyPeripheral is generated by the any_peripheral! macro. The actual name of the trait depends on the name of the AnyPeripheral.
    • The constructor should take fn new(peri: impl Instance + 'd) and call degrade() to type-erase the peripheral.
• If a driver is configurable, configuration options should be implemented as a Config struct in the same module where the driver is located.
  • The driver's constructor should take the config struct by value, and it should return Result<Self, ConfigError>.
  • The ConfigError enum should be separate from other Error enums used by the driver.
  • The driver should implement fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError>.
  • In case the driver's configuration is infallible (all possible combinations of options are supported by the hardware), the ConfigError should be implemented as an empty enum.
  • Configuration structs should
    • Use the Builder Lite pattern. Prefer deriving procmacros::BuilderLite in order to automatically implement the pattern.
    • Implement Copy if possible.
    • The fields should be private. The BuilderLite macro generates setters and getters automatically.
    • If the configuration is expected to be applied more than once (as, for example, different device configurations on a shared SPI bus may be) and calculating register values based on the configuration is costly, the configuration struct should precompute and store the result of those calculations.
• If a driver implements both blocking and async operations, or only implements blocking operations, but may support asynchronous ones in the future, the driver's type signature must include a crate::Mode type parameter.
• By default, constructors must configure the driver for blocking mode. The driver must implement into_async (and a matching into_blocking) function that reconfigures the driver.
  • into_async must configure the driver and/or the associated DMA channels. This most often means enabling an interrupt handler.
  • into_blocking must undo the configuration done by into_async.
• The asynchronous driver implementation must also expose the blocking methods (except for interrupt related functions).
• Drivers must have a Drop implementation resetting the peripheral to idle state. There are some exceptions to this:
  • GPIO where common usage is to "set and drop" so they can't be changed
  • Where we don't want to disable the peripheral as it's used internally, for example SYSTIMER is used by time::now() API. See KEEP_ENABLED in src/system.rs
  • A driver doesn't need to do anything special for deinitialization and has a PeripheralGuard field which implements the disabling and resetting of the peripheral.
• Consider using a builder-like pattern for driver construction.

Interoperability

• Don't use log::XXX! macros directly - use the wrappers in fmt.rs (e.g. just info! instead of log::info! or importing log::*)!
• Consider implementing common ecosystem traits, like the ones in embedded-hal or embassy-embedded-hal.
  • Where the guidelines suggest implementing Debug, defmt::Format should also be implemented.
    • The defmt::Format implementation needs to be gated behind the defmt feature.
    • see this example
    • e.g. #[cfg_attr(feature = "defmt", derive(defmt::Format))]
  • Implementations of common, but unstable traits (e.g. embassy_embedded_hal::SetConfig) need to be gated with the unstable feature.

API Surface

• API documentation must be provided for every new driver and API.
• Private details should not leak into the public API, and should be made private where technically possible.
  • Implementation details that need to be public should be marked with #[doc(hidden)] and a comment as to why it needs to be public.
    • For the time being, this includes any Instance traits, and State or Info structs as well.
  • Functions which technically need to be public but shouldn't be callable by the user need to be sealed.
    • see this example in Rust's core library
• Any public traits, that must not be implemented downstream need to be Sealed.
• Prefer compile-time checks over runtime checks where possible, prefer a fallible API over panics.
• Follow naming conventions in order to be consistent across drivers - take inspiration from existing drivers.
• Design APIs in a way that they are easy to use.
• Driver API decisions should be assessed individually, don't not just follow embedded-hal or other ecosystem trait crates. Expose the capabilities of the hardware. (Ecosystem traits are implemented on top of the inherent API)
• Avoid type states and extraneous generics whenever possible
  • These often lead to usability problems, and tend to just complicate things needlessly - sometimes it can be a good tradeoff to make a type not ZST
  • Common cases of useless type info is storing pin information - this is usually not required after configuring the pins and will bloat the complexity of the type massively.
• Avoiding &mut self when &self is safe to use. &self is generally easier to use as an API. Typical applications of this are where the methods just do writes to registers which don't have side effects.
• Maintain order consistency in the API, such as in the case of pairs like RX/TX.
• If your driver provides a way to listen for interrupts, the interrupts should be listed in a derive(EnumSetType) enum as opposed to one function per interrupt flag.
• If a driver only implements a subset of a peripheral's capabilities, it should be placed in the peripheral::subcategory module.
  • For example, if a driver implements the slave-mode I2C driver, it should be placed into i2c::slave.
  • This helps us reducing the need of introducing breaking changes if we implement additional functionalities.
• Avoid abbreviations and contractions in the API, where possible.
  • Saving a few characters may introduce ambiguity, e.g SpiTransDone, is it Transmit or Transfer?
  • Common abbreviations, that are well understood such as Dma are perfectly fine.
• Config options should not affect the public API.
  • If you add a new configuration option using esp-config, the added option should not alter the public API of the package being configured.

Maintainability

• Avoid excessive use of macros unless there is no other option; modification of the PAC crates should be considered before resorting to macros.
• Every line of code is a liability. Take some time to see if your implementation can be simplified before opening a PR.
• If you are porting code from ESP-IDF (or anything else), please include a link WITH the commit hash in it, and please highlight the relevant line(s) of code
• If necessary provide further context as comments (consider linking to code, PRs, TRM - make sure to use permanent links, e.g. include the hash when linking to a Git repository, include the revision, page number etc. when linking to TRMs)
• Prefer line comments (//) to block comments (/* ... */)
• Generally, follow common "good practices" and idiomatic Rust style
• All Future objects (public or private) must be marked with #[must_use = "futures do nothing unless you `.await` or poll them"].
• Prefer cfg_if! (or, if the branches just pick between separate values of the same variable, cfg!()) over multiple exclusive #[cfg] attributes. cfg_if!/cfg!() visually divide the options, often results in simpler conditions and simplifies adding new branches in the future.
• When marking an API as unstable:
  • Prefer to use #[instability::unstable].
  • Use the attribute on each public function instead of inherent impl blocks.
• The documentation should contain no more than three primary impl blocks (excluding trait implementations):
  • Blocking: Should be listed first, as it serves as the entry point for creating most drivers.
  • Async: Should appear second in the documentation.
  • Both: A combined impl block for both Blocking and Async.
• Methods inside the impl blocks should be grouped by functionality.

Driver implementation

• If a common Instance trait is used for multiple peripherals, those traits should not have any logic implemented in them.
• The Instance traits should only be used to access information about a peripheral instance.
• The internal implementation of the driver should be non-generic over the peripheral instance. This helps the compiler produce smaller code.
• The author is encouraged to return a static shared reference to an Info and a State structure from the Instance trait.
  • The Info struct should describe the peripheral. Do not use any interior mutability.
  • The State struct should contain counters, wakers and other, mutable state. As this is accessed via a shared reference, interior mutability and atomic variables are preferred.

Modules Documentation

Modules should have the following documentation format:

//! # Peripheral Name (Peripheral Acronym)
//!
//! ## Overview
//! Small description of the peripheral, see ESP-IDF docs or TRM
//!
//! ## Configuration
//! Explain how can the peripheral be configured, and which parameters can be configured
//!
//! ## Usage
//! Explain if we implement any external traits
//!
//! ## Examples
//!
//! ### Name of the Example
//! Small description of the example if needed
//! ```rust, no_run
#[doc = crate::before_snippet!()]
//! ...
//! # Ok(())
//! # }
//! ```
//!
//! ## Implementation State
//! List unsupported features

• If any of the headers is empty, remove it
• When possible, use ESP-IDF docs and TRM as references and include links if possible.
  • In case of referencing an ESP-IDF link make it chip-specific, for example:

    #![doc = concat!("[ESP-IDF documentation](https://docs.espressif.com/projects/esp-idf/en/latest/", crate::soc::chip!(), "/api-reference/peripherals/etm.html)")]
    

  • In case of referencing a TRM chapter, use the crate::trm_markdown_link!() macro. If you are referring to a particular chapter, you may use crate::trm_markdown_link!("#chapter_anchor").
• Documentation examples must be short
  • But must also provide value beyond what the rustdoc generated docs show
    • Showing a snippet of a slightly more complex interaction, for example inverting the signals for a driver
    • Showing construction if it is more complex, or requires some non-obvious precursor steps. Think about this for drivers that take a generic instance to construct, rustdoc doesn't do a good job of showing what concrete things can be passed into a constructor.
  • they are no_run - we cannot run them on a target but we can make sure the contained code compiles
  • avoid unwrap etc. - use the ? operator where possible
  • the before_snippet!() provides some necessary boiler plate for you
  • For more complex scenarios, create an example.
• Use rustdoc syntax for linking to other documentation items instead of markdown links where possible
  • https://doc.rust-lang.org/rustdoc/write-documentation/linking-to-items-by-name.html