arduino / ArduinoCore-megaavr

Arduino Core for the ATMEGA4809 CPU
104 stars 62 forks source link

Baudrate out of range is silently accepted #46

Open egilkv opened 5 years ago

egilkv commented 5 years ago

While the original Uno is advertised to support Serial baudrates down to 300, it seems the m4809 can only support baudrates down to 2400. This is due to the design of the baudrate generator, where lower baudrates overflows the 16 bit divider. I have not checked if there are some tricks to circumvent this, other than reducing the CPU clock.

Anyway, with rates lower than 2400 the current code just silently truncates the divisor to 16 bits, causing all sorts of crazy baudrates. Which is really not nice at all, especially for a platform that is beginner friendly.

Other than updating the documentation, what would the right way of handling this be? I noticed in earlier versions of the code there was an internal error flag that simply skipped the entire UART initialization if the baudrate was too high, but that was probably not useful. One option would be to let begin return true/false to indicate problems?

There is also a max baudrate of 1843200 which I guess is less of a practical problem.

MCUdude commented 5 years ago

@egilkv @facchinm As a temporary fix we can cast the end result to uint16_t instead of int16_t. By doing this we can use 1200 baud when running the microcontroller at 16 MHz. 20 MHz still causes an overflow even if we use SIGROW.OSC20ERR5V to calculate the baud rate.

Does this mean it's physically impossible to achieve slow baud rates (300 baud, 600 baud) when running at 16 or 20 MHz?

facchinm commented 5 years ago

I think the way to go is documenting this behaviour + adding a patch that simply doesn't initialize the UART if the baud rate is too low. I created https://github.com/facchinm/ArduinoCore-megaavr/commit/cf749eea008c1558b8187c2c4bbb0835a9b894d8 to test the second approach.

MCUdude commented 5 years ago

That will at least prevent the baud rate from being crazy due to an overflow.

BTW why is baud_setting casted to int16_t instead of uint16_t?

SimonePDA commented 5 years ago

Please @facchinm I need something more than being assigned this issue. Provide what kind of amendment documentation needs and where this amendment should be made. The Getting Started page for the boards that have the m4809? The Serial.begin() function? Other place? Thanks

facchinm commented 5 years ago

@SimonePDA I think the right place could be the UNO WiFi rev2 product page (and also Nano Every).

A possible sentence explaining this could be:

Serial:
When the board is clocked at 16Mhz (default) or 20Mhz the minimum baud rate achievable is 2400bps. If you are interfacing with a slower peripheral, apply the workaround from https://github.com/arduino/ArduinoCore-megaavr/issues/46 [not the correct link, will amend as soon as we find a nice workaround :) ]
MCUdude commented 5 years ago

@facchinm 1200 baud is achievable when running at 16 MHz, but not on 20 MHz.

SimonePDA commented 5 years ago

@facchinm the product page is the one on the STORE. https://store.arduino.cc/usa/arduino-uno-wifi-rev2 we don't have any specific information there that is related to software. Maybe you mean the Getting Started that is here: https://www.arduino.cc/en/Guide/ArduinoUnoWiFiRev2 From the text you suggest it also looks like the solution is not ready yet, therefore I can't proceed with the amendment. Let me know when it is time.