fix(targets): Refine ESP32 compatibility

[ZZ-Cat]

Overview

This Pull Request fixes #97.

Details

I had erroneously assumed that all targets define their UART pin assignments via their respective constructors (which is the case for SAMD21, SAMD51/SAME51 targets), but it has been brought to my attention that this is not the case for ESP32 targets.

ESP32 assigns UART Rx and Tx pins in the begin() function as opposed to using a constructor.
This has resulted in a lot of confusion around why things like RC Channels don't work on ESP32 targets and the inability to assign custom pins.
This Pull Request aims to fix both of these issues.

API changes

There is now a custom UART parameter that accepts custom pin assignments:

Note

This constructor is currently only available to ESP32 targets.
Custom pin assignments for SSAMD21 and SAMD51 targets SHOULD still be done externally.

• CRSFforArduino::CRSFforArduino(HardwareSerial *serialPort, int rxPin, int txPin)
  • Parameters:
    • serialPort
      This is a pointer to your UART port. EG Serial1.
    • rxPin
      This is the pin number for your UART Rx pin.
    • txPin
      This is the pin number for your UART Tx pin.

UART initialisation for ESP32 targets

When you pass in your pin numbers in the constructor, this tells CRSF for Arduino what pins you are using.
When you call CRSFforArduino::begin(), this internally calls the appropriate HardwareSerial::begin() function that's specific to ESP32 targets, where the pins that you provided in the constructor above get passed into this function.
By rights, this SHOULD work.

If you use the default CRSFforArduino::CRSFforArduino() constructor, the pins for Serial1 are internally set to 0 and 1 for Rx and Tx, respectively.

To initialise CRSF for Arduino with your ES32 target... especially if you want to use custom pins, this is what you do:

#include "Arduino.h"
#include "CRSFforArduino.hpp"

#define CRSF_SERIAL_PORT Serial1
#define CRSF_RX_PIN 0
#define CRSF_TX_PIN 1

CRSFforArduino *crsf = nullptr;

void setup()
{
    /* Create an instance of CRSF for Arduino with your custom port and pins. */
    crsf = new CRSFforArduino(&CRSF_SERIAL_PORT, CRSF_RX_PIN, CRSF_TX_PIN);

    /* Initialise CRSF for Arduino how you would normally. */
    if (crsf->begin() == true)
    {
        /* You may initialise the rest of CRSF for Arduino here. */

        /* ... */

    }
    else
    {
        /* CRSF for Arduino failed to initialise.
        Its resources MUST be released. */
        crsf->end();
        delete crsf;
        crsf = nullptr;
    }
}

void loop()
{
    /* As always, it's good practice to guard CRSF for Arduino. */
    if (crsf != nullptr)
    {
        crsf->update();
    }
}

Additional

I have been made aware that, for some reason, installing the .ZIP file in the Arduino IDE is failing.
From what I have tested on my end, apparently the error flag that comes up is something along the lines of CRSF for Arduino already existing, but at a different version.
EG If you have already installed Version 1.0.0, Arduino IDE will tell you Installation failed: CRSFforArduino already exists, but with a different version. CRSFforArduino @ 1.0.0

Workaround 1

Extract the zip file and manually install CRSF for Arduino.

Workaround 2

Remove the existing CRSF for Arduino folder (thus, deleting the contents in the process) and install the .ZIP file.

Treading on uncharted territory

It has been a very long time since I last used the Arduino IDE myself. The last version I had was 1.8.10, which has since been discontinued for quite some time.
Therefore, I am not yet up to speed with the latest version of the Arduino IDE.

Instead, I have been using Visual Studio Code and PlatformIO as my primary code editing and development environment since 2021.

There have been significant changes to the Arduino IDE that I am not yet familiar with - To the extent where it appears as though it's a completely different development environment. So, please bear with me, as I familiarise myself with this environment.

I am not entirely certain on why the .ZIP installation is failing if you already have an "older" version of CRSF for Arduino present.
By rights, this SHOULD automatically be replaced with the new version.
However, I will look into this myself, and figure out what's going on and why this is happening.

[ZZ-Cat]

@SB-JHL-B26
Can you check out the issue97 branch, grab the source code of that branch (IE <> Code -> Download ZIP), load it up into the Arduino IDE and flash the rc_channels.ino example on your Arduino Nano ESP32 target (Or, one of your other ESP32 targets), please? You MAY need to manually install CRSF for Arduino for the time being, until I can figure out what's going on with doing it through the IDE's built-in package manager.

Before you flash the rc_channels.ino sketch, you MAY modify the default CRSFforArduino() constructor and replace it with the custom UART constructor CRSFforArduino(HardwareSerial *serialPort, int8_t rxPin, int8_t txPin).

By rights, when CRSFforArduino::begin() is called later on in the sketch, it SHOULD pass.
If CRSFforArduino::begin() fails, you MAY enable debugging output with the following configuration:

• CRSF_DEBUG_ENABLED set to 1
• CRSF_DEBUG_SERIAL_PORT set to your desired debug port. Usually this is Serial or SerialUSB
• CRSF_DEBUG_ENABLE_COMPATIBILITY_TABLE_OUTPUT set to 1

The configuration file is located in CRSFforArduino/src and is called CFA_Config.hpp

Then, put the Serial Monitor's output here.

[SB-JHL-B26]

I would be glad to! Might have to wait until tomorrow. Cheers, Scott On Thursday, March 7, 2024 at 07:24:56 p.m. EST, ZZ-Cat ***@***.***> wrote: @SB-JHL-B26 Can you check out the issue97 branch, load that up into the Arduino IDE and flash the RC Channels example on your Arduino Nano ESP32 target, please? Before you flash the rc_channels.ino sketch, you MAY modify the default CRSFforArduino() constructor and replace it with the custom UART constructor CRSFforArduino(HardwareSerial *serialPort, int8_t rxPin, int8_t txPin). By rights, when CRSFforArduino::begin() is called later on in the sketch, it SHOULD pass. If CRSFforArduino::begin() fails, you MAY enable debugging output with the following configuration: - CRSF_DEBUG_ENABLED set to 1 - CRSF_DEBUG_SERIAL_PORT set to your desired debug port. Usually this is Serial or SerialUSB - CRSF_DEBUG_ENABLE_COMPATIBILITY_TABLE_OUTPUT set to 1 The configuration file is located in CRSFforArduino/src and is called CFA_Config.hpp Then, put the Serial Monitor's output here. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ZZ-Cat]

I would be glad to! Might have to wait until tomorrow. Cheers, Scott On Thursday, March 7, 2024 at 07:24:56 p.m. EST, ZZ-Cat @.*> wrote: @SB-JHL-B26 Can you check out the issue97 branch, load that up into the Arduino IDE and flash the RC Channels example on your Arduino Nano ESP32 target, please? Before you flash the rc_channels.ino sketch, you MAY modify the default CRSFforArduino() constructor and replace it with the custom UART constructor CRSFforArduino(HardwareSerial serialPort, int8_t rxPin, int8_t txPin). By rights, when CRSFforArduino::begin() is called later on in the sketch, it SHOULD pass. If CRSFforArduino::begin() fails, you MAY enable debugging output with the following configuration: - CRSF_DEBUG_ENABLED set to 1 - CRSF_DEBUG_SERIAL_PORT set to your desired debug port. Usually this is Serial or SerialUSB - CRSF_DEBUG_ENABLE_COMPATIBILITY_TABLE_OUTPUT set to 1 The configuration file is located in CRSFforArduino/src and is called CFA_Config.hpp Then, put the Serial Monitor's output here. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.>

Yea, all good. =^/.^=

[SB-JHL-B26]

I got it to work. However, I cheated a little ... just to "cut to the chase" ...

I simply modified the default constructor CRSFforArduino::CRSFforArduino() from being empty to the following:

    CRSFforArduino::CRSFforArduino()
    {
		Serial1.begin(420000, SERIAL_8N1, D0, D1);
    }

See the serial monitor output below.

My board is the Arduino Nano ESP32 and I am not familiar with how generic Arduino boards are wrt configuring the serial ports since I only have really used the Nano ESP32. I imagine you could provide a couple of generic constructors and let the user call the appropriate one with their own specific arguments being the serial port number, the Rx and Tx pin "numbers". The baudrate and the 8N1 are fixed by the CRSF protocol I would imagine so it would end up perhaps with only 3 arguments.

crsf = new CRSFforArduino("myseriaPort", "myRxPin", "myTxPin");

Of course it would be accompanied with some verbose comments so it would be easy to follow.

I should also mention I use Visual Studio (VS) 2022 with Visual Micro (VM) for my own work but I like the simplicity of the Arduino IDE to test libraries etc. When you release the "final" 1.x, I will port it over to the superior VS VM tools.

Cheers, Scott

RC Channels Example
Ready
RC Channels <A: 1500, E: 1500, T: 988, R: 1500, Aux1: 1000, Aux2: 988, Aux3: 988, Aux4: 988, Aux5: 988, Aux6: 881, Aux7: 881, Aux8: 881, Aux9: 881, Aux10: 881, Aux11: 2012, Aux12: 2012>
RC Channels <A: 1500, E: 1500, T: 988, R: 1500, Aux1: 1000, Aux2: 988, Aux3: 988, Aux4: 988, Aux5: 988, Aux6: 881, Aux7: 881, Aux8: 881, Aux9: 881, Aux10: 881, Aux11: 2012, Aux12: 2012>

[ZZ-Cat]

I got it to work. However, I cheated a little ... just to "cut to the chase" ...

😆

I simply modified the default constructor CRSFforArduino::CRSFforArduino() from being empty to the following:

    CRSFforArduino::CRSFforArduino()
    {
		Serial1.begin(420000, SERIAL_8N1, D0, D1);
    }

There is something that my auto-complete spat out on one of the constructors in the Sketch Layer, where it suggested that I inherit from the Serial Receiver Layer, like this:

namespace sketchLayer
{
    CRSFforArduino::CRSFforArduino(HardwareSerial *serialPort, int rxPin, int txPin)  : SerialReceiver(serialPort, rxPin, txPin) 
    {
    }
}

It would be a good idea to try this, and tell the default Sketch Layer constructor to explicitly inherit from the Serial Receiver Layer's constructor, like so:

namespace sketchLayer
{
    CRSFforArduino::CRSFforArduino() : SerialReceiver()
    {
    }
}

I added the above into 88be772

As for your "cheat"... 😆
That's hilarious, to me. But, sorry, I have to be that girl and call it out.
The Sketch Layer is a high level abstraction layer for the Serial Receiver Layer. The Serial Receiver Layer is the one that actually does all the heavy lifting. IE Managing hardware compatibility, hardware configurations (via Arduino's API) and the transport of RC and telemetry data between your ExpressLRS receiver and your sketch.
Doing it this way means I can dick around "under the hood" without touching CRSF for Arduino's API. IE It mitigates the risk of whatever I change inadvertently breaks someone's sketch.
It also keeps the API clean, it makes for a more streamlined work-flow, and everything is modular (so, if something is broken, I am able to test it in isolation... again without killing the API).

With that being said, having a hardware-related function in the Sketch Layer is a no-no.
It MUST be placed in the Serial Receiver Layer, and data from it SHOULD be passed up to the Sketch Layer via one of CRSF for Arduino's existing API functions.

[SB-JHL-B26]

I fully 100% get the whole communication layer and abstraction thing. I coded for years in telecommunications and understand the concepts. Now in the spirit of having fun with this …  I called this a cheat and it was a 2 second hack to help isolate and pinpoint the setup of the uart which was my suspicion from the start. My approach removed your code from possibly being the problem. Always good to remove layers of complexity in troubleshooting.I love your design, structure and implementation. I would love to use it in my project.If you might document, with examples, how and where to set up a target’s serial port from a users point of view, it would help polish your great work. Not all users will be seasoned coding professionals in the maker/inventor/hobby bubble.It has been great to work with you on this and if there is anything else I can do, let me know.Cheers, Scott.On Mar 8, 2024, at 4:43 AM, ZZ-Cat ***@***.***> wrote: I got it to work. However, I cheated a little ... just to "cut to the chase" ... 😆 I simply modified the default constructor CRSFforArduino::CRSFforArduino() from being empty to the following: CRSFforArduino::CRSFforArduino() { Serial1.begin(420000, SERIAL_8N1, D0, D1); } There is something that my auto-complete spat out on one of the constructors in the Sketch Layer, where it suggested that I inherit from the Serial Receiver Layer, like this: namespace sketchLayer { CRSFforArduino::CRSFforArduino(HardwareSerial *serialPort, int rxPin, int txPin) : SerialReceiver(serialPort, rxPin, txPin) { } } It would be a good idea to try this, and tell the default Sketch Layer constructor to explicitly inherit from the Serial Receiver Layer's constructor, like so: namespace sketchLayer { CRSFforArduino::CRSFforArduino() : SerialReceiver() { } } I added the above into 88be772 As for your "cheat"... 😆 That's hilarious, to me. But, sorry, I have to be that girl and call it out. The Sketch Layer is a high level abstraction layer for the Serial Receiver Layer. The Serial Receiver Layer is the one that actually does all the heavy lifting. IE Managing hardware compatibility, hardware configurations (via Arduino's API) and the transport of RC and telemetry data between your ExpressLRS receiver and your sketch. Doing it this way means I can dick around "under the hood" without touching CRSF for Arduino's API. IE It mitigates the risk of whatever I change inadvertently breaks someone's sketch. It also keeps the API clean, it makes for a more streamlined work-flow, and everything is modular (so, if something is broken, I am able to test it in isolation... again without killing the API). With that being said, having a hardware-related function in the Sketch Layer is a no-no. It MUST be placed in the Serial Receiver Layer, and data from it SHOULD be passed up to the Sketch Layer via one of CRSF for Arduino's existing API functions. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[ZZ-Cat]

I fully 100% get the whole communication layer and abstraction thing. I coded for years in telecommunications and understand the concepts. Now in the spirit of having fun with this … I called this a cheat and it was a 2 second hack to help isolate and pinpoint the setup of the uart which was my suspicion from the start. My approach removed your code from possibly being the problem. Always good to remove layers of complexity in troubleshooting.I love your design, structure and implementation. I would love to use it in my project.If you might document, with examples, how and where to set up a target’s serial port from a users point of view, it would help polish your great work. Not all users will be seasoned coding professionals in the maker/inventor/hobby bubble.It has been great to work with you on this and if there is anything else I can do, let me know.Cheers, Scott.On Mar 8, 2024, at 4:43 AM, ZZ-Cat @.*> wrote: I got it to work. However, I cheated a little ... just to "cut to the chase" ... 😆 I simply modified the default constructor CRSFforArduino::CRSFforArduino() from being empty to the following: CRSFforArduino::CRSFforArduino() { Serial1.begin(420000, SERIAL_8N1, D0, D1); } There is something that my auto-complete spat out on one of the constructors in the Sketch Layer, where it suggested that I inherit from the Serial Receiver Layer, like this: namespace sketchLayer { CRSFforArduino::CRSFforArduino(HardwareSerial serialPort, int rxPin, int txPin) : SerialReceiver(serialPort, rxPin, txPin) { } } It would be a good idea to try this, and tell the default Sketch Layer constructor to explicitly inherit from the Serial Receiver Layer's constructor, like so: namespace sketchLayer { CRSFforArduino::CRSFforArduino() : SerialReceiver() { } } I added the above into 88be772 As for your "cheat"... 😆 That's hilarious, to me. But, sorry, I have to be that girl and call it out. The Sketch Layer is a high level abstraction layer for the Serial Receiver Layer. The Serial Receiver Layer is the one that actually does all the heavy lifting. IE Managing hardware compatibility, hardware configurations (via Arduino's API) and the transport of RC and telemetry data between your ExpressLRS receiver and your sketch. Doing it this way means I can dick around "under the hood" without touching CRSF for Arduino's API. IE It mitigates the risk of whatever I change inadvertently breaks someone's sketch. It also keeps the API clean, it makes for a more streamlined work-flow, and everything is modular (so, if something is broken, I am able to test it in isolation... again without killing the API). With that being said, having a hardware-related function in the Sketch Layer is a no-no. It MUST be placed in the Serial Receiver Layer, and data from it SHOULD be passed up to the Sketch Layer via one of CRSF for Arduino's existing API functions. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.>

That can be easily done in the Wiki.
I actually had been indecisive on including that in my documentation, when others have already gone about it themselves (EG Adafruit have an article on how to do custom UARTs on SAMD21 and SAMD51 targets).
Now that you have brought this to my attention, I now see a valid reason why I SHOULD document how to go about assigning custom pins properly. because it's convenient to have that information in one place.
From what I understand of it, it's both architecture and hardware specific. For example: ESP32 targets do pin assignments slightly differently to how SAMD21 and SAMD51 targets do it. And SAMD21 targets do UART pin assignments slightly different to how SAMD51 targets do it.

Anyways... were you able to to test 88be772?
If so, how did that turn out, for you?

[ZZ-Cat]

@SB-JHL-B26 I have a new commit I would like you to try out: 70c3354

With that, instead of integrating the pin parameters with the existing custom UART constructor, I decided to split it off into a constructor all on its own. On my end, this opens up the door for a potential re-factor (of the way custom UART pins are assigned) later on down the track, without interfering with the Sketch Layer.

I also updated the OP in this Pull Request to include a draft of what I may add to the Wiki, when this Pull Request is merged. Let me know if there's anything I may have missed here.

At this stage, I am marking this as 'ready for review', because I feel like this Pull Request has addressed the hardware side of your issue. The other side is simply a need for me to update the Wiki. This will happen shortly after this Pull Request is merged. After that, I may release a cut of Version 1.0.2 with these changes brought in as a patch, due to it not affecting the API as drastically as I had previously anticipated.

Thank you very much for your time and patience with me. It has been wonderful to work with you on this.

[SB-JHL-B26]

I downloaded 70c3354. I installed the library manually. I opened up a new sketch ... Sketch-->Include Library-->CRSFforArduino File-->Examples-->CRSFforArduino-->rc_channels Powered on my Tx Powered on my Rx (phrase binding, I love it!) Complied the sketch ... all was PERFECT! Downloaded to my Arduino Nano ESP32 Opened the Serial monitor, and ......................................................... ALL WAS WELL !!!!!!!!!!!!!!!!!!!!!!!!! Beautiful !!!!!!!!!!!!!! I did not have to edit a single thing!. "It just worked". Now that is what I call "working right out of the box!". Congrats! Thank you very much !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!   Cheers, Scott On Saturday, March 16, 2024 at 01:45:16 a.m. EDT, ZZ-Cat ***@***.***> wrote: @SB-JHL-B26 I have a new commit I would like you to try out: 70c3354 With that, instead of integrating the pin parameters with the existing custom UART constructor, I decided to split it off into a constructor all on its own. On my end, this opens up the door for a potential re-factor (of the way custom UART pins are assigned) later on down the track, without interfering with the Sketch Layer. I also updated the OP in this Pull Request to include a draft of what I may add to the Wiki, when this Pull Request is merged. Let me know if there's anything I may have missed here. At this stage, I am marking this as 'ready for review', because I feel like this Pull Request has addressed the hardware side of your issue. The other side is simply a need for me to update the Wiki. This will happen shortly after this Pull Request is merged. After that, I may release a cut of Version 1.0.2 with these changes brought in as a patch, due to it not affecting the API as drastically as I had previously anticipated. Thank you very much for your time and patience with me. It has been wonderful to work with you on this. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ZZ-Cat]

I downloaded 70c3354. I installed the library manually. I opened up a new sketch ... Sketch-->Include Library-->CRSFforArduino File-->Examples-->CRSFforArduino-->rc_channels Powered on my Tx Powered on my Rx (phrase binding, I love it!) Complied the sketch ... all was PERFECT! Downloaded to my Arduino Nano ESP32 Opened the Serial monitor, and ......................................................... ALL WAS WELL !!!!!!!!!!!!!!!!!!!!!!!!! Beautiful !!!!!!!!!!!!!! I did not have to edit a single thing!. "It just worked". Now that is what I call "working right out of the box!". Congrats! Thank you very much !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Cheers, Scott On Saturday, March 16, 2024 at 01:45:16 a.m. EDT, ZZ-Cat @.> wrote: @SB-JHL-B26 I have a new commit I would like you to try out: 70c3354 With that, instead of integrating the pin parameters with the existing custom UART constructor, I decided to split it off into a constructor all on its own. On my end, this opens up the door for a potential re-factor (of the way custom UART pins are assigned) later on down the track, without interfering with the Sketch Layer. I also updated the OP in this Pull Request to include a draft of what I may add to the Wiki, when this Pull Request is merged. Let me know if there's anything I may have missed here. At this stage, I am marking this as 'ready for review', because I feel like this Pull Request has addressed the hardware side of your issue. The other side is simply a need for me to update the Wiki. This will happen shortly after this Pull Request is merged. After that, I may release a cut of Version 1.0.2 with these changes brought in as a patch, due to it not affecting the API as drastically as I had previously anticipated. Thank you very much for your time and patience with me. It has been wonderful to work with you on this. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.>

You're very welcome. =^/,..,^=
This is why I love being a developer.

[ZZ-Cat]

I am bypassing my Branch Protection rules on the grounds of overriding my Arduino IDE build check.
I am still experiencing issues with this build check and its failure does not reflect the actual build status of CRSF for Arduino, because people are able to successfully compile CRSF for Arduino in the Arduino IDE 2.x.x.