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

[WIP] Integration tests: Discussion #10791

Closed
wants to merge 28 commits into from
Closed

Conversation

Stifael
Copy link
Contributor

@Stifael Stifael commented Oct 31, 2018

Background

Currently PX4 evolves primarily through feature addition. However, these features are added without guarantee that no other legacy implementation breaks. To reduce this uncertainty, there is now a clear effort to expand flight-testing through the @PX4/testflights team. However, many implementations cannot be directly perceived through actual testing but rather would require in depth log analysis. In addition, many legacy implementations are not known anymore and just dangle around in the code and waiting for someone to break it such that it can be re-fixed afterwards because a few users depend on it. Although with the current PX4 state there is no way around of breaking it and fixing it again (since that feature might not be known to the developer), it would be desired that after that re-fix of the broken legacy implementation, that part of the code will never break again, even if that feature is unknown to anyone.
One possible approach to tackle this problem is to make use of integration tests. PX4 already uses an integration-test framework which is based on ROS or Dronekit. However, there are about 5 integration tests overall, which already have aged quite a lot. One reason why the set of integration tests are not increasing is probably because it is difficult to run these tests on your local machine without going through a nightmare of installing additional dependencies.
Luckily, recently dronecode_sdk has evolved quite a lot, which makes it very easy to set up simple missions or offboard related maneuvers. Offboard is not fully there yet, but that is mainly due to the lack of resources and time. Having dronecode_sdk as part of the PX4-firmware, every developer can easily generate simple maneuvers that then can be used for integration testing.

Integration test pipeline

The suggested integration test pipeline is different to the integration tests known. Originally, an integration test consisted of a simulated maneuver with immediate test-checking. The approach that I suggest is based on the log-files:

  1. Simulated Maneuver is executed. This generates a log-file .ulg
  2. The .ulg file is the input to the integration tests.

There are several advantages of writing integration tests based on .ulg file:
Self-contained development of simulated maneuvers
The simulated maneuvers do no longer require any tests. They can be implemented in such a way that they could also be run in reality. For instance, a simple maneuver could be to fly a mission in a square where each waypoint is 5m apart. Without any effort, that same mission can be run on the actual vehicle as well. This means that if we have a set of maneuvers, the @PX4/testflights team could in addition to the usual flight tests also run these deterministic flight tests.

Self-contained testing framework based on .ulg-file
We are completely free to choose any framework for testing. Since any .ulg file can be converted into python structure through pyulog, a good option would be a python testing framework.

The tests can be split into general tests and simulated tests:

  • General tests are tests that are simulation independent. For instance, the mission speed can never exceed MPC_XY_CRUISE independent of simulation or actual flight. Consequently, one can write a test that checks for the cruise-speed during mission. This test can be applied to a simulated maneuvers as well as to an actual flight-test log-file.
  • Simulated tests are tests that are using deterministic tests. For instance, a simulated test could be a test that checks if the vehicle reached a predefined waypoint within a certain time.

Current state

px4maneuvers

px4maneuvers is a simple project that uses dronecode_sdk to generate simple missions (https://github.com/Stifael/px4maneuvers). I added this project to the firmware (which is this PR). To run the example mission, all you need to do is:

  • update submodule
  • build the example mission
  • run it as explained in the repo.
    I added a maneuver.sh script that simplifies the build process. From the root of px4-firmware, type:
chmod +x Tools/maneuver.sh
./Tools/maneuver mission

In an other terminal, start sitl as usual.

This project can now easily be extended. I also think that it would help the active development of the SDK. For instance, if the PX4-community decides to use the SDK for integration testing as well, then I am confident that developers will be more active in shaping and contributing to the SDK project as well.

uloganalysis

The name uloganalysis is not a really good name and needs to change eventually. That said, everything is just [WIP] anyway.
This project is here. Most of the details are provided in the README.md. The main objective of this project is to re-sample and merge the pyulog output into pandas dataframe (similar to px4tools). The goal is that this project stays simple without adding too fancy analysis methods. It also should not really analyze anything, but rather should provide convenient functions to add and extract info from the .ulg file. For instance, roll-/pitch- and yaw-error can be easily computed from the vehicle_attitude and vehicle_attitude_setpoint message. Another example would be tilt, which is a fundamental constraint within the px4-firmware but is not logged. That information can be extracted with uloganalysis.
Another goal of this project is to enable other projects such as testing, visualization etc. For instance, PX4-FlightReview is a good tool for sharing general flight related information. FlightPlot is a good tool to go a bit more into detail and look at messages which are not shown in FlightReview. However, computations such as the tilt-computation is not possible either. Hence, from a developer perspective, I prefer to have a setup that is more flexible. Consequently, uloganalysis could be used in any python project (for instance jupyter-notebooks for sharing) and therefore allows the developer to make full use of the python tools.
Once this project is matured, the project could then be packaged and distributed over PyPa (everything should already be in place).

px4ulogtests

px4ulogtests is a project that consists of test-scripts and is based on uloganalysis. The purpose of this project is to use it for integration testing, but also for actual flight testing and creating reports. Currently this project only contains one test, where the the vehicle's tilt during Manual and Altitude flight is tested. This test is a general test, which means it can be applied to simulated maneuvers and actual flight-tests.

What is needed

First I need to know if such a pipeline is something that the PX4-community is interested in. If that is the case, then each project needs to be discussed and reviewed again. However, for personal reason, I definitely don't want to have one large project that contains everything. To me it is crucial that uloganalysis stays independent. However, I am fine with any project as long as it fulfills a similar purpose.
Then we also need to discuss how to integrate such a pipeline into the current CI.
Once we agreed on a pipeline (or declined it entirely), and we still want to use the proposed projects, I would then like to move these repos over to the px4 umbrella.

@Stifael
Copy link
Contributor Author

Stifael commented Oct 31, 2018

I don't want to add every single developer as reviewer (seems like spamming everyone). Please just add your comments and thoughts.

@dagar dagar changed the title [WIP] Inegration tests: Discussion [WIP] Integration tests: Discussion Oct 31, 2018
@dagar
Copy link
Member

dagar commented Oct 31, 2018

@Stifael this sounds like a great idea. We have the CI capacity to run significantly more tests, and will soon have even more with @julianoes faster than realtime work.

I'll spend some time reviewing the submodule project in detail. The main thing I want highlevel is to 1) make it easy to add new tests and 2) make it absolutely trivial to reproduce a failure locally.

@mrpollo
Copy link
Contributor

mrpollo commented Oct 31, 2018

.@julianoes @JonasVautherin any word on the SDK submodule? is this the "right" way to include the SDK as a dependency for testing?

@julianoes
Copy link
Contributor

@mrpollo: I would like it to install the latet release and test against that but until that's ready the submodule is ok.

@bkueng
Copy link
Member

bkueng commented Nov 5, 2018

Nice, thanks Dennis for kicking this off. This is definitely helpful.

In addition to Daniel's points, some things from my side:

  • python would be my prefered choice as well, since the SDK now has a python frontend (and we should use that)
  • letting the analysis be based on log files (vs real-time feedback from the vehicle) has pros and cons:
    • + ability to run validation offline, after a (potentially real) flight
    • - no real-time checks, which might be required for some tests (e.g. timeouts if a waypoint is never reached, vtol never transitions, ...)
    • I also tend towards using the log file though
  • tests and ulog analysis are closely coupled in many cases (e.g. check against a mission plan with changing velocities, sending attitude/velocity setpoints, and afterwards checking them, etc). So ideally a test and validation should life closely together (i.e. single file), to simplify writing of tests.
  • uloganalysis: what's your take on plotting? I could see this being used in Flight Review as well, and the library could then also help with algorithm prototyping.

Some more test cases that should be considered by the design:

  • testing RC loss
  • low battery failsafe
  • GPS loss
  • testing different yaw behaviors in missions

@Stifael
Copy link
Contributor Author

Stifael commented Nov 11, 2018

about the negative points:

  • no real-time checks, which might be required for some tests (e.g. timeouts if a waypoint is never reached, vtol never transitions, ...)

That is still possible even with post ulog analysis, it just depends on the test. For instance, one can write a simple mission where the vehicle flies from one point A to point B and expect the vehicle to fly that distance within a certain time T. The genertated ulog file can still be used to check if the vehicle accomplished that simple maneuver.

So ideally a test and validation should life closely together (i.e. single file), to simplify writing of tests.

Agreed. However, I suggest to have two files. One file that has tests that always have to be satisfied, and one file with tests which are specific for a simulated test. For instance, the example above with the simple mission with waypoints A and B and expected time T is a very specific test that only applies to that particular mission. I would keep these tests seperated from the other general tests.

Some more test cases that should be considered by the design:

  • testing RC loss

I think this should be possible with post analysis, it just depends on what you want to test. Since most px4-flight behaviors are controlled by parameters, I think this possible.

  • low battery failsafe

same as above

  • GPS loss

same as above. Do you have anything in mind that would not be possible to test through ulog that is GPS loss related?

  • testing different yaw behaviors in missions

That is for sure possible.

In the simple tilt test that I have added as an example here, the test uses the parameter to check the maximum tilt and uses the navigation state topic to check the tilt for manual/stabilized only. The same logic can be applied to yaw, or any other message.

Right now I think it would be great to have a PR where the entire pipeline would be executed. For that, however, it would require some help with the CI integration tests.

@bkueng
Copy link
Member

bkueng commented Nov 12, 2018

That is still possible even with post ulog analysis, it just depends on the test.

I was thinking about aborting a test or taking some other action. How do you plan to handle that?

One file that has tests that always have to be satisfied, and one file with tests which are specific for a simulated test.

Sure I'd separate these as well. I don't see the general checks as individual tests, but just a set of checks that in every test need to be satisfied. These checks can be made available to all tests through a common base class for example.

Do you have anything in mind that would not be possible to test through ulog that is GPS loss related?

No, I think it will work.

@Stifael
Copy link
Contributor Author

Stifael commented Nov 12, 2018

I was thinking about aborting a test or taking some other action. How do you plan to handle that?

What would be the use case for stopping a test?

These checks can be made available to all tests through a common base class for example.

I think we are talking about the same thing. Maybe we need to discuss it during the dev-call.

Right now the way I planned the testing is as follow:

General tests

All the tests are within one file. Each test can either be called as a single test, or all tests can be run at once. Here, I would distinguish between CI and regular log file.

Regular log file

For a regular log file, I would like to run that file against all tests within the General-tests. This would require to have all tests written in such a way that any test can only fail if the base conditions are met and the test still fails. Let's take again the tilt-example: the test for tilt during Stabilized only makes sense if the message vehicle_local_attitude is present and the vehicle was in Stabilized for some time. If these conditions are not met, then the test will succeed.
If we now have a regular log file, we can then test it against the General tests and it will fail only where conditions for a test are met and the test fails.

CI

For CI, we obviously need simulated maneuvers that generate ulog files for us. Some of the simulated tests require a specific test from the Simulated tests. In addition they might require tests from the General tests as well, but not all of them. In principle one could run each simulated maneuver against all the general tests as well. However, I am wondering if that is scalable if we have about 100 simulated maneuvers and about 200 general tests.

My point is that I want to have both. Either you run the log file against all tests, or you run it against specific tests that you are interested in.

@bkueng
Copy link
Member

bkueng commented Nov 13, 2018

What would be the use case for stopping a test?

For example when in a mission, and the vehicle does not make any progress anymore.

My point is that I want to have both. Either you run the log file against all tests, or you run it against specific tests that you are interested in.

Sure, I see it the same way. I guess it's best to see that once it's implemented (is it already complete?).

read -p "Continue (y/n)?" answer

if [[ $answer == [Yy] ]]; then
mkdir $build_dir
Copy link
Member

Choose a reason for hiding this comment

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

Maybe first check if the submodule is checked out properly otherwise this will fail and the script continues.

fi

# start requested maneuver
cd $build_dir && ./maneuvers/$maneuver udp://:14540
Copy link
Member

Choose a reason for hiding this comment

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

newline before eof missing

@Stifael
Copy link
Contributor Author

Stifael commented Nov 26, 2018

uloganalysis: what's your take on plotting? I could see this being used in Flight Review as well, and the library could then also help with algorithm prototyping.

yes, that is the goal. It can be used by Flight Review.

I think it would make sense to have a simple example of that testing pipeline in action. To make that work, I will probably need @dagar's help. The CI would need to run the following components:

  1. make posix_sitl_default gazebo to start simulation
  2. ./Tools/maneuver mission for the simulated mission
  3. pytest test_general --filepath=[absolute path to log] to run the tests

@Stifael Stifael force-pushed the integration-testing-ulog branch from d1b857e to af25acf Compare November 27, 2018 10:27
@Stifael Stifael force-pushed the integration-testing-ulog branch from af25acf to 37efe77 Compare December 4, 2018 10:55
@Stifael
Copy link
Contributor Author

Stifael commented Dec 4, 2018

@dagar I added now a test_runner.sh script which does the following:

  1. start gazebo server and client, px4
  2. read newest ulog file
  3. run general test

Right now the logs are stored in the px4-src directory.

TODO:

  • create yaml config file that is read by run_tests.py
  • write a script to setup everything

PhiAbs and others added 2 commits December 12, 2018 09:19
- move yaml to config directory
- deactivate tests based on TestClass name and method
@Stifael
Copy link
Contributor Author

Stifael commented Dec 12, 2018

TODO

  • move px4ulogtest project into px4 src-tree
  • rename uloganalysis and push to PyPa once matured
  • add to CI as an example
  • add jmavsim

@Stifael
Copy link
Contributor Author

Stifael commented Feb 1, 2019

uloganalysis is now called pyulgresample: https://github.com/YUNEEC/pyulgresample
The main purpose of that project is resample ulog data. Once I finished the unit-tests, I will upload it to PyPa from wich it then can be downloaded.

@Stifael Stifael added the devcall label Feb 1, 2019
@hamishwillee
Copy link
Contributor

uloganalysis is now called pyulgresample: https://github.com/YUNEEC/pyulgresample
The main purpose of that project is resample ulog data. Once I finished the unit-tests, I will upload it to PyPa from wich it then can be downloaded.

@Stifael Can you also add some information about this in https://docs.px4.io/en/log/flight_log_analysis.html#analysis-tools ?

@stale
Copy link

stale bot commented Jul 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@hamishwillee
Copy link
Contributor

Is this really stale?

@Stifael
Copy link
Contributor Author

Stifael commented Jul 15, 2019

yep. I will open a new PR instead and will close this one sync it is out-dated by now.

@Stifael Stifael closed this Jul 15, 2019
@Stifael Stifael deleted the integration-testing-ulog branch July 15, 2019 11:36
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.

7 participants