`1-wire` API about embedded-hal HOT 20 OPEN

I think traits should be in embedded-hal, while trait impls should be in their own crates. I don't think embedded-hal containing many traits would be an issue, since they aren't compiled to anything unless implemented.

from embedded-hal.

It can be a trait but not life in the embedded-hal crate but in its own crate.

As long as there is no available hardware implementation (as there is for i2c, SPI...) to orientate an abstratction on, I also think it souldn't be in embedded-hal It does not make an existing controller functionality accessible by an abstraction layer.

In my point of view, the 1-wire protocol can be implemented easily by using the embedded-hal, either in software by only using then Input+OutputPin trait (cough like here) or as @bergus mentioned by using hardware support.

Putting all possible protocol-trait-definitions into embeded-hal, would also make it quite bloated in the long run (where to stop? include UDP-, TCP-Socket abstractions? QUIC?). The abstractions could go in a separate crate (like onewire-hal?), which might not even need to depend on embedded-hal, and allow implementations via software, hardware support and for linux (as @Kixunil mentioned) by separate crates.

from embedded-hal.

@reportingsjr The main point of the HAL traits is to abstract features and protocols typically implemented in hardware. I am not aware of a 1-Wire implementation in hardware (on the MCU side that is) but if they do exist I'd actually support a trait for it, at least that's where I would draw a line. For a pure software implementation as you described it I'd agree with @thejpster that this is more of a driver implementation, i.e. a consumer of the HAL traits.

from embedded-hal.

Chiming in, I would actually probably vote the other way - if we can build sensor drivers on top of the abstraction, then it would be nice to have at the -hal level. Perhaps we could also provide a default impl of the trait that uses the Timer and IOPin trait objects, that would be nice, but if someone wants to impl onewire themselves, they can always do this.

I don't have strong feelings, but wanted to provide some personal thoughts here.

from embedded-hal.

Note that another good reason for 1-wire being a trait is that it can be implemented using Linux driver (e.g. on RPi).

from embedded-hal.

Maybe @kellerkindt and @rnestler might be interested in this discussion 🙂

from embedded-hal.

This sounds like a very useful driver to build on top of the Timer and IO pin traits, but I'm not sure it qualifies as a trait itself.

from embedded-hal.

https://github.com/kellerkindt/onewire

from embedded-hal.

@thejpster, why don't you think this should be a trait offered in embedded-hal? It is a fairly commonly used protocol similar to I2C for which there is a trait.

I can understand not wanting every protocol to appear as a trait here, but what do you think the line is?

from embedded-hal.

@gauteh wrote:

The implementations do not work for my device, because timing issues is really tricky to get right. The Arduino one-wire library has special implementations for most of the common devices adjusting the timing depending on e.g. how long it takes to enable and toggle a pin.

This is why I investigated the DS2482S-100 - an I2C-to-1-Wire bridge device - for communication. I wrote a driver for this device, representing a 1-wire implementation (aided by hardware), which unfortunately doesn't work together with the existing drivers of 1-Wire devices. A 1-wire trait would have come in handy in this case as well.

from embedded-hal.

1-wire can be implemented not only in software or using timers. UART+DMA is very convenient for generating required waveforms and receiving data. I used this approach for working with DS18B20 (firmware was written in C).
So it would make sense to treat 1-wire just like any other standard interface. After all, SPI can also be implemented in software or using synchronous mode of USART.

from embedded-hal.

Based on what @protomors said it sounds like 1-wire should be a trait that lives here, and the bit bang implementation could live in the bit-bang-hal crate suggested in #42 (comment).

from embedded-hal.

That sounds reasonable @japaric. I'll flesh out the trait and an implementation in the next couple of weeks.

from embedded-hal.

Note that another good reason for 1-wire being a trait is that it can be implemented using Linux driver (e.g. on RPi).

It can be a trait but not life in the embedded-hal crate but in its own crate. Maybe in the https://github.com/rust-embedded/ organization?

I think it's a more general discussion of the scope of embedded-hal and which traits it should include. Should it include traits for almost every protocol / peripheral? Or just the most common ones often found as separate peripheral functions in micro-controllers?

from embedded-hal.

I am not aware of a 1-Wire implementation in hardware

Just saying, the STM32L4 (LP)USARTs support a half-duplex mode where they use a single pin to either transmit or receive. Also their SPI peripherals support a single-data-line mode.

from embedded-hal.

I had the time to look into this a bit further. An separate onewire-hal crate might be total overkill, with a lot of duplicate code regarding CRC, device search and device communication code. The differences in software driven/GPIO, hardware driven/SPI and Linux/GPIO communication is basically how bits and bytes are written and read from the wire, while the rest is the same.

Maybe we can use the cargo feature flag for that. For example, lets assume we have some common driver definition like that:

pub trait Driver {
    fn reset(&mut self);
}

pub trait BitDriver: Driver {
    fn read(&mut self) -> Result<bool, Error>;

    fn write(&mut self, data: bool) -> Result<(), Error>;
}

pub trait ByteDriver: Driver {
    fn read(&mut self) -> Result<u8, Error>;

    fn write(&mut self, data: u8) -> Result<(), Error>;
}

impl<T: BitDriver> ByteDriver for T is trivial, so the actual OneWire implementation only cares about having a ByteDriver:

pub struct OneWire<D: ByteDriver> {
    driver: D
}

impl<D: ByteDriver> OneWire<D> {
    pub fn new(driver: D) -> OneWire<D> {
        OneWire {
            driver,
        }
    }
}

With feature flags, we can now provide predefined driver implementations. For example

• feature embedded-opendrain for

impl<T: InputPin+OutputPin> BitDriver for T

• feature embedded-spi for

impl<T: FullDuplex<u8>> ByteDriver for T

• and feature linux-gpio for... well however that one is supposed to look like 😄

This would allow us to improve one common codebase, maybe provide support for a few sensors while still allowing third parties to implement their own driver.

What are your opinions on that? I did not work with the cargo features yet, so I don't know whether its going to work like that.

EDIT: It could look like that (requires some cleanup).

from embedded-hal.

A couple of observations when trying to use 1-wire in rust:

• There are two main one-wire impls, both not updated for several years.
• Drivers depend on a specific implementation, not a trait
• The implementations do not work for my device, because timing issues is really tricky to get right. The Arduino one-wire library has special implementations for most of the common devices adjusting the timing depending on e.g. how long it takes to enable and toggle a pin.
• It is possible to use Uart to drive one-wire with usually greater success, since timing is controlled by hardware.
• Some devices have hardware one-wire support

It would therefore have greatly helped if there was a one-wire trait that makes very little assumptions about the implementation. Some of the points raised above are some of the best reasons for using embedded-hal.

-- gaute

from embedded-hal.

This is why I investigated the DS2482S-100 - an I2C-to-1-Wire bridge device - for communication. I wrote a driver for this device, representing a 1-wire implementation (aided by hardware), which unfortunately doesn't work together with the existing drivers of 1-Wire devices. A 1-wire trait would have come in handy in this case as well.

Maybe this is the solution I should look into as well, is the driver public somewhere?

from embedded-hal.

@gauteh it is still WIP and not published yet to crates.io. The repository is here, I made it public: https://github.com/bartweber/one-wire-ds2482. Also, I made an attempt to create a one-wire-hal: https://github.com/bartweber/one-wire-hal. The former crate is depending on it.

from embedded-hal.

Thanks. I'll try to get my hands on a ds2482 and test it out.

from embedded-hal.