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

Remove DynamicOneShot trait. #19

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Removed
- Removed `I2cInterface`.
- Removed `reset_internal_driver_state` method.
- Removed `DynamicOneShot` trait.

## [0.2.2] - 2021-07-29

Expand Down
23 changes: 0 additions & 23 deletions examples/trait.rs

This file was deleted.

24 changes: 11 additions & 13 deletions src/channel.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
//! ADC input channels
use crate::{ic, Ads1x1x, BitFlags as BF, Config};

mod private {
pub trait Sealed {}
}
use private::ChannelSelection;

/// Marker type for an ADC input channel.
pub trait ChannelId<T>: private::Sealed {
pub trait ChannelId<T> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should this trait not be sealed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually sealed since the ChannelSelection type is private.

/// Get the channel.
fn channel_id() -> ChannelSelection;
}

macro_rules! impl_channels {
($(#[doc = $doc:expr] $CH:ident => [$($IC:ident),+]),+ $(,)?) => {
#[derive(Debug, Clone, Copy)]
/// ADC input channel selection.
pub enum ChannelSelection {
$(
#[doc = $doc]
$CH,
)+
mod private {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you move this module inside this macro?

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way the ChannelId is sealed by making ChannelSelection private.

#[derive(Debug, Clone, Copy)]
/// ADC input channel selection.
pub enum ChannelSelection {
$(
#[doc = $doc]
$CH,
)+
}
}

$(
#[doc = $doc]
pub struct $CH;

impl private::Sealed for $CH {}

$(
impl<I2C, CONV, MODE> ChannelId<Ads1x1x<I2C, ic::$IC, CONV, MODE>> for $CH {
fn channel_id() -> ChannelSelection {
Expand Down
52 changes: 18 additions & 34 deletions src/devices/mode/oneshot.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Common functions
use crate::{
conversion, devices::OperatingMode, mode, Ads1x1x, BitFlags, ChannelId, ChannelSelection,
Config, DynamicOneShot, Error, ModeChangeError, Register,
conversion, devices::OperatingMode, mode, Ads1x1x, BitFlags, ChannelId, Config, Error,
ModeChangeError, Register,
};
use core::marker::PhantomData;

Expand Down Expand Up @@ -40,14 +40,28 @@ where
I2C: embedded_hal::i2c::I2c<Error = E>,
CONV: conversion::ConvertMeasurement,
{
fn read_inner(&mut self, channel: ChannelSelection) -> nb::Result<i16, Error<E>> {
/// Request that the ADC begin a conversion on the specified channel.
///
/// The output value will be within `[2047..-2048]` for 12-bit devices
/// (`ADS101x`) and within `[32767..-32768]` for 16-bit devices (`ADS111x`).
/// The voltage that these values correspond to must be calculated using
/// the full-scale range selected.
/// See [`FullScaleRange`](enum.FullScaleRange.html).
///
/// Returns `nb::Error::WouldBlock` while a measurement is in progress.
///
/// In case a measurement was requested and after is it is finished a
/// measurement on a different channel is requested, a new measurement on
/// using the new channel selection is triggered.
#[allow(unused_variables)]
pub fn read<CH: ChannelId<Self>>(&mut self, channel: CH) -> nb::Result<i16, Error<E>> {
if self
.is_measurement_in_progress()
.map_err(nb::Error::Other)?
{
return Err(nb::Error::WouldBlock);
}
let config = self.config.with_mux_bits(channel);
let config = self.config.with_mux_bits(CH::channel_id());
let same_channel = self.config == config;
if self.a_conversion_was_started && same_channel {
// result is ready
Expand All @@ -63,34 +77,4 @@ where
self.a_conversion_was_started = true;
Err(nb::Error::WouldBlock)
}

/// Request that the ADC begin a conversion on the specified channel.
///
/// The output value will be within `[2047..-2048]` for 12-bit devices
/// (`ADS101x`) and within `[32767..-32768]` for 16-bit devices (`ADS111x`).
/// The voltage that these values correspond to must be calculated using
/// the full-scale range selected.
/// See [`FullScaleRange`](enum.FullScaleRange.html).
///
/// Returns `nb::Error::WouldBlock` while a measurement is in progress.
///
/// In case a measurement was requested and after is it is finished a
/// measurement on a different channel is requested, a new measurement on
/// using the new channel selection is triggered.
#[allow(unused_variables)]
pub fn read<CH: ChannelId<Self>>(&mut self, channel: CH) -> nb::Result<i16, Error<E>> {
self.read_inner(CH::channel_id())
}
}

impl<I2C, IC, CONV, E> DynamicOneShot for Ads1x1x<I2C, IC, CONV, mode::OneShot>
where
I2C: embedded_hal::i2c::I2c<Error = E>,
CONV: conversion::ConvertMeasurement,
{
type Error = Error<E>;

fn read(&mut self, channel: ChannelSelection) -> nb::Result<i16, Self::Error> {
self.read_inner(channel)
}
}
5 changes: 2 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl BitFlags {
}

pub mod channel;
pub use channel::{ChannelId, ChannelSelection};
pub use channel::ChannelId;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we should expose ChannelSelection publicly.

Copy link
Contributor Author

@reitermarkus reitermarkus Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't.

mod construction;
mod conversion;
pub use crate::conversion::{ConvertMeasurement, ConvertThreshold};
Expand All @@ -224,8 +224,7 @@ mod types;
use crate::types::Config;
pub use crate::types::{
mode, Ads1x1x, ComparatorLatching, ComparatorMode, ComparatorPolarity, ComparatorQueue,
DataRate12Bit, DataRate16Bit, DynamicOneShot, Error, FullScaleRange, ModeChangeError,
SlaveAddr,
DataRate12Bit, DataRate16Bit, Error, FullScaleRange, ModeChangeError, SlaveAddr,
};

mod private {
Expand Down
11 changes: 0 additions & 11 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

use core::marker::PhantomData;

use crate::{private, ChannelSelection};

/// Errors in this crate
#[derive(Debug)]
pub enum Error<E> {
Expand Down Expand Up @@ -235,15 +233,6 @@ pub struct Ads1x1x<I2C, IC, CONV, MODE> {
pub(crate) _mode: PhantomData<MODE>,
}

/// Multi channel One-shot ADC
pub trait DynamicOneShot: private::Sealed {
/// Error type
type Error;

/// Read a measurement
fn read(&mut self, channel: ChannelSelection) -> nb::Result<i16, Self::Error>;
}

#[cfg(test)]
mod tests {
use crate::{FullScaleRange, SlaveAddr};
Expand Down
Loading