Skip to content

Commit

Permalink
Avoid abbreviations in API (#2844)
Browse files Browse the repository at this point in the history
* Avoid abbreviations in API

* changelog/migration
  • Loading branch information
MabezDev authored Dec 19, 2024
1 parent f1c372f commit f83ab23
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 21 deletions.
3 changes: 3 additions & 0 deletions documentation/API-GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ In general, the [Rust API Guidelines](https://rust-lang.github.io/api-guidelines
- 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.

## Maintainability

Expand Down
2 changes: 2 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated `esp-pacs` with support for Wi-Fi on the ESP32 and made the peripheral non virtual
- `SpiBitOrder`, `SpiDataMode`, `SpiMode` were renamed to `BitOder`, `DataMode` and `Mode` (#2828)
- `crate::Mode` was renamed to `crate::DriverMode` (#2828)
- Renamed some I2C error variants (#2844)

### Fixed

- Xtensa devices now correctly enable the `esp-hal-procmacros/rtc-slow` feature (#2594)
Expand Down
17 changes: 17 additions & 0 deletions esp-hal/MIGRATING-0.22.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,20 @@ is not compatible with the hardware.
- spi_dma.transfer(dma_rx_buf, dma_tx_buf);
+ spi_dma.transfer(5, dma_rx_buf, 5, dma_tx_buf);
```

## I2C Error changes

To avoid abbreviations and contractions (as per the esp-hal guidelines), some error variants have changed

```diff
- Error::ExecIncomplete
+ Error::ExecutionIncomplete
- Error::CommandNrExceeded
+ Error::CommandNumberExceeded
- Error::ExceedingFifo
+ Error::FifoExceeded
- Error::TimeOut
+ Error::Timeout
- Error::InvalidZeroLength
+ Error::ZeroLengthInvalid
```
44 changes: 23 additions & 21 deletions esp-hal/src/i2c/master/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,35 +91,37 @@ const MAX_ITERATIONS: u32 = 1_000_000;
#[non_exhaustive]
pub enum Error {
/// The transmission exceeded the FIFO size.
ExceedingFifo,
FifoExceeded,
/// The acknowledgment check failed.
AckCheckFailed,
/// A timeout occurred during transmission.
TimeOut,
Timeout,
/// The arbitration for the bus was lost.
ArbitrationLost,
/// The execution of the I2C command was incomplete.
ExecIncomplete,
ExecutionIncomplete,
/// The number of commands issued exceeded the limit.
CommandNrExceeded,
CommandNumberExceeded,
/// Zero length read or write operation.
InvalidZeroLength,
ZeroLengthInvalid,
}

impl core::error::Error for Error {}

impl core::fmt::Display for Error {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Error::ExceedingFifo => write!(f, "The transmission exceeded the FIFO size"),
Error::FifoExceeded => write!(f, "The transmission exceeded the FIFO size"),
Error::AckCheckFailed => write!(f, "The acknowledgment check failed"),
Error::TimeOut => write!(f, "A timeout occurred during transmission"),
Error::Timeout => write!(f, "A timeout occurred during transmission"),
Error::ArbitrationLost => write!(f, "The arbitration for the bus was lost"),
Error::ExecIncomplete => write!(f, "The execution of the I2C command was incomplete"),
Error::CommandNrExceeded => {
Error::ExecutionIncomplete => {
write!(f, "The execution of the I2C command was incomplete")
}
Error::CommandNumberExceeded => {
write!(f, "The number of commands issued exceeded the limit")
}
Error::InvalidZeroLength => write!(f, "Zero length read or write operation"),
Error::ZeroLengthInvalid => write!(f, "Zero length read or write operation"),
}
}
}
Expand Down Expand Up @@ -204,7 +206,7 @@ impl embedded_hal::i2c::Error for Error {
use embedded_hal::i2c::{ErrorKind, NoAcknowledgeSource};

match self {
Self::ExceedingFifo => ErrorKind::Overrun,
Self::FifoExceeded => ErrorKind::Overrun,
Self::ArbitrationLost => ErrorKind::ArbitrationLoss,
Self::AckCheckFailed => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Unknown),
_ => ErrorKind::Other,
Expand Down Expand Up @@ -643,7 +645,7 @@ impl<'a> I2cFuture<'a> {
}

if r.time_out().bit_is_set() {
return Err(Error::TimeOut);
return Err(Error::Timeout);
}

if r.nack().bit_is_set() {
Expand Down Expand Up @@ -1337,7 +1339,7 @@ impl Driver<'_> {
let max_len = if start { 254usize } else { 255usize };
if bytes.len() > max_len {
// we could support more by adding multiple write operations
return Err(Error::ExceedingFifo);
return Err(Error::FifoExceeded);
}

let write_len = if start { bytes.len() + 1 } else { bytes.len() };
Expand Down Expand Up @@ -1386,7 +1388,7 @@ impl Driver<'_> {
I: Iterator<Item = &'a COMD>,
{
if buffer.is_empty() {
return Err(Error::InvalidZeroLength);
return Err(Error::ZeroLengthInvalid);
}
let (max_len, initial_len) = if will_continue {
(255usize, buffer.len())
Expand All @@ -1395,7 +1397,7 @@ impl Driver<'_> {
};
if buffer.len() > max_len {
// we could support more by adding multiple read operations
return Err(Error::ExceedingFifo);
return Err(Error::FifoExceeded);
}

if start {
Expand Down Expand Up @@ -1579,7 +1581,7 @@ impl Driver<'_> {

tout -= 1;
if tout == 0 {
return Err(Error::TimeOut);
return Err(Error::Timeout);
}

embassy_futures::yield_now().await;
Expand Down Expand Up @@ -1607,7 +1609,7 @@ impl Driver<'_> {

tout -= 1;
if tout == 0 {
return Err(Error::TimeOut);
return Err(Error::Timeout);
}
}
self.check_all_commands_done()?;
Expand All @@ -1623,7 +1625,7 @@ impl Driver<'_> {
let cmd = cmd_reg.read();

if cmd.bits() != 0x0 && !cmd.opcode().is_end() && !cmd.command_done().bit_is_set() {
return Err(Error::ExecIncomplete);
return Err(Error::ExecutionIncomplete);
}
}

Expand All @@ -1646,7 +1648,7 @@ impl Driver<'_> {
if #[cfg(esp32)] {
// Handle error cases
let retval = if interrupts.time_out().bit_is_set() {
Err(Error::TimeOut)
Err(Error::Timeout)
} else if interrupts.nack().bit_is_set() {
Err(Error::AckCheckFailed)
} else if interrupts.arbitration_lost().bit_is_set() {
Expand All @@ -1657,7 +1659,7 @@ impl Driver<'_> {
} else {
// Handle error cases
let retval = if interrupts.time_out().bit_is_set() {
Err(Error::TimeOut)
Err(Error::Timeout)
} else if interrupts.nack().bit_is_set() {
Err(Error::AckCheckFailed)
} else if interrupts.arbitration_lost().bit_is_set() {
Expand Down Expand Up @@ -2181,7 +2183,7 @@ fn add_cmd<'a, I>(cmd_iterator: &mut I, command: Command) -> Result<(), Error>
where
I: Iterator<Item = &'a COMD>,
{
let cmd = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?;
let cmd = cmd_iterator.next().ok_or(Error::CommandNumberExceeded)?;

cmd.write(|w| match command {
Command::Start => w.opcode().rstart(),
Expand Down

0 comments on commit f83ab23

Please sign in to comment.