-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CrsfReader serial reader for Crossfire serial #49
Conversation
Great job @CapnBry ! Sorry about the folders. When the source files were increasing in number I decided to place them in folders. It seems that it is conventional to match the directory structure to the namespaces. Yet I didn't want to do that because I had to refactor some things. I just wanted to organize the code while keeping everything in the same namespace. I see that maybe this hasn't been the best decision. |
Haha it's not your fault! Visual Studio was just making me crazy last night because it first wanted to insert two spaces into every line of the MainForm.Designer.cs and change the whole file. I thought I had it all figured out (just edit the .cs file directly to add the CRSF option) and then the folder thing snuck in on me. I was cracking up. I forgot to mention that in testing it worked up to our 250Hz output mode no problem, but I started losing some data at 500Hz (was only getting ~475Hz). That's a ton more packets than a joystick needs, but there may be some optimization that could be done. I really like that buffer class you've got since it makes it so easy to build the incoming packet without having to make a separate copy across multiple calls. Just let me know if you have any notes. I put my CRC8 class outside the CrsfReader class since I thought you might want to refactor that into a common location, KISSReader uses the same CRC8 even with the same polynomial, so it might be nice to use the same code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to make these minor changes?
Otherwise great job! Let the crc8
stay as it is at this point. Maybe some day I'll refactor it.
I'm not sure about the frame drop. I've noticed that some UART converters have very crappy Windows drivers consuming insultingly high cpu even at the low speeds. I've had complaints about this from some users using super high end gaming pcs that still got 5-10% cpu usage. We've made a test and it turned out that even if you simply open the port in any program the cpu jumps. So it's obvious something with the drivers. If you use dummy
, vjsf barely consumes any cpu.
Great job! Another question: can you tell me if there's some bit in the CRSF frame indicating Failsafe? I'm not sure if anyone uses these features but at least to know it if it's needed some day. |
Changed made! I didn't want to reorder the Protocol dropdown due to the profile storing the index and I was concerned about someone who had Dummy selected in the profile getting changed to CRSF on next run, but I suppose that's not an issue. I adjust the slide() but also changed the minimum length up to 2 while I was there, since 1 would be a packet with just a checksum and no payload which I don't think exists. As far as frame drop, I tried setting |
There's no failsafe bit. The CRSF system uses the "No pulses" sort of method to determine failsafe at the flight controller. If a serial packet isn't received in 250ms, it assumes failsafe, but handled entirely on the flight controller, not the RX. |
Hm that's a good point. I should have thought of that while implementing the config storage :D I'll see if I can do something about it.
Technically it should be exactly in the way you attempted. That was the point of |
Thanks, so everything is fine as it is then. |
Sorry to keep commenting on this closed PR, but actually I just tested in a Release build and actually I guess the CPU impact isn't too bad if I do the These are the changes needed to get 500Hz working:
|
If it's set correctly it can become: Buffer.Slide(Buffer.FrameLength);
Buffer.FrameLength = 2; I don't know if gh will allow you to push more commits here or another pr. |
Hey @Cleric-K, it's coming up on a year since there's been any changes to the codebase. Any chance you can put together a release so ExpressLRS users can use your official build instead of me distributing binaries of my branch? |
Hi, sorry for that. The project has somehow remained for me with a lower priority and it's really disturbing that so much time has passed. But I'll really get down to push a new release because there are already several PRs waiting. Thanks for bumping! |
Hi, after a long time I finally had some time to work and integrate the various new things. Could you please test the new release to confirm if everything's OK with CF? Thanks for the patience! |
Yup I can confirm it works, great to have a release with CRSF support, I look forward to sharing it with everyone looking for an ExpressLRS joystick. I have a really weird issue though with the files from the release. When I try to launch it, it does nothing at all. I check the Event Log and I see this error message:
I would post an issue for it, but when I built the latest source and ran that, it worked no problem. I then deleted everything in the Visual Studio This is the line of code that breaks it, since there's no UI loaded at all yet, but I am not sure why it works when running from the Release directory
EDIT: Well now it isn't working from anywhere at all any more, with it crashing on that line. Clearly .NET configstore is blowing my mind right now, but I worked around it by forcing it to |
Thanks, I'll try to reproduce. I did some changes to the configuration. As we spoke before, it used to save the index of the protocol combobox but this makes it necessary that the order never changes. I changed it such that the class name is saved in the config instead. The config is automatically upgraded. I'll give it a try now. |
I've pushed another commit. Can you give it a try? |
BTW the thing with the dependence on the directory is because .NET creates a separate config storage for every path/executable in
Not only the path is taken into account but also the name of the exe. It seems they are hashed in some way and used as the storage folder name. The bug was related to the changes I've made. When the program starts without prior configuration it didn't initialize the protocol combobox properly. |
Yup works great in every instance. Move to a new folder, get a fresh new config with no crash. Move back to the other folder, it has my config and launches fine. |
Great! Thanks for spotting this bug! Before I publish it as a release I want to make sure that it also works on Linux but I'll be able to do that in few hours. I'll let you know. |
1.7.1 has been published. |
Confirmed, works great! Thanks for your help and for your work on this project. |
Thanks for your great work @CapnBry Has anybody tested the crossfire on a flight controller? (more info: I am trying to read my tbs tango rc inputs eith vJoy. It is bound to the tbs nano (receiver) with is wire to the FC. And FC is connected via USB to the computer with ubuntu os.) |
You're adding too many pieces 🙂 vJoySerialFeeder connects directly to the Crossfire Nano RX, not the flight controller. You hook the RX/TX to an FTDI adapter and then you'll get CRSF data out. If you still want to use the Flight Controller, if it has betaflight (or I think iNav does it too) you can turn the FC into a joystick by going to the CLI tab in the configurator and setting it to joystick mode with
Now you'll see a joystick device in your operating system and don't use vJoySerialFeeder at all. If you still want to use BOTH the FC and vJoy, instead you can put your Flight Controller into passthrough mode to have it send the receiver data out the serial port. Replace 1 with the correct UART number (the number may not be the same as it is in the Configurator, it may be off by one)
|
I am an ExpressLRS developer and one of our users asked us to support IBUS output so people would be able to use ExpressLRS with vJoySerialFeeder but I say why make our stuff support that if I can make someone else support us instead? 😄
This PR adds CRSF protocol support to vJoySerialFeeder. It should work with any Crossfire receiver, with just the TX wire to an RX on the FTDI connector. I only tested it with one running ExpressLRS though.
Your architecture is fantastic, it took me only 30 minutes to get this done and like an hour of trying to prevent my Visual Studio from reorganizing all your code when I tried to add a single line. I was largely successful in that, but let me know if something didn't work out. I swear it was a nightmare.