Skip to content

Updated logic around sysfs/cdev pin selection #34

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 12, 2020
Merged

Updated logic around sysfs/cdev pin selection #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d801ed
fix cdev/sysfs pin selection
ryankurte Mar 10, 2020
ca7b29f
removed pin exports, updated changelog
ryankurte Mar 11, 2020
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
3 changes: 2 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ repository = "https://github.com/japaric/linux-embedded-hal"
version = "0.3.0"

[features]
default = []
gpio_sysfs = ["sysfs_gpio"]
gpio_cdev = ["gpio-cdev"]

default = [ "gpio_cdev", "gpio_sysfs" ]

[dependencies]
embedded-hal = { version = "0.2.3", features = ["unproven"] }
gpio-cdev = { version = "0.2", optional = true }
Expand Down
6 changes: 3 additions & 3 deletions src/cdev_pin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use core::ops;
//! Linux CDev pin type

/// Newtype around [`gpio_cdev::LineHandle`] that implements the `embedded-hal` traits
///
Expand Down Expand Up @@ -43,15 +43,15 @@ impl hal::digital::v2::InputPin for CdevPin {
}
}

impl ops::Deref for CdevPin {
impl core::ops::Deref for CdevPin {
type Target = gpio_cdev::LineHandle;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl ops::DerefMut for CdevPin {
impl core::ops::DerefMut for CdevPin {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
Expand Down
43 changes: 34 additions & 9 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,20 @@
extern crate cast;
extern crate core;
extern crate embedded_hal as hal;
#[cfg(feature = "gpio_cdev")]
pub extern crate gpio_cdev;
pub extern crate i2cdev;
pub extern crate nb;
pub extern crate serial_core;
pub extern crate serial_unix;
pub extern crate spidev;
pub extern crate serial_unix;
pub extern crate serial_core;
pub extern crate nb;


#[cfg(feature = "gpio_sysfs")]
pub extern crate sysfs_gpio;

#[cfg(feature = "gpio_cdev")]
pub extern crate gpio_cdev;


use std::io::{self, Write};
use std::path::{Path, PathBuf};
use std::time::Duration;
Expand All @@ -37,19 +41,24 @@ use spidev::SpidevTransfer;

mod serial;

#[cfg(feature = "gpio_cdev")]
/// Cdev Pin wrapper module
mod cdev_pin;
pub use serial::Serial;

#[cfg(feature = "gpio_sysfs")]
/// Sysfs Pin wrapper module
mod sysfs_pin;

#[cfg(feature = "gpio_cdev")]
/// Cdev Pin wrapper module
mod cdev_pin;

#[cfg(feature = "gpio_cdev")]
/// Cdev pin re-export
pub use cdev_pin::CdevPin;
pub use serial::Serial;
#[cfg(feature = "gpio_sysfs")]
/// Sysfs pin re-export
pub use sysfs_pin::SysfsPin;


/// Empty struct that provides delay functionality on top of `thread::sleep`
pub struct Delay;

Expand Down Expand Up @@ -107,6 +116,22 @@ impl hal::blocking::delay::DelayMs<u64> for Delay {
}
}


#[cfg(all(feature = "gpio_sysfs", feature = "gpio_cdev"))]
/// Re-export of `sysfs_pin::SysfsPin` when both pin types are enabled
/// This exists to maintain backwards compatibility with existing users
pub type Pin = sysfs_pin::SysfsPin;

#[cfg(all(feature = "gpio_sysfs", not(feature = "gpio_cdev")))]
/// Re-export of `sysfs_pin::SysfsPin` pin type when `gpio_sysfs` feature is selected
pub type Pin = sysfs_pin::SysfsPin;

#[cfg(all(feature = "gpio_cdev", not(feature = "gpio_sysfs")))]
/// Re-export of `cdev_pin::CdevPin` pin type when `gpio_cdev` feature is selected
pub type Pin = cdev_pin::CdevPin;
Copy link
Member

Choose a reason for hiding this comment

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

My gut feeling is that it could make sense to re-export SysfsPin as Pin if the gpio_sysfs feature is enabled. I think I would prefer to not export any symbol with the name Pin that ever points at a different type that is cdev. I think that's probably just going to cause confusion without any real benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's probably fair, I put this in because a) there was a duplicated Pin impl and b) it makes the change non-breaking.

If we're going to break it, does it make sense to have the re-export at all? We could just export whatever is enabled and indicate a preference in the comments?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that would be my preference overall -- With a semver bump I think the move to be explicit about the subsystem being used to talk to the GPIO makes sense. Any breakage with the semver major version is not something I think will be a big deal for users if they are sticking with sysfs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sweet as, updated




/// Newtype around [`i2cdev::linux::LinuxI2CDevice`] that implements the `embedded-hal` traits
///
/// [`i2cdev::linux::LinuxI2CDevice`]: https://docs.rs/i2cdev/0.3.1/i2cdev/linux/struct.LinuxI2CDevice.html
Expand Down
7 changes: 4 additions & 3 deletions src/sysfs_pin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::ops;
//! Linux Sysfs pin type

use std::path::Path;

/// Newtype around [`sysfs_gpio::Pin`] that implements the `embedded-hal` traits
Expand Down Expand Up @@ -53,15 +54,15 @@ impl hal::digital::v2::InputPin for SysfsPin {
}
}

impl ops::Deref for SysfsPin {
impl core::ops::Deref for SysfsPin {
type Target = sysfs_gpio::Pin;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl ops::DerefMut for SysfsPin {
impl core::ops::DerefMut for SysfsPin {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
Expand Down