Update CRSF spec #13614
Update CRSF spec #13614
Conversation
|
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
|
Can this be scheduled for 4.5.1 please, this needs to happen sooner rather than later and given the current release schedule of BF 4.6 might be another year or so. |
|
Bugfixes will be backported. Need testing first. |
|
Can I enquire who's testing this and what the status is? IMHO this should just be merged into |
|
Seems official baud rate - just want to have this tested scientifically :) measuring is knowing |
Does it make any difference? Betaflight worked with TBS products since the inception of CRSF with the "wrong" baud rate, it should still work if BF changes to I believe it is only really an issue if some target couldn't generate this baud and ended up being even lower on the wire. It is only 0.96% difference otherwise which should be tolerable? It should definitely be tested against ExpressLRS STM32, ESP32, and ESP8285 targets but I do not anticipate any issues. @hydra why does this need to happen ASAP exactly? What uhhhh isn't working that is so broken that it needs immediate attention? It has operated like this for 8 years 😆 |
Yes please focus on the change - like to have this tested if there are any improvements. |
Is there an anticipated improvement? I thought this was more academic than beneficial. |
|
IDK - just following official spec here. Purpose of this PR is to enable testing and get the facts. Appreciate testing results. |
First ELRS is hearing about a change when this PR opened. So there has been no discussion or coordination in a controlled way. I agree there is no rush here, and maybe the PR should be marked as a draft. Steve requested the spec #12398 (comment), where is this? TBS expressed that they will provide a spec, so we can wait for this tbs-fpv/freedomtx#26 (comment). Otherwise community projects are just making random changes. The crsf-wg was also started to try and do these changes in a controlled manner https://github.com/crsf-wg/crsf/issues. Something BF said they are up for tbs-fpv/freedomtx#26 (comment). Im not opposed to a change, lets just get some documentation around it so that everyone knows the standard. SmartAudio is great like this, while TRAMP is a not. Lets start doing it right. |
|
Really love to see any test results opposed to master before further consideration updating standards. |
Betaflight Testing Setup--- a/src/main/rx/crsf.c
+++ b/src/main/rx/crsf.c
@@ -73,6 +73,7 @@ static timeUs_t crsfFrameStartAtUs = 0;
static uint8_t telemetryBuf[CRSF_FRAME_SIZE_MAX];
static uint8_t telemetryBufLen = 0;
static float channelScale = CRSF_RC_CHANNEL_SCALE_LEGACY;
+static int32_t crsfCrcErrCnt = 0;
#ifdef USE_RX_LINK_UPLINK_POWER
#define CRSF_UPLINK_POWER_LEVEL_MW_ITEMS_COUNT 9
@@ -460,6 +461,7 @@ STATIC_UNIT_TESTED void crsfDataReceive(uint16_t c, void *data)
break;
}
} else {
+ DEBUG_SET(DEBUG_CRSF_LINK_STATISTICS_UPLINK, 4, ++crsfCrcErrCnt);
#if defined(USE_CRSF_V3)
if (crsfFrameErrorCnt < CRSF_FRAME_ERROR_COUNT_THRESHOLD)
crsfFrameErrorCnt++;
diff --git a/src/main/rx/crsf_protocol.h b/src/main/rx/crsf_protocol.h
index 0b56233f5..59382c844 100644
--- a/src/main/rx/crsf_protocol.h
+++ b/src/main/rx/crsf_protocol.h
@@ -27,7 +27,7 @@
#include <stdint.h>
#include <stdbool.h>
-#define CRSF_BAUDRATE 420000
+#define CRSF_BAUDRATE 416666
enum { CRSF_SYNC_BYTE = 0xC8 };DEBUG set to ExpressLRS Test Setupdiff --git a/src/lib/Telemetry/telemetry.cpp b/src/lib/Telemetry/telemetry.cpp
index b055fc68..9be47f57 100644
--- a/src/lib/Telemetry/telemetry.cpp
+++ b/src/lib/Telemetry/telemetry.cpp
@@ -163,6 +163,8 @@ void Telemetry::ResetState()
}
}
+int8_t crsfCrcErrors = 0;
+
bool Telemetry::RXhandleUARTin(uint8_t data)
{
switch(telemetry_state) {
@@ -215,6 +217,8 @@ bool Telemetry::RXhandleUARTin(uint8_t data)
receivedPackages++;
return true;
}
+ else if (crsfCrcErrors < 127)
+ ++crsfCrcErrors;
#if defined(UNIT_TEST)
if (data != crc)
{
diff --git a/src/src/rx_main.cpp b/src/src/rx_main.cpp
index 7059df00..2cf7a6f7 100644
--- a/src/src/rx_main.cpp
+++ b/src/src/rx_main.cpp
@@ -431,6 +431,8 @@ void ICACHE_RAM_ATTR LinkStatsToOta(OTA_LinkStats_s * const ls)
ls->SNR = SnrMean.previousMean();
}
#endif
+ extern int8_t crsfCrcErrors;
+ ls->SNR = crsfCrcErrors;
}
bool ICACHE_RAM_ATTR HandleSendTelemetryResponse()Much hack, so dumb. Monitors CRC errors on the ExpressLRS side, is sent back as telemetry in the RSNR field on EdgeTX (31 max value reportable). Expected value: always 0. TestingAll tests done with TX16S Internal module, TMVELOXF411 Betaflight (3.2k/3.2k + analog OSD on), iFlight 2.4G RX (ESP8285)
Partial ConclusionSTM32/ESP32 still needs to be tested but I am Limon-ing, however it appears there is a large window where the baud is fine. ExpressLRS operates in that range for either the old or new baud.
|
|
I made a small sheet that calculates the theoretical bit error possibility. With 416666 baud rate RX side, various TX bauds are tested:
As the UART 8N1 format requires reading 10 bits, any value near 10-ish will be problematic, which will likely cause a failed reading on slight clock drift. Value <10 will be a failure, definitely. This result aligns with @CapnBry 's experiment, and it confirms the compatibility between 420k and 416.666k is totally fine. |
|
I did not see this one linked, so here is "TBS official comment" 🤷♂️ : tbs-fpv/freedomtx#26 (comment) |
|
Updating with ExpressLRS ESP32 pico D4 results (RadioMaster ER8 receiver). Slightly better tolerance than ESP8285 results, but possibly due to only testing higher bauds (no baud <416666 was tested).
|
|
And wrapping up the testing, STM32 (R9MM receiver). STM32 are more sensitive to the baud rate change and have more errors when ExpressLRS runs at the original CRSF baud (420000). However, it also appears STM32 has issues even when set to the correct baud which taints the results-- STM32 receivers don't work perfectly with or without this change. NOTE! Testing procedure is different here, as this is a 900MHz receiver and can only run at 200Hz, therefore the percentage of errors 5x higher normalized to the Team2.4 receiver than can output 1000Hz.
This one was a bit of a nightmare to test because changing the baud on STM32 can only be done by flashing it, but once you change the baud you can't flash it back any more without STLINKing which I forgot and it is all ya'll's fault 😅 Overall Conclusions: TechnicalIt does appear that changing the baud rate is fine for ESP8285 and ESP32 ExpressLRS receivers, but can have a small detrimental effect on ExpressLRS STM32 receivers. ExpressLRS STM32 users can not change the baud rate without reattaching an STLINK, as the bootloader used for passthrough flashing can't be swapped via itself, which creates a large burden on the user base if ExpressLRS were to change. In short this change has a insubstantial detrimental effect to the ExpressLRS ecosystem as a whole, further mitigated by the fact that the overwhelming majority of users are on hardware that will continue to function at 100% no matter which of the two baud rates are used. Overall Conclusions: PersonalWhy are we considering this? What benefit can come from it vs the risks? The community has operated for 8 years on the assumption that 420k baud was the proper CRSF baud for receivers. TBS has never raised an objection, or even mentioned it despite numerous enhancements to the Betaflight CRSF code in that time. We've only become aware this was not the accurate baud rate via a comment from a reputable source stating it, who also has not suggested anyone change. Is there a secret document that's under an NDA which theoretically states that the correct baud rate is 416666. The community almost entirely has operated under the assumption that the correct baud rate is 420000. Numerous pieces of software have been built hard-coding this baud rate into their applications, just as Betaflight has. Therefore it is my opinion that the world adopts 420000 baud The Standard CRSF Baud, as that is what almost everyone is using. I would readily change my mind though if a TBS user tested the BF patch I posted above, and posted number of the number of CRC errors when running 250Hz Tracer with Betaflight set to 416666 vs 420000 baud. |
|
0.8% difference in baudrate is within clock tolerance, so it PR is "only" increasing safety margin. |
|
@CapnBry Thanks so much for the careful testing.
|
My personal preference is: keep the default of 420000 but provide a build option for 416666 |
Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
|
Leave it at 420000 as the default and opt for 416666 as build option. I have not come across any issues with 420000. |
|
Status Quo back to 3.5 is 420000 - I agree, don't change that, but make 416666 an option |
|
Either don't change anything, or fix it. |
My opinion is that is that it should stay 420000. If there had been problems with the "incorrect" baud rate over the past 8 years then I feel like this would have been changed already. I can't have been the only person ever who looked to see how many packets weren't passing CRC, or that the most popular FC software was using the correct baud rate for the protocol I use, am I? 🤨
Thank you for bringing your knowledge to this discussion! I'm not sure if this is being misremembered, but OpenTX / EdgeTX have never used 416666 baud and has always used 400000 baud. This for sure is incompatible, as running 416666 or 420000 baud on the TX module simply does not work at all. To my knowledge, I am the only person who has ever created public documentation for CRSF, but it was based entirely on my own opinions from reverse engineering and reading existing open source code 5 years ago. I've recently updated it with information from a secret document I found, which also contained information contradictory to what I know to be true. It would be a great benefit if the CRSF protocol specification was publicly available right from TBS's website so everyone's not referring to the super secret confidential document they can't share. That would prevent this sort of confusion and allow us to not rely on asking Trappy to comment on what is right or wrong, and knowing those comments aren't guaranteed to be totally accurate |
|
Leaving it as 420k, if someone needs 416... they've now got a custom define they can set. Thanks all for the discussion, appreciate the input and testing. |
@CapnBry some reasons:
@ledvinap agreed!
@blckmn this only goes to confuse everyone further and perpetuate the mess. Less is more. We don't need it. respectfully, I request that maintainers please reconsider. |
|
Also, @CapnBry and @SunjunKim thanks for your testing, your results were pretty much what I expected in that it makes no perceptible difference when updating the baud rate to the correct spec confirming my feeling that we should all just be using the right baud rate in all our software and tooling and move on. @CapnBry with regards to the ELRS bootloader on STM32 boards, correct me if I'm wrong but the bootloader at 420000 still allows passthrough flashing 416666 and vice versa right? |
Seems to be just fine on R9MM anyway (tested 3x, but not sure about the vice versa part of your question) |
I mean that if the if the BL is changed to use 416666, and the user is using a version of BF that uses 420000 then flashing an updated version of ELRS still works. I'm assuming it does due to the reasons you and @SunjunKim outlined before, but would be good to confirm. That way there's no problem, again unless I'm mistaken, for both BF and ELRS to correct the baud rate to match the spec of 416666 and users with old bootloaders at 420000 are still ok and going forward every new install will be using the right baud rate. |
|
Oh, I'm not going to go through all the trouble to rebuild the bootloader and hook up the STLINK and all that to try it. |
|
The decision to merge this was stupid. |
|
Hi everyone again, just following up on this again as it should be noted that @CapnBry 's testing may be masking an issue. I came across the issue with the various baud rates when I was implementing CRSF in Rust. Initially the baud rate of 400,000 was used, and the MCUs (F446 and H743)'s uart peripherals were reporting framing errors, that is that UART's I then tried 416,666 after getting an updated copy of the spec from trappy, and was still getting framing errors, which was because I was testing with an ELRS receiver which was using 420,000. After correcting the ELRS RX to use the right baud rate all the framing errors were gone. Depending on implementations of UARTs and CRSF code users may get more errors if bytes are thrown away because of framing errors, if framing errors are ignored and the RX byte is still used sometimes the packets are still OK, but this doesn't mean the errors are not there, and it also isn't free as code has to clear the FE bit in the UART.
|
|
The problem I recognize is the fact that there’s two CRSF protocols in use – community-built CRSF (mostly from the reverse engineering) and TBS official CRSF, which have been secret to everyone. I want to ask.. how many devices are working on 420,000 and how many are on 416,666? I believe status quo is 420,000, even though the official CRSF had been 416,666. We, as a community, missed the chance to correct it in the beginning v3.5.7, and TBS hasn’t claimed their spec upon the wrong implementation, for full eight years. Now we need to deal with a path dependency. As the most of the devices in the field is working on community-standard 420,000, what’s the benefit of change it to the TBS-spec 416,666 to be default to everyone, only for the TBS gears? I feel like it’s a tail wag the dog situation. The current PR - 420 default, but giving 416 option for who needs that makes more sense to me. |
There is only one CRSF protocol.
In terms of device numbers it's hard to say, but 95% of all projects use 416.6k. The only ones that don't that I know of are betaflight and ELRS.
False. The concern has been raised multiple times. Anyone who has ever asked us has been told it's wrong. This isn't even the first thread by hydra on the issue. It's just that whenever we said it was wrong, the consensus seems to have been that "it works fine" and to leave it as is. Which is fine with me - just don't claim it's our fault for not raising the issue :)
It's still a TBS protocol, so changing it to the TBS spec would be implementation of the spec. Leaving it at as it is would be creating a different spec. Also, "most devices on the field" is just a very biased and factually incorrect interpretation of the word "most". Fwiw, I'm not here to advocate for either of the changes, or even influence the outcome. It's your project(s), they are your decisions, and since TBS products work fine with the implementations either way, either way "works" for TBS. |
Hi, Trappy, happy to see you here. I believe hydra mentioned that I’d like to get the protocol document wrt the crsf-wg up to date with the official CRSF spec. If you’re comfortable with that, please let me know. Thank you in advance! |
Fixes CRSF baud rate is incorrect. #12398
According to spec CRSF uses a baudrate of 416666
Please test carefully
Add
USE_CRSF_OFFICIAL_SPECto activate during local builds.For cloud builds use custom define
CRSF_OFFICIAL_SPEC