ZZ-Cat / CRSFforArduino

An Arduino Library for communicating with ExpressLRS and TBS Crossfire receivers.
GNU Affero General Public License v3.0
150 stars 26 forks source link

Seeed Studio XIAO nRF52840 compatibility #62

Closed kingfisher09 closed 9 months ago

kingfisher09 commented 10 months ago

Minimum requirements

What development board would you like me to add?

Hi, I'm trying to use a Seeed Studio XIAO nRF52840 with this library.

As far as I can tell it conforms to the requirements (I think it uses a Cortex M4 though I'm not quite clear on the specifics).

Could you add this functionality?

ZZ-Cat commented 10 months ago

Short answer

No.

TL;DR

Long answer

Not from what I can see in the source code of the NRF52 board support UART driver.

It's a fork of Adafruit's implementation, which uses a set of 'else if' statements to set the physical baud rate of the NRF52's UART to the nearest value that the hardware is capable of, like so:

  uint32_t nrfBaudRate;

  if (baudrate <= 1200) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud1200;
  } else if (baudrate <= 2400) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud2400;
  } else if (baudrate <= 4800) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud4800;
  } else if (baudrate <= 9600) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud9600;
  } else if (baudrate <= 14400) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud14400;
  } else if (baudrate <= 19200) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud19200;
  } else if (baudrate <= 28800) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud28800;
  } else if (baudrate <= 31250) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud31250;
  } else if (baudrate <= 38400) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud38400;
  } else if (baudrate <= 56000) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud56000;
  } else if (baudrate <= 57600) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud57600;
  } else if (baudrate <= 76800) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud76800;
  } else if (baudrate <= 115200) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud115200;
  } else if (baudrate <= 230400) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud230400;
  } else if (baudrate <= 250000) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud250000;
  } else if (baudrate <= 460800) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud460800;
  } else if (baudrate <= 921600) {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud921600;
  } else {
    nrfBaudRate = UARTE_BAUDRATE_BAUDRATE_Baud1M;
  }

When you set the baudrate on an NRF52, the number you pick is not a guarantee that it will be set to your specified value.
EG if you tried to set the baudrate to 420000 or 416600, the NRF52 will automatically set the baudrate to 250000.

What this means is you can't set custom granular baud rates in the NRF52 like you can with all of the other chipsets in the Compatibility Table.
It is this reason alone that I did not initially add NRF52 support, because I view it as a limiting factor and a hidden caveat that renders CRSF for Arduino practically unusable with half of what my library is intended to be compatible with.

I understand that TBS Crossfire receivers use a fixed UART baudrate of 416600, of which would be impossible to set on the NRF52. If you tried, the closest you will have on your NRF52 is 250000 and you will have garbled data, and no matter what you do, it simply won't work.

In ExpressLRS' case, you would need to go into ExpressLRS Configurator and flash your receiver with your custom baudrate to match the limited choice in baudrates that the NRF52 chipset has provided. In this case, it would have to be 250000
I can see this would trip a lot of people up, if thar not made aware of it.

Even if people are made aware of it, the actual NRF52 support would look more like "NRF52 but... [here's a list of hidden caveats that render CRSF for Arduino unusable with all of TBS Crossfire's receivers and partially unusable with ExpressLRS receivers unless you do x, y, and z]" and I prefer to avoid that scenario.
I don't even like the fact that Nordic have provided a chipset with fixed baudrates like this, which means that you're stuck with whatever values they give you and nothing else. Because "too bad if the device you're using has an obscure baudrate that we don't recognise". :woman_facepalming:

I disagree with approaches like this, because I view it as an unnecessary limitation from a developer's perspective.

Additional

I already tried to implement support for NRF52 chipsets as early as version 0.1.0, and that's how I found out about these limitations.

It irritated me that much, that I basically hit "f-- it"; I never published the branch, I discarded the changes I made, deleted the branch, and declared that I avoid NRF52 support like the plague.

If it's anything other than a definite "yes", it's a "no".

kingfisher09 commented 10 months ago

Thanks for taking the time to give me such a comprehensive answer!

That is super annoying though about the limited baudrates. I've had nothing but problems with this chip since I started this project! I just wanted a small format board with on board gyro, I didn't know I was getting myself into such a difficult chip.

ZZ-Cat commented 10 months ago

Thanks for taking the time to give me such a comprehensive answer!

That is super annoying though about the limited baudrates. I've had nothing but problems with this chip since I started this project! I just wanted a small format board with on board gyro, I didn't know I was getting myself into such a difficult chip.

You're welcome.
This is also why it pays to heavily research any new development board well and truly ahead of purchasing it.

Generally, what I do is I look up the following:

All of this can be quite time consuming. However, it is so worth going the extra mile for.

I can see why you picked the NRF52840, because of it's Bluetooth capabilities, and Cortex M4F (with hardware floating point unit).
The devboard in particular has an on-board IMU. Which, again, this does seem ideal.
However, I can't help but think it's one of those "too good to be true" type of deals, here.

It does seem ideal to have an on-board IMU paired with a microcontroller of that calibre, and the board itself being a small form-factor, however it could have been paired with a better microcontroller.

A better alternative would be boards along the lines of Arduino's Nano 33 IoT and Nano RP2040 Connect.
Me and one other developer are working on fixing support for the Nano 33 IoT (because compatibility for it is currently broken) over at #57, and someone requested support for RP2040-based targets (I will include the Nano RP2040 Connect in this) in #52 as well.

If it's the tiny form factor you're after, I highly recommend Adafruit's QtPy ESP32-S3 and pair it with their ICM-20948 Breakout. That should give you a pretty grunty powerhouse with Bluetooth and WiFi connectivity as well as giving you access to a 9 degrees-of-freedom motion sensor (of which is an upgrade from the now discontinued MPU6000 and MPU9000 series of sensors).
The only trade-off here is you won't have a hardware floating point unit to back you in intensive DSP situations. However, 240 MHz processing speed is nothing to be sneezed at. So, that should be enough to fill the void for what you may be doing.

kingfisher09 commented 10 months ago

I've decided to go for the Nano RP2040 connect! I'll be able to use your library and from what I can tell I'll be able to use Dshot for motor control which I also want to do for my project.

ZZ-Cat commented 10 months ago

I've decided to go for the Nano RP2040 connect! I'll be able to use your library and from what I can tell I'll be able to use Dshot for motor control which I also want to do for my project.

Excellent!
RP2040 support is planned for the upcoming v1.0.0 release.