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

Tools: Add RealFlight Autotest #28293

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

robertlong13
Copy link
Collaborator

On the dev call, we were discussing testing #27839 in RealFlight. I thought it would be nice to have a framework for running optional automated tests in RealFlight. I ran the test before and after the PR and loaded both logs into the PID Review Tool and compared. They look the same (though @IamPete1, it would be really nice to be able to load two logs in here for this).

image

Draft PR because I'd like to get some advice on the right way to do add this. Do we do this by adding disabled tests to things like quadplane.py? Do we add separate files like quadplane_realflight.py? Not sure. This is my a-bit-hacky placeholder that I used to test the aformentioned PR (though I'm very late as it's been merged for a week).

(This PR is currently based on top of #28282, so only the last two commits in this will be here after the other is merged)

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a great fan of needing to pass in the realflight address.

Ideally we:

  • integrate this correctly with vehicleinfo.py
  • get rid of "external: True" from vehicleinfo.py, instead specify which external simulator is involved
  • factor out the code from FlyEachFrame which sets things up from the vehicleinfo definition
  • read configuration for external simulator from a .json file - same code as we use for callisto.json but pointing out how to use the external simulator
  • extreme stretch goal allow use of external simulator from sim-on-hardware
  • use the same information from vehicleinfo.py in sim_vehicle.py to make it easy for it to use the same definitions.

Tools/autotest/default_params/realflight-titan-cobra.parm Outdated Show resolved Hide resolved
name = test.name
else:
name = test.__name__
if name.lower().startswith("realflight"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, very much not the right way to do things :-)

Tools/autotest/quadplane.py Outdated Show resolved Hide resolved
Tools/autotest/quadplane.py Outdated Show resolved Hide resolved
libraries/SITL/SIM_FlightAxis.h Outdated Show resolved Hide resolved
libraries/SITL/SIM_config.h Outdated Show resolved Hide resolved
libraries/SITL/SITL.cpp Outdated Show resolved Hide resolved
@robertlong13
Copy link
Collaborator Author

robertlong13 commented Oct 3, 2024

Not a great fan of needing to pass in the realflight address.

Unfortunately I see no good way around this. The address will be different for every person, unless they are crazy and running autotests with Cygwin. If they are using WSL (like me), I can have a shell command I get it automatically, but if someone uses separate machines (which I believe is more common in the core dev group), or uses a VM, we need to give them the option to override.

If we want to force this to only work in WSL and Cygwin, I can remove that.

@robustini
Copy link
Contributor

robustini commented Oct 4, 2024

Yes, self-testing could also be interesting.
I have already tested the reset position on startup and it works well.

@robertlong13 robertlong13 marked this pull request as ready for review November 27, 2024 04:45
@robertlong13
Copy link
Collaborator Author

Big update after long discussion with Peter on voice chat. I'm really only addressing one of his bullet points above, but he seemed generally in favor of this when we spoke.

I've eliminated the dirty hack for how I made CI skip this test (and replaced it with something that still isn't perfect but hopefully tolerable). Now, unless you have REALFLIGHT_IPADDR set or pass the --realflight-address argument, this test automatically passes. Again, the idea here is to be able to manually run some tests locally and watch/analyze to make sure everything works correctly, while still getting to work within our extensive autotest framework. This does not handle automatically launching RealFlight, selecting the aircraft/airport (though I have some ideas for a tool that could do that in the future).

Added the ability for SITL to look for an environment variable for the RealFlight IP address, which saves a little typing vs passing it in the frame name. On WSL, I use this in my bashrc (because frustratingly, the windows host IP is not static):

# IP Address of the Windows host running RealFlight for ArduPilot SITL
export REALFLIGHT_IPADDR=$(host $(hostname).local | grep "has address" | awk 'NR==1 {print $4}')

For those of you running RealFlight on a separate windows box, you can just make that constant.

This is now integrated with vehicleinfo.py (now that I understand that file's purpose). You can launch with sim_vehicle.py -v ArduPlane -D -f realflight-titan-cobra (and I'd like to add a few more from the SITL models repo). When launching with sim_vehicle, it defaults to using RealFlight's built-in joystick handling, but if you want to use MAVProxy's RC, you can disable the FDM with the RC_PROTOCOLS parameter and launch with the extra --mavproxy-args --sitl=127.0.0.1:5501.

There's one last sneaky issue that I'm hoping @tridge might be able to shed some light on. When I run with the autotest, I get occasional socket timeouts between SITL and RealFlight. When I fly manually, I basically never get these. With autotest, I get them randomly, on average maybe every 10 minutes. With the current 1s timeout, I'd get a large disturbances when it happens. I changed it to timeout after a max of three frames and now when a socket timeout happens, it handles it gracefully after 25ms (how I'm getting 10,000 errors in 25ms doesn't make sense; the update function isn't called nearly that fast) with no noticeable symptoms when they occur.

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

Successfully merging this pull request may close these issues.

3 participants