🚀 Alpha Launch Interface Review

[kammce]

Go through each and every alpha launch interface and consider the following:

1. Give rationale for why each API must be there, no API is given an exception, and APIs that do not fit should be eliminated.
2. Give rationale for why an API MUST have a boost::leaf::result<T> return type and determine if any APIs can get away without them
3. Give rationale for why each parameter must be in the API

The list of interfaces to go through are the following:

• adc
• can
• counter steady_clock
• dac
• i2c
• input_pin
• interrupt_pin
• motor
• output_pin
• pwm
• serial
• servo
• spi
• timer

[kammce]

embed::i2c::configure()

The problem with configure is that it contains clock rate. Making this particular API public means that any arbitrary driver can change the frequency of the I2C peripheral. This is a problem because we can have a situation where devices can support different clock rates from each other.

For example, say we have 3 drivers, two of which support 100kHz and one that supports 400kHz. Each driver set the i2c configure to their preferred high speed clock rate. Let's assume that in this application the developer did not specify a consistent clock rate for each driver even though they support specifying one over the default. So each driver will set the clock to their maximum rate. Running a bus faster then some devices can handle is against the protocol rules for I2C and the behavior is undefined.

This begs the question, should this API be apart of the interface or not? The current purpose of this API is to be used with embed::bus<i2c> where each driver can get a profile i2c driver to which they can set to their needs. Anytime those profile i2c drivers are accessed, the i2c bus is reconfigured to their specified clock rate. Unlike the version of bus that works for embed::spi the i2c variant would NEED an i2c bus multiplexer to keep the effective bus clock domains separate.

So the choice is to keep this as an API for configuring the device or make setting the clock rate driver specific with their own rules and behaviors. embed::bus for i2c could still work but at a higher cost by using std::function</*...*/>

• Keep current API
• Make configuration device driver specific
  • embed::bus<i2c> can still work but at a higher cost and using polymorphic functions std::function</*...*/>

[kammce]

From my experience working with Linux systems and consumer products, being able to have multiple devices work at differing speeds with i2c may be critical.

Considering that this would only ever work with a I2C multiplexer, maybe it stands to reason that there should be some work put in when designing the bus class to incorporate I2C multiplexers in that design directly. It may even make sense to have a I2C multiplexer Interface with a single function allows you to change which bus you're currently using. That also may be too extreme. It could also be a polymorphic function that takes an index and switches the bus for you. Something like:

auto i2c_driver = /*...*/;
auto i2c_mux(i2c_driver);
auto i2c_bus = embed::bus<embed::i2c>(i2c_driver, 
[&i2c_mux](int p_port) -> boost::leaf::result<void> {
  return i2c_mux.port(p_port);
});

[herculanodavi]

From my experience working with Linux systems and consumer products, being able to have multiple devices work at differing speeds with i2c may be critical.
Do you mean by necessarily adding a i2c mux or by optionally doing something else not cited?

The bus implementation would serve quite well exactly as in the spi situation in my opinion (given that mux selection is included in the bus configuration call), the problem would only reside in the naming for that functionality. While bus works fine for spi, it does not for that i2c mux case, since we are selecting busses.

[kammce]

Ah true. May make sense to simply give them different names then. Something like spi_bus for spi and maybe i2c_mux or i2c_multibus for i2c.

But I think there is enough rationale to keep this API.

[kammce]

embed::serial::attach_interrupt(/* ... */) or ::capacity()

We currently do not have this API but should we make it a hard requirement? The idea of attach interrupt would be that when the capacity of the DMA or interrupt driven serial port fills to a particular percentage, a user defined callback is executed. That callback can then be used to handle pulling data out of the buffer.

When I think about this problem I think about the esp8266 driver. The main thing to understand about that driver, without actually looking at the specifications is that, when you execute a request to a server the amount of data you get back can be enormous. The esp8266 could inject its own handler that manages pulling data out of the serial port and into its own buffer.

BUT! Not all DMA based MCUs can support interrupting at arbitrary fill percentages so thats another thing to be concerned about. What other hand I drop based systems for cereal and immediately tell you when they hit a certain limit.

[kammce]

These are ideas are discussed in the ramblings found here: #252

TL;DR: Here are the main points:

• There is no reasonable way to call capacity() in a driver that would be helpful to the driver's implementation and not simply limit the flexibility of the developer and no reason for an application developer to need the information thus, it shouldn't be apart of the API
• Attach interrupt is simply "checking faster" but powered by hardware. But hardware support for interrupting at arbitrary fill rates of serial isn't consistent. And this can be solved by simply increasing the serial buffer size or check it faster, thus the functionality of this can be handled by the current API set.

[kammce]

Should there be an embed::input_output_pin?

Should there exist an interface just for supporting a pin that can have its direction switched from input to output.

This particular functionality would be useful for developing a bit-bang i2c driver.

In particular, output_pin has an interesting capability from its current API. If you set it as open collector and set the active level to true (lets assume this means high voltage), and then use the level() to get back the state of the pin, this is effectively an input pin. So we already seem to have a means of creating an input/output pin. But not sure if it makes sense to have its own type.

[kammce]

wrt I3C I have no experience, but I mean exactly that as a proposal (input, output, open_drain).

As for changing behavior (push-pull -> open drain), this would likely trigger some configure call, thus we might be looking at a software driver similar to the embed::input_output_pin.

Originally posted by @herculanodavi in #293 (comment)

[kammce]

I really like the idea of having a soft driver implement input-output pin functionality.

But:

behavior (push-pull -> open drain), this would likely trigger some configure call, thus we might be looking at a software driver similar to the embed::input_output_pin.

Would this mean that the output_pin API would still have a means to configure the pin's open drain state? If so, is open_drain_pin a more constrained interface? Or could it also just be a soft driver that limits the APIs for the pin? Because if we have an open_drain_pin it would make sense to eliminate the open_drain configuration for output_pin to reduce overlap.

Originally posted by @kammce in #293 (comment)

[kammce]

Hmmm, may also make sense to name input_output_pin as tristate_pin although thats more cryptic 🤔

Originally posted by @kammce in #293 (comment)

[herculanodavi]

I could be wrong here, but tristate to me means that the pin is either set to high, to low or out of the circuit... if this is the case, then io_pin and tristate_pin should be different concepts.

[kammce]

You are correct. But to be clear the third state of the pin is High-Z, which can be achieved with an open drain that isn't active with pull resistor set to none.

[kammce]

Should i2c::transfer() take an embed::timeout object

The only reason why you'd need to timeout for I2C is if there is a device on the bus that can clock stretch. This is really important if the clock stretching is not deterministic. Thus it seems critical that we support this feature in the API.

[kammce]

I cannot conceive of a way for i2c::transfer() to know to kick itself out of a transfer without some defined information about how long it needs to operate for. Even the use of interrupts doesn't seem to make sense here unless a sort of stack unwinding takes place. So with that, I think it makes sense to update the i2c interface to to include this change.

[herculanodavi]

Makes total sense. This functionality is both in the STM32 CubeHAL and in ESP32 I2C master driver, but I guess both timeouts are in the configuration phase rather than in the transfer call.

[kammce]

Oh interesting. I'm considering this now... I feel that having an adjustable timeout based on what action you are making makes more sense than a general one at configuration.

[kammce]

embed::serial::settings::frame_size

I've been debating if we should even allow anything beyond 8-bits per frame for serial. As far as I can tell the applications for other bit sizes are very limited and removing this from the settings API will reduce the size of serial::configure.

• https://github.com/RishiGupta12/SerialPundit/blob/master/applications/9-bit-data.md
• https://www.avrfreaks.net/forum/5-bit-uart-application-examples

I don't think there are any modern tools that care to use any bit width other than 9 bits. And a particular need arises for differing bit widths, I firmly believe that a new interface should be made for that purpose to not create overlap between these two. Maybe call it embed::ancient_serial

[kammce]

pwm::configure

Maybe this should just be pwm::frequency. Changing frequency could be something that happens quite often and the idea of configurations is that they should stay as they are. But there are entire application where the duty cycle is either 0 or 50% and the frequency is the only thing that changes. 🤔

[kammce]

This was confirmed on Aug 10th 2022 in this video by the team https://www.youtube.com/watch?v=6DH4BEXwn1E

[kammce]

PWM as a whole has been confirm by the team in the same video.

[kammce]

hal::dac + hal::adc + hal::pwm + hal::motor + hal::i2c

All confirmed by team on Aug 10th 2022 by the team see video https://www.youtube.com/watch?v=6DH4BEXwn1E

[kammce]

serial::configure

Just a thought... when would the driver call configure twice? For I2c and SPI configuring the bus may be necessary when talking to different devices. Serial is 1-to-1 normally, except in the cases of RS485 or RS232 but the frame configuration should be the same for all of the devices on the bus.

Every driver will have to ensure that the baud rate and other settings are set at the start of operation. But other than that, there will no longer be a need for configure to execute once more. Thus it puts it into question if the device shouldn't simply be configured with the frame configuration needed before being passed into a device driver. Thus the device driver does not configure the device but simply uses it directly assuming the frame information has been supplied already.

[kammce]

Now if we remove this, what cost do we incur?

1. Passing a serial HAL into a driver now becomes complicated for the application developer. Previously, one could simply pass a serial port directly into the library and the configuration settings are taken care of for you. All the user needs to do is simply check if any errors occurred during the creation of the driver.
2. The application developer will need to know in advance what the settings are for the serial driver and this will have to happen, either with some standard settings structure for which each device uses that structure or through documentation. The documentation method seems like a way to make usage of this HAL annoying and cumbersome. The usage of a standardized setting structure is basically the vtable entry of configure but without the virtual meaning its out of the contract. This would simply mean chaos from the driver developers and for the application developers.
3. The frame setup functionality MUST EXIST FOR ALL SERIAL drivers. Meaning no matter what, this functionality is not a cost that we can eliminate. Any confirming serial driver must be able to change its baud rate and frame properties after startup.

The only benefit from this being off interface is that one could make baud rate calculation and configurations at compile time and simply load up the registers afterwards, but this small optimization does not seem like a good reason to pessimize the strategy for serial based drivers.

[kammce]

Oh also, there is a possibility of needing to change the baud rate and frame format. Specifically when negotiating between two devices or two controllers, where one requests that the data rate is raised to send more data at the start of the program.

The idea is this, two devices start at a defined baud rate of 9600 to negotiate. They send each other their max speeds. They set themselves to the lower of the two. This would require two calls to configure.

Another situation could involve changing changing formats just like the example above, but that would be more rare.

[kammce]

output_pin starting level does not it make sense

This cannot be controlled after the creation of the pin so having a configuration option for it does not make sense.

[kammce]

Should hal::timer have a get_range() function

The idea of get_range() or some equivalent is that the timer can return to the user what it is capable of in terms of scheduling a callback. It can return a pair, or struct indicating the shortest possible schedule time and the longest possible schedule time.

This is in contrast to what the current API has which informs the user via an error return type. But this seems very wrong as error should be considered for exception situations and not for situations where the user simply needs to know the bounds of a driver.

[kammce]

The idea of a "longest" scheduled time can also be extended to the bounds of std::chrono::nanoseconds because no matter what the real bounds of a timer is, any future scheduled time can be reached by adding up the "longest" duration times + the remainder time in order to reach it. For example, if a timer's longest duration is 1s and you want something to execute in 5.5s, simply, internally, schedule a callback that updates a counter. Have this callback, when it is executed, re-schedule itself for 1s until 5s has elapsed. On the last schedule event, schedule the actual callback for 0.5s, the remainder of the time. And thus the time can be achieved.

[kammce]

Decided to can this idea and simply put the responsibility of this in the app developers hands.