Open reportingsjr opened 6 years ago
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.
@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?
@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.
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.
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.
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 https://github.com/japaric/embedded-hal/issues/42#issuecomment-369695110.
That sounds reasonable @japaric. I'll flesh out the trait and an implementation in the next couple of weeks.
Note that another good reason for 1-wire being a trait is that it can be implemented using Linux driver (e.g. on RPi).
Maybe @kellerkindt and @rnestler might be interested in this discussion :slightly_smiling_face:
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?
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.
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.
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
embedded-opendrain
for
impl<T: InputPin+OutputPin> BitDriver for T
embedded-spi
for
impl<T: FullDuplex<u8>> ByteDriver for T
linux-gpio
for... well however that one is supposed to look like :smile: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).
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.
A couple of observations when trying to use 1-wire in rust:
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
@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.
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?
@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.
Thanks. I'll try to get my hands on a ds2482 and test it out.
I'd like to propose a trait for a bit banged 1-wire protocol. It is a fairly simply protocol that uses a single pin for communications between one master device and many slaves devices. Because of this #29 is currently a blocker.
The protocol for a master device itself primarily consists of the following:
There is some other common functionality that afaik most 1-wire devices use (CRC, a sort of binary search to identify all of the devices on one bus, etc). I haven't worked with 1-wire enough to know what all should be included in the embedded-hal API vs in a device driver. I think the CRC, search, and a couple of other things should probably go in embedded-hal.
I'm not sure it is worth implementing both master and slave APIs. I've only seen a few projects implement a slave in software since it has some pretty big drawbacks. It would have to be either an interrupt driven API or would have to continuously poll the IO pin. Thoughts on this?
Since this is a bitbang protocol and we are doing all of the timing in software it might need to block in some of these functions. Admittedly the timing has fairly loose requirements, but I'm not sure how well using the nb crate here to prevent blocking will work across different speed embedded devices (e.g. using yeild/generators on a fast STM32 vs on a atmega328). @tib888 wrote an implementation of 1-wire for RTFM using atomic blocks to handle this.
I'll start working on an example trait and implementation for this next week, but I want to see what sort of scope embedded-hal will have with regards to the extra functionality and I'd like opinions on the timing/blocking issue.
Some references: Maxim app note on bitbanging 1-wire: https://www.maximintegrated.com/en/app-notes/index.mvp/id/126 Maxim app note on the search algorithm: https://www.maximintegrated.com/en/app-notes/index.mvp/id/187 Some more indepth info on the protocol timing: https://www.maximintegrated.com/en/app-notes/index.mvp/id/74