ZZ-Cat / CRSFforArduino

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

feat(serial receiver interface): Add custom Flight Mode strings as telemetry #83

Closed berickson closed 9 months ago

berickson commented 9 months ago

This pull request adds support for custom flight mode strings from the client through the telemetryWriteCustomFlightMode method. This might be a niche use case, but I'm using CRSF for a car based robot and using flight modes to represent different operational modes. Other Arduino users might be doing similar things so I'm sending it as a pull request.

To make it work directly as a Platformio library, I also modified CFA_Config.hpp to allow users to set CRSF_TELEMETRY_FLIGHTMODE_ENABLED externally, allowing me to set it through my platformio.ini build flags. That same pattern might be useful for other #defines, but I only changed the one that I required.

berickson commented 9 months ago

Looks like merge is blocked because commits aren't GPG signed. I thought SSH commits were good enough. I'll need to figure that out since I haven't encountered that before.

ZZ-Cat commented 9 months ago

Yes. All commits in your Pull Request MUST be signed, in order for them to be merged. It can be a headache to set-up, but totally worth it.

All of my commits are SSH signed. So, by rights, it shouldn't matter if thar SSH or GPG signed. As long as there's a verification signature, ya should be all good.

However, due to your first commit not being signed, that's why merging is currently blocked.

ZZ-Cat commented 9 months ago

Just waiting on the Arduino and PlatformIO checks to pass.

The Arduino checks also checks for correct code formatting, and it will ping you for it if your code isn't formatted correctly.

ZZ-Cat commented 9 months ago

Alright. So, the Arduino check has pinged you in your formatting of CFA_Config.hpp.

Can you go through and do clang-format -i src/*.* then clang-format -i src/*/*.* and then commit your changes, please?

Once you have done that, both checks should pass.
But, you're not out of the woods yet. I need you to go through the source code files and bump the version date to the latest.
Look for the @date boiler plate field at the top of each and every *.hpp, *.cpp, and *.ino file.
In CFA_Config.hpp, you will need to bump CRSFFORARDUINO_VERSION_DATE to match the version date fields too.

This helps me out a lot during things like re-basing development branches when the Main-Trunk has had recent merge commits pushed to it.

For future reference:
Date is formatted as YYYY-MM-DD EG 2024-2-14.
Bumping the version date should be the first commit in your Pull Request, as this reduces the chances of changes from one branch being confused with changes from another branch and them getting mixed up or worse yet... obliterated by a clean commit.

berickson commented 9 months ago

It's great you got this merged so quickly. It's my pleasure to have contributed something.

For overloading, I was wondering too whether you'd prefer an overload or a new name, either works for me.

I'll let you know if I run into anything else while I'm using the library. Right now it's working out great. I was able to go from using PWM as the interface between my receiver and my microcontroller to CRSF where I can easily get many more input channels and telemetry back to my controller that I wasn't getting before.

ZZ-Cat commented 9 months ago

That's the whole idea behind the project.

That and I got really spiteful about the only solution available for adding RC to one's projects is to use a PWM receiver, use pulseIn() (or pin change interrupts) and hope for the best.

At the time I created CRSF for Arduino, I was like "F*** that shit! People deserve better than this!"

...and here we are. =^/.^=