Closed CapnBry closed 3 years ago
Great job @CapnBry ! I'll take a closer look later today before merging.
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.
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 Buffer.FrameLength
to 2, which would make the first read call get the packet length, then adjusting Buffer.FrameLength
to the length of the remaining packet, thinking that would mean the serial read wouldn't wait for a timeout, it should always try to read the exact number of bytes needed to build the packet. It spammed a ton of "Read beyond frame" to debug which I thought might cause more CPU use than needed so I switched back to just a fixed size. I tried adjusting the serial.readTimeout but didn't have any better luck.
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.
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.
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.
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.
As far as frame drop, I tried setting
Buffer.FrameLength
to 2, which would make the first read call get the packet length, then adjustingBuffer.FrameLength
to the length of the remaining packet, thinking that would mean the serial read wouldn't wait for a timeout, it should always try to read the exact number of bytes needed to build the packet. It spammed a ton of "Read beyond frame" to debug which I thought might cause more CPU use than needed so I switched back to just a fixed size. I tried adjusting the serial.readTimeout but didn't have any better luck.
Technically it should be exactly in the way you attempted. That was the point of FrameLength
- to get only as much bytes as needed without waiting. Unfortunately I can't test this as I don't have CRSF. Maybe it could be tested with an arduino that simulates the CRSF frames but I'm not sure if the arduino itself will be able to handle the higher baudrates.
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.
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.
Thanks, so everything is fine as it is then.
Technically it should be exactly in the way you attempted. That was the point of
FrameLength
- to get only as much bytes as needed without waiting. Unfortunately I can't test this as I don't have CRSF. Maybe it could be tested with an arduino that simulates the CRSF frames but I'm not sure if the arduino itself will be able to handle the higher baudrates.
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 Buffer.FrameLength = 2
and then switch to Buffer.FrameLength = len
to read the rest of the packet, and it reads over 500Hz no problems at all. The CPU usage is appears to be slightly lower if I comment out the System.Diagnostics.Debug.WriteLine("Read beyond frame");
when "// reading beyond the FrameLength?". It seems the code implies that this shouldn't happen, but is the case if you do the 2/len cadence. Let me know if you'd like me to change it, but I'd recommend removing the "Read beyond frame" because it is really "Read beyond initial read" which is perfectly valid when reading 2 then len bytes.
These are the changes needed to get 500Hz working:
diff --git a/vJoySerialFeeder/SerialProtocols/CrsfReader.cs b/vJoySerialFeeder/SerialProtocols/CrsfReader.cs
index 10f2168..9ae0392 100644
--- a/vJoySerialFeeder/SerialProtocols/CrsfReader.cs
+++ b/vJoySerialFeeder/SerialProtocols/CrsfReader.cs
@@ -66,7 +66,7 @@ namespace vJoySerialFeeder
public override void Start()
{
serialPort.ReadTimeout = 500;
- Buffer.FrameLength = CRSF_PAYLOAD_SIZE_MAX + 2;
+ Buffer.FrameLength = 2;
}
public override void Stop()
@@ -84,7 +84,7 @@ namespace vJoySerialFeeder
Buffer.Slide(1);
return 0;
}
-
+ Buffer.FrameLength = len;
// Read the whole frame up to the CRC byte
crcCalc.Out = 0;
@@ -126,6 +126,7 @@ namespace vJoySerialFeeder
} /* if packed channels */
} /* if valid checksum */
+ Buffer.FrameLength = 2;
Buffer.Slide(2 + len + 1);
return retVal;
}
Buffer.FrameLength
should be the exact number of bytes of the frame. From the Buffer.Slide(2 + len + 1)
I think that this is the full frame length. So this is the same number that should be set to Buffer.FrameLength
after the initial 2 byte read.
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:
Application: vJoySerialFeeder.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.IndexOutOfRangeException
at vJoySerialFeeder.MainForm.buildProfile()
at vJoySerialFeeder.MainForm.resetProfile()
at vJoySerialFeeder.MainForm..ctor(System.String[])
at vJoySerialFeeder.Program.Main(System.String[])
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 bin/windows/Release
folder and copied the files from the release zip there, and your binaries worked fine too. I tried moving those files anywhere else around my hard drive and they don't work, the only place I can run the app from is C:\Users\bmayland\Documents\GitHub Projects\vJoySerialFeeder\vJoySerialFeeder\bin\windows\Release
(or Debug). Even if I rename the folder ReleaseX it doesn't work. I even tried deleting my %appdata%/vJoySerialFeeder
directory and it still won't load from any other directory except the Release or Debug directory.
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
p.Protocol = Protocols[comboProtocol.SelectedIndex].Name;
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 Protocols[0]
then saving a new profile and reverting that change. After that, the app loads again but if I try to move it, it crashes again, so works fine assuming it never creates a new default profile because it is running from another directory.
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 %appdata%/local/vjoyserialfeeder
:
vJoySerialFeeder.exe_Url_luekjd05bucqadhno5gd021aku05t14b
vJoySerialFeeder.exe_Url_m4xe4hdc40msoq33ztdqxkw5naht0k1p
vJoySerialFeeder.exe_Url_yxchnrudyucahaifeo4w5oz0jme3jhah
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.
I've pushed another commit. Can you give it a try?
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.
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? I am connecting my FC via USB in an ubuntu machine. But vJoy does not recognize it and shows failsafe message.
(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.)
Has anybody tested the crossfire on a flight controller? I am connecting my FC via USB in an ubuntu machine. But vJoy does not recognize it and shows failsafe message.
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
set usb_hid_cdc = ON
save
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)
serialpassthrough 1 420000
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.