Should setting baud rate for serial communication be part of this hal?

[kjetilkjeka]

I believe some way to set baud rate for the serial traits is needed.

Relevant problem

Dynamixel servos comes with two different RS-485 protocols that have some difference in support of baud rate. And even if every servo on your RS-485 bus uses the same protocol there should be nothing stopping you from configuring them with different baud rates. This require setting baud rate before communicating with a servo.

This is currently the biggest blocker for having the dynamixel library work for every embedded-hal implementation out of the box (as it currently does with adapters supporing serialport)

[kjetilkjeka]

One possible solution could be to introduce a serial::Serial trait that implements the set_baud_rate(&mut self) -> Result<(), Self::Error>.

Then i could auto implement the dynamixel interface for Serial + Write + Read types

[therealprof]

Hm, so you're using the Serial trait for RS-485 communication? Interesting. Is this application relying on the fact that a RS-232 to RS-485 adapter is available?

I do agree that there're some uses for on-the-fly configuration changes, so I'd be very much in favour for adding this; however we'd need to check for implementation feasibility. There might be chips which only allow such changes through full re-initialisation which may require resources which are not owned but only borrowed.

[kjetilkjeka]

Hm, so you're using the Serial trait for RS-485 communication? Interesting. Is this application relying on the fact that a RS-232 to RS-485 adapter is available?

currently I've only implemented it for this serial trait. I've tested it with an ftdi like rs485 usb dongle.

You could use a RS-485 transceiver directly connected to some uart, meaning it's no need for converting to RS-232. Since this is SW and we don't really care about things as differential-lines and voltage levels the only real difference is that with RS-485 you cannot read and write at the same time.

[therealprof]

@kjetilkjeka That only works with dedicated hardware. When using a MCU and a RS485 transceiver you will need a "Driver Enable" signal. Expecting the user to manage that manually is not very user friendly and some chips, e.g. STM32 MCUs have hardware support for it which is especially handy if one wants to use async IO.

[kjetilkjeka]

The simple way is to connect tx together with the enable line. Then you will get dominant 1s and recessive 0s. That is if the mcu doesn't drive the enable line for you.

I would expect the write/read implementation to handle this. If they don't they're pretty much useless. I'm not suggesting such a thing.

Edit: i dont expect the hal to assume what kind of hw you've connected to the uart. This mist be handled in hw or at the initialization.

[therealprof]

My point is that you'll need specific setup for that case. You can't just use the regular serial impl and it'll work out automagically for RS-485 as well.

[kjetilkjeka]

Is this simply an observation, an argument against adding the set baud method or an argument against using the serial traits for RS485?

Perhaps I'm wrong, but i see the serial traits as a generic serial protocol. Much like the stdlib Read/Write traits can be used on network sockets, files, rs485, etc. I expect the serial traits to work on uart, usart, RS232, RS485, etc. Even for the simple uart case you will need "specific setup" as voltage levels might not be the same for the commuinicating units.

[therealprof]

It's a somewhat unrelated observation that the serial trait may be lacking more details than just the baudrate selection to be useful for various serial protocols.

[austinglaser]

I'm in favor of keeping the base Read and Write traits as simple as possible -- they should have to do with transmitting and receiving bytes on the bus, and basically nothing else.

However, I can also see the need for changing the baud rate (and possibly other parameters) at runtime. I'm going to make the same suggestion we've talked about elsewhere: Add a separate, but composable trait (maybe serial::Configure). Something like:

trait Configure {
    type Freq;

    fn set_baud_rate<T>(&mut self, baud_rate: T) where T: Into<self::Freq>;
}

This doesn't break existing implementations, and leaves it to implementers to decide whether such configuration makes sense for their use-case.

[kjetilkjeka]

In favor of what @austinglaser suggests with the following additions.

• To make the trait useable we must create a way to construct it. What about requiring a fallible conversion from u32 to Freq.
• I'm generally in favor of making functions infallible by only allowing legal arguments to be allowed. I think if we want to support weird baudrates (which we do) we must allow the set_baud_rate to fail.

pub trait ConfigureBaud {
    type BaudRate: TryFrom<u32>;
    type Error;

    fn set_baud_rate(&mut self, baud_rate: Self::BaudRate) -> Result<(), Self::Error>;
}

Another alternative could be to distinguish between interfaces supporting weird and non-weird bauds:

pub enum StandardBaudRate {
    B1200,
    B2400,
    B4800,
    B9600,
    B19200,
    B38400,
    B57600,
    B115200,
}

pub trait ConfigureBaud {
    type BaudRate: From<StandardBaudRate> + TryFrom<u32>;

    fn set_baud_rate(&mut self, baud_rate: Self::BaudRate);
}

pub trait TryConfigureBaud: ConfigureBaud {
    type Error;

    fn try_set_baud_rate(&mut self, baud_rate: u32) -> Result<(), Self::Error>;
}

[therealprof]

This is a rather messy topic and I don't have good solutions in my head at this time. Your "standard" baud rates are ending far too low, 115200 is rather pedestrian nowadays. Also even "standard" baud rates can very often not be represented exactly and also often this depends on the clock the UART or even the whole system is running at.

For me this means it either should be fallible or there must be a way to determine whether a baud rate can be supported or what the next closest supported value would be.

[kjetilkjeka]

This is a rather messy topic and I don't have good solutions in my head at this time. Your "standard" baud rates are ending far too low, 115200 is rather pedestrian nowadays.

These are BaudRates that every unit conforming to the embedded-hal needs to support. And it looks like Rust is going to work on 8-bit avr in the future. Perhaps this is a good argument for dropping the infallible set_baud.

Also even "standard" baud rates can very often not be represented exactly

How big of a problem is this? I would be happy with the "best effort" and "you should probably have your impl fail if it's to far away (2-3% or something)"

and also often this depends on the clock the UART or even the whole system is running at.

I guess the solution here is, If the implementation don't know what kind of freq the uart clock runs at this Trait should (read: can) not be implemented.

---

This alternative would supporting getting an indication of how much off the BaudRate is. But would allow scary things like. interface.set_baud_rate(BaudRate.closest(9999999999)) (the naming is ad-hoc)

#[derive(Debug)]
pub struct UnrepresentableBaud;

pub trait BaudRate: Into<f32>{
    //// Try to crate a `BaudRate` with less than 2% error.
    fn create_approximately(baud_rate: u32) -> Reult<(), UnrepresentableBaud>; 

    //// Create the closest `BaudRate` possible to `baud_rate`.
    fn closest(baud_rate: u32) -> Self;

}

pub trait ConfigureBaud {
    type BaudRate: BaudRate;

    fn set_baud_rate(&mut self, baud_rate: Self::BaudRate);
}

[therealprof]

These are BaudRates that every unit conforming to the embedded-hal needs to support.

Huh?

And it looks like Rust is going to work on 8-bit avr in the future.

Even the ominous ATMega328 officially supports 2 MBaud...

How big of a problem is this? I would be happy with the "best effort" and "you should probably have your impl fail if it's to far away (2-3% or something)"

It's a big problem. I've seen people clock their MCUs specifically for proper serial communication and if this is all automagic then it will cause all sorts of surprises.

I guess the solution here is, If the implementation don't know what kind of freq the uart clock runs at this Trait should (read: can) not be implemented.

Well, often you pass in the relevant clocks when initialising the UART, e.g. from the stm32f103xx-hal:

            impl<PINS> Serial<$USARTX, PINS> {
                pub fn $usartX(
                    usart: $USARTX,
                    pins: PINS,
                    mapr: &mut MAPR,
                    baud_rate: Bps,
                    clocks: Clocks,
                    apb: &mut $APB,
                ) -> Self

So the application sets up the clocks and passes it in so there's no way of knowing whether the baud rate is compatible with the current clocks and now coming back to this topic and my previous concern: If you reconfigure the baud rate on the fly you will most likely have to pass in the very same information again that was used during construction to make the proper calculation (and some chips actually require that you turn the peripheral off again to make such a change) but this is all implementation dependant...

[kjetilkjeka]

So it looks like the temporary spec sheet currently looks:

• Cannot be mandatory to implement just to use Write/Read. (must live in its own trait)
• Support weird baud rates.
• Give an indication of how far away from the desired baud the actual baud is.
• Be explicit whether you need an "exact" baud or approximate baud.
• Be explicit of when the baud rate change occurs (can this affect bytes already called transmit on?)
• Must not require passing any "init" information like uart clock frequency etc.
• Of course, be easy to use correctly and hard to use wrongly.

and some chips actually require that you turn the peripheral off again to make such a change

If this is the case, the reconfigurable baud rate should probably not be implemented (or handle these special concerns in software)

So the application sets up the clocks and passes it in so there's no way of knowing whether the baud rate is compatible with the current clocks and now coming back to this topic and my previous concern: If you reconfigure the baud rate on the fly you will most likely have to pass in the very same information again that was used during construction to make the proper calculation

I guess what you mean is "no way of knowing statically"? As the interface could remember its clock frequency. Then you could simply ask the interface whether the new clock freuquency is compatible or not. For instance with:

#[derive(Debug)]
pub struct UnrepresentableBaud;

pub trait ConfigureBaud {
     fn set_baud_rate(&mut self, baud_rate: u32) -> Result<(), UnrepresentableBaud>;
}

[therealprof]

I guess what you mean is "no way of knowing statically"? As the interface could remember its clock frequency.

The problem with "remembering" is, that the clock might actually change...

At the moment, the most compatible and safest way to change the baud rate would be to drop and reinitialise the serial interface.

[kjetilkjeka]

The problem with "remembering" is, that the clock might actually change...

If the clock changes the current baud rate would also change, therefore needing interaction with the serial implementation (hw driver), this should be used to update the remembered value. If, for some weird reason, the baud rate is controlled by unkown external factors the ConfigurableBaud trait cannot be implemented for that controller.

At the moment, the most compatible and safest way to change the baud rate would be to drop and reinitialise the serial interface.

Since initialization is not the responsibility of this hal this would make it impossible to implement the dynamixel protocol generically for devices supporting this hal.

@therealprof if an implementation was made adhering to my "temp spec sheet" would that be something you would consider to include in the hal (and are there any other concerns that must be made), or would you prefer keeping baud rate configuration out of the hal at the current being?

[therealprof]

Since initialization is not the responsibility of this hal this would make it impossible to implement the dynamixel protocol generically for devices supporting this hal.

True.

@therealprof if an implementation was made adhering to my "temp spec sheet" would that be something you would consider to include in the hal (and are there any other concerns that must be made), or would you prefer keeping baud rate configuration out of the hal at the current being?

As I mentioned before I think it would be useful to have and thus don't have any general objection to include it.

However we need to ensure that the traits are designed in a way that they actually can be implemented in a safe and sane way on common hardware and at the moment I don't see that how that would be possible.

[mvirkkunen]

Another relevant problem:

More than one device that uses an UART interface supports changing the speed on the fly. Usually they start at a low baud rate, and then you can select a higher one by sending a command via the UART. After that you need to switch your end to the higher baud rate too.

Currently there is no way to do this with an embedded-hal serial driver.

[little-arhat]

Somewhat related: mpu9250 (accelerometer & gyroscope sensors) allows using SPI configured at 1Mhz to read/write configuration register, but up to 20Mhz to retrieve readings. To account for that, we've added ability reinit spi after initialization, but it could be easier if embedded-hal provided methods for updating configuration.

[therealprof]

@little-arhat How would you do this safely? A method on an embedded-hal trait can not contain any of the information usually required to do such a change and for most MCUs it can also not be stored in the implementation since it might require access to shared registers.

[little-arhat]

@therealprof that what this issue is about, I guess: should embedded-hal include methods like set_speed(bps) or set_bus_frequency(hz).

[therealprof]

@little-arhat And my question was: Are those methods even (safely) implementable? I have strong doubts that they are (in general at least)...

[little-arhat]

@therealprof I don't know, but do they have to be safe?

For example, this hal implementation has buncf of unsafe calls in Watchdog, OutputPin, InputPin, and maybe others.

On the other hand, stm32 HAL doesn't provide such functions.

[therealprof]

@little-arhat unsafe does not necessarily mean that something is not safe but that the compiler can not reason about the safety by itself so it's up to the software author to do that.

Anyway my point is that for a "safe" implementation you actually need to have exclusive access to the resources you're changing to not stomp on someone elses feet. Usually that means that you have to provide mutable references to every resource required to set up the peripheral in the first place which is typically very unwieldy and thus not expected to happen in the middle of a running application but during initialisation. Also due to the very different nature of the vendor peripherals this is not something that can be easily abstracted over in the vendor independent way which makes it highly unsuitable for embedded-hal.

If someone can show a convincing demonstration that this can be generically defined in a trait and usefully implemented for any real hardware I'd be more than happy to help turn it into an extension trait.

[MathiasKoch]

I'd like to bring this back up.

I have a device that does auto baudrate detection, but only up to 115200 bps, so in order to run it with a higher baud rate, one would have to:

1. Auto baud @ < 115200 bps
2. Set device baud rate to > 115200 bps
3. Reconfigure UART peripheral to baudrate from point 2

I would like this logic to be contained within the drive crate, thus i need a way to reconfigure the baud rate after initial initialization of the UART.

[unizippro]

How does a trait like this look?

pub trait ConfigureBaud {
    type BaudRate;
    type Error;
    fn set_baud_rate(&mut self, baud_rate: BaudRate) -> Result<(), self::Error>;
}

Many of the current hal's have their own type for baudrates, so giving them the opportunity to use them makes sense and gives them the responsibility of supporting 'weird' rates.
Having the option of returning an error you can limit the available bauds to the ones compatible with a given clock setup or else return an error.
As a response to an earlier issue, if a given clock setup changes and there are not made a way of updating the copy held by the Serial interface. The Serial interface baud is lost anyhow.

[barafael]

For e.g. HC-12 serial transceiver module, this would make it possible to change the baud rate (which the module requires for supporting all modes).

[horazont]

Another case in point is using a USART as 1-wire Bus "master". There you need to frequently change the baud rate in order to generate the correct bit patterns.

I think the trait proposal by @unizippro looks good -- note that the BaudRate type used by the implementation could encode information about the clocks if needed, which allows to erase that dependency from the general trait interface.

In addition, I suggest to make the function return nb::Result, in order to allow the impl to wait for an ongoing transmission to complete or any other precondition necessary to change the baud rate.

[ryankurte]

How does a trait like this look?

pub trait ConfigureBaud {
    type BaudRate;
    type Error;
    fn set_baud_rate(&mut self, baud_rate: BaudRate) -> Result<(), self::Error>;
}

seems broadly good to me.

• from a consumer perspective imo it's better to use an u32 value for baud rate, otherwise you have to add a bunch of complexity to abstract over the BaudRate type
• i'd be inclined to return the actual baud instead of () on success to let users know what was achievable with the oscillators available (it's normal for there to be a slight difference between requested and actual bauds due to clocking).

if you would like to contribute this i would suggest going forward with an e-h and demonstration PR, any further trait-specific discussion can occur there ^_^

[Dirbaio]

I'd just make baudrate an u32 (or maybe some Hertz concrete type that is in embedded-hal that wraps an u32 or something)

Having an unconstrained type BaudRate seems like the same mistake in #324, #201, #177 (comment) . Different HALs will use different types among u32, whatever Hertz they have, or stuff from embedded-time or similar, making the trait impossible to use from HAL-independent drivers.

[therealprof]

I have to agree with @ryankurte that this should be fleshed out in a demonstrator PR. I still have strong doubts this is generally implementable without gnarly hacks but would love to be proven wrong.

[horazont]

I implemented a similar trait as PoC based on the stm32f1xx-hal. I had to use the unconstrained (custom) BaudRate type to force the user to go through a function which calculates the correct register value from the current clocks. The Serial in stm32f1xx-hal is mostly a zero-sized type and does not hold a reference to the (frozen) clocks.

It looks roughly like this:

pub trait AsyncSetBaudRate {
	type Error: Debug;
	type BaudRate;

	fn poll_set_baud_rate(
		&'_ mut self,
		rate: &Self::BaudRate,
		cx: &mut Context<'_>,
	) -> Poll<Result<(), Self::Error>>;
}

pub trait BaudRateGenerator<T> {
	fn calc_baud_rate(&self, bps: u32) -> T;
}

#[derive(Clone, Copy)]
pub struct BaudRateConfig {
	config: serial::Config,  // serial from stm32f1xx_hal
	clocks: rcc::Clocks,  // rcc from stm32f1xx_hal
}

pub struct BaudRateFromClocks<'x>(pub &'x rcc::Clocks);

impl<'x> usart::BaudRateGenerator<BaudRateConfig> for BaudRateFromClocks<'x> {
	fn calc_baud_rate(&self, bps: u32) -> BaudRateConfig {
		BaudRateConfig {
			config: serial::Config::default().baudrate(bps.bps()),
			clocks: *self.0,
		}
	}
}

(sorry, I'm a bit in a hurry right now so no proper minimal test code)

I'm not convinced this is the best design (for instance, this is not actually calculating the BRR register value and storing it in BaudRate, but instead only putting the necessary values to call reconfigure on the Serial), but it was possible to implement this without modifying stm32f1xx_hal.

To improve the implementation, I would've had to modify stm32f1xx_hal to support setting only the baud rate and to provide a wrapper type around the register's integer type in order to allow precalculating that value from frozen clocks.

Without that, the trait would indeed force the Serial implementations to know about the current clock rates, which sounds painful at least.

[Dirbaio]

I had to use the unconstrained (custom) BaudRate type to force the user to go through a function which calculates the correct register value from the current clocks

If calling the trait needs calling HAL-specific methods/types, it can't be used from HAL-independent drivers. With this design, set_baud_rate might as well be an inherent method in Uart instead of a trait method, since the advantage of having traits is writing HAL-independent drivers, and this trait is not fulfilling that.

the trait would indeed force the Serial implementations to know about the current clock rates, which sounds painful at least.

embassy-stm32 stores the current clocks in a global, so Uart/Spi/I2c/etc have access to it at all times. WIth that design it's possible (and not painful at all) to implement a trait like fn set_baudrate(&mut self, baud_rate: u32), which would be useful for HAL-independent drivers (no unconstrained BaudRate type)