New VehicleAction in MissionItem #1424

rligocki · 2021-04-29T21:04:47Z

based on:

JonasVautherin

Except for a wrong comment, it looks good to me 😊. Did you test it? Does it work? 🚀

src/plugins/mission/mission_impl.cpp

JonasVautherin · 2021-04-29T23:18:20Z

Seeing the errors in CI (e.g. in mission.h), I come to wonder: did you auto-generate the code from the proto file, or did you write it manually? mission.h should be auto-generated, you should not have to write anything there yourself, right?

julianoes · 2021-04-30T05:53:01Z

Thank @rligocki. Now it would be nice if there was a new integration test using it.

rligocki · 2021-04-30T10:07:15Z

Yep I wrote it manually. Didn't know that it is possible to generate templates based on MAVSDK-Proto. I will have a look on generation of on C++ code based on Proto.

julianoes · 2021-04-30T10:40:39Z

@rligocki there is a comment telling you that the file is auto-generated.
And please have a look at the docs: https://mavsdk.mavlink.io/main/en/cpp/contributing/plugins.html#generate-h-and-cpp-files

rligocki · 2021-05-01T13:52:59Z

Now it looks like it might be ready for testing. What is best way to do it?

JonasVautherin · 2021-05-01T21:52:03Z

What is best way to do it?

You can write a small script that uses it, or directly go for an integration test (like one of the mission_*.cpp here). Maybe mission_action.cpp?

Also the style is wrong, please run ./tools/fix_style.sh ., and the "proto" CI test is failing because you did not update the proto submodule in this PR. Just make it point to your MAVSDK-Proto PR commit and it should be fine 👍.

rligocki · 2021-05-02T16:11:45Z

It looks like, there is some change in MAVSDK-Proto in core.proto file. Connection state is missing value 1. In my MAVSDK-Proto PR there are only additional code with VehicleAction. MAVSDK PR there is one removed line with that value.

JonasVautherin · 2021-05-02T16:29:09Z

Connection state is missing value 1.

Wasn't this resolved in #1420? Is your branch up to date?

Also you forgot to fix the style: ./tools/fix_style.sh .

Other than that, everything seems to build, so I'd say that the next step would be an integration test showing a mission for a VTOL, with transition waypoints 😊. Once we can verify that this works, I think that will be ready to be merged (first the mavsdk-proto PR, then you'll have to update this one to point to main of mavsdk-proto, and then we'll merge this one).

TL;DR: next up is the integration test 😉

rligocki · 2021-05-02T16:39:52Z

Connection state is missing value 1.

Wasn't this resolved in #1420? Is your branch up to date?

https://github.com/mavlink/MAVSDK-Proto/blob/main/protos/core/core.proto - It looks like it is in main, so I was up to date.

Also you forgot to fix the style: ./tools/fix_style.sh .

On my mac I was not able to compile these tools. So I need to do fix_style on different computer at home.

JonasVautherin · 2021-05-02T17:18:37Z

It looks like it is in main, so I was up to date.

There two of them: the proto submodules, and then the MAVSDK branch itself. Both need to be up to date 👍. This said, the CI tests pass... where do you see the issue with the connection state? When building the integration tests? If yes, just update your branch to mavsdk:main 👍

rligocki · 2021-05-05T12:15:26Z

Still don't understand that problem with MAVSDK-Proto. On main branch, there is removed one connection state. Fixed problem with style. Should I create PR for connection state error?

JonasVautherin · 2021-05-05T12:47:23Z

On main branch, there is removed one connection state. Fixed problem with style. Should I create PR for connection state error?

You have a fork of mavlink/MAVSDK, say ~/mavsdk, right? Inside of it, there is the proto submodule, ~/mavsdk/proto. Those are two different git repositories, but they are related.

You need ~/mavsdk/proto to point to the HEAD of mavlink/MAVSDK-Proto, and ~/mavsdk to point to the HEAD of mavlink/MAVSDK. The connection changes are both in the proto submodule (where the interface removes it) and in the mavsdk repo (where it is removed in the implementation)

Does that make sense?

rligocki · 2021-05-05T13:00:34Z

You have a fork of mavlink/MAVSDK, say ~/mavsdk, right? Inside of it, there is the proto submodule, ~/mavsdk/proto. Those are two different git repositories, but they are related.

This is what I understand. I see no problem there.

You need ~/mavsdk/proto to point to the HEAD of mavlink/MAVSDK-Proto, and ~/mavsdk to point to the HEAD of mavlink/MAVSDK. The connection changes are both in the proto submodule (where the interface removes it) and in the mavsdk repo (where it is removed in the implementation)

But you said that ~/mavsdk/proto should point to MAVSDK-Proto PR. So you suggest to change proto git to point at mavlink/MAVSDK-Proto main branch?

rligocki · 2021-05-05T20:29:37Z

I am really lost. I need help at this situation. Cannot find where exactly I made mistake and how to solve it. Tried to find solution, but unfortunately it is first time I contribute to public repository so I have no experience at this case.

JonasVautherin · 2021-05-05T21:31:05Z

No worries, it can be tricky in the beginning 😉.

Should be fine now, both your branches were not up-to-date with upstream. Read about syncing with upstream e.g. here (it has to be done on both your mavsdk and mavsdk-proto forks until your proto is merged upstream).

JonasVautherin · 2021-05-05T21:53:49Z

The docs CI check is failing, I commented on the proto PR accordingly: mavlink/MAVSDK-Proto#226

Other than that, it would be nice if you could add an integration test. Note that the integration tests run on the iris SITL multicopter, so ideally use actions that work for a multicopter (i.e. takeoff and land), and maybe write a test that checks that the iris behaves correctly when you use VTOL actions (I don't know what PX4 is expected to do, but a test for it would be nice).

I won't be asking you to change the integration tests such that they can run a VTOL, but that would be a nice addition later 😊.

rligocki · 2021-05-06T10:35:28Z

Thanks a lot for your help. I though, that you would give me some advice, but this was faster way.

Where are those integration tests located? I can check them and then create group of new tests for this purpose.

julianoes · 2021-05-06T11:02:57Z

Does this answer it? https://mavsdk.mavlink.io/main/en/cpp/guide/test.html

JonasVautherin

Does the integration test prove that the takeoff action works? My feeling is that it would also takeoff without the takeoff action 🤔.

Could we make it such that it lands and takes off in the middle of a mission? That I am pretty sure is not yet working 😆

src/integration_tests/mission_takeoff_land.cpp

JonasVautherin · 2021-05-13T12:29:42Z

src/integration_tests/mission_transition.cpp

+    Mission::MissionPlan mission_plan{};
+    mission_plan.mission_items.push_back(new_item);
+
+    new_item.vehicle_action = Mission::MissionItem::VehicleAction::TransitionToFw;


What's the expected behavior when flying a multicopter? I guess it is ignored, isn't it?

I guess so. I was thinking, that if landing and takeoff would work, this action will provide that information.

src/integration_tests/mission_takeoff_land.cpp

JonasVautherin

LGTM, but will need to be merged after the acceptance_radius PR (because it requires an update of the proto submodule)

…lem on IRIS

JonasVautherin · 2021-05-28T18:28:43Z

@rligocki: I merged the proto PR and updated the branch here (don't forget to pull the changes). Once the integration tests are fixed, that can go in 👍

JonasVautherin · 2021-06-03T17:03:24Z

@rligocki I removed the commits on mavsdk-proto:main and moved them to https://github.com/mavlink/MAVSDK-Proto/tree/add-vehicle-action until this is solved.

Hence you will have to update the proto submodule here to point to https://github.com/mavlink/MAVSDK-Proto/tree/add-vehicle-action when you get back to this PR

rligocki · 2021-06-08T16:14:07Z

Now I was able to found some time for this problem. I will fix those tests so that they will pass.

julianoes · 2021-07-12T09:44:50Z

@rligocki what's the status here? Would be nice to get this in soon.

rligocki · 2021-07-15T11:01:31Z

Cannot upload those vehicle mission item on PX4. There is an error.

rligocki · 2021-08-25T13:15:49Z

Currently there should be no error during runtime. One last thing, that needs to be done is transition mission validation. Now there is no measurement if transition happened. Also I cannot find way to run transition test with gazebo_standard_vtol plane.

rligocki changed the title ~~Added first iteration of code to generate MAVLink commands based on VehicleAction in MissionItem~~ New VehicleAction in MissionItem Apr 29, 2021

JonasVautherin requested changes Apr 29, 2021

View reviewed changes

src/plugins/mission/mission_impl.cpp Outdated Show resolved Hide resolved

JonasVautherin force-pushed the feature-extension-mission-item branch 2 times, most recently from fafd513 to 8cf92da Compare May 5, 2021 21:29

JonasVautherin force-pushed the feature-extension-mission-item branch from 8cf92da to 8bf4b21 Compare May 5, 2021 21:39

JonasVautherin reviewed May 10, 2021

View reviewed changes

rligocki commented May 13, 2021

View reviewed changes

src/integration_tests/mission_takeoff_land.cpp Outdated Show resolved Hide resolved

JonasVautherin reviewed May 13, 2021

View reviewed changes

JonasVautherin requested changes May 13, 2021

View reviewed changes

src/integration_tests/mission_takeoff_land.cpp Outdated Show resolved Hide resolved

JonasVautherin force-pushed the feature-extension-mission-item branch from aaab585 to 8c3802b Compare May 27, 2021 08:55

JonasVautherin approved these changes May 27, 2021

View reviewed changes

JonasVautherin requested changes May 27, 2021

View reviewed changes

Roman Ligocki and others added 16 commits May 28, 2021 20:18

Added code to generate command based on VehicleAction in MissionItem

e7592ec

Modified code block to suite VehicleAction

7afa50c

Better format, added comments

cdfcd0f

Addition changes based on current implementation of cameraAction

6ed1390

Generated mission files based on modified proto

7cd92ca

Changed wrong comment

aa55476

Fixed errors to pass compilation process

fc7e015

Used fix_style.sh

81c4b80

Added integration test for takeoff and land

e0ac4f0

New transition integration test. Should be completed without any prob…

8fe318a

…lem on IRIS

Added verification of takeoff and land actions during mission.

3f495fe

Fixed problems during build

65f2195

Modified arrays to vectors

6248a4f

Removed uncessary index variable

f0b0833

Fixed for loop

bbedab1

Update branch

3404dda

JonasVautherin force-pushed the feature-extension-mission-item branch from 8c3802b to 3404dda Compare May 28, 2021 18:27

julianoes marked this pull request as draft July 12, 2021 14:53

rligocki added 2 commits August 23, 2021 22:18

Changed mission items type, Removed low height test

38a3079

Added transition test

efed0c0

rligocki mentioned this pull request Aug 23, 2021

Added support for takeoff and land commands in mission PX4/PX4-Autopilot#18130

Closed

Fixed issue with vehicle action, modified tests

6337c04

rligocki marked this pull request as ready for review August 25, 2021 13:13

rligocki closed this Sep 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New VehicleAction in MissionItem #1424

New VehicleAction in MissionItem #1424

rligocki commented Apr 29, 2021

JonasVautherin left a comment

JonasVautherin commented Apr 29, 2021

julianoes commented Apr 30, 2021

rligocki commented Apr 30, 2021

julianoes commented Apr 30, 2021

rligocki commented May 1, 2021

JonasVautherin commented May 1, 2021

rligocki commented May 2, 2021

JonasVautherin commented May 2, 2021 •

edited

Loading

rligocki commented May 2, 2021

JonasVautherin commented May 2, 2021

rligocki commented May 5, 2021

JonasVautherin commented May 5, 2021

rligocki commented May 5, 2021 •

edited

Loading

rligocki commented May 5, 2021

JonasVautherin commented May 5, 2021

JonasVautherin commented May 5, 2021

rligocki commented May 6, 2021

julianoes commented May 6, 2021

JonasVautherin left a comment

JonasVautherin May 13, 2021

rligocki May 13, 2021

JonasVautherin left a comment

JonasVautherin commented May 28, 2021 •

edited

Loading

JonasVautherin commented Jun 3, 2021 •

edited

Loading

rligocki commented Jun 8, 2021

julianoes commented Jul 12, 2021

rligocki commented Jul 15, 2021

rligocki commented Aug 25, 2021

New VehicleAction in MissionItem #1424

New VehicleAction in MissionItem #1424

Conversation

rligocki commented Apr 29, 2021

JonasVautherin left a comment

Choose a reason for hiding this comment

JonasVautherin commented Apr 29, 2021

julianoes commented Apr 30, 2021

rligocki commented Apr 30, 2021

julianoes commented Apr 30, 2021

rligocki commented May 1, 2021

JonasVautherin commented May 1, 2021

rligocki commented May 2, 2021

JonasVautherin commented May 2, 2021 • edited Loading

rligocki commented May 2, 2021

JonasVautherin commented May 2, 2021

rligocki commented May 5, 2021

JonasVautherin commented May 5, 2021

rligocki commented May 5, 2021 • edited Loading

rligocki commented May 5, 2021

JonasVautherin commented May 5, 2021

JonasVautherin commented May 5, 2021

rligocki commented May 6, 2021

julianoes commented May 6, 2021

JonasVautherin left a comment

Choose a reason for hiding this comment

JonasVautherin May 13, 2021

Choose a reason for hiding this comment

rligocki May 13, 2021

Choose a reason for hiding this comment

JonasVautherin left a comment

Choose a reason for hiding this comment

JonasVautherin commented May 28, 2021 • edited Loading

JonasVautherin commented Jun 3, 2021 • edited Loading

rligocki commented Jun 8, 2021

julianoes commented Jul 12, 2021

rligocki commented Jul 15, 2021

rligocki commented Aug 25, 2021

JonasVautherin commented May 2, 2021 •

edited

Loading

rligocki commented May 5, 2021 •

edited

Loading

JonasVautherin commented May 28, 2021 •

edited

Loading

JonasVautherin commented Jun 3, 2021 •

edited

Loading