libhal-google / libhal-util

libhal utility functions, interface wrappers, and types to help manage usage of embedded resources
Apache License 2.0
1 stars 4 forks source link

Add `hal::bus<hal::interface>` #22

Open kammce opened 2 years ago

kammce commented 2 years ago

Problem

Consider a microcontroller with an spi bus connected to two sensors. Let's assert that relocating the sensors onto another bus is not possible. One of them is a low bandwidth sensor with a max clock rate of 100kHz and the other is a high bandwidth sensor that can communicate at 10MHz. The spi bus can also support any clock rate from 10MHz and below.

The simple way to handle this is to set the spi bus to 100kHz, the slowest clock speed, and everything else should work. The problem is that the high bandwidth sensor gets throttled by this reduced clock speed. If the application needs to pull the data off of the device at high speeds then this one-size-fits-all approach will not work.

Solution

Create some way to have each driver register its configuration settings for the spi or i2c bus such that, each time that particular driver attempts to communicate with their device over SPI, the configuration settings are applied for that particular device. So when the application attempts to talk to the low bandwidth driver it sets the speed of communication to a safe 100kHz. When the application attempts to talk to the high bandwidth device, it drives the speed up to 10MHz.

This solution can also support more settings than just speed such as phase and polarity for SPI.

Potential API

// First template parameter defines which interface we are specializing
// Second parameter is the number of different profiles (could also use a std::span or something rather than a template)
embed::bus<embed::spi, 3> spi_bus;

// embed::bus<T>::profile() returns an implementation of the embed::spi interface
// When the factory functions call `::configure()` on the SPI bus, the configuration is recorded
auto storage = embed::sd_card::create(spi_bus.profile<0>());
auto accelerometer = embed::high_speed_accelerometer::create(spi_bus.profile<1>());
auto compass = embed::low_speed_compass::create(spi_bus.profile<2>());

// When an spi bus profile has its `::transfer()` function called, if it wasn't the last profile to be called, 
// then it first configures the bus for its own profile, then it performs the transfer operations.
kammce commented 2 years ago

The name of this library as well as the API names are totally up for change. These were just some simple names given to illustrate a point.

May even make sense to call this a embed::network like what is available for CAN. But may also make just as much sense to make embed::network into embed::bus and to have a specialization for embed::can

herculanodavi commented 2 years ago

esp-idf has a similar interface to deal with that issue, but in C:

they do: spi_bus_initialize -> spi_bus_add_device -> spi_device_acquire_bus (config assertion before transaction) -> spi_device_polling_transmit

kammce commented 2 years ago

Neat. That seems very much like what I would imagine this as except upside down.

To explain this to any other readers reading this comment thread, it is upside down because you don't have some bus that you pass all of your devices to (spi_bus_add_device in the ESP-IDF example). In our case, we pass an SPI implementation with context regarding how it was last configured it. And each time it performs its work, it ensures that it reconfigures the bus to its specifications. The drivers are none-the-wiser that this is happening.

Also, changing bus speeds is a bit tricky with i2c as that goes against the spec. But it can work if you use an i2c multiplexer. Still thinking of a clever way to make that work.

herculanodavi commented 2 years ago

.take

github-actions[bot] commented 2 years ago

Thanks for taking this issue! Let us know if you have any questions!

kammce commented 2 years ago

As we talked about before, may be a good idea to go with a name like embed::spi_bus for embed::spi and embed::i2c_bus for embed::i2c since they would be so dissimilar.