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

New example that runs multiple plan files on multiple drones #761

Merged
merged 8 commits into from
Jun 8, 2019
Merged

New example that runs multiple plan files on multiple drones #761

merged 8 commits into from
Jun 8, 2019

Conversation

shusilshapkota
Copy link
Contributor

Added a new example that runs multiple QGC plan files on multiple drones.

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Would you mind updating CMakeLists.txt to use find_package(DronecodeSDK REQUIRED) instead of those includes, as in the (recently updated) other examples?

Also, you'll need to run clang-format ($ ./tools/fix_style.sh .)

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

That's nice, thank you! I fixed the build for the recent changes in the CMake organization, it should pass the CI now.

However, I can't manage to run it. I followed your instructions and got:

% ./example/build/fly_multiple_drones/fly_multiple_drones udp://:14540 udp://:14541 ~/Downloads/test_plan1.plan ~/Downloads/test_plan2.plan
[05:49:48|Info ] DronecodeSDK version: 0.17.0-89-gbbe891e5-dirty (dronecode_sdk_impl.cpp:25)
Waiting to discover system...
[05:49:48|Info ] New system on: 127.0.0.1:14580 (udp_connection.cpp:228)
[05:49:48|Debug] New: System ID: 1 Comp ID: 1 (dronecode_sdk_impl.cpp:338)
[05:49:48|Debug] Component Autopilot (1) added. (system_impl.cpp:400)
[05:49:48|Error] No known remotes (udp_connection.cpp:132)
[05:49:48|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:48|Error] connection send error (520) (mavlink_commands.cpp:256)
[05:49:48|Debug] Discovered 1 component(s) (UUID: 5283920058631409231) (system_impl.cpp:563)
Discovered system with UUID: 5283920058631409231
[05:49:49|Error] No known remotes (udp_connection.cpp:132)
[05:49:49|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:49|Debug] MAVLink: info: data link #1 lost (system_impl.cpp:307)
[05:49:49|Debug] MAVLink: info: data link #0 regained (system_impl.cpp:307)
[05:49:49|Debug] MAVLink: info: ARMED by arm/disarm component command (system_impl.cpp:307)
[05:49:49|Debug] MAVLink: info: Takeoff detected (system_impl.cpp:307)
[05:49:49|Debug] MAVLink: info: data link #1 regained (system_impl.cpp:307)
[05:49:50|Error] No known remotes (udp_connection.cpp:132)
[05:49:50|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:50|Error] Error: Send message failed (mavlink_parameters.cpp:233)
[05:49:50|Error] Error: Param for gyro cal failed. (telemetry_impl.cpp:615)
[05:49:50|Error] No known remotes (udp_connection.cpp:132)
[05:49:50|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:50|Error] connection send error (511) (mavlink_commands.cpp:256)
[05:49:50|Error] No known remotes (udp_connection.cpp:132)
[05:49:50|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:50|Error] No known remotes (udp_connection.cpp:132)
[05:49:50|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:50|Error] Error: Send message failed (mavlink_parameters.cpp:233)
Importing mission from mission plan: /home/jones/Downloads/test_plan1.plan
[05:49:50|Error] Error: Param for accel cal failed. (telemetry_impl.cpp:626)
Vehicle is getting ready to arm
[05:49:50|Error] No known remotes (udp_connection.cpp:132)
[05:49:50|Error] send fail (dronecode_sdk_impl.cpp:91)
[05:49:50|Error] Error: Send message failed (mavlink_parameters.cpp:233)
[05:49:50|Error] Error: Param for mag cal failed. (telemetry_impl.cpp:637)

Does that work for you?

@shusilshapkota
Copy link
Contributor Author

Does that work for you?

Yes it works fine for me. Are you running multiple instances? It looks like it didn't discover the other system (May be the system running in udp://:14541)

@JonasVautherin
Copy link
Collaborator

Hmm it worked when I ran jmavsim with port 4561 (instead of 14561 in your instructions). Seems like the px4 docs says 4561 as well. Is that a typo in your instructions?

@shusilshapkota
Copy link
Contributor Author

shusilshapkota commented Jun 6, 2019

It think I was running an old version of Firmware v1.8.2, so my instructions were for this version as per v1.8.2 PX4 docs. I can update the instructions to get it to work for v1.9.0 if needed.

@JonasVautherin
Copy link
Collaborator

Yes, I believe it would be better to have updated instructions. Maybe you can also add the link to the documentation, in case it changes again in the future :-).

After that it will be fine for me! Thanks a lot for the contribution!

@shusilshapkota
Copy link
Contributor Author

Yea sure. I will update the instructions and add the link as well

JonasVautherin
JonasVautherin previously approved these changes Jun 6, 2019
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the example. I think it's nice overall but I have a couple of small things that can be improved.

std::cout << "Waiting to discover system..." << std::endl;
dc.register_on_discover([&discovered_system](uint64_t uuid) {
std::cout << "Discovered system with UUID: " << uuid << std::endl;
discovered_system = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're only checking for one system, not for as many as you need. What about using:

std::atomic<unsigned> num_systems_discovered {0};

and then in this lambda you can do:

++num_systems_discovered;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to check if all the systems are discovered right? I've added it on the new commit.

Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot for the fixes!

@julianoes
Copy link
Collaborator

Merging this even though CI is not working which is probably due to the name change.

@julianoes julianoes merged commit 7011745 into mavlink:develop Jun 8, 2019
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