Skip to content
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

feat(simu): cross-platform connection of host serial ports to simu radio #5584

Merged

Conversation

nrw505
Copy link
Contributor

@nrw505 nrw505 commented Oct 3, 2024

Summary of changes:

  • Replaces the old way of connecting simulated radio AUX ports to the host's serial ports.

It's a bit convoluted because QSerialPort has to belong to the main event loop's thread, not the simulator's thread. We rely on QT's signal management to let us shunt data between the threads nicely.

@nrw505 nrw505 force-pushed the use-real-host-serial-device-for-aux-in-simulator branch 2 times, most recently from 9a00027 to 1b6a405 Compare October 3, 2024 11:08
@wimalopaan
Copy link
Contributor

Would this allow to connect an external module (e.g. ELRS RX-as-TX bidirectional connected) to the simulator?

@rotorman
Copy link
Member

rotorman commented Oct 7, 2024

The task was explicitly not to require to communicate within the simulator with physical internal or external RF modules, as the serial communication with them has too tight timing constraints, which typical computer OS, w/o real-time capabilities, would not be able to adhere to: https://edgetx.org/contest_companion_phy_ser/

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 7, 2024

Would this allow to connect an external module (e.g. ELRS RX-as-TX bidirectional connected) to the simulator?

If ELRS RX-as-TX currently works on AUX1 or AUX2, then maybe, with absolutely no guarantees of reliability given the impossibility of guaranteeing timing on a general purpose OS. If you're going to try it for god's sake make sure you've got your props off.

@wimalopaan
Copy link
Contributor

I tried this PR unter Linux: selecting a serial device like ttyUSB0, then OK and then reentering the serial device dialog shows not assigned again.

@wimalopaan
Copy link
Contributor

I tried this PR unter Linux: selecting a serial device like ttyUSB0, then OK and then reentering the serial device dialog shows not assigned again.

I tried both the prebuilt AppImage as well as a self-compiled version: in both variants I can't successfully choose a serial device.

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

I tried this PR unter Linux: selecting a serial device like ttyUSB0, then OK and then reentering the serial device dialog shows not assigned again.

I tried both the prebuilt AppImage as well as a self-compiled version: in both variants I can't successfully choose a serial device.

Did you see if data flowed between the simulator and the serial port, or did you just check to see if the dropdown had the correct value selected when you opened the serial device dialog again?

I've just fixed the issue where the dialog wouldn't select the currently-connected port when it opens :)

@wimalopaan
Copy link
Contributor

wimalopaan commented Oct 8, 2024

Just tried with yout latest update (both pre-built app-image as well as self-build)

  1. Now the device selection is permanent ;-) But not across restarts of the simulator.

  2. Unfortunately there is no data flow on the serial device (checked that with LA).

@wimalopaan
Copy link
Contributor

And I want to say that it would be a real cool feature if simu could talk to an ELRS Rx-as-TX. At least sending CRSF_CHANNEL packets should be possible with loose timing requirements. Maybe elrsv3.lua would also be possible (no need for elrsbuddy anymore).

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

But not across restarts of the simulator.

It's not expected to persist across restarts of the simulator.

  1. Unfortunately there is no data flow on the serial device (checked that with LA).

Interesting, it's working for me (on linux and windows) with a GPS plugged in to the host.

a) How are you configuring the simulated radio?
b) What do you have plugged into the host's serial port?
c) Can you see any errors in the log with HostSerialConnector::serialStart() ?

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

And I want to say that it would be a real cool feature if simu could talk to an ELRS Rx-as-TX. At least sending CRSF_CHANNEL packets should be possible with loose timing requirements. Maybe elrsv3.lua would also be possible (no need for elrsbuddy anymore).

It might be cool, but it won't happen in this PR. (I've had a poke through and it looks like you'd need to wait for #4102 to go in)

@wimalopaan
Copy link
Contributor

But not across restarts of the simulator.

It's not expected to persist across restarts of the simulator.

Ok, no problem ;-)

  1. Unfortunately there is no data flow on the serial device (checked that with LA).

Interesting, it's working for me (on linux and windows) with a GPS plugged in to the host.

a) How are you configuring the simulated radio? b) What do you have plugged into the host's serial port? c) Can you see any errors in the log with HostSerialConnector::serialStart() ?

Maybe we are talking of different things.
You are reading from the serial port.
My objective is to use the serial port to get the data sent to the external module. So, my hope was to set AUX1 to external module and set in the model config external module to crsf.

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

If ELRS RX-as-TX currently works on AUX1 or AUX2

I've just had a poke around, and it doesn't.

Maybe we are talking of different things. You are reading from the serial port. My objective is to use the serial port to get the data sent to the external module. So, my hope was to set AUX1 to external module and set in the model config external module to crsf.

If you set AUX1 to External module and set the model config external module to CRSF then the radio does not actually try to use AUX1 to talk to the external module (at least not on any radio that has an external module). This is why the simulator doesn't send anything over the serial port when you try to do that in the simulator - it's doing the same thing that a real radio does in that situation which is not even initialising AUX1.

@wimalopaan
Copy link
Contributor

If you set AUX1 to External module and set the model config external module to CRSF then the radio does not actually try to use AUX1 to talk to the external module (at least not on any radio that has an external module). This is why the simulator doesn't send anything over the serial port when you try to do that in the simulator - it's doing the same thing that a real radio does in that situation which is not even initialising AUX1.

Mmh, that's a pity!
Then my understanding of the setting AUX1 -> external module was completely bogus!

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

Mmh, that's a pity! Then my understanding of the setting AUX1 -> external module was completely bogus!

I suspect that the point of AUX1 -> External module is to allow UART-based modules e.g. multi / CRSF on radios whose module bay doesn't have a UART, just PPM. But I can't say for sure.

@rotorman
Copy link
Member

rotorman commented Oct 8, 2024

@wimalopaan See: #2562

@wimalopaan
Copy link
Contributor

The baudrate should be user settable I think, and not fixed 9600Bd

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 8, 2024

The baudrate should be user settable I think, and not fixed 9600Bd

The baudrate on the host will be set to whatever the radio has set it to.

opentxsimulator.cpp:167 -> opentxsimulator.cpp:144 -> opentxsimulator.cpp:678 -> simulatormainwindow.cpp:160 -> hostserialconnector.cpp:136

@wimalopaan
Copy link
Contributor

The baudrate on the host will be set to whatever the radio has set it to.

Yes, I was aware of that fact but did not find the link to your new code ...
Thanks.

@wimalopaan
Copy link
Contributor

Here the files and sections from previous post with links for easier reading:

Many thanks ;-)

@wimalopaan
Copy link
Contributor

I still wonder about the best way to achieve the goal to attach an ELRS-module to the simu ...

@nrw505
Copy link
Contributor Author

nrw505 commented Oct 9, 2024

I still wonder about the best way to achieve the goal to attach an ELRS-module to the simu ...

Something like the approach in this PR but instead of connecting the simulated AUX1/AUX2 to a host serial port, you'd be connecting the simulated internal or external module to the host hardware in some way?

OR you could extend the module handling code to support having a third (or fourth) serial-connected (no module heartbeat for sync, for example) module on AUX1/AUX2 and use this PR to connect it to the host.

@wimalopaan
Copy link
Contributor

Actually I hestitate to use a serial device. I think of a pipe/socket or shared-memory to communicate with a kind of module-simulator ...

@pfeerick
Copy link
Member

Sorry it's taken so long to come back to this one.

My intended route to test this was to simply use a USB Uart (i.e. CH340/FTDI) and do a loopback on the TX/RX, thus could test Lua serialRead and serialWrite. However, it has not been co-operating, and so first had to debug my own setup. But now it looks like a bug in the implementation, as it looks like Lua is only sending the last byte of a string? And I don't know if it is able to read anything at all, as it appears to be reading an empty string.

A secondary bug appears to be the serial port is not released/returned when you close the simulator, so you need to restart Companion in order to use the simulator with a serial port on a second run. This was on Windows, not sure about Linux. It would also be nice it if there was message out to the user that serial port was already in use.

Later this week I'll dig out a GPS, and try it with that.

i..e this should have been "EdgeTX"
image
and this should have been "Eddie"
image

I used a terminal app just to sanity check "EdgeTX"
image

This was all on Windows. Linux appears to be the same, but I've not verified after confirming my test setup is indeed correct.

Stupid simple lua test (which was working on TX16S handset with a longer test string):

-- toolName = "TNS|_PR5584 EdgeTX|TNE"

local firstRun = true

local function init()
end

local function run(event)
  if firstRun == true then
    firstRun = false
    -- setSerialBaudrate(9600)
    lcd.clear();
    lcd.drawText(10,10,"PR5584 Serial Loop test")
    lcd.drawText(10,30,"Press ENTER to write to serial")
  end
  
  if event == EVT_VIRTUAL_EXIT then
    firstRun = true
  elseif event == EVT_VIRTUAL_ENTER then
    lcd.clear();
    lcd.drawText(10,50,"Writing to serial")
    serialWrite("EdgeTX")
  end

  local output = serialRead()

  if output ~= nil and output ~= '' then
    lcd.drawText(10,70,output)
    print("serialRead: " .. output)
  end

  return 0
end

return { run=run, init=init }

@nrw505
Copy link
Contributor Author

nrw505 commented Nov 8, 2024

A secondary bug appears to be the serial port is not released/returned when you close the simulator, so you need to restart Companion in order to use the simulator with a serial port on a second run.

Whoops, this is what happens when I test with simulator211 not companion211. Fixed in latest push.

re: the LUA loopback test: turns out that QByteArray::fromRawData doesn't copy the data it's given but points directly to it. And with the timing for the the GPS driver it just happened to be that the data got sent before the buffer got overwritten. But when sending data from LUA, the buffer got overwritten for each character while the queue of QT signals grew. Also fixed in latest push, at least that's what it looks like on my machine :)

@pfeerick
Copy link
Member

pfeerick commented Nov 9, 2024

Looking good so far... both fixes working fine here on Linux :)

edit: and Windows

@nrw505
Copy link
Contributor Author

nrw505 commented Nov 9, 2024

Now just need to find someone with a mac 😆

@pfeerick
Copy link
Member

pfeerick commented Nov 9, 2024

Welp... github blocked my email reply...

Got someone in mind already, just need to send him a reminder to test this as we were discussing this just before EdgeTX fest so would have been forgotten in the chaos of getting ready for that.

@wimalopaan
Copy link
Contributor

Works for me now, I can output on AUX1 and AUX2. Very cool, thanks!

@wimalopaan
Copy link
Contributor

I went a little bit further:

  1. I wrote a lua-mixer script that encodes the ch1-16 and sends them as crsf-frames via serialWrite()
  2. I wrote lua functions crossfireTelemetryPush() and crossfireTelemetryPop()
  3. then I added these lua function to elrsV3.lua

and - voila - the simulator now can talk to my rx-as-tx ELRS and to my receiver and all crsf-devices attached to that receiver.

This is really cool.
Well, no need for ELRSBuddy any more ;-)

@wimalopaan
Copy link
Contributor

Are chances that thhis PR gets into 2.11?

@pfeerick pfeerick added this to the 2.11 milestone Dec 5, 2024
@pfeerick pfeerick force-pushed the use-real-host-serial-device-for-aux-in-simulator branch from f499d9f to 0593241 Compare December 5, 2024 03:17
@wimalopaan
Copy link
Contributor

  • I wrote a lua-mixer script that encodes the ch1-16 and sends them as crsf-frames via serialWrite()
  • I wrote lua functions crossfireTelemetryPush() and crossfireTelemetryPop()
  • then I added these lua function to elrsV3.lua

The above mentioned script you may find on: https://github.com/wimalopaan/LUA

@wimalopaan
Copy link
Contributor

Maybe we can extend this PR (later) to add a new option: not only select a host-serial for AUX1 or AUX2 but also for internal or external modul.

@wimalopaan
Copy link
Contributor

wimalopaan commented Dec 8, 2024

Tested this PR for a while now and had no problems.
Is it possible to merge it into master so that is available via the nightlies to all people/platforms?

@pfeerick
Copy link
Member

pfeerick commented Dec 8, 2024

What platform(s) are you using? Just waiting for @JimB40 to confirm it is working on MacOS, as it has been tested on Linux and Windows. Until it is merged, builds for all platforms are available via the Checks tab as with all other PRs ;)

btw, while it would at best be material for a different PR, I strongly doubt internal/external module will be possible, as the timing requirements are too critical. serial is relatively easy, especially as there is OS level drivers and support for it, but microsecond timing for module communications on general purpose operating systems is a whole different story.

@wimalopaan
Copy link
Contributor

wimalopaan commented Dec 8, 2024

I tested it only on Linux (arch-linux).

I wrote the above mentioned LUA-scripts and now I can steer all models from the desktop ;-)

A video of the bench: https://youtu.be/6hBYG1nIjNw

As long as the ELRS module receives channel data with a least 5Hz it switches to transmit-mode and sends RADIO_ID packets to the simulator. As you can see the communication is slower than on a real radio. If this is due to the fact that all is done via LUA or due to the wrong synchronization I don't know.

@JimB40
Copy link
Collaborator

JimB40 commented Dec 8, 2024

Just waiting for @JimB40 to confirm it is working on MacOS

Got this msg from @pfeerick

Simplest test of the concept with a USB serial adapter is link the TX and RX lines and run the dumb script here... (use the new Tools -> Serial Ports entry to connect the usb uart to the simulator, and don't forget to assign either AUX1 or AUX1 to Lua in radio settings -> hardware 😉 ), and thus testing if RX hears what TX sends. If that works, the PR is complete.
The equivalent on the handset would be to link the TX and RX on either AUX1 or AUX2, and assign the correct one to Lua, and then run the script

not much time to dig into
Can some of you record simple short video what to connect/setup and test

@rotorman
Copy link
Member

rotorman commented Dec 9, 2024

That was unfortunately total bulls... from me, thus ignore and read on from next post's second paragraph instead...: Take a radio with accessible dual serial ports, like RM TX16S. Take two wires with Dupont female connectors on both ends. Wire up one wire between AUX1 RX and AUX2 TX lines. And wire up the second wire between AUX2 RX and AUX1 TX lines. Flash the firmware binary for TX16S from this PR. Now power on and attach the radio via top USB to your Mac. HW setup for testing done.

@nrw505
Copy link
Contributor Author

nrw505 commented Dec 9, 2024

Take a radio with accessible dual serial ports, like RM TX16S. Take two wires with Dupont female connectors on both ends. Wire up one wire between AUX1 RX and AUX2 TX lines. And wire up the second wire between AUX2 RX and AUX1 TX lines. Flash the firmware binary for TX16S from this PR. Now power on and attach the radio via top USB to your Mac. HW setup for testing done.

This is not the case. This PR only changes the simulator so that the simulated radio (e.g. simulated RM TX16S) AUX1 and AUX2 can drive a serial port on the host (e.g. a USB serial adapter)

To set up a test, take a USB serial adapter and connect it's RX to it's TX, then test that the simulator can receive what it sends using @pfeerick 's LUA script. I'll try to get a screen recording of how I'm testing that on linux tomorrow when I'm at the right computer.

@pfeerick
Copy link
Member

pfeerick commented Dec 9, 2024 via email

@nrw505
Copy link
Contributor Author

nrw505 commented Dec 9, 2024

Risto is only detailing how to reproduce the expected functionality on HW side... As simulator should now behave exactly the same as the hw, when the USB serial adapter is wired in a loopback.

That makes more sense :) . But in that case you'd need to wire AUX1 TX to AUX1 RX and AUX2 TX to AUX2 RX

@pfeerick
Copy link
Member

pfeerick commented Dec 10, 2024 via email

@pfeerick
Copy link
Member

pfeerick commented Dec 16, 2024

@JimB40 has just tested this on MacOS and confirmed that my simple loopback test is working perfectly. Thus this PR is good to go! :)

@pfeerick pfeerick merged commit 9109d4f into EdgeTX:main Dec 16, 2024
51 checks passed
@rotorman
Copy link
Member

rotorman commented Dec 16, 2024

@nrw505 Congratulations on winning our fourth developer competition!
https://edgetx.org/contest_companion_phy_ser/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants