Guide for driver authors and `embedded-hal` users

[ryankurte]

It would be good to have a guide for driver authors and other users of embedded hal providing best practices for defining and consuming drivers, and detailing the use of generics in both (as this can be a complex gotcha for new rust users).

Related to https://github.com/rust-embedded/embedded-hal/issues/135, https://github.com/rust-embedded/wg/issues/411

[markhildreth]

Hopefully this is a good place to ask a question that I hope the docs could cover; how should hal implementations, device drivers and user code deal with concurrency? If the answer is "in the user code" (which seems reasonable), this would be nice to hint on in the docs how one can plug the limited resources given by a HAL implementation into the ownership expectations of drivers.

Take this code that I have written to use an I2C LCD driver:

let i2c = hal::i2c_master(
    &mut clocks,
    270.khz(),
    peripherals.SERCOM3,
    &mut peripherals.PM,
    pins.sda,
    pins.scl,
    &mut pins.port,
);  

let delay = Delay::new(core.SYST, &mut clocks);
let lcd = LCDImpl::new(LCDDriver::new_i2c(i2c, LCD_I2C_ADDRESS, delay))

The LCD driver will now take ownership of both the Delay and I2C resources provided by the HAL implementation. This problem has been discussed before for busses in https://github.com/rust-embedded/embedded-hal/issues/35.

However, it seems like Delay would have a similar issue; if multiple drivers required to sleep for certain amounts of time, they can't all own the single Delay resource provided by the HAL implementation. Are there patterns that is typically used in these cases?

[ryankurte]

yeah concurrency is a wee bit tricky, unfortunately i haven't dug into RTIC to comment on that part.

the best approach for sharing peripherals is Rahix/shared-bus, though there are some outstanding issues with transaction ordering and locking that can come into play with SPI but i do not think these are an issue for I2C.

i don't know about the HAL you're using but afaik Delay doesn't need exclusive ownership of any underlying resources so should be trivially clone-able rather than shared between drivers.

[markhildreth]

@ryankurte Thanks for confirming shared-bus. But I'm not sure I understand what you mean by this:

but afaik Delay doesn't need exclusive ownership of any underlying resources so should be trivially clone-able rather than shared between drivers.

I'm using the atsamd HAL. Here is the Delay implementation. I looked at some of the HALs in the stm32-rs organization, and they seem to be similar (here's the stm32f3xx).

They all appear to be using the SysTick peripheral, which itself requires access to a number of registers.

[ryankurte]

Ahh, I see you point! IMO this seems like a slight oversight on the Delay implementation front.

In embedded C it's pretty normal to have a SysTick interrupt handler that increments a counter variable (usually at 1kHz), then any delay_ms implementations only need to read this value, which should be reasonably implementable in rust (though will require atomics on an M3 or unsafe and maybe interrupt guards on an M0)

[therealprof]

As long as the counter is not reconfigurable after initialisation, Copy/Clone should be reasonable. The only guarantee delay provides is that the delay is at least as long as requested. Even if you interrupt and do something else and the timer wraps aroung in the meantime, this guarantee still holds.

[markhildreth]

The reason that I ask about all of this is that I noticed recently an LCD driver implementation that uses Delay, seeing that Delay implementations are providing single instances, changing their APIs to not take ownership of the Delay instance. This led to (IMO) a less ergonomic API, as you would need to pass it in every call.

It seems like in this specific case, the HAL implementations could provide at least one instance of something that implements Delay for multiple owners, thus making things easier on driver implementors. So, to bring this back around to my more general question:

Would it be true to say that device driver implementations should assume that there will be an implementation of the HAL trait that can be owned when making decisions as to their API? Or is that more a trait-specific thing?

Would it be true to say that HAL implementors should think about how they can provide multiple instances/references of a trait, as has been thought about in the above Delay example?

These are the types of questions I was hoping could be answered in this section of docs (fully realizing that perhaps my question is too general, or the answers too implementation specific).

[ryankurte]

The only guarantee delay provides is that the delay is at least as long as requested.

This is a good point and also, somehow viscerally upsetting 🙃

Would it be true to say that device driver implementations should assume that there will be an implementation of the HAL trait that can be owned when making decisions as to their API?

Yep, basically all drivers make this assumption and to not do so, as you note, creates a nightmarish API.

Would it be true to say that HAL implementors should think about how they can provide multiple instances/references of a trait, as has been thought about in the above Delay example?

This one is much more dependent on the trait. Where it's possible to have multiple implementations (ie. DelayX) this would be a good idea, in some cases this may be partially feasible (something like where timers have multiple counters that could be used independently), and in others this is likely not possible due to mutual exclusion requirements for which we have shared-bus.

I believe we will look to adopt shared-bus into the rust-embedded project once we have a few more of the SPI related issues ironed out (but that's blocked on some embedded-hal issues and PRs).