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

ActionServer: allow direct setting of arm/disarm state and vehicle flight mode #2512

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonathanreeves
Copy link
Contributor

Summary

SDK changes to implement the interface provided in the corresponding proto PR. Testing continues to be done using QGroundControl to provide visibility into vehicle mode changes.

It's worth noting that there is probably a broad class of state changes like this that could use public APIs. I've opted to keep the scope constrained and focus only on the most essential information for now.

@jonathanreeves jonathanreeves force-pushed the feature/public-set-armed branch from bf98bf6 to e827c91 Compare February 18, 2025 18:25
@@ -75,16 +82,6 @@ class ActionServerImpl : public ServerPluginImplBase {
std::atomic<bool> _force_disarmable = false;
std::atomic<bool> _allow_takeoff = false;

union px4_custom_mode {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opportunistic cleanup, this declaration is a redundant copy of the one in px4_custom_mode.h

@julianoes
Copy link
Collaborator

There is now a new step that needs doing:

sudo apt install doxygen
tools/generate_docs.sh --overwrite --skip-checks

@jonathanreeves
Copy link
Contributor Author

There is now a new step that needs doing:

sudo apt install doxygen
tools/generate_docs.sh --overwrite --skip-checks

Thanks @julianoes, I did notice that. Quick question though: the github workflow shows it running on ubuntu 24.04, I assume the doxygen version is important here? I tried doing it on a container running 22.04 and it gave me a much larger diff than I was expecting. Wonder if it's worth having a dedicated container to do this similar to the run-docker-clang-format.sh script that's already there?

@jonathanreeves jonathanreeves force-pushed the feature/public-set-armed branch from fc5c350 to a48b101 Compare February 19, 2025 05:40
@jonathanreeves
Copy link
Contributor Author

There is now a new step that needs doing:

sudo apt install doxygen
tools/generate_docs.sh --overwrite --skip-checks

Thanks @julianoes, I did notice that. Quick question though: the github workflow shows it running on ubuntu 24.04, I assume the doxygen version is important here? I tried doing it on a container running 22.04 and it gave me a much larger diff than I was expecting. Wonder if it's worth having a dedicated container to do this similar to the run-docker-clang-format.sh script that's already there?

Nevermind, sorry for the churn. It looks like running with a 24.04 container does in fact make the docs consistent.

@julianoes
Copy link
Collaborator

Good point adding oxygen to the clang-format docker image, adding a doxygen version check and using that image for both. I'll do that.

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.

2 participants